[ARVADOS] created: 2.1.0-823-g89e9f9406

Git user git at public.arvados.org
Fri May 21 06:39:18 UTC 2021


        at  89e9f940678b8f60166d3c2f7dd9be856bbc5557 (commit)


commit 89e9f940678b8f60166d3c2f7dd9be856bbc5557
Author: Tom Clegg <tom at curii.com>
Date:   Fri May 21 02:38:53 2021 -0400

    17704: Enable OIDC access tokens for tests.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/controller/auth_test.go b/lib/controller/auth_test.go
index 01990620f..ee267ba5d 100644
--- a/lib/controller/auth_test.go
+++ b/lib/controller/auth_test.go
@@ -94,6 +94,7 @@ 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 = "*"
 
 	s.testHandler = &Handler{Cluster: cluster}
 	s.testServer = newServerFromIntegrationTestEnv(c)
diff --git a/lib/controller/integration_test.go b/lib/controller/integration_test.go
index 7b1dcbea6..51748126d 100644
--- a/lib/controller/integration_test.go
+++ b/lib/controller/integration_test.go
@@ -113,6 +113,7 @@ func (s *IntegrationSuite) SetUpSuite(c *check.C) {
         ClientSecret: ` + s.oidcprovider.ValidClientSecret + `
         EmailClaim: email
         EmailVerifiedClaim: email_verified
+        AcceptAccessTokenScope: "*"
 `
 		} else {
 			yaml += `

commit 8daccc2ab3f2178745d12bc54ec9a8d06d88864a
Author: Tom Clegg <tom at curii.com>
Date:   Thu May 20 17:12:36 2021 -0400

    17704: Check scope before accepting OIDC access tokens.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/doc/api/requests.html.textile.liquid b/doc/api/requests.html.textile.liquid
index cee472852..fc5957af5 100644
--- a/doc/api/requests.html.textile.liquid
+++ b/doc/api/requests.html.textile.liquid
@@ -42,7 +42,7 @@ $ curl -v -H "Authorization: Bearer xxxxapitokenxxxx" https://192.168.5.2:8000/a
 > ...
 </pre>
 
-On a cluster configured to use an OpenID Connect provider (including Google) as a login backend, an OpenID Connect access token can also be used in place of an Arvados API token. This is also supported on a cluster that delegates login to another cluster (LoginCluster) which in turn uses an OpenID Connect provider.
+On a cluster configured to use an OpenID Connect provider (other than Google) as a login backend, Arvados can be configured to accept an OpenID Connect access token in place of an Arvados API token. OIDC access tokens are also accepted by a cluster that delegates login to another cluster (LoginCluster) which in turn has this feature configured. See @Login.OpenIDConnect.AcceptAccessTokenScope@ in the "default config.yml file":{{site.baseurl}}/admin/config.html for details.
 
 <pre>
 $ curl -v -H "Authorization: Bearer xxxx-openid-connect-access-token-xxxx" https://192.168.5.2:8000/arvados/v1/collections
diff --git a/doc/api/tokens.html.textile.liquid b/doc/api/tokens.html.textile.liquid
index c9321ae1d..35d161f7c 100644
--- a/doc/api/tokens.html.textile.liquid
+++ b/doc/api/tokens.html.textile.liquid
@@ -34,9 +34,10 @@ h3. Direct username/password authentication
 
 h3. Using an OpenID Connect access token
 
-On a cluster that uses OpenID Connect or Google as a login provider, or defers to a LoginCluster that does so, clients may present an access token instead of an Arvados API 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).
 # 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.
 # 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 54deb34da..d00c7d9ad 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -633,6 +633,17 @@ 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 "".
+        #
+        # 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: ""
+
       PAM:
         # (Experimental) Use PAM to authenticate users.
         Enable: false
diff --git a/lib/config/export.go b/lib/config/export.go
index 5c0e9f270..cdefc0b08 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.AcceptAccessTokenScope":          false,
 	"Login.OpenIDConnect.AuthenticationRequestParameters": false,
 	"Login.OpenIDConnect.ClientID":                        false,
 	"Login.OpenIDConnect.ClientSecret":                    false,
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 26c159c8c..4f1cd2e7d 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -639,6 +639,17 @@ 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 "".
+        #
+        # 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: ""
+
       PAM:
         # (Experimental) Use PAM to authenticate users.
         Enable: false
diff --git a/lib/controller/localdb/login.go b/lib/controller/localdb/login.go
index 01fa84ea4..47af553b6 100644
--- a/lib/controller/localdb/login.go
+++ b/lib/controller/localdb/login.go
@@ -54,15 +54,16 @@ func chooseLoginController(cluster *arvados.Cluster, parent *Conn) loginControll
 		}
 	case wantOpenIDConnect:
 		return &oidcLoginController{
-			Cluster:            cluster,
-			Parent:             parent,
-			Issuer:             cluster.Login.OpenIDConnect.Issuer,
-			ClientID:           cluster.Login.OpenIDConnect.ClientID,
-			ClientSecret:       cluster.Login.OpenIDConnect.ClientSecret,
-			AuthParams:         cluster.Login.OpenIDConnect.AuthenticationRequestParameters,
-			EmailClaim:         cluster.Login.OpenIDConnect.EmailClaim,
-			EmailVerifiedClaim: cluster.Login.OpenIDConnect.EmailVerifiedClaim,
-			UsernameClaim:      cluster.Login.OpenIDConnect.UsernameClaim,
+			Cluster:                cluster,
+			Parent:                 parent,
+			Issuer:                 cluster.Login.OpenIDConnect.Issuer,
+			ClientID:               cluster.Login.OpenIDConnect.ClientID,
+			ClientSecret:           cluster.Login.OpenIDConnect.ClientSecret,
+			AuthParams:             cluster.Login.OpenIDConnect.AuthenticationRequestParameters,
+			EmailClaim:             cluster.Login.OpenIDConnect.EmailClaim,
+			EmailVerifiedClaim:     cluster.Login.OpenIDConnect.EmailVerifiedClaim,
+			UsernameClaim:          cluster.Login.OpenIDConnect.UsernameClaim,
+			AcceptAccessTokenScope: cluster.Login.OpenIDConnect.AcceptAccessTokenScope,
 		}
 	case wantSSO:
 		return &ssoLoginController{Parent: parent}
diff --git a/lib/controller/localdb/login_oidc.go b/lib/controller/localdb/login_oidc.go
index a435b014d..6ca53bf18 100644
--- a/lib/controller/localdb/login_oidc.go
+++ b/lib/controller/localdb/login_oidc.go
@@ -35,6 +35,7 @@ import (
 	"golang.org/x/oauth2"
 	"google.golang.org/api/option"
 	"google.golang.org/api/people/v1"
+	"gopkg.in/square/go-jose.v2/jwt"
 )
 
 var (
@@ -45,16 +46,17 @@ var (
 )
 
 type oidcLoginController struct {
-	Cluster            *arvados.Cluster
-	Parent             *Conn
-	Issuer             string // OIDC issuer URL, e.g., "https://accounts.google.com"
-	ClientID           string
-	ClientSecret       string
-	UseGooglePeopleAPI bool              // Use Google People API to look up alternate email addresses
-	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
-	AuthParams         map[string]string // Additional parameters to pass with authentication request
+	Cluster                *arvados.Cluster
+	Parent                 *Conn
+	Issuer                 string // OIDC issuer URL, e.g., "https://accounts.google.com"
+	ClientID               string
+	ClientSecret           string
+	UseGooglePeopleAPI     bool              // Use Google People API to look up alternate email addresses
+	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
+	AuthParams             map[string]string // Additional parameters to pass with authentication request
 
 	// override Google People API base URL for testing purposes
 	// (normally empty, set by google pkg to
@@ -134,6 +136,7 @@ func (ctrl *oidcLoginController) Login(ctx context.Context, opts arvados.LoginOp
 	if !ok {
 		return loginError(errors.New("error in OAuth2 exchange: no ID token in OAuth2 token"))
 	}
+	ctxlog.FromContext(ctx).WithField("rawIDToken", rawIDToken).Debug("oauth2Token provided ID token")
 	idToken, err := ctrl.verifier.Verify(ctx, rawIDToken)
 	if err != nil {
 		return loginError(fmt.Errorf("error verifying ID token: %s", err))
@@ -448,6 +451,10 @@ func (ta *oidcTokenAuthorizer) registerToken(ctx context.Context, tok string) er
 	if err != nil {
 		return fmt.Errorf("error setting up OpenID Connect provider: %s", err)
 	}
+	if ok, err := ta.checkAccessTokenScope(ctx, tok); err != nil || !ok {
+		ta.cache.Add(tok, time.Now().Add(tokenCacheNegativeTTL))
+		return err
+	}
 	oauth2Token := &oauth2.Token{
 		AccessToken: tok,
 	}
@@ -494,3 +501,37 @@ func (ta *oidcTokenAuthorizer) registerToken(ctx context.Context, tok string) er
 	ta.cache.Add(tok, aca)
 	return nil
 }
+
+// Check that the provided access token is a JWT with the required
+// scope. If it is a valid JWT but missing the required scope, we
+// return a 403 error, otherwise true (acceptable as an API token) or
+// false (pass through unmodified).
+//
+// 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 "":
+		return false, nil
+	}
+	var claims struct {
+		Scope string `json:"scope"`
+	}
+	if t, err := jwt.ParseSigned(tok); err != nil {
+		ctxlog.FromContext(ctx).WithError(err).Debug("error parsing jwt")
+		return false, nil
+	} else if err = t.UnsafeClaimsWithoutVerification(&claims); err != nil {
+		ctxlog.FromContext(ctx).WithError(err).Debug("error extracting jwt claims")
+		return false, nil
+	}
+	for _, s := range strings.Split(claims.Scope, " ") {
+		if s == ta.ctrl.AcceptAccessTokenScope {
+			return true, nil
+		}
+	}
+	ctxlog.FromContext(ctx).WithFields(logrus.Fields{"have": claims.Scope, "need": ta.ctrl.AcceptAccessTokenScope}).Infof("unacceptable access token scope")
+	return false, httpserver.ErrorWithStatus(errors.New("unacceptable access token scope"), http.StatusUnauthorized)
+}
diff --git a/lib/controller/localdb/login_oidc_test.go b/lib/controller/localdb/login_oidc_test.go
index e3c72addd..3a345d7dc 100644
--- a/lib/controller/localdb/login_oidc_test.go
+++ b/lib/controller/localdb/login_oidc_test.go
@@ -208,22 +208,24 @@ 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.fakeProvider.ValidClientID = "oidc#client#id"
 	s.fakeProvider.ValidClientSecret = "oidc#client#secret"
 	db := arvadostest.DB(c, s.cluster)
 
 	tokenCacheTTL = time.Millisecond
 	tokenCacheRaceWindow = time.Millisecond
+	tokenCacheNegativeTTL = time.Millisecond
 
 	oidcAuthorizer := OIDCAccessTokenAuthorizer(s.cluster, func(context.Context) (*sqlx.DB, error) { return db, nil })
 	accessToken := s.fakeProvider.ValidAccessToken()
 
 	mac := hmac.New(sha256.New, []byte(s.cluster.SystemRootToken))
 	io.WriteString(mac, accessToken)
-	hmac := fmt.Sprintf("%x", mac.Sum(nil))
+	apiToken := fmt.Sprintf("%x", mac.Sum(nil))
 
 	cleanup := func() {
-		_, err := db.Exec(`delete from api_client_authorizations where api_token=$1`, hmac)
+		_, err := db.Exec(`delete from api_client_authorizations where api_token=$1`, apiToken)
 		c.Check(err, check.IsNil)
 	}
 	cleanup()
@@ -237,7 +239,7 @@ func (s *OIDCLoginSuite) TestOIDCAuthorizer(c *check.C) {
 		c.Assert(creds.Tokens, check.HasLen, 1)
 		c.Check(creds.Tokens[0], check.Equals, accessToken)
 
-		err := db.QueryRowContext(ctx, `select expires_at at time zone 'UTC' from api_client_authorizations where api_token=$1`, hmac).Scan(&exp1)
+		err := db.QueryRowContext(ctx, `select expires_at at time zone 'UTC' from api_client_authorizations where api_token=$1`, apiToken).Scan(&exp1)
 		c.Check(err, check.IsNil)
 		c.Check(exp1.Sub(time.Now()) > -time.Second, check.Equals, true)
 		c.Check(exp1.Sub(time.Now()) < time.Second, check.Equals, true)
@@ -245,17 +247,55 @@ func (s *OIDCLoginSuite) TestOIDCAuthorizer(c *check.C) {
 	})(ctx, nil)
 
 	// If the token is used again after the in-memory cache
-	// expires, oidcAuthorizer must re-checks the token and update
+	// expires, oidcAuthorizer must re-check the token and update
 	// the expires_at value in the database.
 	time.Sleep(3 * time.Millisecond)
 	oidcAuthorizer.WrapCalls(func(ctx context.Context, opts interface{}) (interface{}, error) {
 		var exp time.Time
-		err := db.QueryRowContext(ctx, `select expires_at at time zone 'UTC' from api_client_authorizations where api_token=$1`, hmac).Scan(&exp)
+		err := db.QueryRowContext(ctx, `select expires_at at time zone 'UTC' from api_client_authorizations where api_token=$1`, apiToken).Scan(&exp)
 		c.Check(err, check.IsNil)
 		c.Check(exp.Sub(exp1) > 0, check.Equals, true)
 		c.Check(exp.Sub(exp1) < time.Second, check.Equals, true)
 		return nil, nil
 	})(ctx, nil)
+
+	s.fakeProvider.AccessTokenPayload = map[string]interface{}{"scope": "openid profile foobar"}
+	accessToken = s.fakeProvider.ValidAccessToken()
+	ctx = auth.NewContext(context.Background(), &auth.Credentials{Tokens: []string{accessToken}})
+
+	mac = hmac.New(sha256.New, []byte(s.cluster.SystemRootToken))
+	io.WriteString(mac, accessToken)
+	apiToken = fmt.Sprintf("%x", mac.Sum(nil))
+
+	for _, trial := range []struct {
+		configScope string
+		acceptable  bool
+		shouldRun   bool
+	}{
+		{"foobar", true, true},
+		{"foo", false, false},
+		{"*", true, true},
+		{"", false, true},
+	} {
+		c.Logf("trial = %+v", trial)
+		cleanup()
+		s.cluster.Login.OpenIDConnect.AcceptAccessTokenScope = trial.configScope
+		oidcAuthorizer = OIDCAccessTokenAuthorizer(s.cluster, func(context.Context) (*sqlx.DB, error) { return db, nil })
+		checked := false
+		oidcAuthorizer.WrapCalls(func(ctx context.Context, opts interface{}) (interface{}, error) {
+			var n int
+			err := db.QueryRowContext(ctx, `select count(*) from api_client_authorizations where api_token=$1`, apiToken).Scan(&n)
+			c.Check(err, check.IsNil)
+			if trial.acceptable {
+				c.Check(n, check.Equals, 1)
+			} else {
+				c.Check(n, check.Equals, 0)
+			}
+			checked = true
+			return nil, nil
+		})(ctx, nil)
+		c.Check(checked, check.Equals, trial.shouldRun)
+	}
 }
 
 func (s *OIDCLoginSuite) TestGenericOIDCLogin(c *check.C) {
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index 2c6db42d1..8a1b025a0 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
+			AcceptAccessTokenScope          string
 			AuthenticationRequestParameters map[string]string
 		}
 		PAM struct {
diff --git a/sdk/go/arvadostest/oidc_provider.go b/sdk/go/arvadostest/oidc_provider.go
index 96205f919..de21302e5 100644
--- a/sdk/go/arvadostest/oidc_provider.go
+++ b/sdk/go/arvadostest/oidc_provider.go
@@ -17,6 +17,7 @@ import (
 
 	"gopkg.in/check.v1"
 	"gopkg.in/square/go-jose.v2"
+	"gopkg.in/square/go-jose.v2/jwt"
 )
 
 type OIDCProvider struct {
@@ -25,9 +26,10 @@ type OIDCProvider struct {
 	ValidClientID     string
 	ValidClientSecret string
 	// desired response from token endpoint
-	AuthEmail         string
-	AuthEmailVerified bool
-	AuthName          string
+	AuthEmail          string
+	AuthEmailVerified  bool
+	AuthName           string
+	AccessTokenPayload map[string]interface{}
 
 	PeopleAPIResponse map[string]interface{}
 
@@ -44,11 +46,13 @@ func NewOIDCProvider(c *check.C) *OIDCProvider {
 	c.Assert(err, check.IsNil)
 	p.Issuer = httptest.NewServer(http.HandlerFunc(p.serveOIDC))
 	p.PeopleAPI = httptest.NewServer(http.HandlerFunc(p.servePeopleAPI))
+	p.AccessTokenPayload = map[string]interface{}{"sub": "example"}
 	return p
 }
 
 func (p *OIDCProvider) ValidAccessToken() string {
-	return p.fakeToken([]byte("fake access token"))
+	buf, _ := json.Marshal(p.AccessTokenPayload)
+	return p.fakeToken(buf)
 }
 
 func (p *OIDCProvider) serveOIDC(w http.ResponseWriter, req *http.Request) {
@@ -118,7 +122,8 @@ func (p *OIDCProvider) serveOIDC(w http.ResponseWriter, req *http.Request) {
 	case "/auth":
 		w.WriteHeader(http.StatusInternalServerError)
 	case "/userinfo":
-		if authhdr := req.Header.Get("Authorization"); strings.TrimPrefix(authhdr, "Bearer ") != p.ValidAccessToken() {
+		authhdr := req.Header.Get("Authorization")
+		if _, err := jwt.ParseSigned(strings.TrimPrefix(authhdr, "Bearer ")); err != nil {
 			p.c.Logf("OIDCProvider: bad auth %q", authhdr)
 			w.WriteHeader(http.StatusUnauthorized)
 			return

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list