chore: minor improvements to readability and updated code style (#827)

go.mod

@@ -1,11 +1,5 @@
 module github.com/ory/fosite
 
-replace github.com/dgrijalva/jwt-go => github.com/form3tech-oss/jwt-go v3.2.1+incompatible
-
-replace github.com/gobuffalo/packr => github.com/gobuffalo/packr v1.30.1
-
-replace github.com/gorilla/sessions => github.com/gorilla/sessions v1.2.1
-
 require (
 	github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2
 	github.com/cristalhq/jwt/v4 v4.0.2

token/hmac/bytes.go

@@ -14,7 +14,7 @@ import (
 func RandomBytes(n int) ([]byte, error) {
 	bytes := make([]byte, n)
 	if _, err := io.ReadFull(rand.Reader, bytes); err != nil {
-		return []byte{}, errorsx.WithStack(err)
+		return nil, errorsx.WithStack(err)
 	}
 	return bytes, nil
 }

token/hmac/bytes_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestRandomBytes(t *testing.T) {
@@ -17,13 +18,11 @@ func TestRandomBytes(t *testing.T) {
 
 func TestPseudoRandomness(t *testing.T) {
 	runs := 65536
-	results := map[string]bool{}
+	results := map[string]struct{}{}
 	for i := 0; i < runs; i++ {
 		bytes, err := RandomBytes(128)
-		assert.NoError(t, err)
-
-		_, ok := results[string(bytes)]
-		assert.False(t, ok)
-		results[string(bytes)] = true
+		require.NoError(t, err)
+		results[string(bytes)] = struct{}{}
 	}
+	assert.Len(t, results, runs)
 }

token/hmac/hmacsha.go

@@ -36,10 +36,7 @@ type HMACStrategy struct {
 }
 
 const (
-	// key should be at least 256 bit long, making it
-	minimumEntropy int = 32
-
-	// the secrets (client and global) should each have at least 16 characters making it harder to guess them
+	minimumEntropy      = 32
 	minimumSecretLength = 32
 )
 
@@ -51,27 +48,27 @@ func (c *HMACStrategy) Generate(ctx context.Context) (string, string, error) {
 	c.Lock()
 	defer c.Unlock()
 
-	secrets, err := c.Config.GetGlobalSecret(ctx)
+	globalSecret, err := c.Config.GetGlobalSecret(ctx)
 	if err != nil {
 		return "", "", err
 	}
 
-	if len(secrets) < minimumSecretLength {
-		return "", "", errors.Errorf("secret for signing HMAC-SHA512/256 is expected to be 32 byte long, got %d byte", len(secrets))
+	if len(globalSecret) < minimumSecretLength {
+		return "", "", errors.Errorf("secret for signing HMAC-SHA512/256 is expected to be 32 byte long, got %d byte", len(globalSecret))
 	}
 
 	var signingKey [32]byte
-	copy(signingKey[:], secrets)
+	copy(signingKey[:], globalSecret)
 
 	entropy := c.Config.GetTokenEntropy(ctx)
 	if entropy < minimumEntropy {
 		entropy = minimumEntropy
 	}
 
-	// When creating secrets not intended for usage by human users (e.g.,
+	// When creating tokens not intended for usage by human users (e.g.,
 	// client secrets or token handles), the authorization server should
 	// include a reasonable level of entropy in order to mitigate the risk
-	// of guessing attacks.  The token value should be >=128 bits long and
+	// of guessing attacks. The token value should be >=128 bits long and
 	// constructed from a cryptographically strong random or pseudo-random
 	// number sequence (see [RFC4086] for best current practice) generated
 	// by the authorization server.
@@ -91,34 +88,36 @@ func (c *HMACStrategy) Generate(ctx context.Context) (string, string, error) {
 func (c *HMACStrategy) Validate(ctx context.Context, token string) (err error) {
 	var keys [][]byte
 
-	secrets, err := c.Config.GetGlobalSecret(ctx)
+	globalSecret, err := c.Config.GetGlobalSecret(ctx)
 	if err != nil {
 		return err
 	}
 
+	if len(globalSecret) > 0 {
+		keys = append(keys, globalSecret)
+	}
+
 	rotatedSecrets, err := c.Config.GetRotatedGlobalSecrets(ctx)
 	if err != nil {
 		return err
 	}
 
-	if len(secrets) > 0 {
-		keys = append(keys, secrets)
+	keys = append(keys, rotatedSecrets...)
+
+	if len(keys) == 0 {
+		return errors.New("a secret for signing HMAC-SHA512/256 is expected to be defined, but none were")
 	}
 
-	keys = append(keys, rotatedSecrets...)
 	for _, key := range keys {
 		if err = c.validate(ctx, key, token); err == nil {
 			return nil
 		} else if errors.Is(err, fosite.ErrTokenSignatureMismatch) {
+			// Continue to the next key. The error will be returned if it is the last key.
 		} else {
 			return err
 		}
 	}
 
-	if err == nil {
-		return errors.New("a secret for signing HMAC-SHA512/256 is expected to be defined, but none were")
-	}
-
 	return err
 }
 
@@ -130,13 +129,11 @@ func (c *HMACStrategy) validate(ctx context.Context, secret []byte, token string
 	var signingKey [32]byte
 	copy(signingKey[:], secret)
 
-	split := strings.Split(token, ".")
-	if len(split) != 2 {
+	tokenKey, tokenSignature, ok := strings.Cut(token, ".")
+	if !ok {
 		return errorsx.WithStack(fosite.ErrInvalidTokenFormat)
 	}
 
-	tokenKey := split[0]
-	tokenSignature := split[1]
 	if tokenKey == "" || tokenSignature == "" {
 		return errorsx.WithStack(fosite.ErrInvalidTokenFormat)
 	}
@@ -161,13 +158,11 @@ func (c *HMACStrategy) validate(ctx context.Context, secret []byte, token string
 }
 
 func (c *HMACStrategy) Signature(token string) string {
-	split := strings.Split(token, ".")
-
-	if len(split) != 2 {
+	_, sig, ok := strings.Cut(token, ".")
+	if !ok {
 		return ""
 	}
-
-	return split[1]
+	return sig
 }
 
 func (c *HMACStrategy) generateHMAC(ctx context.Context, data []byte, key *[32]byte) []byte {

token/hmac/hmacsha_test.go

@@ -6,6 +6,7 @@ package hmac
 import (
 	"context"
 	"crypto/sha512"
+	"fmt"
 	"testing"
 
 	"github.com/ory/fosite"
@@ -23,113 +24,111 @@ func TestGenerateFailsWithShortCredentials(t *testing.T) {
 }
 
 func TestGenerate(t *testing.T) {
-	for _, c := range []struct {
-		globalSecret []byte
-		tokenEntropy int
-	}{
-		{
-			globalSecret: []byte("1234567890123456789012345678901234567890"),
-			tokenEntropy: 32,
-		},
-		{
-			globalSecret: []byte("1234567890123456789012345678901234567890"),
-			tokenEntropy: 64,
-		},
-	} {
-		config := &fosite.Config{
-			GlobalSecret: c.globalSecret,
-			TokenEntropy: c.tokenEntropy,
-		}
-		cg := HMACStrategy{Config: config}
-
-		token, signature, err := cg.Generate(context.Background())
-		require.NoError(t, err)
-		require.NotEmpty(t, token)
-		require.NotEmpty(t, signature)
-		t.Logf("Token: %s\n Signature: %s", token, signature)
-
-		err = cg.Validate(context.Background(), token)
-		require.NoError(t, err)
-
-		validateSignature := cg.Signature(token)
-		assert.Equal(t, signature, validateSignature)
-
-		config.GlobalSecret = []byte("baz")
-		err = cg.Validate(context.Background(), token)
-		require.Error(t, err)
+	ctx := context.Background()
+	config := &fosite.Config{
+		GlobalSecret: []byte("1234567890123456789012345678901234567890"),
+	}
+	cg := HMACStrategy{Config: config}
+
+	for _, entropy := range []int{32, 64} {
+		t.Run(fmt.Sprintf("entropy=%d", entropy), func(t *testing.T) {
+			config.TokenEntropy = entropy
+
+			token, signature, err := cg.Generate(ctx)
+			require.NoError(t, err)
+			require.NotEmpty(t, token)
+			require.NotEmpty(t, signature)
+
+			err = cg.Validate(ctx, token)
+			require.NoError(t, err)
+
+			actualSignature := cg.Signature(token)
+			assert.Equal(t, signature, actualSignature)
+
+			config.GlobalSecret = append([]byte("not"), config.GlobalSecret...)
+			err = cg.Validate(ctx, token)
+			assert.ErrorIs(t, err, fosite.ErrTokenSignatureMismatch)
+		})
 	}
 }
 
 func TestValidateSignatureRejects(t *testing.T) {
-	var err error
 	cg := HMACStrategy{
 		Config: &fosite.Config{GlobalSecret: []byte("1234567890123456789012345678901234567890")},
 	}
 	for k, c := range []string{
 		"",
 		" ",
-		"foo.bar",
+		".",
 		"foo.",
 		".foo",
 	} {
-		err = cg.Validate(context.Background(), c)
-		assert.Error(t, err)
-		t.Logf("Passed test case %d", k)
+		t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
+			err := cg.Validate(context.Background(), c)
+			assert.ErrorIs(t, err, fosite.ErrInvalidTokenFormat)
+		})
 	}
+
+	err := cg.Validate(context.Background(), "foo.bar")
+	assert.ErrorIs(t, err, fosite.ErrTokenSignatureMismatch)
 }
 
 func TestValidateWithRotatedKey(t *testing.T) {
-	old := HMACStrategy{Config: &fosite.Config{GlobalSecret: []byte("1234567890123456789012345678901234567890")}}
+	ctx := context.Background()
+	oldGlobalSecret := []byte("1234567890123456789012345678901234567890")
+	old := HMACStrategy{Config: &fosite.Config{GlobalSecret: oldGlobalSecret}}
 	now := HMACStrategy{Config: &fosite.Config{
 		GlobalSecret: []byte("0000000090123456789012345678901234567890"),
 		RotatedGlobalSecrets: [][]byte{
 			[]byte("abcdefgh90123456789012345678901234567890"),
-			[]byte("1234567890123456789012345678901234567890"),
+			oldGlobalSecret,
 		},
-	},
-	}
+	}}
 
-	token, _, err := old.Generate(context.Background())
+	token, _, err := old.Generate(ctx)
 	require.NoError(t, err)
 
-	require.EqualError(t, now.Validate(context.Background(), "thisisatoken.withaninvalidsignature"), fosite.ErrTokenSignatureMismatch.Error())
-	require.NoError(t, now.Validate(context.Background(), token))
+	assert.ErrorIs(t, now.Validate(ctx, "thisisatoken.withaninvalidsignature"), fosite.ErrTokenSignatureMismatch)
+	assert.NoError(t, now.Validate(ctx, token))
 }
 
 func TestValidateWithRotatedKeyInvalid(t *testing.T) {
-	old := HMACStrategy{Config: &fosite.Config{GlobalSecret: []byte("1234567890123456789012345678901234567890")}}
+	ctx := context.Background()
+	oldGlobalSecret := []byte("1234567890123456789012345678901234567890")
+	old := HMACStrategy{Config: &fosite.Config{GlobalSecret: oldGlobalSecret}}
 	now := HMACStrategy{Config: &fosite.Config{
 		GlobalSecret: []byte("0000000090123456789012345678901234567890"),
 		RotatedGlobalSecrets: [][]byte{
 			[]byte("abcdefgh90123456789012345678901"),
-			[]byte("1234567890123456789012345678901234567890"),
+			oldGlobalSecret,
 		}},
 	}
 
-	token, _, err := old.Generate(context.Background())
+	token, _, err := old.Generate(ctx)
 	require.NoError(t, err)
 
-	require.EqualError(t, now.Validate(context.Background(), token), "secret for signing HMAC-SHA512/256 is expected to be 32 byte long, got 31 byte")
+	require.EqualError(t, now.Validate(ctx, token), "secret for signing HMAC-SHA512/256 is expected to be 32 byte long, got 31 byte")
 
-	require.EqualError(t, (&HMACStrategy{Config: &fosite.Config{}}).Validate(context.Background(), token), "a secret for signing HMAC-SHA512/256 is expected to be defined, but none were")
+	require.EqualError(t, (&HMACStrategy{Config: &fosite.Config{}}).Validate(ctx, token), "a secret for signing HMAC-SHA512/256 is expected to be defined, but none were")
 }
 
 func TestCustomHMAC(t *testing.T) {
-	def := HMACStrategy{Config: &fosite.Config{
-		GlobalSecret: []byte("1234567890123456789012345678901234567890")},
-	}
-	sha512 := HMACStrategy{Config: &fosite.Config{
-		GlobalSecret: []byte("1234567890123456789012345678901234567890"),
+	ctx := context.Background()
+	globalSecret := []byte("1234567890123456789012345678901234567890")
+	defaultHasher := HMACStrategy{Config: &fosite.Config{
+		GlobalSecret: globalSecret,
+	}}
+	sha512Hasher := HMACStrategy{Config: &fosite.Config{
+		GlobalSecret: globalSecret,
 		HMACHasher:   sha512.New,
-	},
-	}
+	}}
 
-	token, _, err := def.Generate(context.Background())
+	token, _, err := defaultHasher.Generate(ctx)
 	require.NoError(t, err)
-	require.EqualError(t, sha512.Validate(context.Background(), token), fosite.ErrTokenSignatureMismatch.Error())
+	require.ErrorIs(t, sha512Hasher.Validate(ctx, token), fosite.ErrTokenSignatureMismatch)
 
-	token512, _, err := sha512.Generate(context.Background())
+	token512, _, err := sha512Hasher.Generate(ctx)
 	require.NoError(t, err)
-	require.NoError(t, sha512.Validate(context.Background(), token512))
-	require.EqualError(t, def.Validate(context.Background(), token512), fosite.ErrTokenSignatureMismatch.Error())
+	require.NoError(t, sha512Hasher.Validate(ctx, token512))
+	require.ErrorIs(t, defaultHasher.Validate(ctx, token512), fosite.ErrTokenSignatureMismatch)
 }