Skip to content

JMCP: Papers - jmcp/wire-up-real-orcid-oauth-callback-route--qXIh7dlf#7

Merged
landigf merged 1 commit into
mainfrom
jmcp/wire-up-real-orcid-oauth-callback-route--qXIh7dlf
Mar 25, 2026
Merged

JMCP: Papers - jmcp/wire-up-real-orcid-oauth-callback-route--qXIh7dlf#7
landigf merged 1 commit into
mainfrom
jmcp/wire-up-real-orcid-oauth-callback-route--qXIh7dlf

Conversation

@landigf

@landigf landigf commented Mar 24, 2026

Copy link
Copy Markdown
Owner

Review Summary

Change Summary

Three files, two concerns:

  1. packages/auth/src/index.ts — Adds a databaseHooks.account.create.after hook so that when an ORCID account is linked via Better Auth, the user's profiles.orcid column is populated with the ORCID iD (account.accountId).

  2. apps/web/app/api/auth/orcid/link/route.ts (new) — A /api/auth/orcid/link GET route that proxies into Better Auth's sign-in/social endpoint for ORCID, forwarding cookies and redirecting the user to the ORCID authorization URL (or back to settings on failure).

  3. apps/web/app/settings/identity/page.tsx — Shows the linked ORCID iD when present, and renders a "Link ORCID" button when managed auth is active, ORCID is configured, and no iD is linked yet.

Validation Confidence: Medium

  • The approach is sound — piggybacking on Better Auth's genericOAuth + a database hook is the right-weight solution.
  • disableImplicitSignUp: true is already set, which is correct for linking (existing user only).
  • Cookie forwarding in the link route looks correct.

Risks That Still Remain

# Risk Severity
1 No auth guard on /api/auth/orcid/link — an unauthenticated visitor can hit this route. Better Auth's social sign-in may still require a session, but an explicit session check + early 401 would be safer and give a clear error. High
2 Hook fires on create only — if a user re-links ORCID (e.g., changed ORCID iD), the existing account row may be updated rather than created, and the hook won't fire. Consider also hooking account.update.after. Medium
3 No error surfacing — the fallback redirect appends ?error=orcid-link-failed but the settings page doesn't read or display that query param, so the user sees no feedback on failure. Medium
4 profiles row must already exist — the db.update(profiles) in the hook silently affects 0 rows if the profile hasn't been seeded yet. An upsert or a pre-check would be more defensive. Medium
5 No CSRF protection on the GET link route — initiating an OAuth flow from a plain GET with no state/nonce validation at the application layer relies entirely on Better Auth's internal CSRF handling. Worth confirming that sign-in/social generates and validates state. Low–Med
6 @orcid.local sentinel email in mapProfileToUser — if Better Auth enforces email uniqueness this could collide if two users somehow get the same sub, or cause confusion in logs/admin. Minor but worth a note. Low

…eds actual token exchange and profile linking
@landigf landigf merged commit 1872060 into main Mar 25, 2026
1 check passed
@landigf landigf deleted the jmcp/wire-up-real-orcid-oauth-callback-route--qXIh7dlf branch March 25, 2026 12:19
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.

1 participant