Viewing Archived Workflows when RBAC is Namespace Delegated - Bug?

[BhavikaSharma]

Hi, I'm wondering if there's a potential bug when when using namespace delegation for RBAC. I have a service account in a namespace that has permissions to view workflows in the namespace. I'm able to log in, and through a group, bind to the service account. This allows me to view workflows in the namespace.

From the logs, I see:

time="2022-04-26T20:53:26.926Z" level=info msg="selected SSO RBAC service account for user" email=<redacted> loginServiceAccount=user-default-login serviceAccount=default ssoDelegated=true ssoDelegationAllowed=true subject=<redacted>

However, when accessing archived workflows from the UI, I receive an error:

time="2022-04-26T17:54:01.366Z" level=info msg="finished unary call with code OK" grpc.code=OK grpc.method=GetInfo grpc.service=info.InfoService grpc.start_time="2022-04-26T17:54:01Z" grpc.time_ms=2.549 span.kind=server system=grpc
time="2022-04-26T17:54:01.376Z" level=warning msg="finished unary call with code PermissionDenied" error="rpc error: code = PermissionDenied desc = Permission denied, you are not allowed to list workflows in namespace \"<redacted>\". Maybe you want to specify a namespace with `listOptions.fieldSelector=metadata.namespace=your-ns`?" grpc.code=PermissionDenied grpc.method=ListArchivedWorkflows grpc.service=workflowarchive.ArchivedWorkflowService grpc.start_time="2022-04-26T17:54:01Z" grpc.time_ms=15.412 span.kind=server system=grpc

From the logs, I see:

time="2022-04-26T20:52:30.920Z" level=info msg="selected SSO RBAC service account for user" email=<redacted> loginServiceAccount=user-default-login serviceAccount=user-default-login ssoDelegated=false ssoDelegationAllowed=false subject=<redacted>

When I expected to see the same logs as regularly accessing the workflows page.

It seems like this function is returning false for archived workflows. Wondering if anyone else is seeing this/if this is a potential bug. Thanks!

Originally posted by @BhavikaSharma in https://github.com/argoproj/argo-workflows/discussions/8497

[alexec]

@basanthjenuhb ?

[basanthjenuhb]

So ListArchivedWorkflowsRequest is implemented differently. It doesn't have a namespace parameter. Namespace is passed as a FieldSelector Hence currently this will not work

so to fix this, 2 options

1. Write custom code to handle ListArchivedWorkflowsRequest requests which don't have a namespace request
2. Modify ListArchivedWorkflowsRequest and then it should automatically work. (And make sure future apis built should have a namespace parameter)

@alexec I think we should do option 2. WDYT ?

@BhavikaSharma A workaround that you can use is to give READ access to workflows for user-default-login. Since SSO is not delegated, the read access of user-default-login will be used.

[basanthjenuhb]

Other requests like RetryArchivedWorkflowRequest, ResubmitArchivedWorkflowRequest have a namespace parameter. Not sure why ListArchivedWorkflowsRequest was implemented differently.

[alexec]

So we're saying they cannot be supported. We should probabyl document that.

[basanthjenuhb]

We can support, just that the request needs to be modified to add a namespace parameter. For ListArchivedWorkflowsRequest and couple of others. Shall i make a PR with this approach ?

[alexec]

I don't think we should fix this. @juliev0 work will deprecate this endpoint. Users should move to use the next endpoint. We could put a 301 redirect in place

[jessesuen]

I don't think we should fix this. @juliev0 work will deprecate this endpoint. Users should move to use the next endpoint. We could put a 301 redirect in place

@alexec Why do we need a new endpoint to replace this one -- what would be so different about the Request signature that it needs a new endpoint vs. enhancing our current one?

[alexec]

The current endpoints don't container the namespace in the URL, so we cannot use GetNamespace to perform namespaced RBAC on them. The current endpoints don't support directories. I'd like to consolidate them into a single endpoint to reduce the amount of code, the amount of bugs, the amount of security problems.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is a mentoring request, please provide an update here. Thank you for your contributions.

[stale[bot]]

This issue has been closed due to inactivity. Feel free to re-open if you still encounter this issue.

[christophkreutzer]

@alexec @jessesuen @basanthjenuhb We are affected as well with our cluster-wide installation and RBAC Namespace Delegation. Could you please reopen the issue?

[hbrewster-splunk]

+1

[msuthar-splunk]

Can we please reopen this? This issue is impacting us as well.

[terrytangyuan]

I don't think we should fix this. @juliev0 work will deprecate this endpoint. Users should move to use the next endpoint. We could put a 301 redirect in place

@alexec What work are you referring to from @juliev0 that will deprecate the endpoint?

@basanthjenuhb Would you be interested in working on this issue?

[juliev0]

I don't think we should fix this. @juliev0 work will deprecate this endpoint. Users should move to use the next endpoint. We could put a 301 redirect in place

@alexec What work are you referring to from @juliev0 that will deprecate the endpoint?

@basanthjenuhb Would you be interested in working on this issue?

I guess Alex was referring to the fact that we now have this endpoint: mux.HandleFunc("/artifact-files/", artifactServer.GetArtifactFile)

in addition to the endpoints we had before:

mux.HandleFunc("/artifacts/", artifactServer.GetOutputArtifact)
mux.HandleFunc("/input-artifacts/", artifactServer.GetInputArtifact)
mux.HandleFunc("/artifacts-by-uid/", artifactServer.GetOutputArtifactByUID)
mux.HandleFunc("/input-artifacts-by-uid/", artifactServer.GetInputArtifactByUID)

I think he's thinking we'd replace those last 4 with the first once, since I believe it should be able to do the same thing. I'm not sure if this is a priority at the moment, though, although certainly reducing the code is always a good thing. Moreover, I don't know that the new endpoint would necessarily take care of this issue (would it @alexec?). If @basanthjenuhb is able to help with this issue it would be great.

[basanthjenuhb]

@juliev0 Is the new endpoint integrated with the UI ? Let me have a look at the new endpoint. It should be easily fixable.

[basanthjenuhb]

Also, generally issues are prioritized by 👍 count on the issue subject. This issue didn't have any, probably why it got deprioritized. Would request people to add a 👍 on the issue subject at the top. This would help maintainers to prioritize.

[juliev0]

@juliev0 Is the new endpoint integrated with the UI ? Let me have a look at the new endpoint. It should be easily fixable.

It is as far as the "artifact visualization" at least (being able to click on the artifact and view it). That's when the new endpoint was added (GED week).

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is a mentoring request, please provide an update here. Thank you for your contributions.

[christophkreutzer]

Still an issue, sorry bot :(