From 72c4b83fd30c85a9759a6b0c56a56ea317bba36c Mon Sep 17 00:00:00 2001 From: Adrian Bannister Date: Tue, 7 Apr 2026 23:00:36 +1000 Subject: [PATCH] Fix access token invalidating JWT signature by re-serializing the header getTokenIfExists now returns the original raw token string from disk alongside the parsed JWS object. Callers use the raw string instead of CompactSerialize(), which re-marshals the header with Go's alphabetical key ordering and changes the base64url bytes the signature covers. Co-authored-by: Forge --- token/token.go | 24 +++++++------ token/token_test.go | 85 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 10 deletions(-) diff --git a/token/token.go b/token/token.go index ac1de27e9eb..40fea32c827 100644 --- a/token/token.go +++ b/token/token.go @@ -400,7 +400,7 @@ func GetOrgTokenIfExists(authDomain string) (string, error) { if err != nil { return "", err } - token, err := getTokenIfExists(path) + raw, token, err := getTokenIfExists(path) if err != nil { return "", err } @@ -414,7 +414,7 @@ func GetOrgTokenIfExists(authDomain string) (string, error) { err := os.Remove(path) return "", err } - return token.CompactSerialize() + return raw, nil } func GetAppTokenIfExists(appInfo *AppInfo) (string, error) { @@ -422,7 +422,7 @@ func GetAppTokenIfExists(appInfo *AppInfo) (string, error) { if err != nil { return "", err } - token, err := getTokenIfExists(path) + raw, token, err := getTokenIfExists(path) if err != nil { return "", err } @@ -436,20 +436,24 @@ func GetAppTokenIfExists(appInfo *AppInfo) (string, error) { err := os.Remove(path) return "", err } - return token.CompactSerialize() + return raw, nil } -// GetTokenIfExists will return the token from local storage if it exists and not expired -func getTokenIfExists(path string) (*jose.JSONWebSignature, error) { +// getTokenIfExists will return the token from local storage if it exists and not expired. +// It returns both the raw token string (as stored on disk) and the parsed JWS object. +// Callers should use the raw string when returning the token to preserve the original +// serialization and avoid invalidating the JWT signature. +func getTokenIfExists(path string) (string, *jose.JSONWebSignature, error) { content, err := os.ReadFile(path) if err != nil { - return nil, err + return "", nil, err } - token, err := jose.ParseSigned(string(content), signatureAlgs) + raw := strings.TrimSpace(string(content)) + token, err := jose.ParseSigned(raw, signatureAlgs) if err != nil { - return nil, err + return "", nil, err } - return token, nil + return raw, token, nil } // RemoveTokenIfExists removes the a token from local storage if it exists diff --git a/token/token_test.go b/token/token_test.go index da92ed73a02..90405fb7d28 100644 --- a/token/token_test.go +++ b/token/token_test.go @@ -1,10 +1,16 @@ package token import ( + "encoding/base64" "encoding/json" "net/http" "net/url" + "os" + "path/filepath" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestHandleRedirects_AttachOrgToken(t *testing.T) { @@ -133,3 +139,82 @@ func TestJwtPayloadUnmarshal_FailsWhenAudIsOmitted(t *testing.T) { t.Errorf("Expected %v, got %v", wantErr, err) } } + +// craftTestToken builds a compact JWS string from the given raw JSON header, +// payload, and fake signature bytes. This lets tests control exact header key +// ordering. +func craftTestToken(header, payload, sig string) string { + h := base64.RawURLEncoding.EncodeToString([]byte(header)) + p := base64.RawURLEncoding.EncodeToString([]byte(payload)) + s := base64.RawURLEncoding.EncodeToString([]byte(sig)) + return h + "." + p + "." + s +} + +func TestGetTokenIfExists_PreservesOriginalTokenString(t *testing.T) { + t.Parallel() + + // Header with non-alphabetical key ordering: "typ" before "alg". + // go-jose's CompactSerialize() re-marshals header JSON with Go's default + // alphabetical key order ("alg" before "typ"), producing different base64url + // bytes and invalidating the JWT signature. + header := `{"typ":"JWT","alg":"RS256"}` + payload := `{"aud":"test-aud","exp":9999999999,"iat":1,"nbf":1,"sub":"test","email":"test@test.com","iss":"test"}` + originalToken := craftTestToken(header, payload, "fake-sig") + + tmpFile := filepath.Join(t.TempDir(), "test-token") + require.NoError(t, os.WriteFile(tmpFile, []byte(originalToken), 0600)) + + raw, token, err := getTokenIfExists(tmpFile) + require.NoError(t, err) + require.NotNil(t, token) + + // The returned raw string must be byte-for-byte identical to what was on disk. + assert.Equal(t, originalToken, raw) + + // CompactSerialize() re-encodes the header with alphabetical keys, which + // produces a different string — exactly the bug this change fixes. + reserialized, err := token.CompactSerialize() + require.NoError(t, err) + assert.NotEqual(t, originalToken, reserialized, + "CompactSerialize() reorders header keys, which would invalidate the JWT signature") +} + +func TestGetTokenIfExists_TrimsWhitespace(t *testing.T) { + t.Parallel() + + header := `{"alg":"RS256"}` + payload := `{"aud":"test-aud","exp":9999999999,"iat":1,"nbf":1,"sub":"test","email":"test@test.com","iss":"test"}` + token := craftTestToken(header, payload, "fake-sig") + + // Simulate a file with trailing newline/whitespace (common with text editors). + tokenWithWhitespace := " " + token + " \n" + tmpFile := filepath.Join(t.TempDir(), "test-token-ws") + require.NoError(t, os.WriteFile(tmpFile, []byte(tokenWithWhitespace), 0600)) + + raw, parsed, err := getTokenIfExists(tmpFile) + require.NoError(t, err) + require.NotNil(t, parsed) + + assert.Equal(t, token, raw, "raw token should have surrounding whitespace trimmed") +} + +func TestGetTokenIfExists_FileNotFound(t *testing.T) { + t.Parallel() + + raw, token, err := getTokenIfExists(filepath.Join(t.TempDir(), "nonexistent")) + assert.Error(t, err) + assert.Empty(t, raw) + assert.Nil(t, token) +} + +func TestGetTokenIfExists_InvalidToken(t *testing.T) { + t.Parallel() + + tmpFile := filepath.Join(t.TempDir(), "bad-token") + require.NoError(t, os.WriteFile(tmpFile, []byte("not-a-valid-jws"), 0600)) + + raw, token, err := getTokenIfExists(tmpFile) + assert.Error(t, err) + assert.Empty(t, raw) + assert.Nil(t, token) +}