17704: Split access token scope config into 2 keys.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

diff --git a/doc/api/tokens.html.textile.liquid b/doc/api/tokens.html.textile.liquid
index 35d161f7cddd0cd02abfc389a79b97cba68c61db..0935f9ba1d2a3bf7eb5c5bb7db4eb20b528ac3ed 100644 (file)
--- 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 d00c7d9ade226cf56f11fedee4c83aa44f01f997..8ad2cb53fca8d20fce9a091f5a6b781e7e8c9835 100644 (file)
--- 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 cdefc0b08336139afec6ab00bb3f1ea83bf4f9ba..890d4ce4711eb4f06a00ada8e9e5adc63b2d7999 100644 (file)
--- 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 4f1cd2e7d2e3b888563e34b2cad5b3142c65c74f..9e59f8c9238606ed8b0926ad2841ce66d20e3565 100644 (file)
--- 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 ee267ba5dd8f39bb7ccd219717e69b74f0702265..69458655ba0c8ce7caf9839d05eb5985f3fd0b7b 100644 (file)
--- 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 51748126d06a7f56329693ec7ffa0b14c960b5b9..44c99bf30f8c3a6ae9aa70b8306268b7c4c8fb6d 100644 (file)
--- 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 47af553b6f8d35a7e86201c46e87264ede6903c5..0d6f2ef027e8500c60fdf644e8f808fecd2f226a 100644 (file)
--- 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 6ca53bf18475c955e0b6edaad8051963ef3e3740..61dc5c816b35661f39c4a800ab17f1bf55325f06 100644 (file)
--- 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 3a345d7dc32439d8a49f631bb4e22c9468442df7..c9d6133c480319b9129397ea076068d67bb4a3f5 100644 (file)
--- 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 8a1b025a043c99d340d3eadf1e489a9a5f85c1fc..65e2ff5381e84e9ab4259e0b54b3d033e20d9508 100644 (file)
--- 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
                }