X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/578c505d74e0e4daf680c5c39fb1619bb073a592..aeb2b25c254f8e98a67c0cc3e6cd8cdadcb8bd36:/lib/controller/localdb/login_oidc_test.go diff --git a/lib/controller/localdb/login_oidc_test.go b/lib/controller/localdb/login_oidc_test.go index 49629bb222..cf9cf30eca 100644 --- a/lib/controller/localdb/login_oidc_test.go +++ b/lib/controller/localdb/login_oidc_test.go @@ -21,12 +21,11 @@ import ( "testing" "time" - "git.arvados.org/arvados.git/lib/config" "git.arvados.org/arvados.git/lib/controller/rpc" + "git.arvados.org/arvados.git/lib/ctrlctx" "git.arvados.org/arvados.git/sdk/go/arvados" "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" ) @@ -39,20 +38,11 @@ func Test(t *testing.T) { var _ = check.Suite(&OIDCLoginSuite{}) type OIDCLoginSuite struct { - cluster *arvados.Cluster - localdb *Conn - railsSpy *arvadostest.Proxy + localdbSuite trustedURL *arvados.URL fakeProvider *arvadostest.OIDCProvider } -func (s *OIDCLoginSuite) TearDownSuite(c *check.C) { - // Undo any changes/additions to the user database so they - // don't affect subsequent tests. - arvadostest.ResetEnv() - c.Check(arvados.NewClientFromEnv().RequestAndDecode(nil, "POST", "database/reset", nil, nil), check.IsNil) -} - func (s *OIDCLoginSuite) SetUpTest(c *check.C) { s.trustedURL = &arvados.URL{Scheme: "https", Host: "app.example.com", Path: "/"} @@ -65,10 +55,8 @@ func (s *OIDCLoginSuite) SetUpTest(c *check.C) { s.fakeProvider.ValidCode = fmt.Sprintf("abcdefgh-%d", time.Now().Unix()) s.fakeProvider.PeopleAPIResponse = map[string]interface{}{} - cfg, err := config.NewLoader(nil, ctxlog.TestLogger(c)).Load() - c.Assert(err, check.IsNil) - s.cluster, err = cfg.GetCluster("") - c.Assert(err, check.IsNil) + s.localdbSuite.SetUpTest(c) + s.cluster.Login.Test.Enable = false s.cluster.Login.Google.Enable = true s.cluster.Login.Google.ClientID = "test%client$id" @@ -78,19 +66,14 @@ func (s *OIDCLoginSuite) SetUpTest(c *check.C) { s.fakeProvider.ValidClientID = "test%client$id" s.fakeProvider.ValidClientSecret = "test#client/secret" - s.localdb = NewConn(s.cluster) + s.localdb = NewConn(s.ctx, s.cluster, (&ctrlctx.DBConnector{PostgreSQL: s.cluster.PostgreSQL}).GetDB) c.Assert(s.localdb.loginController, check.FitsTypeOf, (*oidcLoginController)(nil)) s.localdb.loginController.(*oidcLoginController).Issuer = s.fakeProvider.Issuer.URL s.localdb.loginController.(*oidcLoginController).peopleAPIBasePath = s.fakeProvider.PeopleAPI.URL - s.railsSpy = arvadostest.NewProxy(c, s.cluster.Services.RailsAPI) *s.localdb.railsProxy = *rpc.NewConn(s.cluster.ClusterID, s.railsSpy.URL, true, rpc.PassthroughTokenProvider) } -func (s *OIDCLoginSuite) TearDownTest(c *check.C) { - s.railsSpy.Close() -} - func (s *OIDCLoginSuite) TestGoogleLogout(c *check.C) { s.cluster.Login.TrustedClients[arvados.URL{Scheme: "https", Host: "foo.example", Path: "/"}] = struct{}{} s.cluster.Login.TrustPrivateNetworks = false @@ -197,7 +180,7 @@ func (s *OIDCLoginSuite) TestConfig(c *check.C) { s.cluster.Login.OpenIDConnect.ClientID = "oidc-client-id" s.cluster.Login.OpenIDConnect.ClientSecret = "oidc-client-secret" s.cluster.Login.OpenIDConnect.AuthenticationRequestParameters = map[string]string{"testkey": "testvalue"} - localdb := NewConn(s.cluster) + localdb := NewConn(context.Background(), s.cluster, (&ctrlctx.DBConnector{PostgreSQL: s.cluster.PostgreSQL}).GetDB) ctrl := localdb.loginController.(*oidcLoginController) c.Check(ctrl.Issuer, check.Equals, "https://accounts.example.com/") c.Check(ctrl.ClientID, check.Equals, "oidc-client-id") @@ -212,7 +195,7 @@ func (s *OIDCLoginSuite) TestConfig(c *check.C) { s.cluster.Login.Google.ClientSecret = "google-client-secret" s.cluster.Login.Google.AlternateEmailAddresses = enableAltEmails s.cluster.Login.Google.AuthenticationRequestParameters = map[string]string{"testkey": "testvalue"} - localdb = NewConn(s.cluster) + localdb = NewConn(context.Background(), s.cluster, (&ctrlctx.DBConnector{PostgreSQL: s.cluster.PostgreSQL}).GetDB) ctrl = localdb.loginController.(*oidcLoginController) c.Check(ctrl.Issuer, check.Equals, "https://accounts.google.com") c.Check(ctrl.ClientID, check.Equals, "google-client-id") @@ -256,16 +239,73 @@ func (s *OIDCLoginSuite) TestOIDCAuthorizer(c *check.C) { io.WriteString(mac, accessToken) apiToken := fmt.Sprintf("%x", mac.Sum(nil)) + checkTokenInDB := func() time.Time { + var exp time.Time + err := db.QueryRow(`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(time.Now()) > -time.Second, check.Equals, true) + c.Check(exp.Sub(time.Now()) < time.Second, check.Equals, true) + return exp + } cleanup := func() { + oidcAuthorizer.cache.Purge() _, err := db.Exec(`delete from api_client_authorizations where api_token=$1`, apiToken) c.Check(err, check.IsNil) } cleanup() defer cleanup() - ctx := auth.NewContext(context.Background(), &auth.Credentials{Tokens: []string{accessToken}}) - var exp1 time.Time + ctx := ctrlctx.NewWithToken(s.ctx, s.cluster, accessToken) + + // Check behavior on 5xx/network errors (don't cache) vs 4xx + // (do cache) + { + call := oidcAuthorizer.WrapCalls(func(ctx context.Context, opts interface{}) (interface{}, error) { + return nil, nil + }) + + // If fakeProvider UserInfo endpoint returns 502, we + // should fail, return an error, and *not* cache the + // negative result. + tokenCacheNegativeTTL = time.Minute + s.fakeProvider.UserInfoErrorStatus = 502 + _, err := call(ctx, nil) + c.Check(err, check.NotNil) + + // The negative result was not cached, so retrying + // immediately (with UserInfo working now) should + // succeed. + s.fakeProvider.UserInfoErrorStatus = 0 + _, err = call(ctx, nil) + c.Check(err, check.IsNil) + checkTokenInDB() + cleanup() + + // UserInfo 401 => cache the negative result, but + // don't return an error (just pass the token through + // as a v1 token) + s.fakeProvider.UserInfoErrorStatus = 401 + _, err = call(ctx, nil) + c.Check(err, check.IsNil) + ent, ok := oidcAuthorizer.cache.Get(accessToken) + c.Check(ok, check.Equals, true) + c.Check(ent, check.FitsTypeOf, time.Time{}) + + // UserInfo succeeds now, but we still have a cached + // negative result. + s.fakeProvider.UserInfoErrorStatus = 0 + _, err = call(ctx, nil) + c.Check(err, check.IsNil) + ent, ok = oidcAuthorizer.cache.Get(accessToken) + c.Check(ok, check.Equals, true) + c.Check(ent, check.FitsTypeOf, time.Time{}) + + tokenCacheNegativeTTL = time.Millisecond + cleanup() + } + + var exp1 time.Time concurrent := 4 s.fakeProvider.HoldUserInfo = make(chan *http.Request) s.fakeProvider.ReleaseUserInfo = make(chan struct{}) @@ -285,17 +325,12 @@ func (s *OIDCLoginSuite) TestOIDCAuthorizer(c *check.C) { defer wg.Done() _, err := oidcAuthorizer.WrapCalls(func(ctx context.Context, opts interface{}) (interface{}, error) { c.Logf("concurrent req %d/%d", i, concurrent) - var exp time.Time 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`, apiToken).Scan(&exp) - c.Check(err, check.IsNil) - c.Check(exp.Sub(time.Now()) > -time.Second, check.Equals, true) - c.Check(exp.Sub(time.Now()) < time.Second, check.Equals, true) + exp := checkTokenInDB() if i == 0 { exp1 = exp } @@ -314,9 +349,7 @@ func (s *OIDCLoginSuite) TestOIDCAuthorizer(c *check.C) { // 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`, apiToken).Scan(&exp) - c.Check(err, check.IsNil) + exp := checkTokenInDB() c.Check(exp.Sub(exp1) > 0, check.Equals, true, check.Commentf("expect %v > 0", exp.Sub(exp1))) c.Check(exp.Sub(exp1) < time.Second, check.Equals, true, check.Commentf("expect %v < 1s", exp.Sub(exp1))) return nil, nil @@ -324,7 +357,7 @@ func (s *OIDCLoginSuite) TestOIDCAuthorizer(c *check.C) { s.fakeProvider.AccessTokenPayload = map[string]interface{}{"scope": "openid profile foobar"} accessToken = s.fakeProvider.ValidAccessToken() - ctx = auth.NewContext(context.Background(), &auth.Credentials{Tokens: []string{accessToken}}) + ctx = ctrlctx.NewWithToken(s.ctx, s.cluster, accessToken) mac = hmac.New(sha256.New, []byte(s.cluster.SystemRootToken)) io.WriteString(mac, accessToken) @@ -427,7 +460,7 @@ func (s *OIDCLoginSuite) TestGenericOIDCLogin(c *check.C) { s.railsSpy.Close() } s.railsSpy = arvadostest.NewProxy(c, s.cluster.Services.RailsAPI) - s.localdb = NewConn(s.cluster) + s.localdb = NewConn(context.Background(), s.cluster, (&ctrlctx.DBConnector{PostgreSQL: s.cluster.PostgreSQL}).GetDB) *s.localdb.railsProxy = *rpc.NewConn(s.cluster.ClusterID, s.railsSpy.URL, true, rpc.PassthroughTokenProvider) state := s.startLogin(c, func(form url.Values) { @@ -490,7 +523,7 @@ func (s *OIDCLoginSuite) TestGoogleLogin_Success(c *check.C) { // Try using the returned Arvados token. c.Logf("trying an API call with new token %q", token) - ctx := auth.NewContext(context.Background(), &auth.Credentials{Tokens: []string{token}}) + ctx := ctrlctx.NewWithToken(s.ctx, s.cluster, token) cl, err := s.localdb.CollectionList(ctx, arvados.ListOptions{Limit: -1}) c.Check(cl.ItemsAvailable, check.Not(check.Equals), 0) c.Check(cl.Items, check.Not(check.HasLen), 0) @@ -499,7 +532,7 @@ func (s *OIDCLoginSuite) TestGoogleLogin_Success(c *check.C) { // Might as well check that bogus tokens aren't accepted. badtoken := token + "plussomeboguschars" c.Logf("trying an API call with mangled token %q", badtoken) - ctx = auth.NewContext(context.Background(), &auth.Credentials{Tokens: []string{badtoken}}) + ctx = ctrlctx.NewWithToken(s.ctx, s.cluster, badtoken) cl, err = s.localdb.CollectionList(ctx, arvados.ListOptions{Limit: -1}) c.Check(cl.Items, check.HasLen, 0) c.Check(err, check.NotNil) @@ -669,14 +702,15 @@ func (s *OIDCLoginSuite) TestValidateLoginRedirectTarget(c *check.C) { {true, false, "https://app.example.com/"}, {true, false, "https://app.example.com:443/foo?bar=baz"}, // non-listed hostname => deny (regardless of TrustPrivateNetworks) - {false, false, "https://localhost/"}, - {false, true, "https://localhost/"}, + {false, false, "https://bad.example/"}, {false, true, "https://bad.example/"}, // non-listed non-private IP addr => deny (regardless of TrustPrivateNetworks) {false, true, "https://1.2.3.4/"}, {false, true, "https://1.2.3.4/"}, {false, true, "https://[ab::cd]:1234/"}, - // non-listed private IP addr => accept only if TrustPrivateNetworks is set + // localhost or non-listed private IP addr => accept only if TrustPrivateNetworks is set + {false, false, "https://localhost/"}, + {true, true, "https://localhost/"}, {false, false, "https://[10.9.8.7]:80/foo"}, {true, true, "https://[10.9.8.7]:80/foo"}, {false, false, "https://[::1]:80/foo"},