-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
feat(ui): add namespace input to UserInfo page for SSO RBAC NS delegation. Fixes #12041 #13628
Open
MasonM
wants to merge
4
commits into
argoproj:main
Choose a base branch
from
MasonM:feat-12041
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When using [SSO RBAC Namespace Delegation](https://argo-workflows.readthedocs.io/en/latest/argo-server-sso/#sso-rbac-namespace-delegation), there's currently no way of seeing which service account maps to the user in a given namespace. The backend for the `/api/v1/userinfo` endpoint already supports a query parameter called `?namespace` that it will use to look up service account details, though this isn't documented. This documents the existing `?namespace` query parameter, adds a namespace inbox filter on the top of the page, and updates the `UserInfo` page to pass it when calling `/api/v1/userinfo`. The only thing I wasn't quite sure about is error handling: if someone enter an invalid namespace, then `/api/v1/userinfo` will ignore it and silently fall back to the installation namespace, which could cause confusion. Ideally, the UI would detect that and show an informative error message, but that'd require non-trivial API changes. Testing procedure: 1. Created the following manifest and ran `kubectl apply -f` on it: ```yaml apiVersion: v1 kind: Namespace metadata: name: delegation-test --- apiVersion: v1 kind: ServiceAccount metadata: name: delegated-sa namespace: delegation-test annotations: workflows.argoproj.io/rbac-rule: "true" workflows.argoproj.io/rbac-rule-precedence: "2" --- apiVersion: v1 kind: Secret metadata: name: delegated-sa.service-account-token namespace: delegation-test annotations: kubernetes.io/service-account.name: delegated-sa type: kubernetes.io/service-account-token data: ca.crt: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUJkekNDQVIyZ0F3SUJBZ0lCQURBS0JnZ3Foa2pPUFFRREFqQWpNU0V3SHdZRFZRUUREQmhyTTNNdGMyVnkKZG1WeUxXTmhRREUzTWpFM05qazBPVFF3SGhjTk1qUXdOekl6TWpFeE9ERTBXaGNOTXpRd056SXhNakV4T0RFMApXakFqTVNFd0h3WURWUVFEREJock0zTXRjMlZ5ZG1WeUxXTmhRREUzTWpFM05qazBPVFF3V1RBVEJnY3Foa2pPClBRSUJCZ2dxaGtqT1BRTUJCd05DQUFTWnAyeVNTekR5OFR6VjJHWG1naUdWaW5Qb0VkREZrdCtSeGpkbE5zMjAKamc1bk9iUEhLMTYybDdObHR5dGxpb0RxTHIwQXNwSGNPVWlvUTlrcElnNVdvMEl3UURBT0JnTlZIUThCQWY4RQpCQU1DQXFRd0R3WURWUjBUQVFIL0JBVXdBd0VCL3pBZEJnTlZIUTRFRmdRVWZlYUhaTnpOeG9kd0w0YXNHdDhSCjBrZDJ6K0V3Q2dZSUtvWkl6ajBFQXdJRFNBQXdSUUlnRGdWZjJBMGZESkN1S3lLblI3RTVBay9KTDlWek1KQzIKZ2dKbmJFbjFrNm9DSVFDZldtL1FWbkxVVG5Bb0RMSCtFNzN6UW0wdXVOVEhzemxXVFhYM2hjWkFaUT09Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K namespace: ZGVsZWdhdGlvbi10ZXN0Cg== token: ZXlKaGJHY2lPaUpTVXpJMU5pSXNJbXRwWkNJNklreHlhVWhQV0RSdlYwOUhhSFZsVFdOWWJqQTVhV1p1VUcxRFh6aFNlVkZUVG5kVGVWTjNTVEZPZDJjaWZRLmV5SnBjM01pT2lKcmRXSmxjbTVsZEdWekwzTmxjblpwWTJWaFkyTnZkVzUwSWl3aWEzVmlaWEp1WlhSbGN5NXBieTl6WlhKMmFXTmxZV05qYjNWdWRDOXVZVzFsYzNCaFkyVWlPaUpoY21kdklpd2lhM1ZpWlhKdVpYUmxjeTVwYnk5elpYSjJhV05sWVdOamIzVnVkQzl6WldOeVpYUXVibUZ0WlNJNkltRnlaMjh0YzJWeWRtVnlMbk5sY25acFkyVXRZV05qYjNWdWRDMTBiMnRsYmlJc0ltdDFZbVZ5Ym1WMFpYTXVhVzh2YzJWeWRtbGpaV0ZqWTI5MWJuUXZjMlZ5ZG1salpTMWhZMk52ZFc1MExtNWhiV1VpT2lKaGNtZHZMWE5sY25abGNpSXNJbXQxWW1WeWJtVjBaWE11YVc4dmMyVnlkbWxqWldGalkyOTFiblF2YzJWeWRtbGpaUzFoWTJOdmRXNTBMblZwWkNJNklqSmtOV1F3TVRjMkxUQmpPRGN0TkRVMlpTMWlOR05tTFdFMlpEUmlaRGhsTURJelpTSXNJbk4xWWlJNkluTjVjM1JsYlRwelpYSjJhV05sWVdOamIzVnVkRHBoY21kdk9tRnlaMjh0YzJWeWRtVnlJbjAuYXZZVVZNZUtpTnZKYWdkU1U0bElYQ1RjdEFqSUE4V0FCVC01MEh1SFRRTVFrNUNKSFY1NFVTOW9hZlIwS085TldLcE5aVVlCU0hIaWJIWXRXRDdVVUVRRnk2bEFidzRUU3lwb2lBN2dST3N3X1dXSXpkS3BYVG5CM1UzSVVVeEdaQW56ZlBKNlZRdXhGQUZIQ205b1lTZXl1eDE4MkNNdnphUEhEdjd0N0V3TTRCSHFOU1dCMFc2aG95ZFJzeWdBV0xoWFhjSnNZZDdTTzBNRUlBWTViWmNHajF0eXQ1NUV1T1N2SDdqdUJES2JianhYMFlKUkp5dl9tbzdHNl9EemtHN3NMQ2NsUGZOMXYyZnJkZ2cxdjVEZHZBNmhVeXhsRjZhUzdublM5X1BfbW42UlctUk9TUWI3YkpTNVFmNzdVNG01QUFYUGttRWhnM2htVXcwdjJB ``` I thought about creating a new profile under `test/e2e/manifests/sso-delegated` that could be used to test this via `make start PROFILE=sso-delegated`, but I don't know if that's worth it. 2. Run `make start UI=true PROFILE=sso SSO_DELEGATE_RBAC_TO_NAMESPACE=true NAMESPACED=false` 3. Visit http://localhost:8080/ 4. Click "Login" 5. Click "Log in with Example" 6. Click "Grant Access" 7. Click the icon for the `UserInfo` page on the left navigation bar 8. Verify namespace input is populated with the installation namespace Also, I verified the Signed-off-by: Mason Malone <[email protected]>
6 tasks
Signed-off-by: Mason Malone <[email protected]>
Signed-off-by: Mason Malone <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #12041
Motivation
When using SSO RBAC Namespace Delegation, there's currently no way of seeing which service account maps to the user in a given namespace. This can be very confusing to a user who only has access to a specific namespace, since they have no way of knowing what permissions they have for that namespace.
Modifications
This updates the
/api/v1/userinfo
endpoint to support a?namespace
query parameter, which the SSO backend code already supports and will use to look up service account details when present:argo-workflows/server/auth/gatekeeper.go
Lines 228 to 237 in 7b2c4aa
argo-workflows/server/auth/gatekeeper.go
Lines 305 to 314 in 7b2c4aa
On the UI side, this adds a namespace input filter on the top of the page, and updates the
UserInfo
page to pass it when calling/api/v1/userinfo
. Note that the namespace input is shown even if SSO RBAC NS delegation isn't enabled. The linked issue says "Only show this input box if SSO RBAC Namespace delegation is enabled", but there's no way of doing this modifying the/api/v1/info
API, which has security ramifications per @agilgur5: #12041 (comment)The only thing I wasn't quite sure about is error handling: if someone enters an invalid namespace, then
/api/v1/userinfo
will ignore it and silently fall back to the installation namespace, which could cause confusion. Ideally, the UI would detect that and show an informative error message, but that'd require non-trivial API changes.Screen.Recording.2024-09-19.at.1.24.18.PM.mp4
Verification
Testing procedure:
kubectl apply -f
on it:test/e2e/manifests/sso-delegated
with these manifests that could be used to test this viamake start PROFILE=sso-delegated
, but I don't know if that's worth it.make start UI=true PROFILE=sso SSO_DELEGATE_RBAC_TO_NAMESPACE=true NAMESPACED=false
UserInfo
page on the left navigation barSee the above recording for the results. Also, I verified the namespace input is replaced with fixed text when using managed namespaces: