[arvados] created: 2.7.0-5321-ge634df22d8

git repository hosting git at public.arvados.org
Wed Nov 8 03:44:59 UTC 2023


        at  e634df22d847fd535d0daefc9fee5b5e2ca89ac3 (commit)


commit e634df22d847fd535d0daefc9fee5b5e2ca89ac3
Author: Brett Smith <brett.smith at curii.com>
Date:   Tue Nov 7 22:43:42 2023 -0500

    21021: WIP - DO NOT MERGE
    
    Arvados-DCO-1.1-Signed-off-by: Brett Smith <brett.smith at curii.com>

diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index b4b7476440..d957e73b53 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,48 @@ 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)
-	}
-	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)
+	logger := ctxlog.FromContext(ctx)
+	var remoteBE backend = conn.local
+	if conn.cluster.Login.LoginCluster != "" {
+		remoteBE = conn.chooseBackend(conn.cluster.Login.LoginCluster)
+		logger.Debugf("remote for LoginCluster %v is %v",
+			conn.cluster.Login.LoginCluster, remoteBE.BaseURL())
+	} 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])
+		}
+		logger.Debugf("remote for v2 token %v is %v",
+			reqauth.Tokens[0][3:30], remoteBE.BaseURL())
+	}
+
+	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 remoteBE != conn.local {
+		logger.Debugf("requesting remote logout")
+		response, err := remoteBE.Logout(ctx, options)
+		if response.RedirectLocation != "" || err != nil {
+			logger.Debugf("returning remote logout response")
+			return response, err
+		}
 	}
-	target.RawQuery = url.Values{"return_to": {options.ReturnTo}}.Encode()
-	return arvados.LogoutResponse{RedirectLocation: target.String()}, nil
+	logger.Debugf("returning local logout response")
+	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..63ff643368 100644
--- a/lib/controller/federation/login_test.go
+++ b/lib/controller/federation/login_test.go
@@ -40,40 +40,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..cd9a364291
--- /dev/null
+++ b/lib/controller/federation/logout_test.go
@@ -0,0 +1,159 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package federation
+
+import (
+	"context"
+	"errors"
+	"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"
+	check "gopkg.in/check.v1"
+)
+
+var _ = check.Suite(&LogoutSuite{})
+
+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(len(actual), check.Not(check.Equals), 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 {
+	stub := LogoutStub{redirectLocation: stubURL}
+	stub.Error = stubErr
+	if id == s.cluster.ClusterID {
+		s.fed.local = &stub
+	} else {
+		s.addDirectRemote(c, id, &stub)
+	}
+	return stub
+}
+
+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", url.URL{}, expErr)
+	returnTo := s.goodReturnURL("TestRemoteLogoutError")
+	_, err := s.fed.Logout(s.ctx, arvados.LogoutOptions{ReturnTo: returnTo.String()})
+	c.Check(err, check.ErrorMatches, `.*: 500 Internal Server Error: TestRemoteLogoutError expErr`)
+	loginStub.CheckCalls(c, returnTo)
+}
+
+func (s *LogoutSuite) TestRemoteLogoutLocalRedirect(c *check.C) {
+	s.setupFederation("zhome")
+	emptyURL := url.URL{}
+	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", url.URL{}, nil)
+	localStub := s.setupStub(c, "aaaaa", url.URL{}, expErr)
+	returnTo := s.goodReturnURL("TestRemoteLogoutLocalError")
+	_, err := s.fed.Logout(s.ctx, arvados.LogoutOptions{ReturnTo: returnTo.String()})
+	c.Check(err, check.ErrorMatches, `.*: 500 Internal Server Error: TestRemoteLogoutLocalError expErr`)
+	loginStub.CheckCalls(c, returnTo)
+	localStub.CheckCalls(c, returnTo)
+}

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list