From: Tom Clegg <tom@curii.com>
Date: Wed, 4 Oct 2023 15:07:20 +0000 (-0400)
Subject: 20300: Fix controller-to-railsapi callback usage.
X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/e84fb97573407f2ddabdcfb3b905d201d0a300a5

20300: Fix controller-to-railsapi callback usage.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>
---

diff --git a/lib/boot/helpers.go b/lib/boot/helpers.go
index 77036e9340..6a5514ada0 100644
--- a/lib/boot/helpers.go
+++ b/lib/boot/helpers.go
@@ -45,7 +45,7 @@ func (super *Supervisor) ClientsWithToken(clusterID, token string) (context.Cont
 // communicating with the cluster on behalf of the 'example' user.
 func (super *Supervisor) UserClients(clusterID string, rootctx context.Context, c *check.C, conn *rpc.Conn, authEmail string, activate bool) (context.Context, *arvados.Client, *keepclient.KeepClient, arvados.User) {
 	login, err := conn.UserSessionCreate(rootctx, rpc.UserSessionCreateOptions{
-		ReturnTo: ",https://example.com",
+		ReturnTo: ",https://controller.api.client.invalid",
 		AuthInfo: rpc.UserSessionAuthInfo{
 			Email:     authEmail,
 			FirstName: "Example",
diff --git a/lib/controller/localdb/login_oidc.go b/lib/controller/localdb/login_oidc.go
index 65e2e250e5..a87d13959f 100644
--- a/lib/controller/localdb/login_oidc.go
+++ b/lib/controller/localdb/login_oidc.go
@@ -154,10 +154,39 @@ func (ctrl *oidcLoginController) Login(ctx context.Context, opts arvados.LoginOp
 		return loginError(err)
 	}
 	ctxRoot := auth.NewContext(ctx, &auth.Credentials{Tokens: []string{ctrl.Cluster.SystemRootToken}})
-	return ctrl.Parent.UserSessionCreate(ctxRoot, rpc.UserSessionCreateOptions{
-		ReturnTo: state.Remote + "," + state.ReturnTo,
+	resp, err := ctrl.Parent.UserSessionCreate(ctxRoot, rpc.UserSessionCreateOptions{
+		ReturnTo: state.Remote + ",https://controller.api.client.invalid",
 		AuthInfo: *authinfo,
 	})
+	if err != nil {
+		return resp, err
+	}
+	// Extract token from rails' UserSessionCreate response, and
+	// attach it to our caller's desired ReturnTo URL.  The Rails
+	// handler explicitly disallows sending the real ReturnTo as a
+	// belt-and-suspenders defence against Rails accidentally
+	// exposing an additional login relay.
+	u, err := url.Parse(resp.RedirectLocation)
+	if err != nil {
+		return resp, err
+	}
+	token := u.Query().Get("api_token")
+	if token == "" {
+		resp.RedirectLocation = state.ReturnTo
+	} else {
+		u, err := url.Parse(state.ReturnTo)
+		if err != nil {
+			return resp, err
+		}
+		q := u.Query()
+		if q == nil {
+			q = url.Values{}
+		}
+		q.Set("api_token", token)
+		u.RawQuery = q.Encode()
+		resp.RedirectLocation = u.String()
+	}
+	return resp, nil
 }
 
 func (ctrl *oidcLoginController) UserAuthenticate(ctx context.Context, opts arvados.UserAuthenticateOptions) (arvados.APIClientAuthorization, error) {