Maniacs_Mansion/MealQuest
11
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Feature 03 task 1: OIDC login and callback against Authentik #16

Merged
approver_agent merged 3 commits from task/oidc-login-callback_13c4c09f-46a4-4d53-9411-8a409f5dd4c7 into feature/9c4eba48-9a49-4b73-a85f-a4bc19eb4062-authentik-oidc-sessions 2026-10-03 09:58:05 -04:00
Conversation 2 Commits 3 Files Changed 13
+1629 -14
dev_agent commented 2026-10-03 09:38:20 -04:00
Member
Copy Link
Copy Source

Implements Feature 03 task 1 (13c4c09f). Done-condition: internal/auth does OIDC discovery on MEALQUEST_OIDC_ISSUER with a confidential client (secret from MEALQUEST_OIDC_CLIENT_SECRET_FILE); GET /auth/login stores state, nonce and a PKCE S256 verifier in a short-lived signed HttpOnly cookie and redirects with scopes openid profile email groups offline_access and an optional same-origin return path; GET /auth/callback validates state, exchanges the code, verifies the ID token (RS256, issuer, audience, expiry, nonce) and hands claims to a session-layer seam; httptest fake-provider tests cover success, mismatched state, wrong nonce, HS256, expired and off-origin return; quality floor passes.

Evidence (developer-run): gofmt empty; go vet clean; go test -race all 7 packages pass; CGO_ENABLED=0 build ok; make build ok. Adds approved deps coreos/go-oidc/v3 and golang.org/x/oauth2.

Implements Feature 03 task 1 (13c4c09f). Done-condition: internal/auth does OIDC discovery on MEALQUEST_OIDC_ISSUER with a confidential client (secret from MEALQUEST_OIDC_CLIENT_SECRET_FILE); GET /auth/login stores state, nonce and a PKCE S256 verifier in a short-lived signed HttpOnly cookie and redirects with scopes openid profile email groups offline_access and an optional same-origin return path; GET /auth/callback validates state, exchanges the code, verifies the ID token (RS256, issuer, audience, expiry, nonce) and hands claims to a session-layer seam; httptest fake-provider tests cover success, mismatched state, wrong nonce, HS256, expired and off-origin return; quality floor passes. Evidence (developer-run): gofmt empty; go vet clean; go test -race all 7 packages pass; CGO_ENABLED=0 build ok; make build ok. Adds approved deps coreos/go-oidc/v3 and golang.org/x/oauth2.
dev_agent added 1 commit 2026-10-03 09:38:23 -04:00
Add OIDC login and callback against Authentik
CI / ci (push) Successful in 1m27s
Details
Independent review Codex gpt-6-sol high: CHANGES_REQUIRED (1 high, 2 medium)
Details
CI / ci (pull_request) Successful in 1m22s
Details
a10b24c86c 
internal/auth discovers the provider at MEALQUEST_OIDC_ISSUER with
coreos/go-oidc/v3 and runs a confidential-client authorization code flow
with golang.org/x/oauth2 (client_secret_basic).

GET /auth/login stores state, nonce and a PKCE S256 verifier in a
10-minute HMAC-signed HttpOnly __Host- cookie and redirects with scopes
"openid profile email groups offline_access". An optional "return" path
must be same-origin; anything else fails the login with 400.

GET /auth/callback clears the single-use cookie, checks state, exchanges
the code with the verifier, verifies the ID token (RS256 only, issuer,
audience, expiry) and nonce, then hands the claims to a SessionStarter.
cmd/server wires a placeholder that refuses every login until the
session layer lands.

Config gains required MEALQUEST_OIDC_ISSUER, MEALQUEST_OIDC_CLIENT_ID,
MEALQUEST_OIDC_CLIENT_SECRET_FILE and MEALQUEST_OIDC_ALLOWED_GROUPS.

Pins go-oidc v3.21.0 and oauth2 v0.36.0, the newest releases that
support Go 1.25 (oauth2 v0.37.0 requires Go 1.26).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
review_agent left a comment
Member
Copy Link
Copy Source

Independent review — Codex gpt-6-sol, reasoning high

Verdict: CHANGES_REQUIRED
Reviewed SHA: a10b24c86c08d7fc676a09f934f5501dd65eb39e

{
  "verdict": "CHANGES_REQUIRED",
  "reviewed_sha": "a10b24c86c08d7fc676a09f934f5501dd65eb39e",
  "summary": "The OIDC flow and rejection tests meet the main task requirements. gofmt, go vet, and go test -race -count=1 ./... passed. Three security and configuration issues remain.",
  "findings": [
    {
      "id": "F1",
      "severity": "high",
      "location": "internal/config/config.go:198-213; internal/auth/auth.go:88-103",
      "failure_case": "A remote HTTP issuer can advertise an HTTP token endpoint. The code exchange then sends the confidential client secret in Basic authentication over plaintext HTTP.",
      "evidence": "httpURL accepts any http:// host. Auth uses the discovered token endpoint without checking its scheme; the approved oauth2 library sends the secret in the Authorization header.",
      "suggested_correction": "Require HTTPS for remote issuers and discovered authorization, token, and key endpoints. Permit HTTP only for explicitly local test or development endpoints."
    },
    {
      "id": "F2",
      "severity": "medium",
      "location": "internal/auth/auth.go:181-185,198-203",
      "failure_case": "A failed token exchange can write sensitive provider response content to the service log.",
      "evidence": "The callback logs the wrapped exchange error. oauth2.RetrieveError.Error() includes the token endpoint response body, or its error_description, which may contain echoed credentials or tokens.",
      "suggested_correction": "Log a fixed exchange failure category and safe status metadata; do not log the provider response body or description."
    },
    {
      "id": "F3",
      "severity": "medium",
      "location": "internal/config/config.go:198-213; internal/auth/cookie.go:57-66",
      "failure_case": "With MEALQUEST_PUBLIC_URL set to plain HTTP on a non-local host, the browser will not send the Secure login cookie, so every callback fails.",
      "evidence": "The URL validator accepts arbitrary http:// public URLs while the login cookie is always Secure.",
      "suggested_correction": "Reject non-HTTPS public URLs except supported local development hosts."
    }
  ]
}

Posted unaltered by review_agent. This is transport, not a second review.

## Independent review — Codex `gpt-6-sol`, reasoning high Verdict: **CHANGES_REQUIRED** Reviewed SHA: `a10b24c86c08d7fc676a09f934f5501dd65eb39e` ```json { "verdict": "CHANGES_REQUIRED", "reviewed_sha": "a10b24c86c08d7fc676a09f934f5501dd65eb39e", "summary": "The OIDC flow and rejection tests meet the main task requirements. gofmt, go vet, and go test -race -count=1 ./... passed. Three security and configuration issues remain.", "findings": [ { "id": "F1", "severity": "high", "location": "internal/config/config.go:198-213; internal/auth/auth.go:88-103", "failure_case": "A remote HTTP issuer can advertise an HTTP token endpoint. The code exchange then sends the confidential client secret in Basic authentication over plaintext HTTP.", "evidence": "httpURL accepts any http:// host. Auth uses the discovered token endpoint without checking its scheme; the approved oauth2 library sends the secret in the Authorization header.", "suggested_correction": "Require HTTPS for remote issuers and discovered authorization, token, and key endpoints. Permit HTTP only for explicitly local test or development endpoints." }, { "id": "F2", "severity": "medium", "location": "internal/auth/auth.go:181-185,198-203", "failure_case": "A failed token exchange can write sensitive provider response content to the service log.", "evidence": "The callback logs the wrapped exchange error. oauth2.RetrieveError.Error() includes the token endpoint response body, or its error_description, which may contain echoed credentials or tokens.", "suggested_correction": "Log a fixed exchange failure category and safe status metadata; do not log the provider response body or description." }, { "id": "F3", "severity": "medium", "location": "internal/config/config.go:198-213; internal/auth/cookie.go:57-66", "failure_case": "With MEALQUEST_PUBLIC_URL set to plain HTTP on a non-local host, the browser will not send the Secure login cookie, so every callback fails.", "evidence": "The URL validator accepts arbitrary http:// public URLs while the login cookie is always Secure.", "suggested_correction": "Reject non-HTTPS public URLs except supported local development hosts." } ] } ``` --- Posted unaltered by `review_agent`. This is transport, not a second review.
dev_agent added 1 commit 2026-10-03 09:46:11 -04:00
Require HTTPS for OIDC and public URLs; stop logging token responses
CI / ci (push) Successful in 1m24s
Details
CI / ci (pull_request) Successful in 1m24s
Details
e8dafda534 
Address review findings on a10b24c:

- F1: the OIDC issuer and the discovered authorization, token and JWKS
  endpoints must be https://; plain http:// is allowed only for the
  loopback hosts localhost, 127.0.0.1 and [::1]. Config rejects a remote
  http:// issuer at startup; auth.New rejects a discovered remote http://
  endpoint, naming it, before it is used.
- F2: a failed code exchange is reduced to a fixed category plus the HTTP
  status; the token endpoint's body and error_description are never logged
  or wrapped into the returned error.
- F3: MEALQUEST_PUBLIC_URL must be https:// unless its host is loopback,
  so the Secure login cookie is always sent back on the callback.

The rule lives once, in internal/secureurl.Check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dev_agent added 1 commit 2026-10-03 09:51:40 -04:00
Refuse OIDC redirect downgrades; stop logging provider error text
CI / ci (push) Successful in 1m27s
Details
CI / ci (pull_request) Successful in 1m25s
Details
Independent review Codex gpt-6-sol high: APPROVED (0 findings)
Details
bb0a9c3129 
The provider HTTP client now has a redirect policy: every redirect target
must pass secureurl.Check, and an https:// request never follows a
redirect to plain http://, so a 307/308 from the token endpoint cannot
resend the client secret in plaintext. Transport errors from the code
exchange no longer include the (possibly provider-supplied) URL.

The callback's provider-error path logs a fixed category instead of the
raw error and error_description query values.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
review_agent left a comment
Member
Copy Link
Copy Source

Independent review — Codex gpt-6-sol, reasoning high

Verdict: APPROVED
Reviewed SHA: bb0a9c3129c5fec7143ec25c35dc2b0a9482c7b1

{
  "verdict": "APPROVED",
  "reviewed_sha": "bb0a9c3129c5fec7143ec25c35dc2b0a9482c7b1",
  "summary": "Verified the OIDC flow, session seam, and all prior-round fixes. The fake-provider tests cover the required success and rejection cases. gofmt, go vet, and go test -race -count=1 ./... passed.",
  "findings": []
}

Posted unaltered by review_agent. This is transport, not a second review.

## Independent review — Codex `gpt-6-sol`, reasoning high Verdict: **APPROVED** Reviewed SHA: `bb0a9c3129c5fec7143ec25c35dc2b0a9482c7b1` ```json { "verdict": "APPROVED", "reviewed_sha": "bb0a9c3129c5fec7143ec25c35dc2b0a9482c7b1", "summary": "Verified the OIDC flow, session seam, and all prior-round fixes. The fake-provider tests cover the required success and rejection cases. gofmt, go vet, and go test -race -count=1 ./... passed.", "findings": [] } ``` --- Posted unaltered by `review_agent`. This is transport, not a second review.
approver_agent merged commit 99c8f1625f into feature/9c4eba48-9a49-4b73-a85f-a4bc19eb4062-authentik-oidc-sessions 2026-10-03 09:58:05 -04:00
Sign in to join this conversation.
Reviewers
No Reviewers
review_agent
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
approver_agent (The Approver) chatgpt claude-ai Claude-Code codex (Codex) dev_agent grok opencode orchestrator-agent overseer (Joshua) review_agent (The Reviewer)
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Maniacs_Mansion/MealQuest#16