From: Tom Clegg Date: Fri, 21 May 2021 18:47:09 +0000 (-0400) Subject: 17704: Split access token scope config into 2 keys. X-Git-Tag: 2.2.0~14^2 X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/e0b63c68db6c398aeb7a5820ac0ff5553d33bb40 17704: Split access token scope config into 2 keys. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- diff --git a/doc/api/tokens.html.textile.liquid b/doc/api/tokens.html.textile.liquid index 35d161f7cd..0935f9ba1d 100644 --- a/doc/api/tokens.html.textile.liquid +++ b/doc/api/tokens.html.textile.liquid @@ -34,10 +34,10 @@ h3. Direct username/password authentication h3. Using an OpenID Connect access token -A cluster that uses OpenID Connect as a login provider can be configured to accept OIDC access tokens as well as Arvados API tokens (this is disabled by default; see @Login.OpenIDConnect.AcceptAccessTokenScope@ in the "default config.yml file":{{site.baseurl}}/admin/config.html). +A cluster that uses OpenID Connect as a login provider can be configured to accept OIDC access tokens as well as Arvados API tokens (this is disabled by default; see @Login.OpenIDConnect.AcceptAccessToken@ in the "default config.yml file":{{site.baseurl}}/admin/config.html). # The client obtains an access token from the OpenID Connect provider via some method outside of Arvados. # The client presents the access token with an Arvados API request (e.g., request header @Authorization: Bearer xxxxaccesstokenxxxx@). -# Depending on configuration, the API server decodes the access token (which must be a signed JWT) and confirms that it includes the required scope. +# Depending on configuration, the API server decodes the access token (which must be a signed JWT) and confirms that it includes the required scope (see @Login.OpenIDConnect.AcceptAccessTokenScope@ in the "default config.yml file":{{site.baseurl}}/admin/config.html). # The API server uses the provider's UserInfo endpoint to validate the presented token. # If the token is valid, it is cached in the Arvados database and accepted in subsequent API calls for the next 10 minutes. diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml index d00c7d9ade..8ad2cb53fc 100644 --- a/lib/config/config.default.yml +++ b/lib/config/config.default.yml @@ -633,15 +633,21 @@ Clusters: AuthenticationRequestParameters: SAMPLE: "" - # Accept an OIDC access token as an API token if it is a JWT - # whose "scope" value includes this scope. To accept any - # access token (even if it's not a JWT), use "*". To disable - # this feature, use the empty string "". + # Accept an OIDC access token as an API token if the OIDC + # provider's UserInfo endpoint accepts it. # - # If an incoming token's scope is satisfactory, Arvados - # verifies the token is valid by presenting it at the OIDC - # provider's UserInfo endpoint. (Signature and expiry are not - # checked separately.) Valid tokens are cached for 10 minutes. + # AcceptAccessTokenScope should also be used when enabling + # this feature. + AcceptAccessToken: false + + # Before accepting an OIDC access token as an API token, first + # check that it is a JWT whose "scope" value includes this + # value. Example: "https://zzzzz.example.com/" (your Arvados + # API endpoint). + # + # If this value is empty and AcceptAccessToken is true, all + # access tokens will be accepted regardless of scope, + # including non-JWT tokens. This is not recommended. AcceptAccessTokenScope: "" PAM: diff --git a/lib/config/export.go b/lib/config/export.go index cdefc0b083..890d4ce471 100644 --- a/lib/config/export.go +++ b/lib/config/export.go @@ -157,6 +157,7 @@ var whitelist = map[string]bool{ "Login.LDAP.UsernameAttribute": false, "Login.LoginCluster": true, "Login.OpenIDConnect": true, + "Login.OpenIDConnect.AcceptAccessToken": false, "Login.OpenIDConnect.AcceptAccessTokenScope": false, "Login.OpenIDConnect.AuthenticationRequestParameters": false, "Login.OpenIDConnect.ClientID": false, diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go index 4f1cd2e7d2..9e59f8c923 100644 --- a/lib/config/generated_config.go +++ b/lib/config/generated_config.go @@ -639,15 +639,21 @@ Clusters: AuthenticationRequestParameters: SAMPLE: "" - # Accept an OIDC access token as an API token if it is a JWT - # whose "scope" value includes this scope. To accept any - # access token (even if it's not a JWT), use "*". To disable - # this feature, use the empty string "". + # Accept an OIDC access token as an API token if the OIDC + # provider's UserInfo endpoint accepts it. # - # If an incoming token's scope is satisfactory, Arvados - # verifies the token is valid by presenting it at the OIDC - # provider's UserInfo endpoint. (Signature and expiry are not - # checked separately.) Valid tokens are cached for 10 minutes. + # AcceptAccessTokenScope should also be used when enabling + # this feature. + AcceptAccessToken: false + + # Before accepting an OIDC access token as an API token, first + # check that it is a JWT whose "scope" value includes this + # value. Example: "https://zzzzz.example.com/" (your Arvados + # API endpoint). + # + # If this value is empty and AcceptAccessToken is true, all + # access tokens will be accepted regardless of scope, + # including non-JWT tokens. This is not recommended. AcceptAccessTokenScope: "" PAM: diff --git a/lib/controller/auth_test.go b/lib/controller/auth_test.go index ee267ba5dd..69458655ba 100644 --- a/lib/controller/auth_test.go +++ b/lib/controller/auth_test.go @@ -94,7 +94,8 @@ func (s *AuthSuite) SetUpTest(c *check.C) { cluster.Login.OpenIDConnect.ClientSecret = s.fakeProvider.ValidClientSecret cluster.Login.OpenIDConnect.EmailClaim = "email" cluster.Login.OpenIDConnect.EmailVerifiedClaim = "email_verified" - cluster.Login.OpenIDConnect.AcceptAccessTokenScope = "*" + cluster.Login.OpenIDConnect.AcceptAccessToken = true + cluster.Login.OpenIDConnect.AcceptAccessTokenScope = "" s.testHandler = &Handler{Cluster: cluster} s.testServer = newServerFromIntegrationTestEnv(c) diff --git a/lib/controller/integration_test.go b/lib/controller/integration_test.go index 51748126d0..44c99bf30f 100644 --- a/lib/controller/integration_test.go +++ b/lib/controller/integration_test.go @@ -113,7 +113,8 @@ func (s *IntegrationSuite) SetUpSuite(c *check.C) { ClientSecret: ` + s.oidcprovider.ValidClientSecret + ` EmailClaim: email EmailVerifiedClaim: email_verified - AcceptAccessTokenScope: "*" + AcceptAccessToken: true + AcceptAccessTokenScope: "" ` } else { yaml += ` diff --git a/lib/controller/localdb/login.go b/lib/controller/localdb/login.go index 47af553b6f..0d6f2ef027 100644 --- a/lib/controller/localdb/login.go +++ b/lib/controller/localdb/login.go @@ -63,6 +63,7 @@ func chooseLoginController(cluster *arvados.Cluster, parent *Conn) loginControll EmailClaim: cluster.Login.OpenIDConnect.EmailClaim, EmailVerifiedClaim: cluster.Login.OpenIDConnect.EmailVerifiedClaim, UsernameClaim: cluster.Login.OpenIDConnect.UsernameClaim, + AcceptAccessToken: cluster.Login.OpenIDConnect.AcceptAccessToken, AcceptAccessTokenScope: cluster.Login.OpenIDConnect.AcceptAccessTokenScope, } case wantSSO: diff --git a/lib/controller/localdb/login_oidc.go b/lib/controller/localdb/login_oidc.go index 6ca53bf184..61dc5c816b 100644 --- a/lib/controller/localdb/login_oidc.go +++ b/lib/controller/localdb/login_oidc.go @@ -55,7 +55,8 @@ type oidcLoginController struct { EmailClaim string // OpenID claim to use as email address; typically "email" EmailVerifiedClaim string // If non-empty, ensure claim value is true before accepting EmailClaim; typically "email_verified" UsernameClaim string // If non-empty, use as preferred username - AcceptAccessTokenScope string // If non-empty, accept any access token containing this scope as an API token + AcceptAccessToken bool // Accept access tokens as API tokens + AcceptAccessTokenScope string // If non-empty, don't accept access tokens as API tokens unless they contain this scope AuthParams map[string]string // Additional parameters to pass with authentication request // override Google People API base URL for testing purposes @@ -507,15 +508,16 @@ func (ta *oidcTokenAuthorizer) registerToken(ctx context.Context, tok string) er // return a 403 error, otherwise true (acceptable as an API token) or // false (pass through unmodified). // +// Return false if configured not to accept access tokens at all. +// // Note we don't check signature or expiry here. We are relying on the // caller to verify those separately (e.g., by calling the UserInfo // endpoint). func (ta *oidcTokenAuthorizer) checkAccessTokenScope(ctx context.Context, tok string) (bool, error) { - switch ta.ctrl.AcceptAccessTokenScope { - case "*": - return true, nil - case "": + if !ta.ctrl.AcceptAccessToken { return false, nil + } else if ta.ctrl.AcceptAccessTokenScope == "" { + return true, nil } var claims struct { Scope string `json:"scope"` diff --git a/lib/controller/localdb/login_oidc_test.go b/lib/controller/localdb/login_oidc_test.go index 3a345d7dc3..c9d6133c48 100644 --- a/lib/controller/localdb/login_oidc_test.go +++ b/lib/controller/localdb/login_oidc_test.go @@ -208,7 +208,8 @@ func (s *OIDCLoginSuite) TestOIDCAuthorizer(c *check.C) { json.Unmarshal([]byte(fmt.Sprintf("%q", s.fakeProvider.Issuer.URL)), &s.cluster.Login.OpenIDConnect.Issuer) s.cluster.Login.OpenIDConnect.ClientID = "oidc#client#id" s.cluster.Login.OpenIDConnect.ClientSecret = "oidc#client#secret" - s.cluster.Login.OpenIDConnect.AcceptAccessTokenScope = "*" + s.cluster.Login.OpenIDConnect.AcceptAccessToken = true + s.cluster.Login.OpenIDConnect.AcceptAccessTokenScope = "" s.fakeProvider.ValidClientID = "oidc#client#id" s.fakeProvider.ValidClientSecret = "oidc#client#secret" db := arvadostest.DB(c, s.cluster) @@ -268,17 +269,20 @@ func (s *OIDCLoginSuite) TestOIDCAuthorizer(c *check.C) { apiToken = fmt.Sprintf("%x", mac.Sum(nil)) for _, trial := range []struct { - configScope string - acceptable bool - shouldRun bool + configEnable bool + configScope string + acceptable bool + shouldRun bool }{ - {"foobar", true, true}, - {"foo", false, false}, - {"*", true, true}, - {"", false, true}, + {true, "foobar", true, true}, + {true, "foo", false, false}, + {true, "", true, true}, + {false, "", false, true}, + {false, "foobar", false, true}, } { c.Logf("trial = %+v", trial) cleanup() + s.cluster.Login.OpenIDConnect.AcceptAccessToken = trial.configEnable s.cluster.Login.OpenIDConnect.AcceptAccessTokenScope = trial.configScope oidcAuthorizer = OIDCAccessTokenAuthorizer(s.cluster, func(context.Context) (*sqlx.DB, error) { return db, nil }) checked := false diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go index 8a1b025a04..65e2ff5381 100644 --- a/sdk/go/arvados/config.go +++ b/sdk/go/arvados/config.go @@ -167,6 +167,7 @@ type Cluster struct { EmailClaim string EmailVerifiedClaim string UsernameClaim string + AcceptAccessToken bool AcceptAccessTokenScope string AuthenticationRequestParameters map[string]string }