argo-workflows icon indicating copy to clipboard operation
argo-workflows copied to clipboard

Support Large Bearer token for SSO authentication

Open sarabala1979 opened this issue 3 years ago • 8 comments

If the user has part of a lot of groups. The bearer token is failed to be set as a cookie because it is crossing its size limit.

image

Checklist

  • [ ] Double-checked my configuration.
  • [ ] Tested using :latest images.
  • [ ] Used the Emissary executor.

Summary

What happened/what you expected to happen?

What version are you running?

Diagnostics

Paste the smallest workflow that reproduces the bug. We must be able to run the workflow.


# Logs from the workflow controller:
kubectl logs -n argo deploy/workflow-controller | grep ${workflow} 

# If the workflow's pods have not been created, you can skip the rest of the diagnostics.

# The workflow's pods that are problematic:
kubectl get pod -o yaml -l workflows.argoproj.io/workflow=${workflow},workflow.argoproj.io/phase!=Succeeded

# Logs from in your workflow's wait container, something like:
kubectl logs -c wait -l workflows.argoproj.io/workflow=${workflow},workflow.argoproj.io/phase!=Succeeded

Message from the maintainers:

Impacted by this bug? Give it a šŸ‘. We prioritise the issues with the most šŸ‘.

sarabala1979 avatar Sep 06 '22 15:09 sarabala1979

@alexec @basanthjenuhb how can we address this issue? Can we split two cookies if the token size is more than 4096?

sarabala1979 avatar Sep 06 '22 15:09 sarabala1979

Cookie is compressed already. Browsers only allow 4k of cookie in total. I.e. you cannot split the cookie.

Work-around - user should remove some groups.

The only solution is to store the JWT in a cache (e.g. Redis key-value store) and the cookie is just the key within that store. That is XXL work.

alexec avatar Sep 06 '22 15:09 alexec

I will change to enhancement.

sarabala1979 avatar Sep 06 '22 15:09 sarabala1979

@alexec can we store in a K8s secret object. it will be just another key in the secret

basanthjenuhb avatar Sep 08 '22 03:09 basanthjenuhb

Bala and I saw today that a commit to sso.go copied the entire claimns into the argo issued JWE. This will >=2x the size of the JWE:

https://github.com/argoproj/argo-workflows/blob/9d66b69f0bca92d7ef0c9aa67e87a2e334797530/server/auth/sso/sso.go#L280

This was added in #6444 by @liafizan .

@liafizan I don't think we need this copy?

alexec avatar Sep 09 '22 00:09 alexec

@alexec I think we understood this requirement based on this comment. I don't think we need this since it is meant to support edge cases but I think when we have those cases, we can add explicit support for those

liafizan avatar Sep 09 '22 17:09 liafizan

I get that. I’m just not sure we need to populate it on this specific line of code. The groups are copied into the Group field.

alexec avatar Sep 12 '22 02:09 alexec

Yes. Think we can leave it out

liafizan avatar Sep 16 '22 00:09 liafizan

Hi looks like I discovered the same thing in https://github.com/argoproj/argo-workflows/issues/10153 as well.

I wonder what's the best fix here? Should we by default not serialize RawClaim and add a flag to enable it?

kolorful avatar Dec 02 '22 19:12 kolorful

@kolorful see my comment on Sep 8. Would you like to submit a PR?

alexec avatar Dec 03 '22 15:12 alexec

@alexec sure thing, I created https://github.com/argoproj/argo-workflows/pull/10170

kolorful avatar Dec 03 '22 18:12 kolorful