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.

Sign up for GitHub

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

Jump to bottom

frontend: AuthVisible: Validate verb before auth check #2610

Merged
merged 1 commit into from
Nov 27, 2024
Merged

frontend: AuthVisible: Validate verb before auth check #2610

merged 1 commit into from
Nov 27, 2024

Conversation

skoeva
Copy link
Contributor

@skoeva skoeva commented Nov 25, 2024

This change validates the authVerb passed into the AuthVisible component before calling getAuthorization(), which prevents components from rendering with an invalid verb.

Fixes: #2147

Testing

  • Open a cluster in Headlamp and navigate to the Namespaces page
  • Open frontend/src/components/namespace/CreateNamespaceButton.tsx and set the authVerb to something random (e.g. '1234')
  • Reload the page and ensure that the create button does not show up
  • Ensure that you see the following warning in the console:

image

@skoeva skoeva added frontend Issues related to the frontend auth Authentication or authorization related labels Nov 25, 2024
@skoeva skoeva self-assigned this Nov 25, 2024
@skoeva skoeva linked an issue Nov 25, 2024 that may be closed by this pull request
AuthVisible authenticates with any auth verb #2147
Closed
@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Nov 25, 2024
@joaquimrocha joaquimrocha self-requested a review November 26, 2024 17:28
joaquimrocha
joaquimrocha requested changes Nov 26, 2024
View reviewed changes
Copy link
Collaborator

@joaquimrocha joaquimrocha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a comment. I am surprised that the kubernetes server replies with an allowed response if the verb is unknown though. Did you verify whether this is the case or we may be interpreting a non-negative value as positive?

frontend/src/components/common/Resource/AuthVisible.tsx Outdated Show resolved Hide resolved
@skoeva
frontend: AuthVisible: Validate verb before auth check
98053b2 
This change validates the authVerb passed into the AuthVisible
component before calling getAuthorization(), which prevents components
from rendering with an invalid verb.

Fixes: #2147

Signed-off-by: Evangelos Skopelitis <eskopelitis@microsoft.com>
@skoeva
Copy link
Contributor Author

skoeva commented Nov 26, 2024 •
edited
Loading

@joaquimrocha the this.fetchAuthorization() call returns allowed: true for the invalid verb here. Maybe the rules permit all verbs (e.g. verbs: ["*"]) without actually validating them.

image

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Nov 27, 2024
@joaquimrocha joaquimrocha merged commit 877d8e1 into main Nov 27, 2024
17 of 18 checks passed
@joaquimrocha joaquimrocha deleted the authvisible-verb-bug branch November 27, 2024 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joaquimrocha joaquimrocha joaquimrocha approved these changes

Assignees

@skoeva skoeva

Labels
auth Authentication or authorization related frontend Issues related to the frontend lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AuthVisible authenticates with any auth verb
2 participants
@skoeva @joaquimrocha