Skip to content

fix: use codes to represent signin errors - #11

Merged
disintegrator merged 1 commit into
mainfrom
signin-error-codes
Aug 7, 2025
Merged

fix: use codes to represent signin errors#11
disintegrator merged 1 commit into
mainfrom
signin-error-codes

Conversation

@disintegrator

Copy link
Copy Markdown
Contributor

This change switches the auth redirection to use error codes instead of error messages. This prevents potential content spoofing attacks because error messages in the URL represent arbitrary content that an attacker can control to inject malicious content (like urls) and send to victims.

Now we send back simple error codes and the client side maps the ones it recognizes to fixed error messages that are shown to the user.

This change switches the auth redirection to use error codes instead of
error messages. This prevents potential content spoofing attacks because
error messages in the URL represent arbitrary content that an attacker
can control to inject malicious content (like urls) and send to victims.
@disintegrator
disintegrator requested a review from a team as a code owner August 7, 2025 11:48
@vercel

vercel Bot commented Aug 7, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
gram ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 7, 2025 11:50am
gram-landing ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 7, 2025 11:50am

@disintegrator
disintegrator merged commit 05d690a into main Aug 7, 2025
10 checks passed
@disintegrator
disintegrator deleted the signin-error-codes branch August 7, 2025 14:35
qstearns added a commit that referenced this pull request May 5, 2026
Stand up server/internal/remotesessions/ mirroring the usersessions
skeleton: stubs for remoteSessionIssuers, remoteSessionClients, and
remoteSessions services per spike §6.2. One Service struct implements
all three Goa interfaces; per-resource Attach* helpers are wired in
start.go.

All handler bodies return oops.CodeNotImplemented. Audit, RBAC, and real
SQLc queries land in tickets #9-#11.

Adds URN types for each resource and the sqlc.yaml stanza.
qstearns added a commit that referenced this pull request May 5, 2026
Ticket #9 of Milestone #1. Implements remoteSessionIssuers.{discover,
create,update,list,get,delete} per spike §6.2.

discover hits the upstream /.well-known/oauth-authorization-server (RFC
8414), validates outbound destinations through guardian.Policy, parses
the metadata, and returns a RemoteSessionIssuerDraft with
discovery_warnings for any RFC 8414 deviations. No persistence.

delete is blocked (CodeConflict) when remote_session_clients still
reference the issuer.

Adds two new dependencies on the remotesessions Service struct:
encryption.Client (for ticket #10's client_secret_encrypted) and
guardian.Policy (for outbound HTTP validation in discover and DCR).
start.go is updated to plumb them through.

Audit subjects (remote_session_issuer + remote_session for #11's
cascade reuse), RBAC scope checks, mv/remotesessionissuer.go view
builder, and per-method tests wired. 17 tests pass in this package.
qstearns added a commit that referenced this pull request May 6, 2026
Stand up server/internal/remotesessions/ mirroring the usersessions
skeleton: stubs for remoteSessionIssuers, remoteSessionClients, and
remoteSessions services per spike §6.2. One Service struct implements
all three Goa interfaces; per-resource Attach* helpers are wired in
start.go.

All handler bodies return oops.CodeNotImplemented. Audit, RBAC, and real
SQLc queries land in tickets #9-#11.

Adds URN types for each resource and the sqlc.yaml stanza.
qstearns added a commit that referenced this pull request May 6, 2026
Ticket #9 of Milestone #1. Implements remoteSessionIssuers.{discover,
create,update,list,get,delete} per spike §6.2.

discover hits the upstream /.well-known/oauth-authorization-server (RFC
8414), validates outbound destinations through guardian.Policy, parses
the metadata, and returns a RemoteSessionIssuerDraft with
discovery_warnings for any RFC 8414 deviations. No persistence.

delete is blocked (CodeConflict) when remote_session_clients still
reference the issuer.

Adds two new dependencies on the remotesessions Service struct:
encryption.Client (for ticket #10's client_secret_encrypted) and
guardian.Policy (for outbound HTTP validation in discover and DCR).
start.go is updated to plumb them through.

Audit subjects (remote_session_issuer + remote_session for #11's
cascade reuse), RBAC scope checks, mv/remotesessionissuer.go view
builder, and per-method tests wired. 17 tests pass in this package.
qstearns added a commit that referenced this pull request May 6, 2026
Stand up server/internal/remotesessions/ mirroring the usersessions
skeleton: stubs for remoteSessionIssuers, remoteSessionClients, and
remoteSessions services per spike §6.2. One Service struct implements
all three Goa interfaces; per-resource Attach* helpers are wired in
start.go.

All handler bodies return oops.CodeNotImplemented. Audit, RBAC, and real
SQLc queries land in tickets #9-#11.

Adds URN types for each resource and the sqlc.yaml stanza.
qstearns added a commit that referenced this pull request May 6, 2026
Ticket #9 of Milestone #1. Implements remoteSessionIssuers.{discover,
create,update,list,get,delete} per spike §6.2.

discover hits the upstream /.well-known/oauth-authorization-server (RFC
8414), validates outbound destinations through guardian.Policy, parses
the metadata, and returns a RemoteSessionIssuerDraft with
discovery_warnings for any RFC 8414 deviations. No persistence.

delete is blocked (CodeConflict) when remote_session_clients still
reference the issuer.

Adds two new dependencies on the remotesessions Service struct:
encryption.Client (for ticket #10's client_secret_encrypted) and
guardian.Policy (for outbound HTTP validation in discover and DCR).
start.go is updated to plumb them through.

Audit subjects (remote_session_issuer + remote_session for #11's
cascade reuse), RBAC scope checks, mv/remotesessionissuer.go view
builder, and per-method tests wired. 17 tests pass in this package.
qstearns added a commit that referenced this pull request May 6, 2026
Stand up server/internal/remotesessions/ mirroring the usersessions
skeleton: stubs for remoteSessionIssuers, remoteSessionClients, and
remoteSessions services per spike §6.2. One Service struct implements
all three Goa interfaces; per-resource Attach* helpers are wired in
start.go.

All handler bodies return oops.CodeNotImplemented. Audit, RBAC, and real
SQLc queries land in tickets #9-#11.

Adds URN types for each resource and the sqlc.yaml stanza.
qstearns added a commit that referenced this pull request May 6, 2026
Ticket #9 of Milestone #1. Implements remoteSessionIssuers.{discover,
create,update,list,get,delete} per spike §6.2.

discover hits the upstream /.well-known/oauth-authorization-server (RFC
8414), validates outbound destinations through guardian.Policy, parses
the metadata, and returns a RemoteSessionIssuerDraft with
discovery_warnings for any RFC 8414 deviations. No persistence.

delete is blocked (CodeConflict) when remote_session_clients still
reference the issuer.

Adds two new dependencies on the remotesessions Service struct:
encryption.Client (for ticket #10's client_secret_encrypted) and
guardian.Policy (for outbound HTTP validation in discover and DCR).
start.go is updated to plumb them through.

Audit subjects (remote_session_issuer + remote_session for #11's
cascade reuse), RBAC scope checks, mv/remotesessionissuer.go view
builder, and per-method tests wired. 17 tests pass in this package.
qstearns added a commit that referenced this pull request May 6, 2026
Stand up server/internal/remotesessions/ mirroring the usersessions
skeleton: stubs for remoteSessionIssuers, remoteSessionClients, and
remoteSessions services per spike §6.2. One Service struct implements
all three Goa interfaces; per-resource Attach* helpers are wired in
start.go.

All handler bodies return oops.CodeNotImplemented. Audit, RBAC, and real
SQLc queries land in tickets #9-#11.

Adds URN types for each resource and the sqlc.yaml stanza.
qstearns added a commit that referenced this pull request May 6, 2026
Ticket #9 of Milestone #1. Implements remoteSessionIssuers.{discover,
create,update,list,get,delete} per spike §6.2.

discover hits the upstream /.well-known/oauth-authorization-server (RFC
8414), validates outbound destinations through guardian.Policy, parses
the metadata, and returns a RemoteSessionIssuerDraft with
discovery_warnings for any RFC 8414 deviations. No persistence.

delete is blocked (CodeConflict) when remote_session_clients still
reference the issuer.

Adds two new dependencies on the remotesessions Service struct:
encryption.Client (for ticket #10's client_secret_encrypted) and
guardian.Policy (for outbound HTTP validation in discover and DCR).
start.go is updated to plumb them through.

Audit subjects (remote_session_issuer + remote_session for #11's
cascade reuse), RBAC scope checks, mv/remotesessionissuer.go view
builder, and per-method tests wired. 17 tests pass in this package.
qstearns added a commit that referenced this pull request May 7, 2026
Stand up server/internal/remotesessions/ mirroring the usersessions
skeleton: stubs for remoteSessionIssuers, remoteSessionClients, and
remoteSessions services per spike §6.2. One Service struct implements
all three Goa interfaces; per-resource Attach* helpers are wired in
start.go.

All handler bodies return oops.CodeNotImplemented. Audit, RBAC, and real
SQLc queries land in tickets #9-#11.

Adds URN types for each resource and the sqlc.yaml stanza.
qstearns added a commit that referenced this pull request May 7, 2026
Ticket #9 of Milestone #1. Implements remoteSessionIssuers.{discover,
create,update,list,get,delete} per spike §6.2.

discover hits the upstream /.well-known/oauth-authorization-server (RFC
8414), validates outbound destinations through guardian.Policy, parses
the metadata, and returns a RemoteSessionIssuerDraft with
discovery_warnings for any RFC 8414 deviations. No persistence.

delete is blocked (CodeConflict) when remote_session_clients still
reference the issuer.

Adds two new dependencies on the remotesessions Service struct:
encryption.Client (for ticket #10's client_secret_encrypted) and
guardian.Policy (for outbound HTTP validation in discover and DCR).
start.go is updated to plumb them through.

Audit subjects (remote_session_issuer + remote_session for #11's
cascade reuse), RBAC scope checks, mv/remotesessionissuer.go view
builder, and per-method tests wired. 17 tests pass in this package.
qstearns added a commit that referenced this pull request May 12, 2026
Stand up server/internal/remotesessions/ mirroring the usersessions
skeleton: stubs for remoteSessionIssuers, remoteSessionClients, and
remoteSessions services per spike §6.2. One Service struct implements
all three Goa interfaces; per-resource Attach* helpers are wired in
start.go.

All handler bodies return oops.CodeNotImplemented. Audit, RBAC, and real
SQLc queries land in tickets #9-#11.

Adds URN types for each resource and the sqlc.yaml stanza.
qstearns added a commit that referenced this pull request May 12, 2026
Ticket #9 of Milestone #1. Implements remoteSessionIssuers.{discover,
create,update,list,get,delete} per spike §6.2.

discover hits the upstream /.well-known/oauth-authorization-server (RFC
8414), validates outbound destinations through guardian.Policy, parses
the metadata, and returns a RemoteSessionIssuerDraft with
discovery_warnings for any RFC 8414 deviations. No persistence.

delete is blocked (CodeConflict) when remote_session_clients still
reference the issuer.

Adds two new dependencies on the remotesessions Service struct:
encryption.Client (for ticket #10's client_secret_encrypted) and
guardian.Policy (for outbound HTTP validation in discover and DCR).
start.go is updated to plumb them through.

Audit subjects (remote_session_issuer + remote_session for #11's
cascade reuse), RBAC scope checks, mv/remotesessionissuer.go view
builder, and per-method tests wired. 17 tests pass in this package.
qstearns added a commit that referenced this pull request May 12, 2026
…dings

Security:
- Sec #1: Auth-code redemption is now atomic via Redis GETDEL (new
  Cache.GetAndDelete). Two clients racing on the same code can't both win.
- Sec #14: Consent-state consumption uses the same GETDEL primitive,
  preventing parallel POSTs from minting multiple grants per /authorize.
- Sec #13: ValidateBearer fails closed on revocation-cache errors (Redis
  outage → tokens reject, not admit).
- Sec #4: /revoke verifies the presented token belongs to the
  authenticated client (RFC 7009 §2.1). Ownership mismatches look like
  the "unknown token" silent-no-op per §2.2. New GetUserSessionByJTI
  and GetUserSessionByRefreshTokenHash peek queries support this.
- Sec #9: JWT audience is now urn.NewToolset(toolset.ID) — globally
  unique even when slugs collide across projects. No cross-project
  token replay.
- Sec #11: JWT iss claim honours custom domains (matches the AS
  metadata document's issuer).

Correctness:
- Bug #2: Access-token lifetime is hardcoded (1h). issuer.SessionDuration
  is now the refresh-token (i.e. total session) lifetime — the value the
  IDP-driven session window was meant to control.
- Bug #5: HandleIDPCallback forwards IDP error responses back to the MCP
  client's redirect_uri instead of swallowing them as a generic 400.
- Bug #6: Consent POST refuses to mint an anonymous session for a
  private toolset — IDP authentication is required first.
- Bug #8: Consent approve path requires action=approve explicitly.
  Missing or unknown values 400, not fail-open into the approval branch.
- Bug #10: HandleToken now calls requireUserSessionIssuer like the
  other authn-dance handlers.
- Bug #15: Consent POST → GET redirects use 303 See Other (was 302
  Found, which leaves the verb implementation-defined).

Hardening:
- Hard #3: isUniqueViolation uses errors.As + *pgconn.PgError.Code
  instead of strings.Contains on the error message.
- Hard #7: Migrate the remaining "jwt-signing-key" literal callsites
  (chatsessions/assistanttokens/assets) to usersessions.JWTSigningKeyFlag.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants