Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: metrics should be protected behind authZ #1895

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

adodon2go
Copy link
Collaborator

What type of PR is this?
bug

Which issue does this PR fix:
#1876

What does this PR do / Why do we need it:
In case anonymous policy is present in access control,we allow anonymous users to access to access /metrics endpoints.
To prevent this in accessControl section of config you can specify a list of metrics users that are allowed to access /metrics endpoint.
In case accessControl is not specified in config but auth is present than only authenticated users can access /metrics endpoint

If an issue # is not available please add repro steps and logs showing the issue:

Testing done on this change:

Automation added to e2e:

Will this break upgrades or downgrades?

Does this PR introduce any user-facing change?:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov
Copy link

codecov bot commented Oct 5, 2023

Codecov Report

Merging #1895 (7b1441d) into main (a44ca57) will decrease coverage by 0.03%.
The diff coverage is 88.37%.

@@            Coverage Diff             @@
##             main    #1895      +/-   ##
==========================================
- Coverage   91.87%   91.85%   -0.03%     
==========================================
  Files         155      155              
  Lines       26678    26701      +23     
==========================================
+ Hits        24511    24525      +14     
- Misses       1603     1611       +8     
- Partials      564      565       +1     
Files Coverage Δ
pkg/api/authn.go 95.71% <100.00%> (ø)
pkg/api/config/config.go 97.35% <ø> (ø)
pkg/api/routes.go 94.50% <100.00%> (ø)
pkg/extensions/extension_metrics.go 100.00% <100.00%> (ø)
pkg/extensions/extension_metrics_disabled.go 100.00% <100.00%> (ø)
pkg/api/authz.go 94.00% <86.11%> (-1.64%) ⬇️

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

andaaron
andaaron previously approved these changes Oct 6, 2023
pkg/api/authz.go Outdated Show resolved Hide resolved
pkg/api/authz.go Outdated Show resolved Hide resolved
pkg/api/authz.go Outdated Show resolved Hide resolved
pkg/api/authz.go Outdated Show resolved Hide resolved
pkg/extensions/README.md Outdated Show resolved Hide resolved
pkg/extensions/extension_metrics.go Outdated Show resolved Hide resolved
@adodon2go adodon2go force-pushed the authn-protected-metrics branch 2 times, most recently from ef6f7bd to e8b1bcb Compare October 11, 2023 12:22
@adodon2go adodon2go marked this pull request as ready for review October 11, 2023 12:26
andaaron
andaaron previously approved these changes Oct 11, 2023
@adodon2go adodon2go self-assigned this Oct 11, 2023
@adodon2go adodon2go force-pushed the authn-protected-metrics branch 4 times, most recently from fc826a5 to 4ea9451 Compare October 16, 2023 10:09
@adodon2go adodon2go requested a review from shimish2 as a code owner October 16, 2023 10:09
@adodon2go adodon2go force-pushed the authn-protected-metrics branch from 4ea9451 to 4389c19 Compare October 16, 2023 10:12
@adodon2go adodon2go changed the title fix: Metrics should be protected behind authZ fix: metrics should be protected behind authZ Oct 16, 2023
@adodon2go adodon2go force-pushed the authn-protected-metrics branch 2 times, most recently from 398838f to 7e4f8cb Compare October 16, 2023 10:39
pkg/test/common/utils.go Outdated Show resolved Hide resolved
@adodon2go adodon2go force-pushed the authn-protected-metrics branch 3 times, most recently from 3b01c06 to 6b5fac6 Compare October 18, 2023 10:01
pkg/test/common/utils.go Outdated Show resolved Hide resolved
pkg/test/common/utils.go Outdated Show resolved Hide resolved
pkg/extensions/search/userprefs_test.go Outdated Show resolved Hide resolved
pkg/extensions/search/userprefs_test.go Outdated Show resolved Hide resolved
pkg/extensions/search/userprefs_test.go Outdated Show resolved Hide resolved
pkg/api/controller_test.go Outdated Show resolved Hide resolved
pkg/api/controller_test.go Outdated Show resolved Hide resolved
@adodon2go adodon2go force-pushed the authn-protected-metrics branch 2 times, most recently from a208dbc to 823e3f4 Compare October 18, 2023 15:33
andaaron
andaaron previously approved these changes Oct 18, 2023
@rchincha
Copy link
Contributor

There is a test failure?

@adodon2go adodon2go force-pushed the authn-protected-metrics branch 2 times, most recently from 9a11415 to 0dd615e Compare October 19, 2023 20:40
@adodon2go adodon2go force-pushed the authn-protected-metrics branch from 0dd615e to 7b1441d Compare October 20, 2023 06:32
@andaaron andaaron merged commit a345ba0 into project-zot:main Oct 20, 2023
31 of 33 checks passed
@andaaron andaaron linked an issue Oct 20, 2023 that may be closed by this pull request
@adodon2go adodon2go deleted the authn-protected-metrics branch October 20, 2023 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Metrics should be protected behind authN
3 participants