Add support for Epinio's Dex Auth Provider #9
Conversation
|
@nwmac This is the backend work for dex, i'm sure there's some golang smells here and possible the way i've integrated dex
|
| log.Infof("Epinio API url: '%s'. Epinio WSS url: '%s'. Skipping SSL Validation: '%+v'", epinioApiUrlValue, epinioApiWsUrlValue, epinioApiUrlskipSSLValidation) | ||
| epinioAuthUrlValue, _ := portalProxy.Env().Lookup(epinioDexAuthUrl) | ||
| if len(epinioAuthUrlValue) == 0 { | ||
| epinioAuthUrlValue = strings.Replace(epinioApiUrlValue, "epinio.", "auth.", 1) |
There was a problem hiding this comment.
I think this is wrong. IIUC it doesn't replace the internal epinio-server prefix but the namespace epinio value instead. So in the end the URL is wrong http://epinio-server.auth.svc.cluster.local/.
There was a problem hiding this comment.
This was an optimisation for the external scenario.
What's the correct internal url (as in, where does the auth.<domain> external url point to), something like...?
http://dex.epinio.svc.cluster.local
There was a problem hiding this comment.
yes http://dex.epinio.svc.cluster.local should be correct but it's not listening on port 80 but on 5556 and 5558.
Ingress for auth.<domain> points to its port 5556.
…redirect shim - Rancher API / local dashboard builds redirect `verify-auth` to `auth/verify` - In Rancher API this handles redirecting to ember or vue - Without these the user just gets redirected back to log in page - For our use case we can just redirect the user to the epinio ui
- Fix stripping of query params on redirect to /auth/verify - Add support to disable Dex - Customise client secret
| ) | ||
| } | ||
|
|
||
| dexUrl, _ := oidcProvider.AuthCodeURLWithPKCE(state) |
There was a problem hiding this comment.
I have the feeling that you should not use the PKCE flow, since you have a secret and it's a server-to-server request. You can probably use directly the oidcProvider.Config.AuthCodeURL(state)
https://dexidp.io/docs/using-dex/
The PKCE flow is needed when you cannot embed a secret safely (in our cli for example, or in a mobile application). If it works you can drop a lot of unneeded files.
There was a problem hiding this comment.
Ah, yeah. The secret wasn't that safe before but now it is.
Is there any inherent downside to leaving this in for the moment?
There was a problem hiding this comment.
The PKCE is a bit less secure I guess. It's not because of it, but just because usually temporary means forever.
Do you think is possible to do a quick change and test? If it's complicated we can try to revisit in a second quick round. We could maybe do this by ourself, and your review.
There was a problem hiding this comment.
Hum, having a look here maybe it's even better to have both. So let's keep it like this!
Contributes to epinio/ui#167, epinio/ui#176
Related PRs rancher/dashboard#7299, epinio/helm-charts#295