[ARVADOS] updated: 2.1.0-539-g64e4f7598

Git user git at public.arvados.org
Tue Mar 30 23:00:32 UTC 2021


Summary of changes:
 lib/controller/localdb/login_testuser.go |  2 +-
 lib/controller/localdb/logout.go         | 52 +++++++++++++++++++++++---------
 2 files changed, 39 insertions(+), 15 deletions(-)

       via  64e4f759884fadc7fb3b984eb773a1dd89b7848f (commit)
      from  94b3b18d028468440d793c2f78da61acc0ba4270 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 64e4f759884fadc7fb3b984eb773a1dd89b7848f
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Tue Mar 30 19:57:04 2021 -0300

    16159: Validates token uuid & secret before expiring.
    
    Don't error out when no token is provided, to support old clients.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/lib/controller/localdb/login_testuser.go b/lib/controller/localdb/login_testuser.go
index 2e99034b6..e2f38f35b 100644
--- a/lib/controller/localdb/login_testuser.go
+++ b/lib/controller/localdb/login_testuser.go
@@ -22,7 +22,7 @@ type testLoginController struct {
 }
 
 func (ctrl *testLoginController) Logout(ctx context.Context, opts arvados.LogoutOptions) (arvados.LogoutResponse, error) {
-	err := ctrl.Parent.ExpireAPIClientAuthorization(ctx)
+	err := ctrl.Parent.expireAPIClientAuthorization(ctx)
 	if err != nil {
 		return arvados.LogoutResponse{}, err
 	}
diff --git a/lib/controller/localdb/logout.go b/lib/controller/localdb/logout.go
index 417fdc066..66089539c 100644
--- a/lib/controller/localdb/logout.go
+++ b/lib/controller/localdb/logout.go
@@ -6,40 +6,59 @@ package localdb
 
 import (
 	"context"
+	"database/sql"
 	"errors"
 	"fmt"
 	"strings"
 
 	"git.arvados.org/arvados.git/lib/ctrlctx"
 	"git.arvados.org/arvados.git/sdk/go/auth"
+	"git.arvados.org/arvados.git/sdk/go/ctxlog"
 )
 
-func (conn *Conn) ExpireAPIClientAuthorization(ctx context.Context) error {
+func (conn *Conn) expireAPIClientAuthorization(ctx context.Context) error {
 	creds, ok := auth.FromContext(ctx)
-
 	if !ok {
 		return errors.New("credentials not found from context")
 	}
 
-	if len(creds.Tokens) < 1 {
-		return errors.New("no tokens found to expire")
+	if len(creds.Tokens) == 0 {
+		// Old client may not have provided the token to expire
+		return nil
+	}
+
+	tx, err := ctrlctx.CurrentTx(ctx)
+	if err != nil {
+		return err
 	}
 
 	token := creds.Tokens[0]
-	tokensecret := token
-	if strings.Contains(token, "/") {
-		tokenparts := strings.Split(token, "/")
-		if len(tokenparts) >= 3 {
-			tokensecret = tokenparts[2]
+	tokenSecret := token
+	var tokenUuid string
+	if strings.HasPrefix(token, "v2/") {
+		tokenParts := strings.Split(token, "/")
+		if len(tokenParts) >= 3 {
+			tokenUuid = tokenParts[1]
+			tokenSecret = tokenParts[2]
 		}
 	}
 
-	tx, err := ctrlctx.CurrentTx(ctx)
-	if err != nil {
+	var retrievedUuid string
+	err = tx.QueryRowContext(ctx, `SELECT uuid FROM api_client_authorizations WHERE api_token=$1 AND (expires_at IS NULL OR expires_at > current_timestamp AT TIME ZONE 'UTC') LIMIT 1`, tokenSecret).Scan(&retrievedUuid)
+	if err == sql.ErrNoRows {
+		ctxlog.FromContext(ctx).Debugf("expireAPIClientAuthorization(%s): not found in database", token)
+		return nil
+	} else if err != nil {
+		ctxlog.FromContext(ctx).WithError(err).Debugf("expireAPIClientAuthorization(%s): database error", token)
 		return err
 	}
+	if tokenUuid != "" && retrievedUuid != tokenUuid {
+		// secret part matches, but UUID doesn't -- somewhat surprising
+		ctxlog.FromContext(ctx).Debugf("expireAPIClientAuthorization(%s): secret part found, but with different UUID: %s", tokenSecret, retrievedUuid)
+		return nil
+	}
 
-	res, err := tx.ExecContext(ctx, "UPDATE api_client_authorizations SET expires_at=current_timestamp WHERE api_token=$1", tokensecret)
+	res, err := tx.ExecContext(ctx, "UPDATE api_client_authorizations SET expires_at=current_timestamp AT TIME ZONE 'UTC' WHERE (expires_at IS NULL OR expires_at > current_timestamp AT TIME ZONE 'UTC' AND api_token=$1)", tokenSecret)
 	if err != nil {
 		return err
 	}
@@ -48,8 +67,13 @@ func (conn *Conn) ExpireAPIClientAuthorization(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
-	if rows != 1 {
-		return fmt.Errorf("token expiration affected rows: %d -  token: %s", rows, token)
+	if rows == 0 {
+		ctxlog.FromContext(ctx).Debugf("expireAPIClientAuthorization(%s): no rows were updated", tokenSecret)
+		return fmt.Errorf("couldn't expire provided token")
+	} else if rows > 1 {
+		ctxlog.FromContext(ctx).Debugf("expireAPIClientAuthorization(%s): multiple (%d) rows updated", tokenSecret, rows)
+	} else {
+		ctxlog.FromContext(ctx).Debugf("expireAPIClientAuthorization(%s): ok", tokenSecret)
 	}
 
 	return nil

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list