RFC: User Authentication and Online MyOpenCRE Mapping#876
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
Summary by CodeRabbit
WalkthroughThis PR adds a new RFC document proposing an authenticated "MyOpenCRE" flow under ChangesUser Auth & MyOpenCRE RFC
Estimated code review effort: 1 (Trivial) | ~5 minutes Related issues: None specified. Suggested labels: documentation, rfc Suggested reviewers: None specified. 🐰 A rabbit sketches routes on parchment scroll, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
user-auth-myopencre.md (1)
48-48: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd language specifiers to fenced code blocks for readability.
The static analysis tool flagged four fenced code blocks without language specifiers. While this is an RFC document, adding specifiers improves rendering and copy-paste ergonomics:
- Line 48 (
usersschema):sql- Line 96 (
user_standardsschema):sql- Line 196 (upload endpoint spec):
httportext- Line 243 (result retrieval endpoints):
httportext📝 Proposed formatting fixes
Line 48:
- ``` + ```sqlLine 96:
- ``` + ```sqlLine 196:
- ``` + ```httpLine 243:
- ``` + ```httpAlso applies to: 96-96, 196-196, 243-243
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@user-auth-myopencre.md` at line 48, Add language specifiers to the fenced code blocks in the RFC so the rendered examples are easier to read and copy. Update the four affected fences in the document sections for the users schema and user_standards schema to use sql, and the upload endpoint spec plus result retrieval endpoints to use http (or text if preferred); keep the surrounding content unchanged and ensure the fenced blocks remain matched in each section.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@user-auth-myopencre.md`:
- Around line 302-304: The privacy note is inconsistent with existing session
usage, so clarify the storage claim in the RFC. Update the note near the
“Privacy note” section to distinguish persistent storage from server-side
session data, and ensure it matches the behavior implied by login_required and
the session['name']/session['google_id'] fields in application/web/web_main.py.
If the intent is “nothing is persisted,” say that explicitly; otherwise, adjust
the documented behavior to reflect what is actually stored.
- Line 51: Clarify the identity field naming by explicitly mapping google_sub to
the existing google_id used in login_required and related session checks. Update
the RFC text around the google_sub column to state whether it is the same
immutable Google sub claim or a new identifier, and if it is the same, add a
brief note or alias reference so readers can cross-reference google_sub with
google_id consistently.
- Around line 177-179: Define the evaluation protocol for the checkpoint by
clarifying what the “known CRE links” ground truth is (for example, manually
verified mappings versus preexisting graph edges), who validates the mappings
and how tie-breaks/disagreements are resolved, and which metric the “≥ 98%
correct mappings” refers to. Update the checkpoint text near the pass criteria
to reference the relevant evaluation method in the fixture upload flow so the
requirement is operationalized and reproducible.
- Around line 59-66: The session contract is inconsistent between the RFC and
the existing login flow: login_required currently checks session['google_id']
and session['name'], while the new auth flow stores only user_id. Update
login_required to use session['user_id'] and fetch the user’s display name from
the User model (instead of relying on session name), and make sure the auth
callback writes the same key that login_required expects. Also clarify whether
google_sub replaces google_id everywhere or whether both are retained during
migration.
---
Nitpick comments:
In `@user-auth-myopencre.md`:
- Line 48: Add language specifiers to the fenced code blocks in the RFC so the
rendered examples are easier to read and copy. Update the four affected fences
in the document sections for the users schema and user_standards schema to use
sql, and the upload endpoint spec plus result retrieval endpoints to use http
(or text if preferred); keep the surrounding content unchanged and ensure the
fenced blocks remain matched in each section.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: f43ed216-e971-4602-acd5-d66366a1a273
📒 Files selected for processing (1)
user-auth-myopencre.md
…arify google_sub, define mapping accuracy metric
Pa04rth
reviewed
Jul 7, 2026
Collaborator
Collaborator
There was a problem hiding this comment.
File location should be under docs folder
- Reconcile session contract with existing login_required (google_id/name -> user_id) - Clarify google_sub == existing session google_id (OIDC sub claim) - Privacy note: clarify nothing is persisted to DB (session data already exists) - Define TODO 3 accuracy checkpoint as exact-match vs human-verified CRE links - Remove Author/Status/Related-issues metadata lines - Move RFC under docs/rfc/
northdpole
approved these changes
Jul 8, 2026
northdpole
left a comment
northdpole
left a comment
Collaborator
There was a problem hiding this comment.
RFC approved. Follow-up tracked in separate issues: auth route migration to /rest/v1/auth/*, capabilities.login rollout, Module C reuse for mapping, and auth-wrapping cre_csv_import.
This was referenced Jul 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This RFC proposes the design for user authentication and
personal MyOpenCRE views (issue #586).
Reviewed and approved by @northdpole — opening for
community review as suggested.
What this RFC covers
google_subas anchordata duplication against the shared CRE graph
cross-encoder) with 98% accuracy checkpoint
/rest/v2/Implementation order
5 TODO steps with explicit code changes, tests,
pass/fail checkpoints, and rollback behaviour for each.
Related
/rest/v2/— no breaking changesto existing
/rest/v1/surfaceReviewers
@northdpole @parth-sohaney @robvanderveer