[ARVADOS] created: 2.1.0-290-gaed89fc99
Git user
git at public.arvados.org
Tue Jan 26 19:03:40 UTC 2021
at aed89fc99abadf571a710233642fa6add9036e65 (commit)
commit aed89fc99abadf571a710233642fa6add9036e65
Author: Tom Clegg <tom at curii.com>
Date: Mon Jan 25 16:01:51 2021 -0500
16669: Fix cache bug.
When caching a valid token we were updating the database with
exp=now+10m but updating the cache with exp="". When checking the
cache, exp="" indicates we don't need to re-check the access token and
extend the database entry's expiry field -- so we never did, we just
passed through the HMAC, which RailsAPI rejected based on the expiry
time in the database row.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/controller/localdb/login_oidc.go b/lib/controller/localdb/login_oidc.go
index 9a32cead9..a5fe45181 100644
--- a/lib/controller/localdb/login_oidc.go
+++ b/lib/controller/localdb/login_oidc.go
@@ -490,6 +490,7 @@ func (ta *oidcTokenAuthorizer) registerToken(ctx context.Context, tok string) er
if err != nil {
return err
}
+ aca.ExpiresAt = exp.Format(time.RFC3339Nano)
ta.cache.Add(tok, aca)
return nil
}
commit 65198e0d95ccef9b56459dd6cd4f52a1f7c7baf7
Author: Tom Clegg <tom at curii.com>
Date: Mon Jan 25 23:58:15 2021 -0500
16669: Test oidcTokenAuthorizer cache.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/controller/localdb/login_oidc.go b/lib/controller/localdb/login_oidc.go
index b99a1c2aa..9a32cead9 100644
--- a/lib/controller/localdb/login_oidc.go
+++ b/lib/controller/localdb/login_oidc.go
@@ -37,10 +37,11 @@ import (
"google.golang.org/api/people/v1"
)
-const (
+var (
tokenCacheSize = 1000
tokenCacheNegativeTTL = time.Minute * 5
tokenCacheTTL = time.Minute * 10
+ tokenCacheRaceWindow = time.Minute
)
type oidcLoginController struct {
@@ -363,8 +364,9 @@ func (ta *oidcTokenAuthorizer) WrapCalls(origFunc api.RoutableFunc) api.Routable
return origFunc(ctx, opts)
}
// Check each token in the incoming request. If any
- // are OAuth2 access tokens, swap them out for Arvados
- // tokens.
+ // are valid OAuth2 access tokens, insert/update them
+ // in the database so RailsAPI's auth code accepts
+ // them.
for _, tok := range creds.Tokens {
err = ta.registerToken(ctx, tok)
if err != nil {
@@ -463,7 +465,7 @@ func (ta *oidcTokenAuthorizer) registerToken(ctx context.Context, tok string) er
// Expiry time for our token is one minute longer than our
// cache TTL, so we don't pass it through to RailsAPI just as
// it's expiring.
- exp := time.Now().UTC().Add(tokenCacheTTL + time.Minute)
+ exp := time.Now().UTC().Add(tokenCacheTTL + tokenCacheRaceWindow)
var aca arvados.APIClientAuthorization
if updating {
diff --git a/lib/controller/localdb/login_oidc_test.go b/lib/controller/localdb/login_oidc_test.go
index 9bc6f90ea..e157b73fc 100644
--- a/lib/controller/localdb/login_oidc_test.go
+++ b/lib/controller/localdb/login_oidc_test.go
@@ -7,8 +7,11 @@ package localdb
import (
"bytes"
"context"
+ "crypto/hmac"
+ "crypto/sha256"
"encoding/json"
"fmt"
+ "io"
"net/http"
"net/http/httptest"
"net/url"
@@ -23,6 +26,7 @@ import (
"git.arvados.org/arvados.git/sdk/go/arvadostest"
"git.arvados.org/arvados.git/sdk/go/auth"
"git.arvados.org/arvados.git/sdk/go/ctxlog"
+ "github.com/jmoiron/sqlx"
check "gopkg.in/check.v1"
)
@@ -194,6 +198,62 @@ func (s *OIDCLoginSuite) TestGoogleLogin_PeopleAPIError(c *check.C) {
c.Check(resp.RedirectLocation, check.Equals, "")
}
+func (s *OIDCLoginSuite) TestOIDCAuthorizer(c *check.C) {
+ s.cluster.Login.Google.Enable = false
+ s.cluster.Login.OpenIDConnect.Enable = true
+ 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.fakeProvider.ValidClientID = "oidc#client#id"
+ s.fakeProvider.ValidClientSecret = "oidc#client#secret"
+ db := arvadostest.DB(c, s.cluster)
+
+ tokenCacheTTL = time.Millisecond
+ tokenCacheRaceWindow = 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))
+
+ cleanup := func() {
+ _, err := db.Exec(`delete from api_client_authorizations where api_token=$1`, hmac)
+ c.Check(err, check.IsNil)
+ }
+ cleanup()
+ defer cleanup()
+
+ ctx := auth.NewContext(context.Background(), &auth.Credentials{Tokens: []string{accessToken}})
+ var exp1 time.Time
+ oidcAuthorizer.WrapCalls(func(ctx context.Context, opts interface{}) (interface{}, error) {
+ creds, ok := auth.FromContext(ctx)
+ c.Assert(ok, check.Equals, true)
+ 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)
+ 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)
+ return nil, nil
+ })(ctx, nil)
+
+ // If the token is used again after the in-memory cache
+ // expires, oidcAuthorizer must re-checks 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)
+ 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)
+}
+
func (s *OIDCLoginSuite) TestGenericOIDCLogin(c *check.C) {
s.cluster.Login.Google.Enable = false
s.cluster.Login.OpenIDConnect.Enable = true
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list