Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
9 changes: 9 additions & 0 deletions coordinator/internal/api/device_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,15 @@ func (s *Server) handleDeviceToken(w http.ResponseWriter, r *http.Request) {
})

case "approved":
// Atomically consume the device code before issuing the token.
// This prevents a second poll (or a race) from minting a second
// long-lived token for the same browser approval.
if err := s.store.ConsumeDeviceCode(dc.DeviceCode); err != nil {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Defer consuming code until token issuance is guaranteed

Consuming the device code before generating and persisting the provider token makes the flow non-recoverable on downstream failures: if rand.Read or CreateProviderToken fails, the code is already marked consumed, so subsequent polls cannot succeed and the user must restart login. This introduces a new failure mode under transient entropy/DB issues and should be handled atomically (consume + token create in one transaction) or with rollback semantics.

Useful? React with 👍 / 👎.

// Another concurrent request already consumed it.
writeJSON(w, http.StatusGone, errorResponse("expired_token", "device code is no longer valid"))
return
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Distinguish store failures from already-consumed device codes

The handler maps every ConsumeDeviceCode error to expired_token, but the Postgres implementation can return operational errors (for example query timeout/connection errors). In those cases this path returns a terminal client error instead of a retriable server error, causing clients to abort login even though the code may still be valid.

Useful? React with 👍 / 👎.

}

// Generate a long-lived provider token.
tokenBytes := make([]byte, 32)
if _, err := rand.Read(tokenBytes); err != nil {
Expand Down
6 changes: 6 additions & 0 deletions coordinator/internal/store/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,12 @@ type Store interface {
// ApproveDeviceCode links a device code to an account, marking it approved.
ApproveDeviceCode(deviceCode, accountID string) error

// ConsumeDeviceCode atomically transitions a device code from approved to
// consumed. Returns an error if the code is not in the approved state.
// Must be called when issuing a provider token to prevent a second poll
// from minting an additional token for the same approval.
ConsumeDeviceCode(deviceCode string) error

// DeleteExpiredDeviceCodes removes device codes that have passed their expiry.
DeleteExpiredDeviceCodes() error

Expand Down
15 changes: 15 additions & 0 deletions coordinator/internal/store/memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -1095,6 +1095,21 @@ func (s *MemoryStore) ApproveDeviceCode(deviceCode, accountID string) error {
return nil
}

func (s *MemoryStore) ConsumeDeviceCode(deviceCode string) error {
s.mu.Lock()
defer s.mu.Unlock()

dc, ok := s.deviceCodesByCode[deviceCode]
if !ok {
return errors.New("device code not found")
}
if dc.Status != "approved" {
return fmt.Errorf("device code is %s, not approved", dc.Status)
}
dc.Status = "consumed"
return nil
}

func (s *MemoryStore) DeleteExpiredDeviceCodes() error {
s.mu.Lock()
defer s.mu.Unlock()
Expand Down
18 changes: 18 additions & 0 deletions coordinator/internal/store/postgres.go
Original file line number Diff line number Diff line change
Expand Up @@ -1829,6 +1829,24 @@ func (s *PostgresStore) ApproveDeviceCode(deviceCode, accountID string) error {
return nil
}

func (s *PostgresStore) ConsumeDeviceCode(deviceCode string) error {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()

tag, err := s.pool.Exec(ctx,
`UPDATE device_codes SET status = 'consumed'
WHERE device_code = $1 AND status = 'approved'`,
deviceCode,
)
if err != nil {
return fmt.Errorf("store: consume device code: %w", err)
}
if tag.RowsAffected() == 0 {
return errors.New("device code not found or not in approved state")
}
return nil
}

func (s *PostgresStore) DeleteExpiredDeviceCodes() error {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
Expand Down
Loading