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

UI: Add namespace input box to UserInfo page for SSO RBAC NS delegation #12041

Open
agilgur5 opened this issue Oct 20, 2023 · 6 comments · May be fixed by #13628
Open

UI: Add namespace input box to UserInfo page for SSO RBAC NS delegation #12041

agilgur5 opened this issue Oct 20, 2023 · 6 comments · May be fixed by #13628
Labels
area/sso-rbac area/ui solution/suggested A solution to the bug has been suggested. Someone needs to implement it. type/feature Feature request
Milestone
v3.6.0

Comments

@agilgur5
Copy link
Member

agilgur5 commented Oct 20, 2023 •
edited
Loading

Summary

Add an input box to the UserInfo page where a user can type in a namespace. It would default to the Argo installation namespace, as it currently does today.

Use Cases

Right now, when using SSO RBAC with Namespace delegation, there's no way to see in the UI what SA will map to your user in a specific namespace.
You have to try a request in that namespace and then check the Server logs. Users who don't have access to the Server logs have no way to debug or even see what SA they got assigned in an NS.

Moreover, as the UserInfo page only shows you the default SA in the Argo installation namespace (see also #10968), that can be very confusing to a user who only has access to a specific namespace; they will effectively never be able to see their permissions for that namespace.
This popped up over Slack today with a user, but I was similarly confused by this when I was first setting up SSO RBAC namespace delegation (see also #10968).

Implementation Details

Server-side Details

This does require a new query parameter to be able to be passed to the backend in /api/v1/userinfo, i.e. ?namespace=my-namespace.

In server/auth/gatekeeper.go, this line in the rbacAuthorization function gets the namespace via the getNamespace function. That function merely takes a request as an argument, so we just have to make sure the request is properly formatted as it expects.

I'm not entirely sure how it does the conversion from gRPC to Go struct, as this is one of the places where almost everything is auto-generated that keeps confusing me (and other contributors). IIRC the source of truth here is info.proto, but I'm not entirely sure how to ensure that adding a namespace property there corresponds to GetNamespace() returning that property specifically.

UI Details

I think an input box for Namespace at the top of the page would suffice. Default to the Argo installation namespace as mentioned above. Should use the same input box and behavior as the Namespace input on the Workflows List page.

Only show this input box if SSO RBAC Namespace delegation is enabled.

Message from the maintainers:

Love this enhancement proposal? Give it a 👍. We prioritise the proposals with the most 👍.
@agilgur5 agilgur5 added the solution/suggested A solution to the bug has been suggested. Someone needs to implement it. label Oct 29, 2023
@agilgur5 agilgur5 mentioned this issue Jan 20, 2024
Closed
4 tasks
@gabrielfsousa
Copy link

i also would love to have the UserInfo and ns correct in UI

@agilgur5
Copy link
Member Author

agilgur5 commented Feb 6, 2024

i also would love to have the UserInfo and ns correct in UI

To be clear, this only impacts those using SSO RBAC Namespace Delegation and the current SA is not incorrect, it just shows the assigned cluster scope SA instead of any one NS SA

@aaron-arellano
Copy link

this should be a namespace drop-down that pulls in namespaces by scanning rolebindings the user is assigned to or using SubjectAccessReview for the same behavior so the user sees the namespaces they are part of. This would be a better user experience then a blank input box.

@agilgur5
Copy link
Member Author

agilgur5 commented Feb 7, 2024

This would be a better user experience then a blank input box.

It would be, but this is a bit easier said than done. There's actually a separate feature request for that: #9140

that pulls in namespaces by scanning rolebindings the user is assigned to

The problem is that you don't know ahead of time what namespaces the user has access to.
A user can map to multiple SAs' annotation expressions. So you'd have to do a cluster-wide search, which in and of itself may not be performant (big lists etc), then evaluate all the expressions to see which ones apply to the user, which also may not be performant.
If there were a static listing somewhere, that would definitely make it much easier, but that does not currently exist.

@agilgur5 agilgur5 mentioned this issue Sep 14, 2024
Open
@MasonM
Copy link
Contributor

MasonM commented Sep 18, 2024

I'm looking into this, and I'm having trouble with the "Only show this input box if SSO RBAC Namespace delegation is enabled" requirement. Unless I'm missing something, the UI currently has no way of knowing when SSO or RBAC namespace delegation (which is controlled by the SSO_DELEGATE_RBAC_TO_NAMESPACE environment variable) is enabled. It would be possible to update the /api/v1/info API to expose this information via a new field called something like canDelegateRBACToRequestNamespace, and I created a branch to do that: main...MasonM:argo-workflows:feat-12041

That works, but do we really want to want to update that API solely to support this requirement? It'd be simpler to always show the input box.

@agilgur5
Copy link
Member Author

agilgur5 commented Sep 18, 2024 •
edited
Loading

Nice analysis! Yes, you're correct that it would have to be added to the info endpoint (although it would be better to name it identically).

I've also tried to avoid updating that API, especially as there can be security ramifications to exposing more configuration. Instead I've tried to infer the configuration where possible. In this case, I don't think you can infer it before displaying the input, as the installation namespace will always have some SA.

As such, I think it's fine to have the namespace there for now and for an MVP, and we can reevaluate the info endpoint changes later.

Worth noting that I wrote this issue up almost a year ago, so my knowledge of the codebase was certainly more incomplete back then 😅

MasonM added a commit to MasonM/argo-workflows that referenced this issue Sep 19, 2024
@MasonM
feat(ui): add namespace input to UserInfo page. Fixes argoproj#12041
ee50921 
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 <mmalone@adobe.com>
@MasonM MasonM linked a pull request Sep 19, 2024 that will close this issue
Open
@agilgur5 agilgur5 added this to the v3.6.0 milestone Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/sso-rbac area/ui solution/suggested A solution to the bug has been suggested. Someone needs to implement it. type/feature Feature request
Projects
Argo Workflows UI/UX Improvements
Status: No status
Milestone
v3.6.0
4 participants
@MasonM @gabrielfsousa @agilgur5 @aaron-arellano