fix: allow token lookups by all hashing algorithms used in store

Fix a bug that only allowed hashed tokens to be looked up if they used the
currently active hashing algorithm. Also added tests for configuration
migration scenarios (enabling and disabling hashing, changing hashing scheme).

Author: gwossum

authorization/storage.go

@@ -229,7 +229,7 @@ func (s *Store) autogenerateHasher(ctx context.Context, variantName string) (*Au
 	for _, a := range auths {
 		if a.HashedToken != "" {
 			digest, err := tempDecoder.Decode(a.HashedToken)
-			if err != nil {
+			if err == nil {
 				if influxdbDigest, ok := digest.(*influxdb2_algo.Digest); ok {
 					foundVariants[influxdbDigest.Variant] = struct{}{}
 				}

authorization/storage_authorization_test.go

@@ -11,19 +11,20 @@ import (
 	"github.com/influxdata/influxdb/v2/kit/platform"
 	"github.com/influxdata/influxdb/v2/kv"
 	"github.com/influxdata/influxdb/v2/kv/migration/all"
+	influxdb2_algo "github.com/influxdata/influxdb/v2/pkg/crypt/algorithm/influxdb2"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap/zaptest"
 )
 
+const (
+	authIndexName       = "authorizationindexv1"
+	hashedAuthIndexName = "authorizationhashedindexv1"
+)
+
 func TestAuth(t *testing.T) {
 	checkIndexCounts := func(t *testing.T, tx kv.Tx, expAuthIndexCount, expHashedAuthIndexCount int) {
 		t.Helper()
 
-		const (
-			authIndexName       = "authorizationindexv1"
-			hashedAuthIndexName = "authorizationhashedindexv1"
-		)
-
 		indexCount := make(map[string]int)
 		for _, indexName := range []string{authIndexName, hashedAuthIndexName} {
 			index, err := tx.Bucket([]byte(indexName))
@@ -279,3 +280,192 @@ func TestAuth(t *testing.T) {
 		}
 	}
 }
+
+func TestAuthorizationStore_HashingConfigChanges(t *testing.T) {
+	sha256, err := influxdb2_algo.New(influxdb2_algo.WithVariant(influxdb2_algo.VariantSHA256))
+	require.NoError(t, err)
+	sha512, err := influxdb2_algo.New(influxdb2_algo.WithVariant(influxdb2_algo.VariantSHA512))
+	require.NoError(t, err)
+
+	type authData struct {
+		ID          platform.ID
+		Token       string
+		HashedToken string
+	}
+	type testConfig struct {
+		enabled bool
+		algo    string
+	}
+	type testCase struct {
+		desc         string
+		config       testConfig
+		action       func(t *testing.T, ctx context.Context, store *authorization.Store, tx kv.Tx)
+		exp          []authData
+		hashedTokens []string // tokens which only exists as hashes
+	}
+	cases := []testCase{
+		{
+			desc:   "initial unhashed",
+			config: testConfig{enabled: false},
+			action: func(t *testing.T, ctx context.Context, store *authorization.Store, tx kv.Tx) {
+				a := &influxdb.Authorization{
+					ID:     platform.ID(1),
+					OrgID:  platform.ID(1),
+					UserID: platform.ID(1),
+					Token:  "Token#1",
+				}
+				require.NoError(t, store.CreateAuthorization(ctx, tx, a))
+			},
+			exp: []authData{
+				{ID: platform.ID(1), Token: "Token#1"},
+			},
+		},
+		{
+			desc:   "upgrade hashed #1", // update hash and indices
+			config: testConfig{enabled: true, algo: influxdb2_algo.VariantIdentifierSHA256},
+			exp: []authData{
+				{ID: platform.ID(1), HashedToken: sha256.MustHash("Token#1").Encode()},
+			},
+			hashedTokens: []string{"Token#1"},
+		},
+		{
+			desc:   "downgrade hashed #1", // can't unhash
+			config: testConfig{enabled: false, algo: influxdb2_algo.VariantIdentifierSHA256},
+			action: func(t *testing.T, ctx context.Context, store *authorization.Store, tx kv.Tx) {
+				a := &influxdb.Authorization{
+					ID:     platform.ID(2),
+					OrgID:  platform.ID(2),
+					UserID: platform.ID(2),
+					Token:  "Token#2",
+				}
+				require.NoError(t, store.CreateAuthorization(ctx, tx, a))
+			},
+			exp: []authData{
+				{ID: platform.ID(1), HashedToken: sha256.MustHash("Token#1").Encode()},
+				{ID: platform.ID(2), Token: "Token#2"},
+			},
+			hashedTokens: []string{"Token#1"},
+		},
+		{
+			desc:   "upgrade hashed sha512", // can't rehash existing, use new algo for new auths
+			config: testConfig{enabled: true, algo: influxdb2_algo.VariantIdentifierSHA512},
+			action: func(t *testing.T, ctx context.Context, store *authorization.Store, tx kv.Tx) {
+				a := &influxdb.Authorization{
+					ID:     platform.ID(3),
+					OrgID:  platform.ID(3),
+					UserID: platform.ID(3),
+					Token:  "Token#3",
+				}
+				require.NoError(t, store.CreateAuthorization(ctx, tx, a))
+			},
+			exp: []authData{
+				{ID: platform.ID(1), HashedToken: sha256.MustHash("Token#1").Encode()},
+				{ID: platform.ID(2), HashedToken: sha512.MustHash("Token#2").Encode()},
+				{ID: platform.ID(3), HashedToken: sha512.MustHash("Token#3").Encode()},
+			},
+			hashedTokens: []string{"Token#1", "Token#2", "Token#3"},
+		},
+	}
+
+	ctx := context.Background()
+
+	// The underlying kv store persists across tests cases. This allows for testing how opening with
+	// new authentication configurations impacts the data.
+	kvStore := inmem.NewKVStore()
+	err = all.Up(ctx, zaptest.NewLogger(t), kvStore)
+	require.NoError(t, err)
+
+	for _, tc := range cases {
+		t.Run(tc.desc, func(t *testing.T) {
+			// Create new authorization.Store for test cases using existing kvStore.
+			variantName := tc.config.algo
+			if variantName == "" {
+				if !tc.config.enabled {
+					variantName = authorization.DefaultHashVariantName
+				} else {
+					require.Fail(t, "Must specific algo if hashing is enabled for test case")
+				}
+			}
+			store, err := authorization.NewStore(ctx, kvStore, tc.config.enabled, authorization.WithAuthorizationHashVariantName(variantName))
+			require.NoError(t, err)
+			require.NotNil(t, store)
+
+			// Execute action, if given. Simply opening the store with a different configuration may be the "action".
+			if tc.action != nil {
+				err = kvStore.Update(ctx, func(tx kv.Tx) error {
+					tc.action(t, ctx, store, tx)
+					return nil
+				})
+				require.NoError(t, err)
+			}
+
+			// Check results.
+			err = kvStore.View(ctx, func(tx kv.Tx) error {
+				// Collect all authorization data from store.
+				storedAuths, err := store.ListAuthorizations(ctx, tx, influxdb.AuthorizationFilter{})
+				require.NoError(t, err)
+
+				// Collect auth data from data currently in store
+				actualAuthData := make([]authData, 0, len(storedAuths))
+				for _, sa := range storedAuths {
+					ad := authData{ID: sa.ID, Token: sa.Token, HashedToken: sa.HashedToken}
+					actualAuthData = append(actualAuthData, ad)
+				}
+
+				// Check that authData matches exp.
+				require.ElementsMatch(t, tc.exp, actualAuthData)
+
+				// Collect data from kvStore's token index.
+				collectIndex := func(indexName string) map[string]platform.ID {
+					indexMap := make(map[string]platform.ID)
+					index, err := tx.Bucket([]byte(indexName))
+					require.NoError(t, err)
+					cursor, err := index.Cursor()
+					require.NoError(t, err)
+					for k, v := cursor.First(); k != nil; k, v = cursor.Next() {
+						var id platform.ID
+						require.NoError(t, id.Decode(v))
+						indexMap[string(k)] = id
+					}
+					return indexMap
+				}
+				actualTokenIndex := collectIndex(authIndexName)
+				actualHashedIndex := collectIndex(hashedAuthIndexName)
+
+				// Collect expected token and hashed indices.
+				expTokenIndex := make(map[string]platform.ID)
+				expHashedIndex := make(map[string]platform.ID)
+				for _, d := range tc.exp {
+					if d.Token != "" {
+						expTokenIndex[d.Token] = d.ID
+					}
+					if d.HashedToken != "" {
+						expHashedIndex[d.HashedToken] = d.ID
+					}
+				}
+
+				// Compare indices.
+				require.Equal(t, expTokenIndex, actualTokenIndex)
+				require.Equal(t, expHashedIndex, actualHashedIndex)
+
+				// Make sure we can lookup all tokens.
+				var allTokens []string
+				for _, d := range tc.exp {
+					if d.Token != "" {
+						allTokens = append(allTokens, d.Token)
+					}
+				}
+				allTokens = append(allTokens, tc.hashedTokens...)
+
+				for _, token := range allTokens {
+					auth, err := store.GetAuthorizationByToken(ctx, tx, token)
+					require.NoError(t, err, "error looking up token %q", token)
+					require.NotNil(t, auth)
+				}
+
+				return nil
+			})
+		})
+
+	}
+}