address review comments

Author: eternal-flame-AD

api/tokens_test.go

@@ -9,11 +9,11 @@ import (
 
 func TestTokenGeneration(t *testing.T) {
 	clientPub, clientPriv := generateClientToken()
-	assert.Regexp(t, regexp.MustCompile(`^gtfy_client\.(.+)$`), clientPub)
-	assert.Regexp(t, regexp.MustCompile(`^gtfy_client\.(.+)$`), clientPriv)
+	assert.Regexp(t, regexp.MustCompile(`^gtfyc\.(.+)$`), clientPub)
+	assert.Regexp(t, regexp.MustCompile(`^gtfyc\.(.+)$`), clientPriv)
 	applicationPub, applicationPriv := generateApplicationToken()
-	assert.Regexp(t, regexp.MustCompile(`^gtfy_app\.(.+)$`), applicationPub)
-	assert.Regexp(t, regexp.MustCompile(`^gtfy_app\.(.+)$`), applicationPriv)
+	assert.Regexp(t, regexp.MustCompile(`^gtfya\.(.+)$`), applicationPub)
+	assert.Regexp(t, regexp.MustCompile(`^gtfya\.(.+)$`), applicationPriv)
 	imageName := generateImageName()
 	assert.Regexp(t, regexp.MustCompile(`^(.+)$`), imageName)
 }

auth/authentication.go

@@ -90,6 +90,10 @@ func (a *Auth) evaluate(ctx *gin.Context, funcs ...func(ctx *gin.Context) (authS
 	for _, fn := range funcs {
 		state, err := fn(ctx)
 		if err != nil {
+			if errors.Is(err, errCannotParseToken) {
+				ctx.AbortWithError(401, err)
+				return true
+			}
 			ctx.AbortWithError(500, err)
 			return true
 		}
@@ -155,7 +159,7 @@ func (a *Auth) handleClient(checks ...func(*model.Client) (authState, error)) fu
 		if strings.HasPrefix(token, enhancedTokenPrefix) {
 			complexToken, err := ParseEnhancedToken(token)
 			if err != nil || !complexToken.ValidateTimestamp(timeNow().Unix()) {
-				return authStateSkip, nil
+				return authStateSkip, err
 			}
 			token = complexToken.PublicForm()
 		}
@@ -197,7 +201,7 @@ func (a *Auth) handleApplication(ctx *gin.Context) (authState, error) {
 	if strings.HasPrefix(token, enhancedTokenPrefix) {
 		complexToken, err := ParseEnhancedToken(token)
 		if err != nil || !complexToken.ValidateTimestamp(timeNow().Unix()) {
-			return authStateSkip, nil
+			return authStateSkip, err
 		}
 		token = complexToken.PublicForm()
 	}

auth/token.go

@@ -20,17 +20,18 @@ const (
 
 var (
 	errInvalidToken     = errors.New("invalid token")
+	errCannotParseToken = errors.New("cannot parse token")
 	errNoPrivateKey     = errors.New("no private key")
 	tokenCharacters     = []byte("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_")
 	pluginPrefix        = "P"
-	enhancedTokenPrefix = "gtfy_"
+	enhancedTokenPrefix = "gtfy"
 
 	randReader = rand.Reader
 )
 
 type EnhancedToken struct {
-	ident        string // a shared identifier, in formats like A12 for application ID 12
-	pubOrPrivKey []byte // public key
+	ident        string
+	pubOrPrivKey []byte
 	timestamp    int64
 	signature    []byte
 }
@@ -51,7 +52,7 @@ func (c *EnhancedToken) Sign(timestamp int64) (*EnhancedToken, error) {
 	}
 	privKey := ed25519.NewKeyFromSeed(c.pubOrPrivKey)
 	sha512 := sha512.New()
-	sha512.Write([]byte("iss="))
+	sha512.Write([]byte("iat="))
 	fmt.Fprintf(sha512, "%d", timestamp)
 	sign, err := privKey.Sign(nil, sha512.Sum(nil), crypto.SHA512)
 	if err != nil {
@@ -107,49 +108,49 @@ func NewEnhancedToken(ident string) *EnhancedToken {
 func ParseEnhancedToken(token string) (*EnhancedToken, error) {
 	token, found := strings.CutPrefix(token, enhancedTokenPrefix)
 	if !found {
-		return nil, errInvalidToken
+		return nil, fmt.Errorf("%w: token must start with %s", errCannotParseToken, enhancedTokenPrefix)
 	}
 
 	// count number of dots, one dot -> ident then private key, three dots -> ident, public key, challenge then signature
 	fields := strings.SplitN(token, ".", 4)
 	if len(fields) != 2 && len(fields) != 4 {
-		return nil, errInvalidToken
+		return nil, fmt.Errorf("%w: token must have 2 or 4 fields separated by dots", errCannotParseToken)
 	}
 	ident := fields[0]
 	pkOrPubkeyB64 := fields[1]
 	pkOrPubkeyBytesLen := base64.RawURLEncoding.DecodedLen(len(pkOrPubkeyB64))
 	pkOrPubkey, err := base64.RawURLEncoding.DecodeString(pkOrPubkeyB64)
 	if err != nil {
-		return nil, errInvalidToken
+		return nil, fmt.Errorf("%w: base64 decode failed: %w", errCannotParseToken, err)
 	}
 	if len(fields) == 2 {
 		if pkOrPubkeyBytesLen != ed25519.SeedSize {
-			return nil, errInvalidToken
+			return nil, fmt.Errorf("%w: private key must be %d bytes", errCannotParseToken, ed25519.SeedSize)
 		}
 		return &EnhancedToken{
 			ident:        ident,
 			pubOrPrivKey: pkOrPubkey,
 		}, nil
 	}
 	if pkOrPubkeyBytesLen != ed25519.PublicKeySize {
-		return nil, errInvalidToken
+		return nil, fmt.Errorf("%w: public key must be %d bytes", errCannotParseToken, ed25519.PublicKeySize)
 	}
 	timestampStr := fields[2]
 	timestamp, err := strconv.ParseInt(timestampStr, 10, 64)
 	if err != nil {
-		return nil, errInvalidToken
+		return nil, fmt.Errorf("%w: timestamp must be an integer: %w", errCannotParseToken, err)
 	}
 	signatureB64 := fields[3]
 	signatureBytesLen := base64.RawURLEncoding.DecodedLen(len(signatureB64))
 	if signatureBytesLen != ed25519.SignatureSize {
-		return nil, errInvalidToken
+		return nil, fmt.Errorf("%w: signature must be %d bytes", errCannotParseToken, ed25519.SignatureSize)
 	}
 	signature, err := base64.RawURLEncoding.DecodeString(signatureB64)
 	if err != nil {
-		return nil, errInvalidToken
+		return nil, fmt.Errorf("%w: base64 decode failed: %w", errCannotParseToken, err)
 	}
 	sha512 := sha512.New()
-	sha512.Write([]byte("iss=")) // query-like encoding to give us some semantic headroom should we need more fields in the future
+	sha512.Write([]byte("iat=")) // query-like encoding to give us some semantic headroom should we need more fields in the future
 	fmt.Fprintf(sha512, "%d", timestamp)
 	if err := ed25519.VerifyWithOptions(pkOrPubkey, sha512.Sum(nil), signature, &ed25519.Options{Hash: crypto.SHA512}); err != nil {
 		return nil, errInvalidToken
@@ -173,13 +174,13 @@ func randIntn(n int) int {
 
 // GenerateApplicationToken generates an application token.
 func GenerateApplicationToken() (publicForm, privateForm string) {
-	token := NewEnhancedToken("app")
+	token := NewEnhancedToken("a")
 	return token.PublicForm(), token.String()
 }
 
 // GenerateClientToken generates a client token.
 func GenerateClientToken() (publicForm, privateForm string) {
-	token := NewEnhancedToken("client")
+	token := NewEnhancedToken("c")
 	return token.PublicForm(), token.String()
 }
 

auth/token_test.go

@@ -3,14 +3,15 @@ package auth
 import (
 	"crypto/rand"
 	"errors"
+	"strings"
 	"testing"
 	"testing/iotest"
 
 	"github.com/stretchr/testify/assert"
 )
 
 func TestNewComplexToken(t *testing.T) {
-	token := NewEnhancedToken("A12")
+	token := NewEnhancedToken("a")
 	canonicalizedExpected := token.PublicForm()
 	tokenParsed, err := ParseEnhancedToken(token.String())
 	assert.NoError(t, err)
@@ -32,11 +33,12 @@ func TestNewComplexToken(t *testing.T) {
 	assert.NoError(t, err)
 	canonicalizedActual = tokenParsed.PublicForm()
 	assert.Equal(t, canonicalizedExpected, canonicalizedActual)
-	tokenSignedStrMutated := tokenSignedStr[1:]
-	if tokenSignedStr[0] != 'A' {
-		tokenSignedStrMutated = "A" + tokenSignedStrMutated
+	var tokenSignedStrMutated string
+	lastDotIdx := strings.LastIndex(tokenSignedStr, ".")
+	if tokenSignedStr[lastDotIdx+1] != 'A' {
+		tokenSignedStrMutated = tokenSignedStr[:lastDotIdx+1] + "A" + tokenSignedStr[lastDotIdx+2:]
 	} else {
-		tokenSignedStrMutated = "B" + tokenSignedStrMutated
+		tokenSignedStrMutated = tokenSignedStr[:lastDotIdx+1] + "B" + tokenSignedStr[lastDotIdx+2:]
 	}
 	_, err = ParseEnhancedToken(tokenSignedStrMutated)
 	assert.ErrorIs(t, err, errInvalidToken)

model/application.go

@@ -18,7 +18,7 @@ type Application struct {
 	//
 	// read only: true
 	// example: AWH0wZ5r0Mbac.r
-	Token  string `gorm:"type:varchar(180);uniqueIndex:uix_applications_token" json:"token"`
+	Token  string `gorm:"type:varchar(180);uniqueIndex:uix_applications_token" json:"token,omitempty"`
 	UserID uint   `gorm:"index;uniqueIndex:uix_application_user_id_sort_key,priority:1" json:"-"`
 	// The application name. This is how the application should be displayed to the user.
 	//

model/client.go

@@ -18,7 +18,7 @@ type Client struct {
 	//
 	// read only: true
 	// example: CWH0wZ5r0Mbac.r
-	Token  string `gorm:"type:varchar(180);uniqueIndex:uix_clients_token" json:"token"`
+	Token  string `gorm:"type:varchar(180);uniqueIndex:uix_clients_token" json:"token,omitempty"`
 	UserID uint   `gorm:"index" json:"-"`
 	// The client name. This is how the client should be displayed to the user.
 	//

ui/src/tests/application.test.ts

@@ -64,7 +64,7 @@ const createApp =
         await page.waitForSelector($dialog.button('.finish'));
         await page.waitForSelector($dialog.p('.token'));
         const token = await innerText(page, $dialog.p('.token'));
-        expect(token.startsWith('gtfy_app.')).toBeTruthy();
+        expect(token.startsWith('gtfya.')).toBeTruthy();
         await page.click($dialog.button('.finish'));
         await waitToDisappear(page, $dialog.selector());
     };

ui/src/tests/client.test.ts

@@ -50,7 +50,7 @@ const fillClientDialog =
         if (hasToken) {
             await page.waitForSelector($dialog.p('.token'));
             const token = await innerText(page, $dialog.p('.token'));
-            expect(token.startsWith('gtfy_client.')).toBeTruthy();
+            expect(token.startsWith('gtfyc.')).toBeTruthy();
             await page.waitForSelector($dialog.button('.finish'));
             await page.click($dialog.button('.finish'));
         }