Integrate Dex Auth with Multi User Story

[krishnadurai]

Current Dex implementation needs to integrate well with Multi-User Story in the following areas:

• User RBAC is generated as per the roles created through ProfilesController
• Ingress Auth is enabled for Dex based Auth

Reference issue/PR:
#195

/cc @jlewi @kkasravi

/assign @krishnadurai

[jlewi]

@krishnadurai Can you please add

• Priority for this issue
• Area for this issue
• A kind (bug, enhancement etc...)
• Any projects that this should belong to

[jlewi]

I think one of the issues that came up in PR #195 was reconciling the default RBAC and ISTIO RBAC roles and bindings created in #195 with the roles and bindings created by the profile custom resource

[krishnadurai]

/priority p1
/area cuj/multiuser
/kind enhancement

[jlewi]

@krishnadurai What are the next steps here? I believe the existing_arrikto.yaml config is already using Dex and ISTIO.

I think one question that came up is can we generalize what's used in existing_arrikto.yaml to make it easier to compose the Dex setup with installations? (see #3739).

For GCP we would like to get rid of our custom basic auth setup and just install Dex with an appropriate authenticator.

Another question that has come up is where is the source code for the OIDC connector used in kubeflow_existing_arrikto.yaml? I believe the config is currently referencing a container in an Arrikto GCR container.

How does key keycloak-gatekeeper interact with Dex? Is keycloak trying to do the same thing Dex is doing by creating a proxy for OpenID providers?

/cc @yanniszark

[krishnadurai]

@jlewi keycloak-gatekeeper is being used as an authentication proxy for Kubeflow like what IAP does for GCP IAP config. This will also be used to add additional headers required for multi-user in the future. For now, I've been able to make this proxy work on the web UI for Authentication (login/logout) flow. I'm finalizing it to work with kfctl and HTTPS. Soon Multi-User support will follow.

How does key keycloak-gatekeeper interact with Dex? Is keycloak trying to do the same thing
Dex is doing by creating a proxy for OpenID providers?

keycloak-gatekeeper acts as a client application to Dex, which means - if it doesn't find the user session information - it will interact with Dex (making the user login) and set the appropriate session headers or cookie information for its endpoint. Here, Dex acts as a unifying connector to all these identification mechanisms and not a proxy.
keycloak-gatekeeper's endpoint acts as a proxy to Kubeflow's web UIs. This is much like basic HTTP Password for Nginx or Apache (PasswordBasicAuth).
AFAIK, Dex does not provide an in-built auth proxy which is mature - correct me if I'm wrong here.

For GCP we would like to get rid of our custom basic auth setup and just install Dex with an appropriate authenticator.

We should hopefully be able to get there within 0.7.

I think one question that came up is can we generalize what's used in existing_arrikto.yaml to make it easier to compose the Dex setup with installations? (see #3739).

We should go on to implement the cert creation as mentioned in kubeflow/kubeflow#3739 as a part of kfctl. Though I believe LoadBalancer configuration should be made configurable through istio overlays and platform based installations. WDYT?
/cc @yanniszark

[krishnadurai]

Refer discussion here: https://kubeflow.slack.com/archives/C7REE0EHK/p1568033731165800

[jlewi]

@krishnadurai Do you want to add a link your doc?

[krishnadurai]

Here's the doc which describes the approaches to have an Auth Proxy/OIDC flow with Dex: http://bit.ly/2kxbgbE

[krishnadurai]

@jlewi @yanniszark @quanjielin I think one way of moving ahead is to define what solution we should pursue for 0.7 and then if required make changes for 1.0, since we have limited time for the release.
If we can make a decision for this early this week we should be able to implement and test this for 0.7.

[krishnadurai]

@jlewi @yanniszark @quanjielin can we please move ahead on this?

[yanniszark]

@krishnadurai what is the decision from the aforementioned doc?
I believe that any decision we make should satisfy all user requirements mentioned in the doc.
I'm also copying them below for clarity.

We have confirmed that the solution using the OIDC AuthService satisfies those requirements.
I am currently refactoring the existing_arrikto config to satisfy those requirements and they will be available for other platforms to use as well (eg GCP Basic Auth).

===================================

User Requirements

A lot of users of existing_arrikto have made the following requests. These features are important to users operating in on-prem, airgapped environments:

• Remove the need for a LoadBalancer and make it work with NodePort/port-forward.
• Stop using port 5556 for Dex and instead use the same port as Kubeflow (443) under a certain path (/dex).
• Don't make requests that exit and reenter the cluster, as they cause problems in airgapped environments.

To solve these we can:

• Use internal URLs (dex.kubeflow.cluster.local) for discovery, token exchange and jwks validation.
• Use relative URLs for redirecting the user to Dex and back to Kubeflow (authorization_endpoint, redirect_uri). This requires using the same port (443) and we don't need to know the public address of Dex beforehand.

===================================

I believe that Keycloak Gatekeeper currently has no way to satisfy this requirement:

• Don't make requests that exit and reenter the cluster, as they cause problems in airgapped environments.

[krishnadurai]

@yanniszark As discussed in the on-prem meeting - we should go ahead with Ambassador OIDC and let us add it to the dex-auth manifest. I'll remove keycloak-gatekeeper as soon as Ambassador OIDC Authservice is in.

@jlewi can we look to include the Ambassador OIDC repository in Kubeflow?

https://github.com/arrikto/ambassador-auth-oidc

[jlewi]

@krishnadurai Is this #1 or #2 in your design doc?

#2 Seems better to me because not every request goes through the AuthService; only ones which don't have credentials

[krishnadurai]

It is the second approach.

With this approach, JWT checking is done once only by the AuthService instead of Istio. AuthService provides the added facility to start the OIDC login flow if credentials are missing or incorrect.

[jlewi]

Great. Can we capture in the doc the decision and also reference this issue if not already doing so.

[krishnadurai]

Sure, I'll just make a note of it there.

I've made kubeflow/kubeflow#4160 to track adding Ambassador OIDC repository in Kubeflow organisation.

[krishnadurai]

Increasing this issue's priority for making it a part of manifests for 0.7 release.

/priority p0

[yanniszark]

An update on this:

We are integrating a rewrite of the ajmyyra/ambassador-auth-oidc project which features better code structure and support for sessions instead of JWT blacklists (which is safer since if you lose the blacklist JWTs may become active again).

The new code lives in arrikto/oidc-authservice and will use the same license as the old one (MIT).

[krishnadurai]

/reopen

[k8s-ci-robot]

@krishnadurai: Reopened this issue.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[krishnadurai]

@yanniszark I think it would be better to keep this issue open until the entire setup is working in a seamless manner.

There are a few things left in that note:

1. Have the endpoint work through HTTPS rather than HTTP. Adds self sigining cert to dex-ingress #362 aims to address the certificate issuing part.
2. Change ingress gateway kubeflow-gateway to work through 443 instead of 80. (Configures Istio Gateway to serve through port 443 for HTTPS #562 addresses this)
3. Verify if this setup works through NodePort as well - this does not seem to work now. Verified working with NodePort.
4. Configuration for LoadBalancer through this implementation.

Refer to the earlier comment on the PR which closed this issue: #529 (review)

@jlewi I suggest we add this to the 0.7 tracker.

[jlewi]

What about docs and testing for this?

[krishnadurai]

@jlewi as discussed the docs and testing are pending and will be created soon.

[krishnadurai]

Using #599 to track this.

/close

[k8s-ci-robot]

@krishnadurai: Closing this issue.

In response to this:

Using #599 to track this.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.