From 0cecab7dce07e0e169b6c594aed363d9a14582be Mon Sep 17 00:00:00 2001 From: pwilloughby Date: Thu, 10 Oct 2024 13:31:52 -0600 Subject: [PATCH] pkg/auth: log failed PublicProjectID requests updates storj/storj-private#860 Change-Id: Ia088c3f66938fbf552f22620eb3a05d4ddbce347 --- pkg/auth/authdb/db.go | 8 +++++--- pkg/auth/authdb/db_test.go | 5 +++-- pkg/auth/drpcauth/server_test.go | 4 ++-- pkg/auth/httpauth/resources_test.go | 20 ++++++++++---------- pkg/auth/peer.go | 2 +- 5 files changed, 21 insertions(+), 18 deletions(-) diff --git a/pkg/auth/authdb/db.go b/pkg/auth/authdb/db.go index e9f1cc4f..941f2f85 100644 --- a/pkg/auth/authdb/db.go +++ b/pkg/auth/authdb/db.go @@ -14,6 +14,7 @@ import ( "github.com/spacemonkeygo/monkit/v3" "github.com/zeebo/errs" + "go.uber.org/zap" "storj.io/common/encryption" "storj.io/common/macaroon" @@ -130,6 +131,7 @@ func toBase32(k []byte) string { // and secrets. type Database struct { storage Storage + logger *zap.Logger mu sync.Mutex allowedSatelliteURLs map[storj.NodeURL]struct{} @@ -140,9 +142,10 @@ type Database struct { // NewDatabase constructs a Database. allowedSatelliteAddresses should contain // the full URL (with a node ID), including port, for each satellite we // allow for incoming access grants. -func NewDatabase(storage Storage, allowedSatelliteURLs map[storj.NodeURL]struct{}, retrievePublicProjectID bool) *Database { +func NewDatabase(logger *zap.Logger, storage Storage, allowedSatelliteURLs map[storj.NodeURL]struct{}, retrievePublicProjectID bool) *Database { return &Database{ storage: storage, + logger: logger, allowedSatelliteURLs: allowedSatelliteURLs, retrievePublicProjectID: retrievePublicProjectID, uplinkConfig: uplink.Config{ @@ -211,8 +214,7 @@ func (db *Database) Put(ctx context.Context, key EncryptionKey, accessGrant stri if db.retrievePublicProjectID { publicProjectID, err = privateProject.GetPublicID(ctx, db.uplinkConfig, access) if err != nil { - // TODO(artur, sean): we should probably log why we couldn't - // fetch the public project ID. + db.logger.Warn("retrieve public project id failed", zap.Error(err)) publicProjectID = uuid.UUID{} // just in case, zero it mon.Event("retrieve_public_project_id_failed") } diff --git a/pkg/auth/authdb/db_test.go b/pkg/auth/authdb/db_test.go index 066b3ad3..76819d00 100644 --- a/pkg/auth/authdb/db_test.go +++ b/pkg/auth/authdb/db_test.go @@ -10,6 +10,7 @@ import ( "time" "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" "storj.io/common/encryption" "storj.io/common/grant" @@ -163,7 +164,7 @@ func TestPutSatelliteValidation(t *testing.T) { url, err := storj.ParseNodeURL(validURL) require.NoError(t, err) - db := NewDatabase(mockStorage{}, map[storj.NodeURL]struct{}{url: {}}, false) + db := NewDatabase(zaptest.NewLogger(t), mockStorage{}, map[storj.NodeURL]struct{}{url: {}}, false) key, err := NewEncryptionKey() require.NoError(t, err) @@ -194,7 +195,7 @@ func TestPutShortExpiration(t *testing.T) { s, err := g.Serialize() require.NoError(t, err) - _, err = NewDatabase(mockStorage{}, map[storj.NodeURL]struct{}{url: {}}, false).Put(context.TODO(), enc, s, true) + _, err = NewDatabase(zaptest.NewLogger(t), mockStorage{}, map[storj.NodeURL]struct{}{url: {}}, false).Put(context.TODO(), enc, s, true) t.Log(err) require.Error(t, err) require.True(t, ErrAccessGrant.Has(err)) diff --git a/pkg/auth/drpcauth/server_test.go b/pkg/auth/drpcauth/server_test.go index c258a073..2925c941 100644 --- a/pkg/auth/drpcauth/server_test.go +++ b/pkg/auth/drpcauth/server_test.go @@ -44,7 +44,7 @@ func createBackend(t *testing.T, sizeLimit memory.Size) (_ *Server, _ *authdb.Da storage, err := badgerauth.New(logger, badgerauth.Config{FirstStart: true}) require.NoError(t, err) - db := authdb.NewDatabase(storage, map[storj.NodeURL]struct{}{minimalAccessSatelliteID: {}}, false) + db := authdb.NewDatabase(zaptest.NewLogger(t), storage, map[storj.NodeURL]struct{}{minimalAccessSatelliteID: {}}, false) endpoint, err := url.Parse("http://gateway.test") require.NoError(t, err) @@ -106,7 +106,7 @@ func TestRegisterAccessContextCanceled(t *testing.T) { require.NoError(t, storage.HealthCheck(ctx)) - db := authdb.NewDatabase(storage, map[storj.NodeURL]struct{}{minimalAccessSatelliteID: {}}, false) + db := authdb.NewDatabase(zaptest.NewLogger(t), storage, map[storj.NodeURL]struct{}{minimalAccessSatelliteID: {}}, false) endpoint, err := url.Parse("http://gateway.test") require.NoError(t, err) diff --git a/pkg/auth/httpauth/resources_test.go b/pkg/auth/httpauth/resources_test.go index b2a2b007..3051fcba 100644 --- a/pkg/auth/httpauth/resources_test.go +++ b/pkg/auth/httpauth/resources_test.go @@ -116,7 +116,7 @@ func TestResources_CRUD(t *testing.T) { t.Run("Availability after startup", func(t *testing.T) { allowed := map[storj.NodeURL]struct{}{minimalAccessSatelliteID: {}} - res := newResource(t, logger, authdb.NewDatabase(storage, allowed, false), endpoint) + res := newResource(t, logger, authdb.NewDatabase(zaptest.NewLogger(t), storage, allowed, false), endpoint) const path = "/v1/health/startup" @@ -135,7 +135,7 @@ func TestResources_CRUD(t *testing.T) { t.Run("CRUD", func(t *testing.T) { allowed := map[storj.NodeURL]struct{}{minimalAccessSatelliteID: {}} - res := newResource(t, logger, authdb.NewDatabase(storage, allowed, false), endpoint) + res := newResource(t, logger, authdb.NewDatabase(zaptest.NewLogger(t), storage, allowed, false), endpoint) // create an access createRequest := fmt.Sprintf(`{"access_grant": %q}`, minimalAccess) @@ -155,7 +155,7 @@ func TestResources_CRUD(t *testing.T) { var unknownSatelliteID storj.NodeURL unknownSatelliteID.ID[4] = 7 allowed := map[storj.NodeURL]struct{}{unknownSatelliteID: {}} - res := newResource(t, logger, authdb.NewDatabase(storage, allowed, false), endpoint) + res := newResource(t, logger, authdb.NewDatabase(zaptest.NewLogger(t), storage, allowed, false), endpoint) // create an access createRequest := fmt.Sprintf(`{"access_grant": %q}`, minimalAccess) @@ -169,7 +169,7 @@ func TestResources_CRUD(t *testing.T) { require.False(t, ok) allowed = map[storj.NodeURL]struct{}{unknownSatelliteID: {}, minimalAccessSatelliteID: {}} - res = newResource(t, logger, authdb.NewDatabase(storage, allowed, false), endpoint) + res = newResource(t, logger, authdb.NewDatabase(zaptest.NewLogger(t), storage, allowed, false), endpoint) // create an access createRequest = fmt.Sprintf(`{"access_grant": %q}`, minimalAccess) @@ -178,7 +178,7 @@ func TestResources_CRUD(t *testing.T) { allowed, _, err := nodelist.Resolve(context.Background(), []string{"12EayRS2V1kEsWESU9QMRseFhdxYxKicsiFmxrsLZHeLUtdps3S@us-central-1.tardigrade.io:7777"}) require.NoError(t, err) - res = newResource(t, logger, authdb.NewDatabase(storage, allowed, false), endpoint) + res = newResource(t, logger, authdb.NewDatabase(zaptest.NewLogger(t), storage, allowed, false), endpoint) mac, err := macaroon.NewAPIKey(nil) require.NoError(t, err) access := grant.Access{ @@ -198,7 +198,7 @@ func TestResources_CRUD(t *testing.T) { t.Run("Public", func(t *testing.T) { allowed := map[storj.NodeURL]struct{}{minimalAccessSatelliteID: {}} - res := newResource(t, logger, authdb.NewDatabase(storage, allowed, false), endpoint) + res := newResource(t, logger, authdb.NewDatabase(zaptest.NewLogger(t), storage, allowed, false), endpoint) // create a public access createRequest := fmt.Sprintf(`{"access_grant": %q, "public": true}`, minimalAccess) @@ -216,7 +216,7 @@ func TestResources_CRUD(t *testing.T) { t.Run("Invalidated", func(t *testing.T) { allowed := map[storj.NodeURL]struct{}{minimalAccessSatelliteID: {}} - res := newResource(t, logger, authdb.NewDatabase(storage, allowed, false), endpoint) + res := newResource(t, logger, authdb.NewDatabase(zaptest.NewLogger(t), storage, allowed, false), endpoint) createRequest := fmt.Sprintf(`{"access_grant": %q, "public": true}`, minimalAccess) createResult, ok := exec(res, "POST", "/v1/access", createRequest) @@ -240,7 +240,7 @@ func TestResources_CRUD(t *testing.T) { t.Run("Invalid request", func(t *testing.T) { allowed := map[storj.NodeURL]struct{}{minimalAccessSatelliteID: {}} - res := newResource(t, logger, authdb.NewDatabase(storage, allowed, false), endpoint) + res := newResource(t, logger, authdb.NewDatabase(zaptest.NewLogger(t), storage, allowed, false), endpoint) check := func(body string, expectedCode int) { rec := httptest.NewRecorder() @@ -273,7 +273,7 @@ func TestResources_Authorization(t *testing.T) { require.NoError(t, err) allowed := map[storj.NodeURL]struct{}{minimalAccessSatelliteID: {}} - res := newResource(t, logger, authdb.NewDatabase(storage, allowed, false), endpoint) + res := newResource(t, logger, authdb.NewDatabase(zaptest.NewLogger(t), storage, allowed, false), endpoint) // create an access grant and base url createRequest := fmt.Sprintf(`{"access_grant": %q}`, minimalAccess) @@ -391,7 +391,7 @@ func TestResources_Shutdown(t *testing.T) { req := httptest.NewRequest("GET", "/v1/health/live", nil) allowed := map[storj.NodeURL]struct{}{minimalAccessSatelliteID: {}} - res := newResource(t, logger, authdb.NewDatabase(storage, allowed, false), endpoint) + res := newResource(t, logger, authdb.NewDatabase(zaptest.NewLogger(t), storage, allowed, false), endpoint) res.SetStartupDone() if inShutdown { res.SetShutdown() diff --git a/pkg/auth/peer.go b/pkg/auth/peer.go index e6b18a8a..a70900a3 100644 --- a/pkg/auth/peer.go +++ b/pkg/auth/peer.go @@ -153,7 +153,7 @@ func New(ctx context.Context, log *zap.Logger, config Config, configDir string) return nil, errs.Wrap(err) } - adb := authdb.NewDatabase(storage, allowedSats, config.RetrievePublicProjectID) + adb := authdb.NewDatabase(log.Named("authdb"), storage, allowedSats, config.RetrievePublicProjectID) res := httpauth.New(log.Named("resources"), adb, endpoint, config.AuthToken, config.POSTSizeLimit) tlsInfo := &TLSInfo{