search

sky-uk / osprey

Kubernetes OIDC CLI login
BSD 3-Clause "New" or "Revised" License
50 stars 18 forks source link

Return a custom error when a token doesn't contain a groups claim. #104

Closed yku04 closed 4 months ago

yku04 commented 4 months ago

Return a custom error when a token doesn't contain a groups claim. That would mean the user is a member of more than 200 groups.

PR for #8494

aecay commented 4 months ago

This change targets the osprey server, which is an (afaik unused) way of getting tokens directly from the target cluster. We use the osprey client's oidc code to get the tokens from Azure AD (no cluster involved in that part of the dance). So I think you need to hook the validation in here instead: https://github.com/sky-uk/osprey/blob/076adef8aedea60f748a67c17835f2d62ed1b076/client/oidc/oidc.go#L121-L130 (or possibly somewhere up the call stack from there, such as here: https://github.com/sky-uk/osprey/blob/076adef8aedea60f748a67c17835f2d62ed1b076/client/azure.go#L152)

IOW, all our currently existing clusters use the client/azure.go implementation of the Retriever interface, not the client/osprey.go one. (This is incredibly confusing and even I'm not sure I've got it 100% correct....)

yku04 commented 4 months ago

This change targets the osprey server, which is an (afaik unused) way of getting tokens directly from the target cluster. We use the osprey client's oidc code to get the tokens from Azure AD (no cluster involved in that part of the dance). So I think you need to hook the validation in here instead:

So that's why I couldn't find where it gets served from. Osprey servers in the clusters work only in the cluster-info mode. And tokens are fetched on the client side

aecay commented 4 months ago

(I also think you should wait for another engineer to approve the PR before merging. I actually didn't realize I still had approver rights in this repo!)

aecay commented 4 months ago

Looks fine after the latest changes -- will let another engineer press ✅