[ARVADOS] created: 2.1.0-822-g8daccc2ab
Git user
git at public.arvados.org
Fri May 21 05:42:26 UTC 2021
at 8daccc2ab3f2178745d12bc54ec9a8d06d88864a (commit)
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