Skip to content

core: fix oauth callback with mail only #1110

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
undefined-moe merged 2 commits into from
Dec 26, 2025
Merged

core: fix oauth callback with mail only #1110

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
def79f0
core: fix oauth callback with mail only
renbaoshuo Dec 26, 2025
46b69d6
Simplify OAuth account binding logic
renbaoshuo Dec 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion packages/hydrooj/src/handler/user.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,12 +476,19 @@ class OauthCallbackHandler extends Handler {
return;
}

const uid = await this.ctx.oauth.get(args.type, r._id) || await this.ctx.oauth.get('mail', r.email);
const uid = await this.ctx.oauth.get(args.type, r._id);
if (uid) {
await successfulAuth.call(this, await user.getById('system', uid));
this.response.redirect = '/';
return;
}
const udoc = await user.getByEmail('system', r.email);
if (udoc) {
await this.ctx.oauth.set(args.type, r._id, udoc._id);
await successfulAuth.call(this, udoc);
this.response.redirect = '/';
return;
}
Comment on lines +485 to +491

@coderabbitai coderabbitai Bot Dec 26, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Google OAuth email verification guaranteed in access token response

💡 Result:

Short answer: No — Google does not guarantee an email_verified field in the access token response. email and email_verified are provided as claims in the ID token (or returned by userinfo when you call the UserInfo endpoint) only if you request the appropriate scope (openid email). You should verify email_verified from the ID token (or the /userinfo response), not from the access token itself. [1][2][3]

Sources:

  • OpenID Connect / email scope: ID token includes email and email_verified when you request the email scope. [1]
  • Google Sign‑in / ID token claims (example showing email_verified). [2]
  • Backend auth guide: verify the ID token (email/email_verified) or call tokeninfo/userinfo to confirm. [3]

🌐 Web query:

GitHub OAuth email verification status in API response

💡 Result:

Short answer: GitHub returns email verification status only from the Emails API (GET /user/emails) — each email object includes "verified": true|false. The standard user object (GET /user) may include an email field but does not expose a "verified" flag. OAuth tokens must include the user:email (or appropriate fine‑grained "Email addresses") scope to call the Emails endpoints. [1][2]

Sources:

  • Emails API (response includes "verified"). [1]
  • Get authenticated user (email in /user is just the public email; no verified field). [2]

🌐 Web query:

Facebook OAuth email verification guarantee

💡 Result:

Short answer: Don’t rely on Facebook Login to guarantee an app-ready, permanently verified email — verify it yourself.

Details: Facebook may return an email via OAuth when the user has one, but the docs and Facebook guidance tell apps to confirm email ownership themselves (e.g., send a verification link) because the email field isn’t always present/guaranteed for every account and practices/fields can vary. [1][2].

If you need a definitive flag you can trust for downstream account actions, send your own confirmation email (or use an IdP-aware user.profile/email_verified field in your identity provider stack). [1][3]

Sources:

  • Facebook Login guidance (recommendation to verify provider emails in your app). [1]
  • Facebook Help Center: how/when Facebook sends email confirmations and how it uses account emails. [2]
  • Notes/discussion about Graph API email behavior (email may be null/not guaranteed). [3]

🏁 Script executed:

cd packages/hydrooj/src/handler && wc -l user.ts

Repository: hydro-dev/Hydro

Length of output: 70

🏁 Script executed:

cd packages/hydrooj/src/handler && sed -n '470,510p' user.ts

Repository: hydro-dev/Hydro

Length of output: 1805

🏁 Script executed:

cd packages/hydrooj/src && find . -name "*.ts" -o -name "*.js" | xargs rg -l "oauth" | head -20

Repository: hydro-dev/Hydro

Length of output: 175

🏁 Script executed:

cd packages/hydrooj/src/handler && sed -n '430,495p' user.ts

Repository: hydro-dev/Hydro

Length of output: 2390

🏁 Script executed:

cd packages/hydrooj/src && rg -A 5 -B 5 "const udoc = await user.getByEmail" handler/user.ts

Repository: hydro-dev/Hydro

Length of output: 960

🏁 Script executed:

cd packages/hydrooj/src && cat -n model/oauth.ts | head -100

Repository: hydro-dev/Hydro

Length of output: 3092

🏁 Script executed:

cd packages/hydrooj/src && rg -l "OAuthProvider" --type ts | head -20

Repository: hydro-dev/Hydro

Length of output: 86

🏁 Script executed:

cd packages/hydrooj/src && rg "provide.*OAuthProvider\|OAuthProvider.*provide" -A 10 -B 2 | head -100

Repository: hydro-dev/Hydro

Length of output: 41

🏁 Script executed:

cd packages && find . -path "*/plugin*" -name "*.ts" | xargs rg -l "oauth" 2>/dev/null | head -10

Repository: hydro-dev/Hydro

Length of output: 86

🏁 Script executed:

cd packages/hydrooj/src && rg "OAuthProvider\|oauth.provide" -B 5 -A 10 plugin-api.ts interface.ts

Repository: hydro-dev/Hydro

Length of output: 41

🏁 Script executed:

cd packages && find . -name "*.ts" -o -name "*.js" | xargs rg "email_verified\|emailVerified" 2>/dev/null | head -30

Repository: hydro-dev/Hydro

Length of output: 41

🏁 Script executed:

cd packages && find . -path "*plugin*" -name "*.ts" -o -name "*.js" | xargs rg "provide.*oauth\|oauth.*callback" 2>/dev/null | head -50

Repository: hydro-dev/Hydro

Length of output: 41

🏁 Script executed:

cd packages/hydrooj/src && head -150 plugin-api.ts | tail -80

Repository: hydro-dev/Hydro

Length of output: 2756

🏁 Script executed:

cd packages/hydrooj/src && grep -n "oauth" plugin-api.ts interface.ts

Repository: hydro-dev/Hydro

Length of output: 401

🏁 Script executed:

cd packages && git ls-files | grep -E "plugin.*oauth|oauth.*plugin" | head -20

Repository: hydro-dev/Hydro

Length of output: 41

🏁 Script executed:

cd packages && git ls-files | grep -i plugin | head -30

Repository: hydro-dev/Hydro

Length of output: 84

🏁 Script executed:

cd packages && ls -la | grep -E "^d"

Repository: hydro-dev/Hydro

Length of output: 1583

🏁 Script executed:

cd packages && find . -maxdepth 2 -type d -name "*plugin*" -o -type d -name "*oauth*" | head -20

Repository: hydro-dev/Hydro

Length of output: 41

🏁 Script executed:

cd packages/login-with-google && find . -name "*.ts" -o -name "*.js" | head -20

Repository: hydro-dev/Hydro

Length of output: 69

🏁 Script executed:

cd packages/login-with-github && find . -name "*.ts" -o -name "*.js" | head -20

Repository: hydro-dev/Hydro

Length of output: 69

🏁 Script executed:

cd packages/login-with-google && cat -n index.ts | head -100

Repository: hydro-dev/Hydro

Length of output: 4315

🏁 Script executed:

cd packages/login-with-github && cat -n index.ts | head -150

Repository: hydro-dev/Hydro

Length of output: 5364

Validate email verification for providers that support it.

The auto-binding logic trusts the OAuth provider's email claim without verifying it has been confirmed by the provider. Google's ID token includes an email_verified claim that should be checked before auto-binding; currently, only the email field is used. GitHub's implementation correctly validates the verified flag from the Emails API. Add a check for payload.email_verified in the Google provider's callback (around login-with-google/index.ts line 59-67), or document that only providers with mandatory email verification are configured for auto-binding.

🤖 Prompt for AI Agents 
In packages/hydrooj/src/handler/user.ts around lines 485 to 491, the
auto-binding flow unconditionally trusts the provider email; update the logic so
that for Google you only auto-bind when the token payload includes
email_verified === true (or otherwise assert the provider guarantees verified
emails). Specifically: inspect the OAuth payload for email_verified before
calling user.getByEmail and set the oauth mapping; if email_verified is false or
missing, skip auto-binding (or route the user to a verification/confirmation
path) and ensure a clear log or response is emitted so binding does not occur
for unverified addresses.

if (!provider.canRegister) throw new ForbiddenError('No binded account found');
this.checkPriv(PRIV.PRIV_REGISTER_USER);
let username = '';
Expand Down
Loading