chore: updates based on PR comments

- Removed `authResponse.HashedToken` field
- Improved error messages, log messages, and comments
- Improved error propagation
- Improve description of `--use-hashed-tokens` command line parameter
- Removed dead code
- Implemented test to make sure hashed version of tokens can not be
  presented for authentication

Author: gwossum

authorization/http_server.go

@@ -137,7 +137,6 @@ type postAuthorizationRequest struct {
 type authResponse struct {
 	ID          platform.ID          `json:"id"`
 	Token       string               `json:"token"`
-	HashedToken string               `json:"hashedToken"`
 	Status      influxdb.Status      `json:"status"`
 	Description string               `json:"description"`
 	OrgID       platform.ID          `json:"orgID"`
@@ -167,7 +166,6 @@ func (h *AuthHandler) newAuthResponse(ctx context.Context, a *influxdb.Authoriza
 	res := &authResponse{
 		ID:          a.ID,
 		Token:       a.Token,
-		HashedToken: a.HashedToken,
 		Status:      a.Status,
 		Description: a.Description,
 		OrgID:       a.OrgID,
@@ -199,7 +197,6 @@ func (a *authResponse) toInfluxdb() *influxdb.Authorization {
 	res := &influxdb.Authorization{
 		ID:          a.ID,
 		Token:       a.Token,
-		HashedToken: a.HashedToken,
 		Status:      a.Status,
 		Description: a.Description,
 		OrgID:       a.OrgID,

authorization/storage.go

@@ -82,8 +82,9 @@ into BoltDB. Raw tokens are not stored.
 To verify tokens when hashed tokens are enabled, the presented token's hash is calculated and used
 for token index lookup. The rest of the authorization flow is unchanged.
 
-The hashed token index is separate from the raw token index. Newer versions also verify that the token
-is not a valid PHC string before starting authorization. This prevents the following attack:
+The hashed token index is separate from the raw token index. In addition, the token presented by the API
+is rejected if it is a PHC encoded hash before starting authorization. The separate index and rejected
+PHC tokens from the API prevent the following attack:
 1. Hashed token is extracted from BoltDB.
 2. Token hashing is disabled.
 3. The hashed token is presented to the API, which will misinterpret it as a raw token and allow access.
@@ -300,17 +301,16 @@ func (s *Store) hashedTokenMigration(ctx context.Context) error {
 	// Figure out which authorization records need to be updated.
 	var authsNeedingUpdate []*influxdb.Authorization
 	err := s.View(ctx, func(tx kv.Tx) error {
-		s.forEachAuthorization(ctx, tx, nil, func(a *influxdb.Authorization) bool {
+		return s.forEachAuthorization(ctx, tx, nil, func(a *influxdb.Authorization) bool {
 			if a.IsHashedTokenClear() {
 				if a.IsTokenSet() {
 					authsNeedingUpdate = append(authsNeedingUpdate, a)
 				} else {
-					s.log.Warn("found authorization without any token set during hashed token migration", zap.Uint64("ID", uint64(a.ID)), zap.String("description", a.Description))
+					s.log.Warn("during hashed token migration, found authorization without any token set", zap.Uint64("ID", uint64(a.ID)), zap.String("description", a.Description))
 				}
 			}
 			return true
 		})
-		return nil
 	})
 	if err != nil {
 		return err

authorization/storage_authorization.go

@@ -20,7 +20,6 @@ import (
 var (
 	ErrNilAuthorization    = goerrors.New("authorization cannot be nil")
 	ErrHashedTokenMismatch = goerrors.New("HashedToken does not match Token")
-	ErrIncorrectToken      = goerrors.New("token is incorrect for authorization")
 	ErrNoTokenAvailable    = goerrors.New("no token available for authorization")
 )
 

cmd/influxd/launcher/cmd.go

@@ -707,7 +707,7 @@ func (o *InfluxdOpts) BindCliOpts() []cli.Opt {
 			DestP:   &o.UseHashedTokens,
 			Flag:    "use-hashed-tokens",
 			Default: o.UseHashedTokens,
-			Desc:    "enable token hashing",
+			Desc:    "enable storing hashed API tokens on disk (improves security, but prevents downgrades to < 2.8)",
 		},
 	}
 }

cmd/influxd/launcher/launcher.go

@@ -293,7 +293,8 @@ func (m *Launcher) run(ctx context.Context, opts *InfluxdOpts) (err error) {
 	var authSvc platform.AuthorizationService
 	{
 		hasherVariantName := authorization.DefaultHashVariantName // This value could come from opts in the future.
-		authStore, err := authorization.NewStore(ctx, m.kvStore, opts.UseHashedTokens, authorization.WithAuthorizationHashVariantName(hasherVariantName), authorization.WithLogger(m.log))
+		authStoreLogger := m.log.With(zap.String("store", "auth"))
+		authStore, err := authorization.NewStore(ctx, m.kvStore, opts.UseHashedTokens, authorization.WithAuthorizationHashVariantName(hasherVariantName), authorization.WithLogger(authStoreLogger))
 		if err != nil {
 			m.log.Error("Failed creating new authorization store", zap.Error(err), zap.Bool("UseHashedTokens", opts.UseHashedTokens), zap.String("hasherVariant", hasherVariantName))
 			return err

pkg/crypt/algorithm/influxdb2/decoder.go

@@ -36,11 +36,6 @@ func RegisterDecoderVariant(r algorithm.DecoderRegister, variant Variant) error
 	return nil
 }
 
-// Decode the encoded digest into a algorithm.Digest.
-func Decode(encodedDigest string) (digest algorithm.Digest, err error) {
-	return DecodeVariant(VariantNone)(encodedDigest)
-}
-
 // DecodeVariant the encoded digest into a algorithm.Digest provided it matches the provided plaintext.Variant. If
 // plaintext.VariantNone is used all variants can be decoded.
 func DecodeVariant(v Variant) func(encodedDigest string) (digest algorithm.Digest, err error) {

pkg/crypt/algorithm/influxdb2/digest.go

@@ -2,15 +2,16 @@ package influxdb2
 
 import (
 	"crypto/subtle"
+	"errors"
 	"fmt"
 
 	"github.com/go-crypt/crypt/algorithm"
 )
 
-// NewDigest creates a new plaintext.Digest using the plaintext.Variant.
-func NewDigest(password string) (digest Digest) {
-	return NewSHA256Digest(password)
-}
+// ErrDigestInvalid is an error returned when a hash digest has an invalid or unsupported properties. It is NOT
+// returned on token or password mismatches. It is equivalent to go-crypt's algorithm.ErrPasswordInvalid
+// error, but with a message that makes more sense for our usage with tokens.
+var ErrDigestInvalid = errors.New("hashed token or password is invalid")
 
 // NewSHA256Digest creates a new influxdb2.Digest using the SHA256 for the hash.
 func NewSHA256Digest(password string) (digest Digest) {
@@ -48,7 +49,7 @@ func (d *Digest) MatchAdvanced(password string) (match bool, err error) {
 // MatchBytesAdvanced is the same as MatchBytes except if there is an error it returns that as well.
 func (d *Digest) MatchBytesAdvanced(passwordBytes []byte) (match bool, err error) {
 	if len(d.key) == 0 {
-		return false, fmt.Errorf(algorithm.ErrFmtDigestMatch, AlgName, fmt.Errorf("%w: key has 0 bytes", algorithm.ErrPasswordInvalid))
+		return false, fmt.Errorf(algorithm.ErrFmtDigestMatch, AlgName, fmt.Errorf("%w: key has 0 bytes", ErrDigestInvalid))
 	}
 
 	input := d.Variant.Hash(passwordBytes)

testing/auth.go

@@ -13,6 +13,7 @@ import (
 	"github.com/influxdata/influxdb/v2/kit/platform"
 	"github.com/influxdata/influxdb/v2/kit/platform/errors"
 	"github.com/influxdata/influxdb/v2/mock"
+	"github.com/stretchr/testify/require"
 )
 
 const (
@@ -706,7 +707,6 @@ func FindAuthorizationByToken(
 		authorization *influxdb.Authorization
 	}
 
-	// VALIS: Add tests to make sure look-up by hashed token does /not/ work
 	tests := []struct {
 		name   string
 		fields AuthorizationFields
@@ -859,6 +859,13 @@ func FindAuthorizationByToken(
 			if diff := cmp.Diff(authorization, tt.wants.authorization, authorizationCmpOptions...); diff != "" {
 				t.Errorf("authorization is different -got/+want\ndiff %s", diff)
 			}
+
+			// Verify that lookup by the hashed token does not work.
+			if authorization.IsHashedTokenSet() {
+				a, err := s.FindAuthorizationByToken(ctx, authorization.HashedToken)
+				require.ErrorContains(t, err, "authorization not found")
+				require.Nil(t, a)
+			}
 		})
 	}
 }
@@ -875,7 +882,6 @@ func FindAuthorizations(
 		token  string
 	}
 
-	// VALIS: Do we need tests that set HashedToken, or tests with Token and HashedToken set?
 	type wants struct {
 		authorizations []*influxdb.Authorization
 		err            error