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.

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

Support auth without refresh tokens #775

Closed
droctothorpe opened this issue Jul 20, 2023 · 7 comments
Closed

Support auth without refresh tokens #775

droctothorpe opened this issue Jul 20, 2023 · 7 comments

Comments

@droctothorpe
Copy link
Contributor

Not all cluster auth providers support refresh tokens. KubeCluster fails to instantiate without one out of cluster when parsing the Kubernetes configuration file. It would be helpful if there was a refresh_token flag or something to that effect in either KubeCluster or one of the auth plugins it accepts. Happy to contribute.

@jacobtomlinson
Copy link
Member

Could you share a small example of this happening?

@jacobtomlinson jacobtomlinson added the needs info Needs further information from the user label Jul 24, 2023
@droctothorpe
Copy link
Contributor Author

Here's what the user section of my ~/.kube/config file looks like:

users:
- name: <obfuscated>
  user:
    auth-provider:
      config:
        client-id: <obfuscated>
        client-secret: <obfuscated>
        extra-scopes: <obfuscated>
        id-token: <obfuscated>
        idp-issuer-url: <obfuscated>
      name: <obfuscated>

In particular, please note the absence of a refresh-token field. operator.KubeClient assumes that field is there, and raises an exception when it doesn't find it. Not all cluster admins enable refresh tokens (case in point). It would be helpful if KubeClient worked without one.

@jacobtomlinson
Copy link
Member

We are slowly transitioning dask-kubernetes to use kr8s so any enhancements to authentication should be made there. We haven't done this in dask_kubernetes.operator.KubeCluster yet, only in the controller for now, but will do it soon.

You've obfuscated the name of the auth-provider from your config. Can I just check that it is oidc? All other in-tree authentication methods have been deprecated as of Kubernetes 1.22 so I didn't bother to implement them in kr8s.

I haven't implemented OIDC in kr8s yet but will do it before we transition KubeCluster over to it. kr8s-org/kr8s#125

Now is a good opportunity to influence the design of that implementation so knowing that refresh-token can be optional is useful thanks! Would you mind commenting on the issue in kr8s with some information on what you would expect to happen in the case where the credentials have expired and no refresh token is provided? Just raise a 403 style exception? I assume in your setup you have to do some manual steps to generate new credentials?

@jacobtomlinson jacobtomlinson added bug operator and removed needs info Needs further information from the user labels Jul 25, 2023
@droctothorpe
Copy link
Contributor Author

We use OIDC. The tokens are relatively long-lived and we provide an out of band auth CLI for when they expire. I'm not sure how unique our setup is, so just to clarify, I have no sense of entitlement around supporting what might be an unusual pattern. I'll comment on the issue but I'm also going to look into enabling refresh tokens on our end since that's probably the path of least resistance, especially given the transition to kr8s. Thanks, @jacobtomlinson!

@droctothorpe
Copy link
Contributor Author

Just found out that our refresh tokens are prohibited by design for security reasons. This will prevent us from using the SDK with the operator, unfortunately.

@jacobtomlinson
Copy link
Member

Don't worry I don't think you're coming across as entitled at all.

We will definitely implement OIDC (and make sure it handles cases without refresh tokens) in kr8s before switching it out here. The goal is to leave things better than before.

If you have any interest in contributing to kr8s and helping implement OIDC auth that would be 🔥. But it's also on my todo list.

@jacobtomlinson
Copy link
Member

Ok, it turns out adding basic OIDC support to kr8s without token refreshing was very simple. kr8s-org/kr8s#126

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants