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@di-pentima.com.ar>

diff --git a/lib/controller/localdb/login_testuser.go b/lib/controller/localdb/login_testuser.go
index 2e99034b65a675439b3dac8c8a844559dce458dc..e2f38f35b5d489bdac68ea24d67ff49aca21d771 100644 (file)
--- 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 417fdc06679a6f3cc33ab5bb21d0b6d4fa68ed13..66089539cb703cc8789129886065d160da6f6127 100644 (file)
--- 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