Skip to content

Record where Fizzy stands against our OAuth rubric - #3193

Open
jeremy wants to merge 4 commits into
mainfrom
oauth-standard
Open

jeremy wants to merge 4 commits into
mainfrom
oauth-standard

Conversation

@jeremy

@jeremy jeremy commented Oct 15, 2026 •

Copy link
Copy Markdown
Member

Adds docs/oauth.md: where this repo stands against our OAuth design and conformance rubric (bc3 doc/oauth/rubric.md, basecamp/bc3#13882), by check id.

It maps the OAuth train (#2296 → … → #3169, #3192, plus #3158 and #3191) to the rubric and lists what's still missing: audience and account binding, token exchange, DPoP, CIMD, PAR, audit events, metrics, and in-place PAT migration. Hashing tokens at rest is #3192, step 1 of 3; storing plaintext was an oversight.

The rubric is one bar that apps meet to different degrees over time. New builds start at it. Existing apps migrate carefully and may lag a long time; a ✗ there is recorded migration debt.

Docs only. It follows the rubric as basecamp/bc3#13882 evolves.

@jeremy jeremy changed the title Record OAuth status against the 37signals OAuth Standard Record where Fizzy stands against our OAuth rubric Oct 15, 2026
@jeremy
jeremy marked this pull request as ready for review October 15, 2026 21:40
Copilot AI balanced review requested due to automatic review settings October 15, 2026 21:40
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-17T07:16:33.078682Z 3becdc6 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Factual inaccuracies in the status ledger undermine its purpose as a reliable migration plan.

3 open findings
What changed in this PR

Documents Fizzy’s progress against the shared OAuth rubric, connecting the pending implementation train to remaining migration work.

Changes:

  • Maps rubric checks to implementation status and related PRs.
  • Records remaining gaps and the planned order of work.
  • Clarifies compatibility expectations for existing clients.

[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

File Description
docs/​oauth.md Adds OAuth status mapping and migration priorities.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread docs/oauth.md Outdated
Comment thread docs/oauth.md Outdated
Comment thread docs/oauth.md Outdated
@jeremy
jeremy requested a balanced review from Copilot October 15, 2026 21:59
@jeremy

jeremy commented Oct 15, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: 32229dfb87

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approved

The documentation-only change has one non-blocking wording correction and no runtime impact.

0 open findings

3 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Low severity Correct integration docs to exclude pending #3192 migrations

docs/​oauth.md:14

The integration branch's docs/oauth/integration-migrations.md lists the train through #3169, plus #3158 and #3191, but not #3192 or its digest migrations. Saying the entire train “is integrated” therefore overstates the beta snapshot. Distinguish the integrated stack from #3192's pending integration and migration-inventory update.

🧠 Review effort: Balanced

@jeremy

jeremy commented Oct 15, 2026

Copy link
Copy Markdown
Member Author

Review threads: 3 resolved (3 fixed, 0 declined with the reasoning in each thread).
Review bodies: the Copilot and Codex review bodies repeat the threads; there were no separate findings.
Not reported on the head: Copilot and Codex re-requested on 32229df

@jeremy

jeremy commented Oct 15, 2026

Copy link
Copy Markdown
Member Author

Nothing open on 32229df.

Points to bc3's doc/oauth/standard.md and gives Fizzy's status by
requirement id across the OAuth train, with what's still missing.
bc3's doc was renamed and reworded from a standard of requirements to a
design and conformance rubric. This file follows: the new path, and the
bar and its checks in place of the standard and its requirements. The
check ids are unchanged.
The train requires state, revokes the first redemption's grant on a
replayed code, and already shares one token table between PATs and OAuth.
@jeremy

jeremy commented Oct 17, 2026

Copy link
Copy Markdown
Member Author

Rebased onto current main, plus a status update: GP-7 (optional state, #2296), TK-1 (fizzy_cs_, 257-bit bodies, #3163), SC-5 (no-store and no-referrer on every OAuth response, #3082) and SC-6 (log filtering, #2296). Head 3becdc6.

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