[ARVADOS] updated: 2.1.0-824-ge0b63c68d

Git user git at public.arvados.org
Fri May 21 18:47:43 UTC 2021


Summary of changes:
 doc/api/tokens.html.textile.liquid        |  4 ++--
 lib/config/config.default.yml             | 22 ++++++++++++++--------
 lib/config/export.go                      |  1 +
 lib/config/generated_config.go            | 22 ++++++++++++++--------
 lib/controller/auth_test.go               |  3 ++-
 lib/controller/integration_test.go        |  3 ++-
 lib/controller/localdb/login.go           |  1 +
 lib/controller/localdb/login_oidc.go      | 12 +++++++-----
 lib/controller/localdb/login_oidc_test.go | 20 ++++++++++++--------
 sdk/go/arvados/config.go                  |  1 +
 10 files changed, 56 insertions(+), 33 deletions(-)

       via  e0b63c68db6c398aeb7a5820ac0ff5553d33bb40 (commit)
      from  89e9f940678b8f60166d3c2f7dd9be856bbc5557 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit e0b63c68db6c398aeb7a5820ac0ff5553d33bb40
Author: Tom Clegg <tom at curii.com>
Date:   Fri May 21 14:47:09 2021 -0400

    17704: Split access token scope config into 2 keys.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/doc/api/tokens.html.textile.liquid b/doc/api/tokens.html.textile.liquid
index 35d161f7c..0935f9ba1 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 d00c7d9ad..8ad2cb53f 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 cdefc0b08..890d4ce47 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 4f1cd2e7d..9e59f8c92 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 ee267ba5d..69458655b 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 51748126d..44c99bf30 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 47af553b6..0d6f2ef02 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 6ca53bf18..61dc5c816 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 3a345d7dc..c9d6133c4 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 8a1b025a0..65e2ff538 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
 		}

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list