Skip to content

fix(auth): verify JWT subject matches stored API token tenant #5215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
igor-kupczynski merged 1 commit into from
Oct 15, 2026
Merged

fix(auth): verify JWT subject matches stored API token tenant #5215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions pkg/auth/token/token.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,10 @@ func (j *jwtManagerImpl) ValidateTenantToken(ctx context.Context, token string)
return uuid.Nil, uuid.Nil, fmt.Errorf("failed to parse subject claim: %v", err)
}

if dbToken.TenantId == nil || *dbToken.TenantId != parsedSubject {
return uuid.Nil, uuid.Nil, fmt.Errorf("subject claim does not match token tenant_id")
}

return parsedSubject, dbToken.ID, nil
}

Expand Down
101 changes: 93 additions & 8 deletions pkg/auth/token/token_authdisabled_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"time"

"github.com/google/uuid"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgtype"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand All @@ -23,14 +24,32 @@ import (
"github.com/hatchet-dev/hatchet/pkg/repository/sqlcv1"
)

type stubAPITokenRepository struct{}
type stubAPITokenRepository struct {
tokens map[uuid.UUID]*sqlcv1.APIToken
}

func (s *stubAPITokenRepository) CreateAPIToken(ctx context.Context, opts *v1.CreateAPITokenOpts) (*sqlcv1.APIToken, error) {
return &sqlcv1.APIToken{ID: opts.ID, ExpiresAt: pgtype.Timestamp{Time: opts.ExpiresAt, Valid: true}}, nil
tok := &sqlcv1.APIToken{
ID: opts.ID,
TenantId: opts.TenantId,
ExpiresAt: pgtype.Timestamp{Time: opts.ExpiresAt, Valid: true},
}

if s.tokens == nil {
s.tokens = make(map[uuid.UUID]*sqlcv1.APIToken)
}

s.tokens[opts.ID] = tok

return tok, nil
}

func (s *stubAPITokenRepository) GetAPITokenById(ctx context.Context, id uuid.UUID) (*sqlcv1.APIToken, error) {
return &sqlcv1.APIToken{ID: id, Revoked: false, ExpiresAt: pgtype.Timestamp{Time: time.Now().Add(24 * time.Hour), Valid: true}}, nil
if tok, ok := s.tokens[id]; ok {
return tok, nil
}

return nil, pgx.ErrNoRows
}

func (s *stubAPITokenRepository) ListAPITokensByTenant(ctx context.Context, tenantId uuid.UUID) ([]*sqlcv1.APIToken, error) {
Expand All @@ -50,11 +69,7 @@ func (s *stubAPITokenRepository) DeleteAPIToken(ctx context.Context, tenantId, i
// and vice versa. The embedded token is only accepted via the separate authdisabled manager, which
// is only wired up (in the gRPC middleware) behind the compile-time authmode.IsDisabled constant.
func TestAuthDisabledKeysetIsolation(t *testing.T) {
master, mainPriv, mainPub, _, err := encryption.GenerateLocalKeys()
require.NoError(t, err)

mainEnc, err := encryption.NewLocalEncryption(master, mainPriv, mainPub)
require.NoError(t, err)
mainEnc := newLocalEncryption(t)

adPriv, adPub := generateInsecureJWTKeyset(t)
adEnc, err := encryption.NewInsecureJWTEncryption(adPriv, adPub)
Expand Down Expand Up @@ -89,6 +104,76 @@ func TestAuthDisabledKeysetIsolation(t *testing.T) {
assert.Error(t, err, "the authdisabled manager must reject a main-keyset token")
}

func TestRejectsTokenWhenSubjectDoesNotMatchStoredTenant(t *testing.T) {
ctx := context.Background()
opts := &token.TokenOpts{Issuer: "hatchet", Audience: "hatchet", ServerURL: "http://localhost:8080"}

enc := newLocalEncryption(t)
mgr, err := token.NewJWTManager(enc, &stubAPITokenRepository{}, opts)
require.NoError(t, err)

tenantA := uuid.New()
tenantB := uuid.New()

tokenA, err := mgr.GenerateTenantToken(ctx, tenantA, "tenant-a", false, nil)
require.NoError(t, err)

gotTenant, _, err := mgr.ValidateTenantToken(ctx, tokenA.Token)
require.NoError(t, err)
assert.Equal(t, tenantA, gotTenant)

resignedWithTenantB := resignTokenWithSubject(t, enc, opts, tokenA, tenantB)

_, _, err = mgr.ValidateTenantToken(ctx, resignedWithTenantB)
assert.Error(t, err)
Comment on lines +125 to +128

@greptile-apps greptile-apps Bot Oct 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Nil-tenant rejection lacks a test

The new test covers a mismatched tenant but never a stored token with TenantId == nil. Add a case that clears the stored token鈥檚 TenantId, validates its otherwise valid JWT, and checks for an error. This would catch a future change that accepts an unbound token or dereferences a nil pointer.

@igor-kupczynski igor-kupczynski Oct 15, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

馃 Gemini 4 Argon on behalf of Igor:

All API token creation paths populate TenantId, and the dbToken.TenantId == nil guard sits on the exact same line right before *dbToken.TenantId != parsedSubject. A separate test mutating the in-memory stub just to exercise a one-line nil guard does not add meaningful coverage.

@greptile-apps greptile-apps Bot Oct 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That鈥檚 fair. Since token creation always sets TenantId, and the implementation explicitly rejects nil before dereferencing, a separate test that mutates the stub solely to hit this defensive branch would add little value beyond the existing mismatch test. I鈥檓 withdrawing the nil-tenant test request.

}

func newLocalEncryption(t *testing.T) encryption.EncryptionService {
t.Helper()

master, priv, pub, _, err := encryption.GenerateLocalKeys()
require.NoError(t, err)

enc, err := encryption.NewLocalEncryption(master, priv, pub)
require.NoError(t, err)

return enc
}

func resignTokenWithSubject(
t *testing.T,
enc encryption.EncryptionService,
opts *token.TokenOpts,
tok *token.Token,
subject uuid.UUID,
) string {
t.Helper()

signer, err := jwt.NewSigner(enc.GetPrivateJWTHandle())
require.NoError(t, err)

issuedAt := time.Now().Add(-time.Minute)
subjectStr := subject.String()

rawJWT, err := jwt.NewRawJWT(&jwt.RawJWTOptions{
IssuedAt: &issuedAt,
Audience: &opts.Audience,
Subject: &subjectStr,
ExpiresAt: &tok.ExpiresAt,
Issuer: &opts.Issuer,
CustomClaims: map[string]interface{}{
"token_id": tok.TokenId.String(),
"server_url": opts.ServerURL,
},
})
require.NoError(t, err)

signed, err := signer.SignAndEncode(rawJWT)
require.NoError(t, err)

return signed
}

func generateInsecureJWTKeyset(t *testing.T) (private, public []byte) {
t.Helper()

Expand Down
14 changes: 13 additions & 1 deletion pkg/auth/token/token_embedded_authdisabled_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,16 @@ package token_test
import (
"context"
"testing"
"time"

"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/hatchet-dev/hatchet/pkg/auth/token"
"github.com/hatchet-dev/hatchet/pkg/authmode"
"github.com/hatchet-dev/hatchet/pkg/encryption"
v1 "github.com/hatchet-dev/hatchet/pkg/repository"
)

// TestEmbeddedTokenValidates checks that the committed embedded token validates against a JWT
Expand All @@ -20,7 +23,16 @@ func TestEmbeddedTokenValidates(t *testing.T) {
enc, err := encryption.NewInsecureJWTEncryption(authmode.EmbeddedPrivateKeyset(), authmode.EmbeddedPublicKeyset())
require.NoError(t, err)

mgr, err := token.NewJWTManager(enc, &stubAPITokenRepository{}, &token.TokenOpts{
embeddedTenantID := uuid.MustParse(authmode.EmbeddedTokenTenantID)
repo := &stubAPITokenRepository{}
_, err = repo.CreateAPIToken(context.Background(), &v1.CreateAPITokenOpts{
ID: uuid.MustParse(authmode.EmbeddedTokenID),
TenantId: &embeddedTenantID,
ExpiresAt: time.Now().Add(24 * time.Hour),
})
require.NoError(t, err)

mgr, err := token.NewJWTManager(enc, repo, &token.TokenOpts{
Issuer: authmode.EmbeddedTokenIssuer,
Audience: authmode.EmbeddedTokenAudience,
ServerURL: authmode.EmbeddedTokenServerURL,
Expand Down
Loading