Arvados

developer-run-tests: #2096

• We don't get an expiry time from the access token. It's not clear (to me) whether the introspection API is likely to be supported/available, and the Go library doesn't provide support for it, so for now this code always treats the OIDC access token expiry as "unknown" and re-verifies after 5m.

Tom Clegg wrote:

16669-oidc-access-token @ 61d493d84cff707e291132847ac3e9792ae94aee -- developer-run-tests: #2096

• We don't get an expiry time from the access token. It's not clear (to me) whether the introspection API is likely to be supported/available, and the Go library doesn't provide support for it, so for now this code always treats the OIDC access token expiry as "unknown" and re-verifies after 5m.

PingFederate supports the introspection API, so ideally we should support it if available.

16669-oidc-access-token @ aa76fee75c0e96214d2ffc5ecbb1c8a06f70b309 -- developer-run-tests: #2112

developer-run-tests: #2117

• If LoginCluster is in use, when RailsAPI receives a non-v2 token that isn't [unexpired] in the local database, it asks LoginCluster's controller to verify it.
• If OIDC login is in use, when controller receives a non-v2 token that isn't [unexpired] in the local database, it asks the OIDC provider to verify it.
  • When controller accepts an OIDC access token, it is cached using hmac(systemroottoken,oidcaccesstoken) as the secret part (api_token column) in the local database.
• When controller's federation delegate/fan-out code needs to send a request to a remote cluster and it's working with a non-v2 token, it first checks whether the token belongs to a user account on the target cluster, and if so, it passes it through instead of making a salted token.

Question: should we only support a login flow that exchanges the OIDC access token for an Arvados token, instead of accepting the access token for every API call?

This is based on:

https://www.ory.sh/hydra/docs/concepts/before-oauth2

which among other things states

A common misconception is that access and refresh tokens represent a user session.
...
That is because both applications are built by different people and different companies, and the user behavior on one site does not reflect user intent on another. It would be crazy if some app could just log you out of GitHub, or if signing out of Google would invalidate your sessions on all the websites where you used "Sign in with Google".

However, after studying this some more, there's an important distinction: the OAuth2 case described on that page is for "peer" applications where one is granting limited access for one application to interact with another.

But for us, we are supporting the Single-Sign-On case. In that case, it does behave more like a session. When the user logs off from SSO, they should be logged off everywhere.

So I think the objections about using generic OAuth2 for sessions don't apply.

The result of OIDC login with Google is an access token that can access Google APIs (only the ones with granted scopes, though). So I think that answers the question "are access tokens supposed to be used as API tokens".

I rebased this branch on master since it was a month old. I force-pushed the rebase.

16669-oidc-access-token @ 64028c0cae469d3da33878a30bd40c5409e64641

Let me see if I understand how this works.

1. We receive a bare token
2. Check local controller cache, which can be a miss, possibly valid (check expiration), or probably invalid (recheck after expiration).
3. Hash the token and check if the hash exists in the database. If it present and still valid, return success.
4. Check that the token is valid from the OIDC provider
5. Create or update an arvados token record based on the information from the OIDC provider, and update the local cache.

Is this comment accurate? I don't think we're swapping anything, we're just ensuring that the Rails API server will accept the token?

// Check whether the token is a valid OIDC access token. If
// so, swap it out for an Arvados token (creating/updating an
// api_client_authorizations row if needed) which downstream
// server components will accept.

We should have configuration options GivenNameClaim (default "given_name") and FamilyNameClaim (default "family_name") for setting first_name / last_name. Right now we only get those from the Google People API, so the name will be blank for when using another provider.

Standard claims are here:

https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims

I wonder if we should add an option to check/enforce that an OIDC token has been issued with "arvados" scope (or whatever).

For testing: I ran the unit tests and they passed. I'd like to do some manual integration tests, but that is kind of a hassle to set up. Maybe once the other comments above are addressed, we go ahead and merge and then I can follow with some testing on the dev cluster.

Is this comment accurate? I don't think we're swapping anything, we're just ensuring that the Rails API server will accept the token?

Yes, oidcTokenAuthorizer ensures that if the token is (or was very recently) a valid OIDC access token, then RailsAPI will recognize it (by HMAC) as an Arvados token.

We should have configuration options GivenNameClaim ...

We could -- does someone need it? That feature wouldn't affect this branch though anyway, right?

I wonder if we should add an option to check/enforce that an OIDC token has been issued with "arvados" scope (or whatever).

That feature wouldn't affect this branch though, right?

I'd like to do some manual integration tests

Yes, I've also been looking forward to having this deployed on dev clusters. ;)

16669-oidc-access-token @ b8de8845c856f7fe1232e5f048824211d1207ee7 -- developer-run-tests: #2139

Tom Clegg wrote:

Is this comment accurate? I don't think we're swapping anything, we're just ensuring that the Rails API server will accept the token?

Yes, oidcTokenAuthorizer ensures that if the token is (or was very recently) a valid OIDC access token, then RailsAPI will recognize it (by HMAC) as an Arvados token.

We should have configuration options GivenNameClaim ...

We could -- does someone need it? That feature wouldn't affect this branch though anyway, right?

Sure, it doesn't have to be a blocker for an initial merge.

I don't know if GivenName needs to be configurable with a GivenNameClaim or if given_name/family_name are actually used consistently across implementations. But we definitely need it because someone using generic OIDC for login wouldn't have correct display names otherwise.

I wonder if we should add an option to check/enforce that an OIDC token has been issued with "arvados" scope (or whatever).

That feature wouldn't affect this branch though, right?

Sure.

I'd like to do some manual integration tests

Yes, I've also been looking forward to having this deployed on dev clusters. ;)

16669-oidc-access-token @ b8de8845c856f7fe1232e5f048824211d1207ee7 -- developer-run-tests: #2139

Ok, please merge but let's keep the ticket open for follow-up.

Peter Amstutz wrote:

I don't know if GivenName needs to be configurable with a GivenNameClaim or if given_name/family_name are actually used consistently across implementations. But we definitely need it because someone using generic OIDC for login wouldn't have correct display names otherwise.

Added #17037

s3cmd doesn't like the error that we're returning, I think we're returning a bare string like "signature verification failed: invalid access key" but it is expect some kind of XML structure.

The 404 response has the same issue (a bare string "not found" instead of the XML structure it is expecting).

This ended up being very helpful in testing: https://github.com/ajanthan/cmdline-openid-client

For the most part, this worked on the first try, which impressive for such a complex feature.

Things that worked:

Got an access token from Google with the "openid profile email" scopes.

1. Exported the access token as ARVADOS_API_TOKEN.
2. Was able to do "arv user current" on ce8i5, and got the correct user.
3. Was able to do "arv user current" on tordo (which has ce8i5 as its LoginCluster). This returned the correct user.
4. Was able to use "s3cmd" to access collections on both ce8i5 and tordo by setting the access token on both access_key/secret_key, and changing host_base to each respective cluster, and requesting a collection hosted on that cluster.

Didn't work:

• Making a federated query, requesting a collection from tordo while connected to ce8i5 via the S3 API. I haven't tried this with a regular Arvados token.
• After 5 minutes, it expired. I'm pretty sure the access token itself is still valid, but it seems like Arvados is not re-checking the token, I'm getting 401 Unauthorized.

This LGTM.

developer-run-tests: #2361

• fix & test for using OIDC tokens in federated calls
• test case has all clusters using the same LoginCluster
• should also work with different LoginClusters, but only if the OIDC providers all recognize one another's access tokens, which practically speaking probably means using the same OIDC Issuer, ClientID, and ClientSecret.

Tom Clegg wrote:

16669-oidc-access-token-fed @ 54d8c4e41a276ac82c79506f63907a108ebd9bfd -- developer-run-tests: #2361

• fix & test for using OIDC tokens in federated calls
• test case has all clusters using the same LoginCluster
• should also work with different LoginClusters, but only if the OIDC providers all recognize one another's access tokens, which practically speaking probably means using the same OIDC Issuer, ClientID, and ClientSecret.

LGTM