From d3cd6f7557ded90258d86a25aa755328f702e488 Mon Sep 17 00:00:00 2001 From: Brett Smith Date: Mon, 13 Nov 2023 14:35:52 -0500 Subject: [PATCH] Merge branch '21021-controller-logout' Closes #21021. Arvados-DCO-1.1-Signed-off-by: Brett Smith --- lib/controller/federation/conn.go | 64 ++++-- lib/controller/federation/login_test.go | 39 ---- lib/controller/federation/logout_test.go | 246 +++++++++++++++++++++++ 3 files changed, 289 insertions(+), 60 deletions(-) create mode 100644 lib/controller/federation/logout_test.go diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go index b4b7476440..c65e142924 100644 --- a/lib/controller/federation/conn.go +++ b/lib/controller/federation/conn.go @@ -14,6 +14,7 @@ import ( "net/url" "regexp" "strings" + "sync" "time" "git.arvados.org/arvados.git/lib/config" @@ -253,30 +254,51 @@ func (conn *Conn) Login(ctx context.Context, options arvados.LoginOptions) (arva return conn.local.Login(ctx, options) } +var v2TokenRegexp = regexp.MustCompile(`^v2/[a-z0-9]{5}-gj3su-[a-z0-9]{15}/`) + func (conn *Conn) Logout(ctx context.Context, options arvados.LogoutOptions) (arvados.LogoutResponse, error) { - // If the logout request comes with an API token from a known - // remote cluster, redirect to that cluster's logout handler - // so it has an opportunity to clear sessions, expire tokens, - // etc. Otherwise use the local endpoint. - reqauth, ok := auth.FromContext(ctx) - if !ok || len(reqauth.Tokens) == 0 || len(reqauth.Tokens[0]) < 8 || !strings.HasPrefix(reqauth.Tokens[0], "v2/") { - return conn.local.Logout(ctx, options) - } - id := reqauth.Tokens[0][3:8] - if id == conn.cluster.ClusterID { - return conn.local.Logout(ctx, options) - } - remote, ok := conn.remotes[id] - if !ok { - return conn.local.Logout(ctx, options) + // If the token was issued by another cluster, we want to issue a logout + // request to the issuing instance to invalidate the token federation-wide. + // If this federation has a login cluster, that's always considered the + // issuing cluster. + // Otherwise, if this is a v2 token, use the UUID to find the issuing + // cluster. + // Note that remoteBE may still be conn.local even *after* one of these + // conditions is true. + var remoteBE backend = conn.local + if conn.cluster.Login.LoginCluster != "" { + remoteBE = conn.chooseBackend(conn.cluster.Login.LoginCluster) + } else { + reqauth, ok := auth.FromContext(ctx) + if ok && len(reqauth.Tokens) > 0 && v2TokenRegexp.MatchString(reqauth.Tokens[0]) { + remoteBE = conn.chooseBackend(reqauth.Tokens[0][3:8]) + } } - baseURL := remote.BaseURL() - target, err := baseURL.Parse(arvados.EndpointLogout.Path) - if err != nil { - return arvados.LogoutResponse{}, fmt.Errorf("internal error getting redirect target: %s", err) + + // We always want to invalidate the token locally. Start that process. + var localResponse arvados.LogoutResponse + var localErr error + wg := sync.WaitGroup{} + wg.Add(1) + go func() { + localResponse, localErr = conn.local.Logout(ctx, options) + wg.Done() + }() + + // If the token was issued by another cluster, log out there too. + if remoteBE != conn.local { + response, err := remoteBE.Logout(ctx, options) + // If the issuing cluster returns a redirect or error, that's more + // important to return to the user than anything that happens locally. + if response.RedirectLocation != "" || err != nil { + return response, err + } } - target.RawQuery = url.Values{"return_to": {options.ReturnTo}}.Encode() - return arvados.LogoutResponse{RedirectLocation: target.String()}, nil + + // Either the local cluster is the issuing cluster, or the issuing cluster's + // response was uninteresting. + wg.Wait() + return localResponse, localErr } func (conn *Conn) AuthorizedKeyCreate(ctx context.Context, options arvados.CreateOptions) (arvados.AuthorizedKey, error) { diff --git a/lib/controller/federation/login_test.go b/lib/controller/federation/login_test.go index a6743b320b..ab39619c79 100644 --- a/lib/controller/federation/login_test.go +++ b/lib/controller/federation/login_test.go @@ -8,10 +8,8 @@ import ( "context" "net/url" - "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" check "gopkg.in/check.v1" ) @@ -40,40 +38,3 @@ func (s *LoginSuite) TestDeferToLoginCluster(c *check.C) { c.Check(remotePresent, check.Equals, remote != "") } } - -func (s *LoginSuite) TestLogout(c *check.C) { - otherOrigin := arvados.URL{Scheme: "https", Host: "app.example.com", Path: "/"} - otherURL := "https://app.example.com/foo" - s.cluster.Services.Workbench1.ExternalURL = arvados.URL{Scheme: "https", Host: "workbench1.example.com"} - s.cluster.Services.Workbench2.ExternalURL = arvados.URL{Scheme: "https", Host: "workbench2.example.com"} - s.cluster.Login.TrustedClients = map[arvados.URL]struct{}{otherOrigin: {}} - s.addHTTPRemote(c, "zhome", &arvadostest.APIStub{}) - s.cluster.Login.LoginCluster = "zhome" - // s.fed is already set by SetUpTest, but we need to - // reinitialize with the above config changes. - s.fed = New(s.ctx, s.cluster, nil, (&ctrlctx.DBConnector{PostgreSQL: s.cluster.PostgreSQL}).GetDB) - - for _, trial := range []struct { - token string - returnTo string - target string - }{ - {token: "", returnTo: "", target: s.cluster.Services.Workbench2.ExternalURL.String()}, - {token: "", returnTo: otherURL, target: otherURL}, - {token: "zzzzzzzzzzzzzzzzzzzzz", returnTo: otherURL, target: otherURL}, - {token: "v2/zzzzz-aaaaa-aaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", returnTo: otherURL, target: otherURL}, - {token: "v2/zhome-aaaaa-aaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", returnTo: otherURL, target: "http://" + s.cluster.RemoteClusters["zhome"].Host + "/logout?" + url.Values{"return_to": {otherURL}}.Encode()}, - } { - c.Logf("trial %#v", trial) - ctx := s.ctx - if trial.token != "" { - ctx = auth.NewContext(ctx, &auth.Credentials{Tokens: []string{trial.token}}) - } - resp, err := s.fed.Logout(ctx, arvados.LogoutOptions{ReturnTo: trial.returnTo}) - c.Assert(err, check.IsNil) - c.Logf(" RedirectLocation %q", resp.RedirectLocation) - target, err := url.Parse(resp.RedirectLocation) - c.Check(err, check.IsNil) - c.Check(target.String(), check.Equals, trial.target) - } -} diff --git a/lib/controller/federation/logout_test.go b/lib/controller/federation/logout_test.go new file mode 100644 index 0000000000..af6f6d9ed2 --- /dev/null +++ b/lib/controller/federation/logout_test.go @@ -0,0 +1,246 @@ +// Copyright (C) The Arvados Authors. All rights reserved. +// +// SPDX-License-Identifier: AGPL-3.0 + +package federation + +import ( + "context" + "errors" + "fmt" + "net/url" + + "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" + check "gopkg.in/check.v1" +) + +var _ = check.Suite(&LogoutSuite{}) +var emptyURL = &url.URL{} + +type LogoutStub struct { + arvadostest.APIStub + redirectLocation *url.URL +} + +func (as *LogoutStub) CheckCalls(c *check.C, returnURL *url.URL) bool { + actual := as.APIStub.Calls(as.APIStub.Logout) + allOK := c.Check(actual, check.Not(check.HasLen), 0, + check.Commentf("Logout stub never called")) + expected := returnURL.String() + for _, call := range actual { + opts, ok := call.Options.(arvados.LogoutOptions) + allOK = c.Check(ok, check.Equals, true, + check.Commentf("call options were not LogoutOptions")) && + c.Check(opts.ReturnTo, check.Equals, expected) && + allOK + } + return allOK +} + +func (as *LogoutStub) Logout(ctx context.Context, options arvados.LogoutOptions) (arvados.LogoutResponse, error) { + as.APIStub.Logout(ctx, options) + loc := as.redirectLocation.String() + if loc == "" { + loc = options.ReturnTo + } + return arvados.LogoutResponse{ + RedirectLocation: loc, + }, as.Error +} + +type LogoutSuite struct { + FederationSuite +} + +func (s *LogoutSuite) badReturnURL(path string) *url.URL { + return &url.URL{ + Scheme: "https", + Host: "example.net", + Path: path, + } +} + +func (s *LogoutSuite) goodReturnURL(path string) *url.URL { + u, _ := url.Parse(s.cluster.Services.Workbench2.ExternalURL.String()) + u.Path = path + return u +} + +func (s *LogoutSuite) setupFederation(loginCluster string) { + if loginCluster == "" { + s.cluster.Login.Test.Enable = true + } else { + s.cluster.Login.LoginCluster = loginCluster + } + dbconn := ctrlctx.DBConnector{PostgreSQL: s.cluster.PostgreSQL} + s.fed = New(s.ctx, s.cluster, nil, dbconn.GetDB) +} + +func (s *LogoutSuite) setupStub(c *check.C, id string, stubURL *url.URL, stubErr error) *LogoutStub { + loc, err := url.Parse(stubURL.String()) + c.Check(err, check.IsNil) + stub := LogoutStub{redirectLocation: loc} + stub.Error = stubErr + if id == s.cluster.ClusterID { + s.fed.local = &stub + } else { + s.addDirectRemote(c, id, &stub) + } + return &stub +} + +func (s *LogoutSuite) v2Token(clusterID string) string { + return fmt.Sprintf("v2/%s-gj3su-12345abcde67890/abcdefghijklmnopqrstuvwxy", clusterID) +} + +func (s *LogoutSuite) TestLocalLogoutOK(c *check.C) { + s.setupFederation("") + resp, err := s.fed.Logout(s.ctx, arvados.LogoutOptions{}) + c.Check(err, check.IsNil) + c.Check(resp.RedirectLocation, check.Equals, s.cluster.Services.Workbench2.ExternalURL.String()) +} + +func (s *LogoutSuite) TestLocalLogoutRedirect(c *check.C) { + s.setupFederation("") + expURL := s.cluster.Services.Workbench1.ExternalURL + opts := arvados.LogoutOptions{ReturnTo: expURL.String()} + resp, err := s.fed.Logout(s.ctx, opts) + c.Check(err, check.IsNil) + c.Check(resp.RedirectLocation, check.Equals, expURL.String()) +} + +func (s *LogoutSuite) TestLocalLogoutBadRequestError(c *check.C) { + s.setupFederation("") + returnTo := s.badReturnURL("TestLocalLogoutBadRequestError") + opts := arvados.LogoutOptions{ReturnTo: returnTo.String()} + _, err := s.fed.Logout(s.ctx, opts) + c.Check(err, check.NotNil) +} + +func (s *LogoutSuite) TestRemoteLogoutRedirect(c *check.C) { + s.setupFederation("zhome") + redirect := url.URL{Scheme: "https", Host: "example.com"} + loginStub := s.setupStub(c, "zhome", &redirect, nil) + returnTo := s.goodReturnURL("TestRemoteLogoutRedirect") + resp, err := s.fed.Logout(s.ctx, arvados.LogoutOptions{ReturnTo: returnTo.String()}) + c.Check(err, check.IsNil) + c.Check(resp.RedirectLocation, check.Equals, redirect.String()) + loginStub.CheckCalls(c, returnTo) +} + +func (s *LogoutSuite) TestRemoteLogoutError(c *check.C) { + s.setupFederation("zhome") + expErr := errors.New("TestRemoteLogoutError expErr") + loginStub := s.setupStub(c, "zhome", emptyURL, expErr) + returnTo := s.goodReturnURL("TestRemoteLogoutError") + _, err := s.fed.Logout(s.ctx, arvados.LogoutOptions{ReturnTo: returnTo.String()}) + c.Check(err, check.Equals, expErr) + loginStub.CheckCalls(c, returnTo) +} + +func (s *LogoutSuite) TestRemoteLogoutLocalRedirect(c *check.C) { + s.setupFederation("zhome") + loginStub := s.setupStub(c, "zhome", emptyURL, nil) + redirect := url.URL{Scheme: "https", Host: "example.com"} + localStub := s.setupStub(c, "aaaaa", &redirect, nil) + resp, err := s.fed.Logout(s.ctx, arvados.LogoutOptions{}) + c.Check(err, check.IsNil) + c.Check(resp.RedirectLocation, check.Equals, redirect.String()) + // emptyURL to match the empty LogoutOptions + loginStub.CheckCalls(c, emptyURL) + localStub.CheckCalls(c, emptyURL) +} + +func (s *LogoutSuite) TestRemoteLogoutLocalError(c *check.C) { + s.setupFederation("zhome") + expErr := errors.New("TestRemoteLogoutLocalError expErr") + loginStub := s.setupStub(c, "zhome", emptyURL, nil) + localStub := s.setupStub(c, "aaaaa", emptyURL, expErr) + _, err := s.fed.Logout(s.ctx, arvados.LogoutOptions{}) + c.Check(err, check.Equals, expErr) + loginStub.CheckCalls(c, emptyURL) + localStub.CheckCalls(c, emptyURL) +} + +func (s *LogoutSuite) TestV2TokenRedirect(c *check.C) { + s.setupFederation("") + redirect := url.URL{Scheme: "https", Host: "example.com"} + returnTo := s.goodReturnURL("TestV2TokenRedirect") + localErr := errors.New("TestV2TokenRedirect error") + tokenStub := s.setupStub(c, "zzzzz", &redirect, nil) + s.setupStub(c, "aaaaa", emptyURL, localErr) + tokens := []string{s.v2Token("zzzzz")} + ctx := auth.NewContext(s.ctx, &auth.Credentials{Tokens: tokens}) + resp, err := s.fed.Logout(ctx, arvados.LogoutOptions{ReturnTo: returnTo.String()}) + c.Check(err, check.IsNil) + c.Check(resp.RedirectLocation, check.Equals, redirect.String()) + tokenStub.CheckCalls(c, returnTo) +} + +func (s *LogoutSuite) TestV2TokenError(c *check.C) { + s.setupFederation("") + returnTo := s.goodReturnURL("TestV2TokenError") + tokenErr := errors.New("TestV2TokenError error") + tokenStub := s.setupStub(c, "zzzzz", emptyURL, tokenErr) + s.setupStub(c, "aaaaa", emptyURL, nil) + tokens := []string{s.v2Token("zzzzz")} + ctx := auth.NewContext(s.ctx, &auth.Credentials{Tokens: tokens}) + _, err := s.fed.Logout(ctx, arvados.LogoutOptions{ReturnTo: returnTo.String()}) + c.Check(err, check.Equals, tokenErr) + tokenStub.CheckCalls(c, returnTo) +} + +func (s *LogoutSuite) TestV2TokenLocalRedirect(c *check.C) { + s.setupFederation("") + redirect := url.URL{Scheme: "https", Host: "example.com"} + tokenStub := s.setupStub(c, "zzzzz", emptyURL, nil) + localStub := s.setupStub(c, "aaaaa", &redirect, nil) + tokens := []string{s.v2Token("zzzzz")} + ctx := auth.NewContext(s.ctx, &auth.Credentials{Tokens: tokens}) + resp, err := s.fed.Logout(ctx, arvados.LogoutOptions{}) + c.Check(err, check.IsNil) + c.Check(resp.RedirectLocation, check.Equals, redirect.String()) + tokenStub.CheckCalls(c, emptyURL) + localStub.CheckCalls(c, emptyURL) +} + +func (s *LogoutSuite) TestV2TokenLocalError(c *check.C) { + s.setupFederation("") + tokenErr := errors.New("TestV2TokenLocalError error") + tokenStub := s.setupStub(c, "zzzzz", emptyURL, nil) + localStub := s.setupStub(c, "aaaaa", emptyURL, tokenErr) + tokens := []string{s.v2Token("zzzzz")} + ctx := auth.NewContext(s.ctx, &auth.Credentials{Tokens: tokens}) + _, err := s.fed.Logout(ctx, arvados.LogoutOptions{}) + c.Check(err, check.Equals, tokenErr) + tokenStub.CheckCalls(c, emptyURL) + localStub.CheckCalls(c, emptyURL) +} + +func (s *LogoutSuite) TestV2LocalTokenRedirect(c *check.C) { + s.setupFederation("") + redirect := url.URL{Scheme: "https", Host: "example.com"} + returnTo := s.goodReturnURL("TestV2LocalTokenRedirect") + localStub := s.setupStub(c, "aaaaa", &redirect, nil) + tokens := []string{s.v2Token("aaaaa")} + ctx := auth.NewContext(s.ctx, &auth.Credentials{Tokens: tokens}) + resp, err := s.fed.Logout(ctx, arvados.LogoutOptions{ReturnTo: returnTo.String()}) + c.Check(err, check.IsNil) + c.Check(resp.RedirectLocation, check.Equals, redirect.String()) + localStub.CheckCalls(c, returnTo) +} + +func (s *LogoutSuite) TestV2LocalTokenError(c *check.C) { + s.setupFederation("") + returnTo := s.goodReturnURL("TestV2LocalTokenError") + tokenErr := errors.New("TestV2LocalTokenError error") + localStub := s.setupStub(c, "aaaaa", emptyURL, tokenErr) + tokens := []string{s.v2Token("aaaaa")} + ctx := auth.NewContext(s.ctx, &auth.Credentials{Tokens: tokens}) + _, err := s.fed.Logout(ctx, arvados.LogoutOptions{ReturnTo: returnTo.String()}) + c.Check(err, check.Equals, tokenErr) + localStub.CheckCalls(c, returnTo) +} -- 2.30.2