From 54d8c4e41a276ac82c79506f63907a108ebd9bfd Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Thu, 4 Mar 2021 16:51:12 -0500 Subject: [PATCH] 16669: Accept OIDC access token in federated requests. ...provided both local and remote clusters use the same login cluster. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- lib/controller/federation.go | 4 ++-- lib/controller/federation/conn.go | 3 +++ lib/controller/integration_test.go | 26 +++++++++++++++++--------- lib/controller/localdb/login_oidc.go | 1 + 4 files changed, 23 insertions(+), 11 deletions(-) diff --git a/lib/controller/federation.go b/lib/controller/federation.go index cab5e4c4ca..419d8b0104 100644 --- a/lib/controller/federation.go +++ b/lib/controller/federation.go @@ -263,10 +263,10 @@ func (h *Handler) saltAuthToken(req *http.Request, remote string) (updatedReq *h return updatedReq, nil } - ctxlog.FromContext(req.Context()).Infof("saltAuthToken: cluster %s token %s remote %s", h.Cluster.ClusterID, creds.Tokens[0], remote) + ctxlog.FromContext(req.Context()).Debugf("saltAuthToken: cluster %s token %s remote %s", h.Cluster.ClusterID, creds.Tokens[0], remote) token, err := auth.SaltToken(creds.Tokens[0], remote) - if err == auth.ErrObsoleteToken { + if err == auth.ErrObsoleteToken || err == auth.ErrTokenFormat { // If the token exists in our own database for our own // user, salt it for the remote. Otherwise, assume it // was issued by the remote, and pass it through diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go index b86266d67e..0f063d1238 100644 --- a/lib/controller/federation/conn.go +++ b/lib/controller/federation/conn.go @@ -69,6 +69,9 @@ func saltedTokenProvider(local backend, remoteID string) rpc.TokenProvider { tokens = append(tokens, salted) case auth.ErrSalted: tokens = append(tokens, token) + case auth.ErrTokenFormat: + // pass through unmodified (assume it's an OIDC access token) + tokens = append(tokens, token) case auth.ErrObsoleteToken: ctx := auth.NewContext(ctx, &auth.Credentials{Tokens: []string{token}}) aca, err := local.APIClientAuthorizationCurrent(ctx, arvados.GetOptions{}) diff --git a/lib/controller/integration_test.go b/lib/controller/integration_test.go index 3d0639f6cc..db1f7f0d0c 100644 --- a/lib/controller/integration_test.go +++ b/lib/controller/integration_test.go @@ -683,15 +683,16 @@ func (s *IntegrationSuite) TestOIDCAccessTokenAuth(c *check.C) { accesstoken := s.oidcprovider.ValidAccessToken() for _, clusterID := range []string{"z1111", "z2222"} { - c.Logf("trying clusterid %s", clusterID) - - conn := s.testClusters[clusterID].Conn() - ctx, ac, kc := s.testClusters[clusterID].ClientsWithToken(accesstoken) var coll arvados.Collection // Write some file data and create a collection { + c.Logf("save collection to %s", clusterID) + + conn := s.testClusters[clusterID].Conn() + ctx, ac, kc := s.testClusters[clusterID].ClientsWithToken(accesstoken) + fs, err := coll.FileSystem(ac, kc) c.Assert(err, check.IsNil) f, err := fs.OpenFile("test.txt", os.O_CREATE|os.O_RDWR, 0777) @@ -708,15 +709,22 @@ func (s *IntegrationSuite) TestOIDCAccessTokenAuth(c *check.C) { c.Assert(err, check.IsNil) } - // Read the collection & file data - { + // Read the collection & file data -- both from the + // cluster where it was created, and from the other + // cluster. + for _, readClusterID := range []string{"z1111", "z2222", "z3333"} { + c.Logf("retrieve %s from %s", coll.UUID, readClusterID) + + conn := s.testClusters[readClusterID].Conn() + ctx, ac, kc := s.testClusters[readClusterID].ClientsWithToken(accesstoken) + user, err := conn.UserGetCurrent(ctx, arvados.GetOptions{}) c.Assert(err, check.IsNil) c.Check(user.FullName, check.Equals, "Example User") - coll, err = conn.CollectionGet(ctx, arvados.GetOptions{UUID: coll.UUID}) + readcoll, err := conn.CollectionGet(ctx, arvados.GetOptions{UUID: coll.UUID}) c.Assert(err, check.IsNil) - c.Check(coll.ManifestText, check.Not(check.Equals), "") - fs, err := coll.FileSystem(ac, kc) + c.Check(readcoll.ManifestText, check.Not(check.Equals), "") + fs, err := readcoll.FileSystem(ac, kc) c.Assert(err, check.IsNil) f, err := fs.Open("test.txt") c.Assert(err, check.IsNil) diff --git a/lib/controller/localdb/login_oidc.go b/lib/controller/localdb/login_oidc.go index 74b8929a21..73b5577237 100644 --- a/lib/controller/localdb/login_oidc.go +++ b/lib/controller/localdb/login_oidc.go @@ -129,6 +129,7 @@ func (ctrl *oidcLoginController) Login(ctx context.Context, opts arvados.LoginOp if err != nil { return loginError(fmt.Errorf("error in OAuth2 exchange: %s", err)) } + ctxlog.FromContext(ctx).WithField("oauth2Token", oauth2Token).Debug("oauth2 exchange succeeded") rawIDToken, ok := oauth2Token.Extra("id_token").(string) if !ok { return loginError(errors.New("error in OAuth2 exchange: no ID token in OAuth2 token")) -- 2.30.2