Arvados

Also need to support standard claims:

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

16171-oidc @ 15dbaf151a72d5cfc00b4ea4b4bcb64c7ed9ac14 -- developer-run-tests: #1881

Tom Clegg wrote:

16171-oidc @ 15dbaf151a72d5cfc00b4ea4b4bcb64c7ed9ac14 -- developer-run-tests: #1881

• UserAuthenticate & getAuthInfo -- holy single line argument list, Batman
• Needs a companion branch to enable in Workbench2
• We should support the "preferred_username" claim (https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims) -- I can double check that's what the customer uses to supply the unix username.

Peter Amstutz wrote:

• We should support the "preferred_username" claim (https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims) -- I can double check that's what the customer uses to supply the unix username.

Initial customer response:

"I think your assumption is not correct. I just checked how it looks in our current AzureAD config and there this field [preferred_username] doesn't even exist.

I would suggest that this is configurable with a reasonable default value. It's also what I was told so far that the claims in the JWT token are configurable."

• UserAuthenticate & getAuthInfo -- holy single line argument list, Batman

Moved more of the oauth2 config stuff into the setup func, and removed a couple of args from getAuthInfo.

In the process I noticed the config was using the Login.Google config section instead of ctrl.ClientID, so I fixed that and added a test.

That test revealed that the OIDC library is sensitive about trailing "/" in the issuer URL, so I added a bit to strip the "/" that our config loader adds. There are possible cases that can only be fixed in the oidc library by doing a more rigorous URL comparison ("https://example:443" is equivalent to "https://example/", etc) but in the meantime this handles the test fake and Google (which strip the trailing slash) and Azure Active Directory (which has a non-empty issuer path containing a tenant ID).

I'll note this in the config doc to help reduce operator annoyance about this. Not sure whether to prioritize submitting a patch upstream.

We could create a new URL type that preserves the absence of bare slash in "https://example". This should make it possible to use any endpoint no matter how it spells itself, as long as it uses the same spelling every time. It wouldn't be a full solution (it would still require the operator to use the exact same spelling as the issuer, so it would still be needlessly finicky and would still break if an issuer changes to an equivalent spelling/encoding). It seems like the proper solution is for go-oidc to do a real "equivalent URL" test instead of a string comparison. Haven't yet found a go pkg that does an rfc3986 equivalence test, though.

The Azure docs don't seem to use trailing slashes consistently. https://docs.microsoft.com/en-us/azure/active-directory/develop/howto-convert-app-to-be-multi-tenant:

A single tenant application normally takes an endpoint value like: https://login.microsoftonline.com/contoso.onmicrosoft.com
Each Azure AD tenant has a unique issuer value of the form: https://sts.windows.net/31537af4-6d77-4bb9-a681-d2394888ea26/

Hopefully the actual service is consistent with the docs in any given case.

16171-oidc @ cd3f543b2ea20a7ac5851c118d5189df080207f2 -- developer-run-tests: #1890

• Needs a companion branch to enable in Workbench2

I don't think Workbench2 actually does anything with this information, but I've updated the config struct/mock for completeness.

16171-oidc arvados-workbench2|1f4bc2074e41d1e6ec0f91d4a7d0e543020d523d

Tom Clegg wrote:

We could create a new URL type that preserves the absence of bare slash in "https://example". This should make it possible to use any endpoint no matter how it spells itself, as long as it uses the same spelling every time.

...or just use the string type:

16171-oidc @ 3b4bb3d393adc3bd3ddfb4442a65087275a5c5c3 -- developer-run-tests: #1891

Tom Clegg wrote:

Tom Clegg wrote:

We could create a new URL type that preserves the absence of bare slash in "https://example". This should make it possible to use any endpoint no matter how it spells itself, as long as it uses the same spelling every time.

...or just use the string type:

16171-oidc @ 3b4bb3d393adc3bd3ddfb4442a65087275a5c5c3 -- developer-run-tests: #1891

This LGTM.

• Needs a companion branch to enable in Workbench2

I don't think Workbench2 actually does anything with this information, but I've updated the config struct/mock for completeness.

16171-oidc arvados-workbench2|1f4bc2074e41d1e6ec0f91d4a7d0e543020d523d

Turns out this probably isn't necessary, the default behavior is log in via 3rd party:

const requirePasswordLogin = (config: Config): boolean => {
    if (config && config.clusterConfig) {
        return config.clusterConfig.Login.LDAP.Enable || config.clusterConfig.Login.PAM.Enable || false;
    }
    return false;
};

16171-oidc-config @ 11f80ed98b70be5379abe18f1b645ab3958d078b -- developer-run-tests: #1906

16171-oidc-config @ f4750d53482ddb3990426563bb424f72790b9090 -- developer-run-tests: #1914

with new configs EmailClaim, EmailVerifiedClaim, UsernameClaim

Some minor comments:

• Typo on comment at lib/config/generated_config.go line 589
• I think is worth mentioning these additional config knobs on the documentation, maybe by just saying that there’re more than the provided example and pointing to the config reference page, wdyt?

Other than that, it LGTM.