-
Notifications
You must be signed in to change notification settings - Fork 0
Harden session JWT revocation #64
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| ALTER TABLE sessions | ||
| ADD COLUMN IF NOT EXISTS token_id TEXT NOT NULL DEFAULT ''; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,8 +12,18 @@ import ( | |
| type Manager struct { | ||
| privateKey ed25519.PrivateKey | ||
| publicKey ed25519.PublicKey | ||
| issuer string | ||
| clockSkew time.Duration | ||
| now func() time.Time | ||
| } | ||
|
|
||
| type Option func(*Manager) | ||
|
|
||
| const ( | ||
| defaultIssuer = "asb" | ||
| defaultClockSkew = 30 * time.Second | ||
| ) | ||
|
|
||
| type claims struct { | ||
| SessionID string `json:"sid"` | ||
| TenantID string `json:"tenant_id"` | ||
|
|
@@ -24,21 +34,56 @@ type claims struct { | |
| jwt.RegisteredClaims | ||
| } | ||
|
|
||
| func NewManager(privateKey ed25519.PrivateKey) (*Manager, error) { | ||
| func WithIssuer(issuer string) Option { | ||
| return func(manager *Manager) { | ||
| if issuer != "" { | ||
| manager.issuer = issuer | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func WithClockSkew(clockSkew time.Duration) Option { | ||
| return func(manager *Manager) { | ||
| if clockSkew >= 0 { | ||
| manager.clockSkew = clockSkew | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func WithNowFunc(now func() time.Time) Option { | ||
| return func(manager *Manager) { | ||
| if now != nil { | ||
| manager.now = now | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func NewManager(privateKey ed25519.PrivateKey, options ...Option) (*Manager, error) { | ||
| if len(privateKey) == 0 { | ||
| return nil, fmt.Errorf("%w: private key is required", core.ErrInvalidRequest) | ||
| } | ||
| publicKey, ok := privateKey.Public().(ed25519.PublicKey) | ||
| if !ok { | ||
| return nil, fmt.Errorf("%w: private key public component is %T, want ed25519.PublicKey", core.ErrInvalidRequest, privateKey.Public()) | ||
| } | ||
| return &Manager{ | ||
| manager := &Manager{ | ||
| privateKey: privateKey, | ||
| publicKey: publicKey, | ||
| }, nil | ||
| issuer: defaultIssuer, | ||
| clockSkew: defaultClockSkew, | ||
| now: time.Now, | ||
| } | ||
| for _, option := range options { | ||
| option(manager) | ||
| } | ||
| return manager, nil | ||
| } | ||
|
|
||
| func (m *Manager) Sign(session *core.Session) (string, error) { | ||
| tokenID := session.TokenID | ||
| if tokenID == "" { | ||
| tokenID = session.ID | ||
| } | ||
| token := jwt.NewWithClaims(jwt.SigningMethodEdDSA, claims{ | ||
| SessionID: session.ID, | ||
| TenantID: session.TenantID, | ||
|
|
@@ -49,7 +94,10 @@ func (m *Manager) Sign(session *core.Session) (string, error) { | |
| RegisteredClaims: jwt.RegisteredClaims{ | ||
| ExpiresAt: jwt.NewNumericDate(session.ExpiresAt), | ||
| IssuedAt: jwt.NewNumericDate(session.CreatedAt), | ||
| NotBefore: jwt.NewNumericDate(session.CreatedAt.Add(-m.clockSkew)), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Double clock skew applied to
|
||
| Issuer: m.issuer, | ||
| Subject: session.WorkloadIdentity.Subject, | ||
| ID: tokenID, | ||
| }, | ||
| }) | ||
| return token.SignedString(m.privateKey) | ||
|
|
@@ -61,7 +109,7 @@ func (m *Manager) Verify(raw string) (*core.SessionClaims, error) { | |
| return nil, fmt.Errorf("%w: unexpected signing method %q", core.ErrUnauthorized, token.Method.Alg()) | ||
| } | ||
| return m.publicKey, nil | ||
| }) | ||
| }, jwt.WithIssuer(m.issuer), jwt.WithIssuedAt(), jwt.WithLeeway(m.clockSkew), jwt.WithTimeFunc(m.now)) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("%w: %v", core.ErrUnauthorized, err) | ||
| } | ||
|
|
@@ -70,21 +118,27 @@ func (m *Manager) Verify(raw string) (*core.SessionClaims, error) { | |
| if !ok || !parsed.Valid { | ||
| return nil, fmt.Errorf("%w: invalid session token", core.ErrUnauthorized) | ||
| } | ||
| if tokenClaims.ExpiresAt == nil { | ||
| return nil, fmt.Errorf("%w: missing exp", core.ErrUnauthorized) | ||
| } | ||
| if tokenClaims.NotBefore == nil { | ||
| return nil, fmt.Errorf("%w: missing nbf", core.ErrUnauthorized) | ||
| } | ||
| if tokenClaims.ID == "" { | ||
| return nil, fmt.Errorf("%w: missing jti", core.ErrUnauthorized) | ||
| } | ||
| if tokenClaims.SessionID == "" || tokenClaims.TenantID == "" { | ||
| return nil, fmt.Errorf("%w: missing required session claims", core.ErrUnauthorized) | ||
| } | ||
|
|
||
| return &core.SessionClaims{ | ||
| SessionID: tokenClaims.SessionID, | ||
| TenantID: tokenClaims.TenantID, | ||
| AgentID: tokenClaims.AgentID, | ||
| RunID: tokenClaims.RunID, | ||
| TokenID: tokenClaims.ID, | ||
| ToolContext: append([]string(nil), tokenClaims.ToolContext...), | ||
| WorkloadHash: tokenClaims.WorkloadHash, | ||
| ExpiresAt: tokenClaims.ExpiresAt.Time, | ||
| }, nil | ||
| } | ||
|
|
||
| func (c *claims) Valid() error { | ||
| if c.ExpiresAt == nil { | ||
| return fmt.Errorf("%w: missing exp", core.ErrUnauthorized) | ||
| } | ||
| return jwt.NewValidator(jwt.WithTimeFunc(time.Now)).Validate(c.RegisteredClaims) | ||
| } | ||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exported
WithClockSkewoption is never usedLow Severity
WithClockSkewis an exportedOptionfunction that is defined but never called anywhere in the codebase — not in production code, bootstrap wiring, or tests. It's dead code introduced in this PR.Reviewed by Cursor Bugbot for commit 03b97ea. Configure here.