[arvados] created: 2.1.0-2932-gb855852ff

git repository hosting git at public.arvados.org
Fri Sep 23 13:54:56 UTC 2022


        at  b855852ff73705f05b5de07e4615c1f3e75ead85 (commit)


commit b855852ff73705f05b5de07e4615c1f3e75ead85
Author: Tom Clegg <tom at curii.com>
Date:   Fri Sep 23 09:54:32 2022 -0400

    19388: Log user activity.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 444398bc3..816d0f99e 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -373,6 +373,18 @@ Clusters:
       # cluster.
       RoleGroupsVisibleToAll: true
 
+      # During each period, a log entry with event_type="activity"
+      # will be recorded for each user who is active during that
+      # period. The object_uuid attribute will indicate the user's
+      # UUID.
+      #
+      # Multiple log entries for the same user may be generated during
+      # a period if there are multiple controller processes or a
+      # controller process is restarted.
+      #
+      # Use 0 to disable activity logging.
+      ActivityLoggingPeriod: 24h
+
     AuditLogs:
       # Time to keep audit logs, in seconds. (An audit log is a row added
       # to the "logs" table in the PostgreSQL database each time an
diff --git a/lib/config/export.go b/lib/config/export.go
index a55295d12..fb17a45c8 100644
--- a/lib/config/export.go
+++ b/lib/config/export.go
@@ -226,6 +226,7 @@ var whitelist = map[string]bool{
 	"TLS.Key":                                             false,
 	"Users":                                               true,
 	"Users.ActivatedUsersAreVisibleToOthers":              false,
+	"Users.ActivityLoggingPeriod":                         false,
 	"Users.AdminNotifierEmailFrom":                        false,
 	"Users.AnonymousUserToken":                            true,
 	"Users.AutoAdminFirstUser":                            false,
diff --git a/lib/controller/localdb/collection.go b/lib/controller/localdb/collection.go
index 868e466e9..581595e5e 100644
--- a/lib/controller/localdb/collection.go
+++ b/lib/controller/localdb/collection.go
@@ -22,6 +22,7 @@ import (
 // CollectionGet defers to railsProxy for everything except blob
 // signatures.
 func (conn *Conn) CollectionGet(ctx context.Context, opts arvados.GetOptions) (arvados.Collection, error) {
+	conn.logActivity(ctx)
 	if len(opts.Select) > 0 {
 		// We need to know IsTrashed and TrashAt to implement
 		// signing properly, even if the caller doesn't want
@@ -39,6 +40,7 @@ func (conn *Conn) CollectionGet(ctx context.Context, opts arvados.GetOptions) (a
 // CollectionList defers to railsProxy for everything except blob
 // signatures.
 func (conn *Conn) CollectionList(ctx context.Context, opts arvados.ListOptions) (arvados.CollectionList, error) {
+	conn.logActivity(ctx)
 	if len(opts.Select) > 0 {
 		// We need to know IsTrashed and TrashAt to implement
 		// signing properly, even if the caller doesn't want
@@ -58,6 +60,7 @@ func (conn *Conn) CollectionList(ctx context.Context, opts arvados.ListOptions)
 // CollectionCreate defers to railsProxy for everything except blob
 // signatures and vocabulary checking.
 func (conn *Conn) CollectionCreate(ctx context.Context, opts arvados.CreateOptions) (arvados.Collection, error) {
+	conn.logActivity(ctx)
 	err := conn.checkProperties(ctx, opts.Attrs["properties"])
 	if err != nil {
 		return arvados.Collection{}, err
@@ -82,6 +85,7 @@ func (conn *Conn) CollectionCreate(ctx context.Context, opts arvados.CreateOptio
 // CollectionUpdate defers to railsProxy for everything except blob
 // signatures and vocabulary checking.
 func (conn *Conn) CollectionUpdate(ctx context.Context, opts arvados.UpdateOptions) (arvados.Collection, error) {
+	conn.logActivity(ctx)
 	err := conn.checkProperties(ctx, opts.Attrs["properties"])
 	if err != nil {
 		return arvados.Collection{}, err
diff --git a/lib/controller/localdb/conn.go b/lib/controller/localdb/conn.go
index a36822ad6..0420cf6f2 100644
--- a/lib/controller/localdb/conn.go
+++ b/lib/controller/localdb/conn.go
@@ -33,8 +33,11 @@ type Conn struct {
 	lastVocabularyRefreshCheck time.Time
 	lastVocabularyError        error
 	loginController
-	gwTunnels     map[string]*yamux.Session
-	gwTunnelsLock sync.Mutex
+	gwTunnels        map[string]*yamux.Session
+	gwTunnelsLock    sync.Mutex
+	activeUsers      map[string]bool
+	activeUsersLock  sync.Mutex
+	activeUsersReset time.Time
 }
 
 func NewConn(cluster *arvados.Cluster) *Conn {
diff --git a/lib/controller/localdb/container_request.go b/lib/controller/localdb/container_request.go
index 5b2ce95da..49e21840e 100644
--- a/lib/controller/localdb/container_request.go
+++ b/lib/controller/localdb/container_request.go
@@ -13,6 +13,7 @@ import (
 // ContainerRequestCreate defers to railsProxy for everything except
 // vocabulary checking.
 func (conn *Conn) ContainerRequestCreate(ctx context.Context, opts arvados.CreateOptions) (arvados.ContainerRequest, error) {
+	conn.logActivity(ctx)
 	err := conn.checkProperties(ctx, opts.Attrs["properties"])
 	if err != nil {
 		return arvados.ContainerRequest{}, err
@@ -27,6 +28,7 @@ func (conn *Conn) ContainerRequestCreate(ctx context.Context, opts arvados.Creat
 // ContainerRequestUpdate defers to railsProxy for everything except
 // vocabulary checking.
 func (conn *Conn) ContainerRequestUpdate(ctx context.Context, opts arvados.UpdateOptions) (arvados.ContainerRequest, error) {
+	conn.logActivity(ctx)
 	err := conn.checkProperties(ctx, opts.Attrs["properties"])
 	if err != nil {
 		return arvados.ContainerRequest{}, err
@@ -37,3 +39,18 @@ func (conn *Conn) ContainerRequestUpdate(ctx context.Context, opts arvados.Updat
 	}
 	return resp, nil
 }
+
+func (conn *Conn) ContainerRequestGet(ctx context.Context, opts arvados.GetOptions) (arvados.ContainerRequest, error) {
+	conn.logActivity(ctx)
+	return conn.railsProxy.ContainerRequestGet(ctx, opts)
+}
+
+func (conn *Conn) ContainerRequestList(ctx context.Context, opts arvados.ListOptions) (arvados.ContainerRequestList, error) {
+	conn.logActivity(ctx)
+	return conn.railsProxy.ContainerRequestList(ctx, opts)
+}
+
+func (conn *Conn) ContainerRequestDelete(ctx context.Context, opts arvados.DeleteOptions) (arvados.ContainerRequest, error) {
+	conn.logActivity(ctx)
+	return conn.railsProxy.ContainerRequestDelete(ctx, opts)
+}
diff --git a/lib/controller/localdb/log_activity.go b/lib/controller/localdb/log_activity.go
new file mode 100644
index 000000000..52b104234
--- /dev/null
+++ b/lib/controller/localdb/log_activity.go
@@ -0,0 +1,117 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package localdb
+
+import (
+	"context"
+	"time"
+
+	"git.arvados.org/arvados.git/lib/ctrlctx"
+	"git.arvados.org/arvados.git/sdk/go/arvados"
+	"git.arvados.org/arvados.git/sdk/go/ctxlog"
+)
+
+func (conn *Conn) logActivity(ctx context.Context) {
+	p := conn.cluster.Users.ActivityLoggingPeriod.Duration()
+	if p < 1 {
+		ctxlog.FromContext(ctx).Debug("logActivity disabled by config")
+		return
+	}
+	user, _, err := ctrlctx.CurrentAuth(ctx)
+	if err == ctrlctx.ErrUnauthenticated {
+		ctxlog.FromContext(ctx).Debug("logActivity skipped for unauthenticated request")
+		return
+	} else if err != nil {
+		ctxlog.FromContext(ctx).WithError(err).Error("logActivity CurrentAuth failed")
+		return
+	}
+	now := time.Now()
+	conn.activeUsersLock.Lock()
+	if conn.activeUsers == nil || conn.activeUsersReset.IsZero() || conn.activeUsersReset.Before(now) {
+		conn.activeUsersReset = alignedPeriod(now, p)
+		conn.activeUsers = map[string]bool{}
+	}
+	logged := conn.activeUsers[user.UUID]
+	if !logged {
+		// Prevent other concurrent calls from logging about
+		// this user until we finish.
+		conn.activeUsers[user.UUID] = true
+	}
+	conn.activeUsersLock.Unlock()
+	if logged {
+		return
+	}
+	defer func() {
+		// If we return without logging, reset the flag so we
+		// try again on the user's next API call.
+		if !logged {
+			conn.activeUsersLock.Lock()
+			conn.activeUsers[user.UUID] = false
+			conn.activeUsersLock.Unlock()
+		}
+	}()
+
+	tx, err := ctrlctx.NewTx(ctx)
+	if err != nil {
+		ctxlog.FromContext(ctx).WithError(err).Error("logActivity NewTx failed")
+		return
+	}
+	defer tx.Rollback()
+	_, err = tx.ExecContext(ctx, `
+insert into logs
+ (uuid,
+  modified_by_user_uuid, object_owner_uuid,
+  event_type,
+  summary,
+  object_uuid,
+  properties,
+  event_at, created_at, updated_at, modified_at)
+ values
+ ($1, $2, $2, $3, $4, $5, $6,
+  current_timestamp at time zone 'UTC',
+  current_timestamp at time zone 'UTC',
+  current_timestamp at time zone 'UTC',
+  current_timestamp at time zone 'UTC')
+ returning id`,
+		arvados.RandomUUID(conn.cluster.ClusterID, "57u5n"),
+		conn.cluster.ClusterID+"-tpzed-000000000000000", // both modified_by and object_owner
+		"activity",
+		"activity of "+user.UUID,
+		user.UUID,
+		"{}")
+	if err != nil {
+		ctxlog.FromContext(ctx).WithError(err).Error("logActivity query failed")
+		return
+	}
+	err = tx.Commit()
+	if err != nil {
+		ctxlog.FromContext(ctx).WithError(err).Error("logActivity commit failed")
+		return
+	}
+	logged = true
+}
+
+// alignedPeriod computes a time interval that includes now and aligns
+// to local clock times that are multiples of p. For example, if local
+// time is UTC-5 and ActivityLoggingPeriod=4h, periodStart and
+// periodEnd will be 0000-0400, 0400-0800, etc., in local time. If p
+// is a multiple of 24h, periods will start and end at midnight.
+//
+// If DST starts or ends during this period, the boundaries will be
+// aligned based on either DST or non-DST time depending on whether
+// now is before or after the DST transition. The consequences are
+// presumed to be inconsequential, e.g., logActivity may unnecessarily
+// log activity more than once in a period that includes a DST
+// transition.
+//
+// In all cases, the period ends in the future.
+//
+// Only the end of the period is returned.
+func alignedPeriod(now time.Time, p time.Duration) time.Time {
+	_, tzsec := now.Zone()
+	tzoff := time.Duration(tzsec) * time.Second
+	periodStart := now.Add(tzoff).Truncate(p).Add(-tzoff)
+	return periodStart.Add(p)
+}
diff --git a/lib/controller/localdb/log_activity_test.go b/lib/controller/localdb/log_activity_test.go
new file mode 100644
index 000000000..6a9bc45e1
--- /dev/null
+++ b/lib/controller/localdb/log_activity_test.go
@@ -0,0 +1,46 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package localdb
+
+import (
+	"time"
+
+	check "gopkg.in/check.v1"
+)
+
+var _ = check.Suite(&activityPeriodSuite{})
+
+type activityPeriodSuite struct{}
+
+// The important thing is that, even when daylight savings time is
+// making things difficult, the current period ends in the future.
+func (*activityPeriodSuite) TestPeriod(c *check.C) {
+	toronto, err := time.LoadLocation("America/Toronto")
+	c.Assert(err, check.IsNil)
+
+	format := "2006-01-02 15:04:05 MST"
+	dststartday, err := time.ParseInLocation(format, "2022-03-13 00:00:00 EST", toronto)
+	c.Assert(err, check.IsNil)
+	dstendday, err := time.ParseInLocation(format, "2022-11-06 00:00:00 EDT", toronto)
+	c.Assert(err, check.IsNil)
+
+	for _, period := range []time.Duration{
+		time.Minute * 13,
+		time.Minute * 49,
+		time.Hour,
+		4 * time.Hour,
+		48 * time.Hour,
+	} {
+		for offset := time.Duration(0); offset < 48*time.Hour; offset += 3 * time.Minute {
+			t := dststartday.Add(offset)
+			end := alignedPeriod(t, period)
+			c.Check(end.After(t), check.Equals, true, check.Commentf("period %v offset %v", period, offset))
+
+			t = dstendday.Add(offset)
+			end = alignedPeriod(t, period)
+			c.Check(end.After(t), check.Equals, true, check.Commentf("period %v offset %v", period, offset))
+		}
+	}
+}
diff --git a/lib/ctrlctx/auth.go b/lib/ctrlctx/auth.go
index 61c6253d4..f4c472f73 100644
--- a/lib/ctrlctx/auth.go
+++ b/lib/ctrlctx/auth.go
@@ -9,7 +9,6 @@ import (
 	"crypto/hmac"
 	"crypto/sha256"
 	"database/sql"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -20,6 +19,7 @@ import (
 	"git.arvados.org/arvados.git/lib/controller/api"
 	"git.arvados.org/arvados.git/sdk/go/arvados"
 	"git.arvados.org/arvados.git/sdk/go/auth"
+	"github.com/ghodss/yaml"
 )
 
 var (
@@ -135,7 +135,7 @@ func (ac *authcache) lookup(ctx context.Context, cluster *arvados.Cluster, token
 	var args []interface{}
 	if len(token) > 30 && strings.HasPrefix(token, "v2/") && token[30] == '/' {
 		fields := strings.Split(token, "/")
-		cond = `aca.uuid=$1 and aca.api_token=$2`
+		cond = `aca.uuid = $1 and aca.api_token = $2`
 		args = []interface{}{fields[1], fields[2]}
 	} else {
 		// Bare token or OIDC access token
@@ -145,24 +145,26 @@ func (ac *authcache) lookup(ctx context.Context, cluster *arvados.Cluster, token
 		cond = `aca.api_token in ($1, $2)`
 		args = []interface{}{token, hmac}
 	}
-	var scopesJSON []byte
+	var expiresAt sql.NullTime
+	var scopesYAML []byte
 	err = tx.QueryRowContext(ctx, `
 select aca.uuid, aca.expires_at, aca.api_token, aca.scopes, users.uuid, users.is_active, users.is_admin
  from api_client_authorizations aca
  left join users on aca.user_id = users.id
  where `+cond+`
  and (expires_at is null or expires_at > current_timestamp at time zone 'UTC')`, args...).Scan(
-		&aca.UUID, &aca.ExpiresAt, &aca.APIToken, &scopesJSON,
+		&aca.UUID, &expiresAt, &aca.APIToken, &scopesYAML,
 		&user.UUID, &user.IsActive, &user.IsAdmin)
 	if err == sql.ErrNoRows {
 		return nil, nil, nil
 	} else if err != nil {
 		return nil, nil, err
 	}
-	if len(scopesJSON) > 0 {
-		err = json.Unmarshal(scopesJSON, &aca.Scopes)
+	aca.ExpiresAt = expiresAt.Time
+	if len(scopesYAML) > 0 {
+		err = yaml.Unmarshal(scopesYAML, &aca.Scopes)
 		if err != nil {
-			return nil, nil, err
+			return nil, nil, fmt.Errorf("loading scopes for %s: %w", aca.UUID, err)
 		}
 	}
 	ent = &authcacheent{
diff --git a/lib/ctrlctx/db.go b/lib/ctrlctx/db.go
index 36d79d3d2..a76420860 100644
--- a/lib/ctrlctx/db.go
+++ b/lib/ctrlctx/db.go
@@ -12,6 +12,7 @@ import (
 	"git.arvados.org/arvados.git/lib/controller/api"
 	"git.arvados.org/arvados.git/sdk/go/ctxlog"
 	"github.com/jmoiron/sqlx"
+
 	// sqlx needs lib/pq to talk to PostgreSQL
 	_ "github.com/lib/pq"
 )
@@ -107,6 +108,26 @@ func New(ctx context.Context, getdb func(context.Context) (*sqlx.DB, error)) (co
 	}
 }
 
+// NewTx starts a new transaction. The caller is responsible for
+// calling Commit or Rollback. This is suitable for database queries
+// that are separate from the API transaction (see CurrentTx), e.g.,
+// ones that will be committed even if the API call fails, or held
+// open after the API call finishes.
+func NewTx(ctx context.Context) (*sqlx.Tx, error) {
+	txn, ok := ctx.Value(contextKeyTransaction).(*transaction)
+	if !ok {
+		return nil, ErrNoTransaction
+	}
+	db, err := txn.getdb(ctx)
+	if err != nil {
+		return nil, err
+	}
+	return db.Beginx()
+}
+
+// CurrentTx returns a transaction that will be committed after the
+// current API call completes, or rolled back if the current API call
+// returns an error.
 func CurrentTx(ctx context.Context) (*sqlx.Tx, error) {
 	txn, ok := ctx.Value(contextKeyTransaction).(*transaction)
 	if !ok {
diff --git a/sdk/go/arvados/client.go b/sdk/go/arvados/client.go
index cdc07bb0a..4dead0ada 100644
--- a/sdk/go/arvados/client.go
+++ b/sdk/go/arvados/client.go
@@ -7,6 +7,7 @@ package arvados
 import (
 	"bytes"
 	"context"
+	"crypto/rand"
 	"crypto/tls"
 	"encoding/json"
 	"errors"
@@ -15,6 +16,7 @@ import (
 	"io/fs"
 	"io/ioutil"
 	"log"
+	"math/big"
 	"net"
 	"net/http"
 	"net/url"
@@ -599,3 +601,13 @@ func (c *Client) PathForUUID(method, uuid string) (string, error) {
 	}
 	return path, nil
 }
+
+var maxUUIDInt = (&big.Int{}).Exp(big.NewInt(36), big.NewInt(15), nil)
+
+func RandomUUID(clusterID, infix string) string {
+	n, err := rand.Int(rand.Reader, maxUUIDInt)
+	if err != nil {
+		panic(err)
+	}
+	return clusterID + "-" + infix + "-" + n.Text(36)
+}
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index eb564cb61..a1fc2e89f 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -248,6 +248,7 @@ type Cluster struct {
 		PreferDomainForUsername               string
 		UserSetupMailText                     string
 		RoleGroupsVisibleToAll                bool
+		ActivityLoggingPeriod                 Duration
 	}
 	StorageClasses map[string]StorageClassConfig
 	Volumes        map[string]Volume

commit 3d2b1ba10a5739e49d26658fa7aaf090dbd6ed44
Author: Tom Clegg <tom at curii.com>
Date:   Thu Sep 22 15:37:09 2022 -0400

    19388: Remove unneeded import so ctrlctx can import arvadostest.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/ctrlctx/auth_test.go b/lib/ctrlctx/auth_test.go
index 5b0b06798..e6803e5c4 100644
--- a/lib/ctrlctx/auth_test.go
+++ b/lib/ctrlctx/auth_test.go
@@ -8,6 +8,7 @@ import (
 	"context"
 
 	"git.arvados.org/arvados.git/lib/config"
+	"git.arvados.org/arvados.git/sdk/go/arvadostest"
 	"git.arvados.org/arvados.git/sdk/go/auth"
 	"git.arvados.org/arvados.git/sdk/go/ctxlog"
 	"github.com/jmoiron/sqlx"
@@ -29,10 +30,10 @@ func (*DatabaseSuite) TestAuthContext(c *check.C) {
 
 	// valid tokens
 	for _, token := range []string{
-		"3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi",
-		"v2/zzzzz-gj3su-077z32aux8dg2s1/3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi",
-		"v2/zzzzz-gj3su-077z32aux8dg2s1/3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi/asdfasdfasdf",
-		"v2/zzzzz-gj3su-077z32aux8dg2s1/3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi", // cached
+		arvadostest.ActiveToken,
+		arvadostest.ActiveTokenV2,
+		arvadostest.ActiveTokenV2 + "/asdfasdfasdf",
+		arvadostest.ActiveTokenV2, // cached
 	} {
 		ok, err := dbwrapper(authwrapper(func(ctx context.Context, opts interface{}) (interface{}, error) {
 			user, aca, err := CurrentAuth(ctx)
@@ -49,8 +50,10 @@ func (*DatabaseSuite) TestAuthContext(c *check.C) {
 
 	// bad tokens
 	for _, token := range []string{
-		"3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmI", // note last char mangled
-		"v2/zzzzz-gj3su-077z32aux8dg2s1/",
+		arvadostest.ActiveToken + "X",
+		arvadostest.ActiveTokenV2 + "X",
+		arvadostest.ActiveTokenV2[:30], // "v2/{uuid}"
+		arvadostest.ActiveTokenV2[:31], // "v2/{uuid}/"
 		"bogus",
 		"",
 	} {
diff --git a/sdk/go/arvadostest/db.go b/sdk/go/arvadostest/db.go
index c20f61db2..d39f3c6fc 100644
--- a/sdk/go/arvadostest/db.go
+++ b/sdk/go/arvadostest/db.go
@@ -5,11 +5,9 @@
 package arvadostest
 
 import (
-	"context"
-
-	"git.arvados.org/arvados.git/lib/ctrlctx"
 	"git.arvados.org/arvados.git/sdk/go/arvados"
 	"github.com/jmoiron/sqlx"
+
 	// sqlx needs lib/pq to talk to PostgreSQL
 	_ "github.com/lib/pq"
 	"gopkg.in/check.v1"
@@ -21,14 +19,3 @@ func DB(c *check.C, cluster *arvados.Cluster) *sqlx.DB {
 	c.Assert(err, check.IsNil)
 	return db
 }
-
-// TransactionContext returns a context suitable for running a test
-// case in a new transaction, and a rollback func which the caller
-// should call after the test.
-func TransactionContext(c *check.C, db *sqlx.DB) (ctx context.Context, rollback func()) {
-	tx, err := db.Beginx()
-	c.Assert(err, check.IsNil)
-	return ctrlctx.NewWithTransaction(context.Background(), tx), func() {
-		c.Check(tx.Rollback(), check.IsNil)
-	}
-}

commit 8be4b29d8837f8ecff47cfde3bea99d97c7562bc
Author: Tom Clegg <tom at curii.com>
Date:   Thu Sep 22 15:19:48 2022 -0400

    19388: Cache token lookups.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/ctrlctx/auth.go b/lib/ctrlctx/auth.go
index 5b96463cc..61c6253d4 100644
--- a/lib/ctrlctx/auth.go
+++ b/lib/ctrlctx/auth.go
@@ -15,6 +15,7 @@ import (
 	"io"
 	"strings"
 	"sync"
+	"time"
 
 	"git.arvados.org/arvados.git/lib/controller/api"
 	"git.arvados.org/arvados.git/sdk/go/arvados"
@@ -33,13 +34,18 @@ var (
 // The incoming context must come from WrapCallsInTransactions or
 // NewWithTransaction.
 func WrapCallsWithAuth(cluster *arvados.Cluster) func(api.RoutableFunc) api.RoutableFunc {
+	var authcache authcache
 	return func(origFunc api.RoutableFunc) api.RoutableFunc {
 		return func(ctx context.Context, opts interface{}) (_ interface{}, err error) {
 			var tokens []string
 			if creds, ok := auth.FromContext(ctx); ok {
 				tokens = creds.Tokens
 			}
-			return origFunc(context.WithValue(ctx, contextKeyAuth, &authcontext{cluster: cluster, tokens: tokens}), opts)
+			return origFunc(context.WithValue(ctx, contextKeyAuth, &authcontext{
+				authcache: &authcache,
+				cluster:   cluster,
+				tokens:    tokens,
+			}), opts)
 		}
 	}
 }
@@ -55,7 +61,25 @@ func CurrentAuth(ctx context.Context) (*arvados.User, *arvados.APIClientAuthoriz
 	if !ok {
 		return nil, nil, ErrNoAuthContext
 	}
-	ac.lookupOnce.Do(func() { ac.user, ac.apiClientAuthorization, ac.err = aclookup(ctx, ac.cluster, ac.tokens) })
+	ac.lookupOnce.Do(func() {
+		// We only validate/lookup the token once per API
+		// call, even though authcache should be efficient
+		// enough to do a lookup each time. This guarantees we
+		// always return the same result when called multiple
+		// times in the course of handling a single API call.
+		for _, token := range ac.tokens {
+			user, aca, err := ac.authcache.lookup(ctx, ac.cluster, token)
+			if err != nil {
+				ac.err = err
+				return
+			}
+			if user != nil {
+				ac.user, ac.apiClientAuthorization = user, aca
+				return
+			}
+		}
+		ac.err = ErrUnauthenticated
+	})
 	return ac.user, ac.apiClientAuthorization, ac.err
 }
 
@@ -64,6 +88,7 @@ type contextKeyA string
 var contextKeyAuth = contextKeyT("auth")
 
 type authcontext struct {
+	authcache              *authcache
 	cluster                *arvados.Cluster
 	tokens                 []string
 	user                   *arvados.User
@@ -72,9 +97,32 @@ type authcontext struct {
 	lookupOnce             sync.Once
 }
 
-func aclookup(ctx context.Context, cluster *arvados.Cluster, tokens []string) (*arvados.User, *arvados.APIClientAuthorization, error) {
-	if len(tokens) == 0 {
-		return nil, nil, ErrUnauthenticated
+var authcacheTTL = time.Minute
+
+type authcacheent struct {
+	expireTime             time.Time
+	apiClientAuthorization arvados.APIClientAuthorization
+	user                   arvados.User
+}
+
+type authcache struct {
+	mtx         sync.Mutex
+	entries     map[string]*authcacheent
+	nextCleanup time.Time
+}
+
+// lookup returns the user and aca info for a given token. Returns nil
+// if the token is not valid. Returns a non-nil error if there was an
+// unexpected error from the database, etc.
+func (ac *authcache) lookup(ctx context.Context, cluster *arvados.Cluster, token string) (*arvados.User, *arvados.APIClientAuthorization, error) {
+	ac.mtx.Lock()
+	ent := ac.entries[token]
+	ac.mtx.Unlock()
+	if ent != nil && ent.expireTime.After(time.Now()) {
+		return &ent.user, &ent.apiClientAuthorization, nil
+	}
+	if token == "" {
+		return nil, nil, nil
 	}
 	tx, err := CurrentTx(ctx)
 	if err != nil {
@@ -82,44 +130,59 @@ func aclookup(ctx context.Context, cluster *arvados.Cluster, tokens []string) (*
 	}
 	var aca arvados.APIClientAuthorization
 	var user arvados.User
-	for _, token := range tokens {
-		var cond string
-		var args []interface{}
-		if token == "" {
-			continue
-		} else if len(token) > 30 && strings.HasPrefix(token, "v2/") && token[30] == '/' {
-			fields := strings.Split(token, "/")
-			cond = `aca.uuid=$1 and aca.api_token=$2`
-			args = []interface{}{fields[1], fields[2]}
-		} else {
-			// Bare token or OIDC access token
-			mac := hmac.New(sha256.New, []byte(cluster.SystemRootToken))
-			io.WriteString(mac, token)
-			hmac := fmt.Sprintf("%x", mac.Sum(nil))
-			cond = `aca.api_token in ($1, $2)`
-			args = []interface{}{token, hmac}
-		}
-		var scopesJSON []byte
-		err = tx.QueryRowContext(ctx, `
+
+	var cond string
+	var args []interface{}
+	if len(token) > 30 && strings.HasPrefix(token, "v2/") && token[30] == '/' {
+		fields := strings.Split(token, "/")
+		cond = `aca.uuid=$1 and aca.api_token=$2`
+		args = []interface{}{fields[1], fields[2]}
+	} else {
+		// Bare token or OIDC access token
+		mac := hmac.New(sha256.New, []byte(cluster.SystemRootToken))
+		io.WriteString(mac, token)
+		hmac := fmt.Sprintf("%x", mac.Sum(nil))
+		cond = `aca.api_token in ($1, $2)`
+		args = []interface{}{token, hmac}
+	}
+	var scopesJSON []byte
+	err = tx.QueryRowContext(ctx, `
 select aca.uuid, aca.expires_at, aca.api_token, aca.scopes, users.uuid, users.is_active, users.is_admin
  from api_client_authorizations aca
  left join users on aca.user_id = users.id
  where `+cond+`
  and (expires_at is null or expires_at > current_timestamp at time zone 'UTC')`, args...).Scan(
-			&aca.UUID, &aca.ExpiresAt, &aca.APIToken, &scopesJSON,
-			&user.UUID, &user.IsActive, &user.IsAdmin)
-		if err == sql.ErrNoRows {
-			continue
-		} else if err != nil {
+		&aca.UUID, &aca.ExpiresAt, &aca.APIToken, &scopesJSON,
+		&user.UUID, &user.IsActive, &user.IsAdmin)
+	if err == sql.ErrNoRows {
+		return nil, nil, nil
+	} else if err != nil {
+		return nil, nil, err
+	}
+	if len(scopesJSON) > 0 {
+		err = json.Unmarshal(scopesJSON, &aca.Scopes)
+		if err != nil {
 			return nil, nil, err
 		}
-		if len(scopesJSON) > 0 {
-			err = json.Unmarshal(scopesJSON, &aca.Scopes)
-			if err != nil {
-				return nil, nil, err
+	}
+	ent = &authcacheent{
+		expireTime:             time.Now().Add(authcacheTTL),
+		apiClientAuthorization: aca,
+		user:                   user,
+	}
+	ac.mtx.Lock()
+	defer ac.mtx.Unlock()
+	if ac.entries == nil {
+		ac.entries = map[string]*authcacheent{}
+	}
+	if ac.nextCleanup.IsZero() || ac.nextCleanup.Before(time.Now()) {
+		for token, ent := range ac.entries {
+			if !ent.expireTime.After(time.Now()) {
+				delete(ac.entries, token)
 			}
 		}
-		return &user, &aca, nil
+		ac.nextCleanup = time.Now().Add(authcacheTTL)
 	}
-	return nil, nil, ErrUnauthenticated
+	ac.entries[token] = ent
+	return &ent.user, &ent.apiClientAuthorization, nil
 }
diff --git a/lib/ctrlctx/auth_test.go b/lib/ctrlctx/auth_test.go
index add7a67d1..5b0b06798 100644
--- a/lib/ctrlctx/auth_test.go
+++ b/lib/ctrlctx/auth_test.go
@@ -32,6 +32,7 @@ func (*DatabaseSuite) TestAuthContext(c *check.C) {
 		"3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi",
 		"v2/zzzzz-gj3su-077z32aux8dg2s1/3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi",
 		"v2/zzzzz-gj3su-077z32aux8dg2s1/3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi/asdfasdfasdf",
+		"v2/zzzzz-gj3su-077z32aux8dg2s1/3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi", // cached
 	} {
 		ok, err := dbwrapper(authwrapper(func(ctx context.Context, opts interface{}) (interface{}, error) {
 			user, aca, err := CurrentAuth(ctx)

commit e4b9cffde1c932a0ca880d1542c62b4611142352
Author: Tom Clegg <tom at curii.com>
Date:   Thu Sep 22 14:03:01 2022 -0400

    19388: Add user/auth context to ctrlctx.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/controller/handler.go b/lib/controller/handler.go
index 665fd5c63..e9c56db4d 100644
--- a/lib/controller/handler.go
+++ b/lib/controller/handler.go
@@ -101,7 +101,10 @@ func (h *Handler) setup() {
 	h.federation = federation.New(h.Cluster, &healthFuncs)
 	rtr := router.New(h.federation, router.Config{
 		MaxRequestSize: h.Cluster.API.MaxRequestSize,
-		WrapCalls:      api.ComposeWrappers(ctrlctx.WrapCallsInTransactions(h.db), oidcAuthorizer.WrapCalls),
+		WrapCalls: api.ComposeWrappers(
+			ctrlctx.WrapCallsInTransactions(h.db),
+			oidcAuthorizer.WrapCalls,
+			ctrlctx.WrapCallsWithAuth(h.Cluster)),
 	})
 
 	healthRoutes := health.Routes{"ping": func() error { _, err := h.db(context.TODO()); return err }}
diff --git a/lib/ctrlctx/auth.go b/lib/ctrlctx/auth.go
new file mode 100644
index 000000000..5b96463cc
--- /dev/null
+++ b/lib/ctrlctx/auth.go
@@ -0,0 +1,125 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package ctrlctx
+
+import (
+	"context"
+	"crypto/hmac"
+	"crypto/sha256"
+	"database/sql"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io"
+	"strings"
+	"sync"
+
+	"git.arvados.org/arvados.git/lib/controller/api"
+	"git.arvados.org/arvados.git/sdk/go/arvados"
+	"git.arvados.org/arvados.git/sdk/go/auth"
+)
+
+var (
+	ErrNoAuthContext   = errors.New("bug: there is no authorization in this context")
+	ErrUnauthenticated = errors.New("unauthenticated request")
+)
+
+// WrapCallsWithAuth returns a call wrapper (suitable for assigning to
+// router.router.WrapCalls) that makes CurrentUser(ctx) et al. work
+// from inside the wrapped functions.
+//
+// The incoming context must come from WrapCallsInTransactions or
+// NewWithTransaction.
+func WrapCallsWithAuth(cluster *arvados.Cluster) func(api.RoutableFunc) api.RoutableFunc {
+	return func(origFunc api.RoutableFunc) api.RoutableFunc {
+		return func(ctx context.Context, opts interface{}) (_ interface{}, err error) {
+			var tokens []string
+			if creds, ok := auth.FromContext(ctx); ok {
+				tokens = creds.Tokens
+			}
+			return origFunc(context.WithValue(ctx, contextKeyAuth, &authcontext{cluster: cluster, tokens: tokens}), opts)
+		}
+	}
+}
+
+// CurrentAuth returns the arvados.User whose privileges should be
+// used in the given context, and the arvados.APIClientAuthorization
+// the caller presented in order to authenticate the current request.
+//
+// Returns ErrUnauthenticated if the current request was not
+// authenticated (no token provided, token is expired, etc).
+func CurrentAuth(ctx context.Context) (*arvados.User, *arvados.APIClientAuthorization, error) {
+	ac, ok := ctx.Value(contextKeyAuth).(*authcontext)
+	if !ok {
+		return nil, nil, ErrNoAuthContext
+	}
+	ac.lookupOnce.Do(func() { ac.user, ac.apiClientAuthorization, ac.err = aclookup(ctx, ac.cluster, ac.tokens) })
+	return ac.user, ac.apiClientAuthorization, ac.err
+}
+
+type contextKeyA string
+
+var contextKeyAuth = contextKeyT("auth")
+
+type authcontext struct {
+	cluster                *arvados.Cluster
+	tokens                 []string
+	user                   *arvados.User
+	apiClientAuthorization *arvados.APIClientAuthorization
+	err                    error
+	lookupOnce             sync.Once
+}
+
+func aclookup(ctx context.Context, cluster *arvados.Cluster, tokens []string) (*arvados.User, *arvados.APIClientAuthorization, error) {
+	if len(tokens) == 0 {
+		return nil, nil, ErrUnauthenticated
+	}
+	tx, err := CurrentTx(ctx)
+	if err != nil {
+		return nil, nil, err
+	}
+	var aca arvados.APIClientAuthorization
+	var user arvados.User
+	for _, token := range tokens {
+		var cond string
+		var args []interface{}
+		if token == "" {
+			continue
+		} else if len(token) > 30 && strings.HasPrefix(token, "v2/") && token[30] == '/' {
+			fields := strings.Split(token, "/")
+			cond = `aca.uuid=$1 and aca.api_token=$2`
+			args = []interface{}{fields[1], fields[2]}
+		} else {
+			// Bare token or OIDC access token
+			mac := hmac.New(sha256.New, []byte(cluster.SystemRootToken))
+			io.WriteString(mac, token)
+			hmac := fmt.Sprintf("%x", mac.Sum(nil))
+			cond = `aca.api_token in ($1, $2)`
+			args = []interface{}{token, hmac}
+		}
+		var scopesJSON []byte
+		err = tx.QueryRowContext(ctx, `
+select aca.uuid, aca.expires_at, aca.api_token, aca.scopes, users.uuid, users.is_active, users.is_admin
+ from api_client_authorizations aca
+ left join users on aca.user_id = users.id
+ where `+cond+`
+ and (expires_at is null or expires_at > current_timestamp at time zone 'UTC')`, args...).Scan(
+			&aca.UUID, &aca.ExpiresAt, &aca.APIToken, &scopesJSON,
+			&user.UUID, &user.IsActive, &user.IsAdmin)
+		if err == sql.ErrNoRows {
+			continue
+		} else if err != nil {
+			return nil, nil, err
+		}
+		if len(scopesJSON) > 0 {
+			err = json.Unmarshal(scopesJSON, &aca.Scopes)
+			if err != nil {
+				return nil, nil, err
+			}
+		}
+		return &user, &aca, nil
+	}
+	return nil, nil, ErrUnauthenticated
+}
diff --git a/lib/ctrlctx/auth_test.go b/lib/ctrlctx/auth_test.go
new file mode 100644
index 000000000..add7a67d1
--- /dev/null
+++ b/lib/ctrlctx/auth_test.go
@@ -0,0 +1,79 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package ctrlctx
+
+import (
+	"context"
+
+	"git.arvados.org/arvados.git/lib/config"
+	"git.arvados.org/arvados.git/sdk/go/auth"
+	"git.arvados.org/arvados.git/sdk/go/ctxlog"
+	"github.com/jmoiron/sqlx"
+	_ "github.com/lib/pq"
+	check "gopkg.in/check.v1"
+)
+
+func (*DatabaseSuite) TestAuthContext(c *check.C) {
+	cfg, err := config.NewLoader(nil, ctxlog.TestLogger(c)).Load()
+	c.Assert(err, check.IsNil)
+	cluster, err := cfg.GetCluster("")
+	c.Assert(err, check.IsNil)
+
+	getter := func(context.Context) (*sqlx.DB, error) {
+		return sqlx.Open("postgres", cluster.PostgreSQL.Connection.String())
+	}
+	authwrapper := WrapCallsWithAuth(cluster)
+	dbwrapper := WrapCallsInTransactions(getter)
+
+	// valid tokens
+	for _, token := range []string{
+		"3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi",
+		"v2/zzzzz-gj3su-077z32aux8dg2s1/3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi",
+		"v2/zzzzz-gj3su-077z32aux8dg2s1/3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi/asdfasdfasdf",
+	} {
+		ok, err := dbwrapper(authwrapper(func(ctx context.Context, opts interface{}) (interface{}, error) {
+			user, aca, err := CurrentAuth(ctx)
+			if c.Check(err, check.IsNil) {
+				c.Check(user.UUID, check.Equals, "zzzzz-tpzed-xurymjxw79nv3jz")
+				c.Check(aca.UUID, check.Equals, "zzzzz-gj3su-077z32aux8dg2s1")
+				c.Check(aca.Scopes, check.DeepEquals, []string{"all"})
+			}
+			return true, nil
+		}))(auth.NewContext(context.Background(), auth.NewCredentials(token)), "blah")
+		c.Check(ok, check.Equals, true)
+		c.Check(err, check.IsNil)
+	}
+
+	// bad tokens
+	for _, token := range []string{
+		"3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmI", // note last char mangled
+		"v2/zzzzz-gj3su-077z32aux8dg2s1/",
+		"bogus",
+		"",
+	} {
+		ok, err := dbwrapper(authwrapper(func(ctx context.Context, opts interface{}) (interface{}, error) {
+			user, aca, err := CurrentAuth(ctx)
+			c.Check(err, check.Equals, ErrUnauthenticated)
+			c.Check(user, check.IsNil)
+			c.Check(aca, check.IsNil)
+			return true, err
+		}))(auth.NewContext(context.Background(), auth.NewCredentials(token)), "blah")
+		c.Check(ok, check.Equals, true)
+		c.Check(err, check.Equals, ErrUnauthenticated)
+	}
+
+	// no auth context
+	{
+		ok, err := dbwrapper(authwrapper(func(ctx context.Context, opts interface{}) (interface{}, error) {
+			user, aca, err := CurrentAuth(ctx)
+			c.Check(err, check.Equals, ErrUnauthenticated)
+			c.Check(user, check.IsNil)
+			c.Check(aca, check.IsNil)
+			return true, err
+		}))(context.Background(), "blah")
+		c.Check(ok, check.Equals, true)
+		c.Check(err, check.Equals, ErrUnauthenticated)
+	}
+}

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list