Add a default public-url config value
#87
Conversation
This change simplifies the setup for most (all?) OIDC+dex setups. The combination of changes below result in OIDC/Dex communicating internally in the kubernetes cluster, and the user being redirected to a relative link of `{whatever_this_kubeflows_hostname_is}/dex/auth`.
Details:
* adds the OIDC_AUTH_URL env var to OIDC, which lets OIDC redirect the user to dex via a relative link rather than the absolute OIDC_PROVIDER link
* sets public-url (and thus OIDC_PROVIDER) to 'http://dex-auth.kubeflow.svc.cluster.local:5556' by default. Also adds special handling for k8s-internal urls since we typically append `/dex` to the public url for OIDC's inputs
* Adds `AFTER_LOGIN_URL` to redirect all logins to the dashboard. This solves some corner cases where getting redirected to a login from a login page makes it seem to the user like they're in a login redirect loop even though they aren't.
* Removes the `AFTER_LOGOUT_URL` which, if left out, defaults to a working value when the other env variables discussed above are set
This change simplifies the setup for most (all?) OIDC+dex setups. The combination of changes below result in OIDC/Dex communicating internally in the kubernetes cluster, and the user being redirected to a relative link of `{whatever_this_kubeflows_hostname_is}/dex/auth`.
Details:
* adds the OIDC_AUTH_URL env var to OIDC, which lets OIDC redirect the user to dex via a relative link rather than the absolute OIDC_PROVIDER link
* sets public-url (and thus OIDC_PROVIDER) to 'http://dex-auth.kubeflow.svc.cluster.local:5556' by default. Also adds special handling for k8s-internal urls since we typically append `/dex` to the public url for OIDC's inputs
* Adds `AFTER_LOGIN_URL` to redirect all logins to the dashboard. This solves some corner cases where getting redirected to a login from a login page makes it seem to the user like they're in a login redirect loop even though they aren't.
* Removes the `AFTER_LOGOUT_URL` which, if left out, defaults to a working value when the other env variables discussed above are set
| if not self.public_url.endswith("/dex"): | ||
| self.public_url += "/dex" |
There was a problem hiding this comment.
does that mean that we can work only with dex then?
or was it always the case?
There was a problem hiding this comment.
we've only ever tried to work with dex, but you're right this really codifies that. Maybe there's a better way to maintain this default/backward compatibility but not be so rigid.
Practically speaking it might not be worth the effort, depending on when we plan to replace this with the identity team's tooling
| @@ -20,8 +20,10 @@ options: | |||
| description: OpenID Connect client secret | |||
| public-url: | |||
| type: string | |||
| default: '' | |||
| description: Publicly-accessible endpoint for cluster | |||
| default: 'http://dex-auth.kubeflow.svc.cluster.local:5556' | |||
There was a problem hiding this comment.
Can this be computed a bit more smartly? This assumes that the deployed application is named 'dex-auth' (and that the model name is 'kubeflow')
Could we not use lightkube to inspect the address of the loadbalancer service for istio ingress and set this automatically to the correct x.x.x.x.nip.io address?
While this is a more useful default than nothing, in reality people will still need to adjust it to access externally, no?
There was a problem hiding this comment.
(I couldn't decide whether to leave this comment here or on the other PR for dex-auth but it sort of applies to both)
There was a problem hiding this comment.
@jnsgruk I think there's a misunderstanding here, which is caused because public-url is a misleading name.
This value configures the URL of the OIDC provider. How this URL will be used depends on the login flow that will be used (i.e. normal OIDC flow, or Authorization Code OIDC flow). In one case it needs to be public (users go there to login) and in another it can be private (used by AuthService to talk to OIDC provider (Dex) and can be an in-cluster K8s Service URL).
To summarize: We need to update the name/desc of this config and expose how it's used. Because it doesn't always have to be a public-url, and thus the current name is misleading.
There was a problem hiding this comment.
coming back to this since it recently became relevant again...
@jnsgruk's point is still correct about how this hard-codes the oidc provider (dex-auth) and namespace (kubeflow). These are good guesses for our setup (the model must be kubeflow atm due to limitations upstream, but the oidc provider might be configurable) but not bulletproof. Although since it is in a config, on the off chance someone needs to change it they can
There was a problem hiding this comment.
What we could do instead of defaulting to http://dex-auth.kubeflow.svc.cluster.local:5556 is document the format this config option should have http://<oidc-provider-domain-name>:<port(?)>, change the name of the config option to oidc-issuer-url or something similar.
In the charm code, we could have a new integration between dex-auth and oidc-gatekeepr where the latter constructs the oidc-issuer-url from the dex-auth Service in the absence of the config option.
There was a problem hiding this comment.
After speaking with @kimwnasptd about this config option, we may not even need it at all. In reality what happens is that there will always be a connection between oidc-gatekeeper and dex-auth, if there is any other identity provider, it has to connect through dex, so it always looks like oidc-gatekeeper <-> dex <-> id provider.
Perhaps the best approach here is to just let the oidc-gatekeeper to construct the issuer-url from the service name and port that dex-auth can share through relation data.
|
Thanks @ca-scribner, this work will be superseded by
Closing this PR as it is not needed any more. |
This adds a default
public-urlconfig value as described in canonical/bundle-kubeflow#608. After implementing canonical/bundle-kubeflow#608 as a whole, we remove the need for users to modifypublic-url.Also implemented here are other config values that are required to make the new public-url default work properly.
While we could deprecate and remove the
public-urlconfig, it is recommended we leave it as is with this default. This is because in the medium term we plan on replacing dex+oidc with a different auth setup.