From 6ac1dbd8fca3db5c5d0ca249b32e8bd8ee2cf811 Mon Sep 17 00:00:00 2001 From: Arthur Belleville Date: Fri, 15 May 2026 20:41:58 +0200 Subject: [PATCH] docs(phase-8): add validation strategy --- .../phases/08-social-sign-in/08-RESEARCH.md | 246 ++++++++++++++++++ .../phases/08-social-sign-in/08-VALIDATION.md | 79 ++++++ 2 files changed, 325 insertions(+) create mode 100644 .planning/phases/08-social-sign-in/08-RESEARCH.md create mode 100644 .planning/phases/08-social-sign-in/08-VALIDATION.md diff --git a/.planning/phases/08-social-sign-in/08-RESEARCH.md b/.planning/phases/08-social-sign-in/08-RESEARCH.md new file mode 100644 index 0000000..8c00194 --- /dev/null +++ b/.planning/phases/08-social-sign-in/08-RESEARCH.md @@ -0,0 +1,246 @@ +# Phase 8: Social Sign-in - Research + +**Researched:** 2026-05-15 +**Domain:** Google OAuth/OIDC, Sign in with Apple, local account linking, Go server-managed sessions, sqlc/Postgres migration impact +**Confidence:** HIGH + +## Summary + +Phase 8 adds Google and Apple sign-in without changing the core auth authority: Xtablo still owns `users`, `sessions`, CSRF, and cookies. Provider tokens are only proof used during callback processing. After verification, the Go server creates or links a local user, then issues the existing Xtablo session cookie through `auth.Store`. + +The safest implementation shape is: + +1. Add `user_identities` and make `users.password_hash` nullable. +2. Add provider config parsing and disabled-button state. +3. Add Google start/callback. +4. Add Apple start/callback, including ES256 client-secret generation and Apple ID token verification. +5. Add linked-provider status view. +6. Verify existing email/password behavior has not regressed. + +Use `golang.org/x/oauth2` for authorization-code exchange. Use an OIDC/JWT verification package rather than hand-parsing ID tokens. Keep provider subject as the durable external identity key; email can change and is not the identity key after first link. + +## User Decisions From CONTEXT.md + +- Auto-link provider login to an existing Xtablo user when provider email is verified and matches. +- Reject provider callbacks with missing or unverified email. +- Provider subject wins after it is linked, even if email later changes. +- Add a simple linked-provider status view; unlinking is deferred. +- Make `users.password_hash` nullable and allow social-only users. +- Defer add-password/password management. +- Block email/password signup if the email already belongs to a social-only account. +- Existing password users who link Google/Apple keep password login. +- Show Google and Apple buttons on both login and signup. +- Show disabled provider buttons when config is missing. +- Give Google and Apple equal prominence. +- Redirect successful social sign-in to `/`. +- Store provider display name and avatar URL when available. +- Accept Apple private relay emails as verified when Apple verifies them. +- Update both `user_identities.email` and local `users.email` when provider email changes, with explicit conflict handling. + +## Phase Requirements + +| ID | Description | Research Support | +|----|-------------|------------------| +| AUTH-08 | User can start Google sign-in from login/signup | Provider buttons + `/auth/google/start` route; disabled config state | +| AUTH-09 | Google callback validates state, exchanges code, verifies ID token, creates/links local user | OAuth2 auth code flow + ID token verification + account linking transaction | +| AUTH-10 | User can start Apple sign-in from login/signup | Provider buttons + `/auth/apple/start` route; Apple Services ID config | +| AUTH-11 | Apple callback validates state/nonce, exchanges code, verifies ID token, creates/links local user | Apple token endpoint + ES256 client secret + nonce/state validation | +| AUTH-12 | Social sign-in issues existing server-managed session cookie | Reuse `auth.Store.Create` / `auth.SetSessionCookie` | +| AUTH-13 | Existing email/password auth still works | Regression tests around signup/login/logout/CSRF/rate limiting | + +## Standard Stack + +| Library / API | Purpose | Recommendation | +|---------------|---------|----------------| +| `golang.org/x/oauth2` | OAuth2 authorization URL and code exchange | Add as a direct dependency | +| `github.com/coreos/go-oidc/v3/oidc` | OIDC discovery, JWKS, ID token verification | Good fit for Google; can also verify ID tokens when issuer/JWKS are available | +| `github.com/go-jose/go-jose/v4` | JOSE/JWT primitives, ES256 client secret signing | Good fit for Apple client-secret JWT if `go-oidc` does not cover all Apple needs cleanly | +| Google OpenID Connect | Provider docs and ID token semantics | Use official docs for issuer/audience/email_verified behavior | +| Apple Sign in REST API | Authorization/token/client-secret rules | Use official docs for `client_secret`, token exchange, and ID token validation | + +Avoid: +- Managed auth platforms such as Clerk/Auth0/Lucia. +- Treating provider access tokens as Xtablo sessions. +- Linking on unverified email. +- Relinking a provider subject based on changed email. + +## Architecture Responsibility Map + +| Capability | Primary Tier | Secondary Tier | Rationale | +|------------|--------------|----------------|-----------| +| Provider start URL | Go web handler | Browser redirect | Server owns state/nonce generation and config | +| OAuth state/nonce storage | Go server + short-lived cookie or DB table | Browser returns state | Callback must prove it belongs to an initiated flow | +| Code exchange | Go server | Provider token endpoint | Client secret stays server-side | +| ID token verification | Go server | Provider JWKS | Must validate issuer, audience, expiry, nonce where applicable, subject, verified email | +| Account linking | Postgres transaction | Go handler | Race-safe lookup/create/link logic | +| Local session creation | Existing `auth.Store` | Browser cookie | Xtablo session remains authoritative | +| Provider status view | Go templates | Authenticated browser | Minimal account visibility; no unlink actions | + +## Data Model Research + +### `users` changes + +Current `users.password_hash text NOT NULL` blocks social-only accounts. Phase 8 should migrate to nullable: + +- `password_hash text NULL` +- Email/password login must treat `NULL` as "no password set" and return the same generic login error used for invalid credentials. +- Signup must check existing users by email before insert. If an existing user has `password_hash IS NULL`, block email/password signup and tell the user to sign in with the provider. + +### New `user_identities` + +Recommended columns: + +- `id uuid primary key default gen_random_uuid()` +- `user_id uuid not null references users(id) on delete cascade` +- `provider text not null` or enum constrained to `google`, `apple` +- `provider_subject text not null` +- `email citext not null` +- `email_verified boolean not null` +- `display_name text` +- `avatar_url text` +- `created_at timestamptz not null default now()` +- `updated_at timestamptz not null default now()` +- `last_login_at timestamptz` + +Indexes/constraints: + +- `unique(provider, provider_subject)` +- `index(user_id)` +- optional `check(email_verified = true)` if unverified identities are never stored + +Linking should happen in a DB transaction: + +1. Look up identity by `(provider, provider_subject)`. +2. If found: update identity profile/email metadata, update local `users.email` if needed, create session for identity's `user_id`. +3. If not found: require verified email. +4. Look up local `users.email`. +5. If found: insert identity for that user. +6. If not found: create social-only user with nullable `password_hash`, insert identity. + +Conflict case: if provider subject is linked to user A and provider email changes to an email owned by user B, D-03 says provider identity wins. The planner must handle the `users.email` uniqueness collision from D-17 explicitly. Recommended behavior: keep user A signed in, update `user_identities.email`, do not update `users.email`, and surface/log a conflict for future account cleanup. Silent relinking is forbidden. + +## Flow Details + +### Start routes + +- `GET /auth/google/start` +- `GET /auth/apple/start` + +Each route: + +- Verifies provider config exists. +- Generates cryptographically random state. +- Generates nonce for ID token validation where applicable. +- Stores state/nonce short-term, preferably in a signed HTTP-only cookie or a DB-backed OAuth state table. +- Redirects to the provider authorization endpoint. + +### Callback routes + +- `GET /auth/google/callback` +- `POST or GET /auth/apple/callback` depending on Apple response mode selected during implementation. + +Each callback: + +- Validates state. +- Validates nonce if included in ID token. +- Exchanges authorization code server-side. +- Verifies ID token: + - issuer + - audience/client ID + - expiry + - subject + - email present + - email verified +- Links/creates local user through a transaction. +- Calls existing session creation and cookie helper. +- Redirects to `/`. + +### Provider buttons + +Login and signup pages should both show Google and Apple buttons: + +- Equal prominence. +- Disabled if required env vars are missing. +- No provider button should submit the password form; use links/buttons to provider start routes. + +## Testing Strategy + +Use real DB integration tests for account linking and schema behavior. Mock providers locally with `httptest.Server` for token endpoints and JWKS responses; do not call real Google/Apple in tests. + +Required test classes: + +- Unit tests for provider config detection and disabled button state. +- Unit tests for OAuth state/nonce generation and validation. +- Unit tests for Apple client-secret JWT claims if generated in-process. +- Integration tests for nullable `password_hash` and email/password login behavior. +- Integration tests for provider identity linking: + - verified email matches existing password user -> identity inserted, session created + - missing/unverified email -> rejected + - new verified email -> social-only user created with NULL password hash + - social-only email/password signup blocked + - provider subject already linked -> linked user wins + - provider email update conflict with another user does not silently relink +- Handler tests for `/auth/{provider}/start` redirects and `/auth/{provider}/callback` session issuance. +- Regression tests for existing signup/login/logout/CSRF/rate-limit behavior. + +## Validation Architecture + +Automated validation should run through `go test ./...` from `backend/`. Phase-specific fast feedback can target: + +- `go test ./internal/auth ./internal/web` +- `go test ./internal/db/... ./internal/web -run 'Test.*(Social|OAuth|Provider|Login|Signup|Session)'` + +Validation dimensions: + +1. **Schema:** migrations apply cleanly; `password_hash` nullable; `user_identities` constraints exist. +2. **Provider Verification:** invalid state, invalid nonce, invalid audience, unverified email, and missing email are rejected. +3. **Linking Semantics:** every D-01..D-17 decision has an integration test or explicit acceptance assertion. +4. **Session Continuity:** social callback uses existing session cookie path and protected routes recognize the session. +5. **Regression:** existing auth tests continue to pass. +6. **Config:** `.env.example` documents provider secrets without real values. +7. **UI:** login/signup render equal provider buttons and disabled states. + +Manual validation: + +- Configure local test OAuth credentials or mocked callback path. +- Verify disabled provider buttons with missing env vars. +- Verify login/signup buttons route to provider start routes when configured. + +## Threat Model Notes + +Plans must include a `` block because this phase handles authentication. + +Threats to cover: + +- CSRF/login CSRF via OAuth callback without state validation. +- Replay/substitution via missing nonce validation. +- Token confusion by accepting ID tokens with wrong issuer or audience. +- Account takeover by linking on unverified email. +- Account takeover by relinking a provider subject after email change. +- Session fixation if callback reuses an old session without rotation. +- Open redirect if callback supports return URLs in the future. +- Secret leakage through logs (`client_secret`, auth code, ID token). +- Email uniqueness conflict when updating `users.email`. + +## Open Implementation Choices for Planner + +- State/nonce storage: signed short-lived cookie vs DB-backed OAuth state table. +- Exact OAuth/OIDC library combination for Google and Apple. +- Exact callback method for Apple (`form_post` vs query response mode). +- Whether provider status view lives at `/account` or `/account/providers`. +- Whether `provider` is a text column with CHECK constraint or a Postgres enum. + +## Sources + +- Google OpenID Connect docs: https://developers.google.com/identity/openid-connect/openid-connect +- Google OAuth 2.0 web-server flow: https://developers.google.com/identity/protocols/oauth2/web-server +- Apple token validation docs: https://developer.apple.com/documentation/SigninwithAppleRESTAPI/Generate-and-validate-tokens +- Apple authorization request docs: https://developer.apple.com/documentation/signinwithapplerestapi/request-an-authorization-to-the-sign-in-with-apple-server. +- Apple client secret docs: https://developer.apple.com/documentation/accountorganizationaldatasharing/creating-a-client-secret +- Go OAuth2 package docs: https://pkg.go.dev/golang.org/x/oauth2 +- go-oidc package docs: https://pkg.go.dev/github.com/coreos/go-oidc/v3/oidc +- go-jose package docs: https://pkg.go.dev/github.com/go-jose/go-jose/v4 + +--- + +## RESEARCH COMPLETE diff --git a/.planning/phases/08-social-sign-in/08-VALIDATION.md b/.planning/phases/08-social-sign-in/08-VALIDATION.md new file mode 100644 index 0000000..c03a99f --- /dev/null +++ b/.planning/phases/08-social-sign-in/08-VALIDATION.md @@ -0,0 +1,79 @@ +--- +phase: 8 +slug: social-sign-in +status: draft +nyquist_compliant: true +wave_0_complete: false +created: 2026-05-15 +--- + +# Phase 8 — Validation Strategy + +> Per-phase validation contract for feedback sampling during execution. + +--- + +## Test Infrastructure + +| Property | Value | +|----------|-------| +| **Framework** | Go test | +| **Config file** | `backend/go.mod`, `backend/sqlc.yaml` | +| **Quick run command** | `cd backend && go test ./internal/auth ./internal/web` | +| **Full suite command** | `cd backend && go test ./...` | +| **Estimated runtime** | ~30-90 seconds | + +--- + +## Sampling Rate + +- **After every task commit:** Run `cd backend && go test ./internal/auth ./internal/web` +- **After every plan wave:** Run `cd backend && go test ./...` +- **Before `$gsd-verify-work`:** Full suite must be green +- **Max feedback latency:** 90 seconds + +--- + +## Per-Requirement Verification Map + +| Requirement | Secure Behavior | Test Type | Automated Command | File Exists | Status | +|-------------|-----------------|-----------|-------------------|-------------|--------| +| AUTH-08 | Google start route exists and handles missing config without leaking secrets | handler/unit | `cd backend && go test ./internal/web -run 'Test.*Google.*Start'` | ❌ W0 | ⬜ pending | +| AUTH-09 | Google callback validates state and verified ID token before local link/session | integration | `cd backend && go test ./internal/web -run 'Test.*Google.*Callback'` | ❌ W0 | ⬜ pending | +| AUTH-10 | Apple start route exists and handles missing config without leaking secrets | handler/unit | `cd backend && go test ./internal/web -run 'Test.*Apple.*Start'` | ❌ W0 | ⬜ pending | +| AUTH-11 | Apple callback validates state/nonce and verified ID token before local link/session | integration | `cd backend && go test ./internal/web -run 'Test.*Apple.*Callback'` | ❌ W0 | ⬜ pending | +| AUTH-12 | Provider callback creates existing Xtablo session cookie, not provider-token session | integration | `cd backend && go test ./internal/web -run 'Test.*Social.*Session'` | ❌ W0 | ⬜ pending | +| AUTH-13 | Email/password signup/login/logout/CSRF/rate-limit tests continue passing | regression | `cd backend && go test ./internal/auth ./internal/web` | ✅ | ⬜ pending | + +*Status: ⬜ pending · ✅ green · ❌ red · ⚠️ flaky* + +--- + +## Wave 0 Requirements + +- [ ] `backend/migrations/0006_social_identities.sql` — nullable `users.password_hash` plus `user_identities` +- [ ] `backend/internal/db/queries/user_identities.sql` — identity lookup/link/update queries +- [ ] `backend/internal/web/handlers_social_test.go` — RED tests for Google/Apple start and callback flows +- [ ] `backend/internal/auth/oauth_test.go` or equivalent — RED tests for state/nonce/config/client-secret helpers + +--- + +## Manual-Only Verifications + +| Behavior | Requirement | Why Manual | Test Instructions | +|----------|-------------|------------|-------------------| +| Disabled provider buttons render when env vars are missing | AUTH-08, AUTH-10 | Visual/HTML state is fast to inspect manually during first pass | Start web with missing provider env vars; visit `/login` and `/signup`; confirm Google/Apple buttons are visible but disabled | +| Configured provider buttons route to provider start URLs | AUTH-08, AUTH-10 | Local provider credentials may not be available in every dev environment | Set dummy/local test provider config; visit `/login` and `/signup`; click each provider button; confirm start route redirects or returns controlled config/test response | + +--- + +## Validation Sign-Off + +- [ ] All tasks have `` verify or Wave 0 dependencies +- [ ] Sampling continuity: no 3 consecutive tasks without automated verify +- [ ] Wave 0 covers all MISSING references +- [ ] No watch-mode flags +- [ ] Feedback latency < 90s +- [x] `nyquist_compliant: true` set in frontmatter + +**Approval:** pending