Skip to content

feat: Oauth github and google#43

Merged
deadlyjack merged 18 commits into
mainfrom
ajit/feat-oauth
May 16, 2026
Merged

feat: Oauth github and google#43
deadlyjack merged 18 commits into
mainfrom
ajit/feat-oauth

Conversation

@deadlyjack
Copy link
Copy Markdown
Member

No description provided.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 14, 2026

Greptile Summary

This PR adds GitHub and Google OAuth login/registration, replaces the old registerUser/loginUser pages with unified account/login components, introduces social profile fields (x, linkedin, avatar_url), and refactors routing (/user/profile, /edit-user/profile/edit).

  • OAuth flow: State-cookie CSRF protection, server-side redirect validation via validatedRedirect, and token expiry enforcement in getLoggedInUser are all correctly implemented. Several previously flagged issues (missing redirect roundtrip, stale route paths, raw OAuth ID exposure, missing email guards) appear addressed.
  • Profile page: User avatar now correctly references the profile owner; social links render inline. LinkedIn URL construction assumes a stored path value (/in/johndoe) but has no validation — a user entering a full URL will produce a broken link.
  • Schema: github_id and google_id gain unique indexes via updateSchema.js; safeColumns returns boolean expressions for OAuth IDs rather than the raw values.

Confidence Score: 3/5

The core OAuth flow looks sound but real behavioral defects exist in the profile rendering and deep-link path that should be fixed before shipping.

The LinkedIn URL construction silently produces a broken profile link for any user who enters a full URL, and the getCookie token call in the login page means the acode deep-link never carries the real token through the OAuth path.

client/pages/user/index.js (LinkedIn URL), client/pages/login/index.js (httpOnly token + app deep-link), server/lib/oauth.js (missing secure cookie flag)

Security Review

  • The session token cookie lacks secure: true in server/lib/oauth.js, making it transmissible over plain HTTP.
  • The oauth_state, oauth_intent, and oauth_redirect cookies in server/apis/oauth.js also lack secure: true.
  • No other injection, credential leakage, or auth-bypass issues were identified.

Important Files Changed

Filename Overview
server/apis/oauth.js New OAuth routes for GitHub and Google — state cookie CSRF protection is in place, redirect validation added. Previously flagged issues appear resolved.
server/lib/oauth.js Token issuance, provider helpers, and state generation. httpOnly added to token cookie; secure flag is missing.
server/apis/user.js Adds GitHub/Google unlink endpoints and new social fields. Contains a truncated 'User already' error message.
server/entities/user.js Adds OAuth ID columns, avatar_url, social fields. safeColumns returns boolean expressions for github_id/google_id.
client/pages/account/index.js New combined register/edit profile page with OAuth link/unlink buttons. Error handling looks correct.
client/pages/login/index.js New login page with OAuth buttons. getCookie('token') always returns null due to httpOnly flag (previously flagged).
client/pages/user/index.js Refactored profile page. LinkedIn link construction breaks if user stores a full URL instead of a path.
client/lib/helpers.js Adds user cache, invalidateLoggedInUser, and withRedirect. withRedirect does not URL-encode the redirect parameter.
server/updateSchema.js Adds migration queries for new OAuth columns with correct unique indexes.
server/lib/helpers.js Adds server-side token expiry check in getLoggedInUser — correct improvement.
client/main.js Route updates and OAuth wildcard route added. Logout invalidates client-side user cache correctly.

Reviews (6): Last reviewed commit: "feat: enhance OAuth account linking, imp..." | Re-trigger Greptile

Comment thread server/updateSchema.js
Comment thread server/apis/oauth.js Outdated
Comment thread client/lib/helpers.js
Comment thread server/lib/oauth.js
Comment thread server/apis/oauth.js Outdated
deadlyjack and others added 4 commits May 15, 2026 04:03
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@deadlyjack
Copy link
Copy Markdown
Member Author

@greptile_apps review again

Comment thread server/apis/oauth.js
Comment thread server/lib/oauth.js
Comment thread client/pages/account/index.js
deadlyjack and others added 4 commits May 15, 2026 04:31
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@deadlyjack
Copy link
Copy Markdown
Member Author

@greptile_apps review once more

Comment thread server/lib/oauth.js
@deadlyjack
Copy link
Copy Markdown
Member Author

@greptile_apps review once more

Comment thread client/pages/user/index.js Outdated
<section id='user'>
<div className='profile'>
<img ref={img} src={userImage} alt='' className='profile-image' />
<img src={loggedInUser.avatar_url || gravatar(user.github)} alt={loggedInUser.email} className='profile-image' />
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.

P1 Wrong user's avatar shown; crash when viewing another user's profile while logged out

The newly-added img element reads from loggedInUser instead of user. When any visitor navigates to /profile/123 without a session, loggedInUser is null and loggedInUser.avatar_url throws a TypeError before the component renders. Even when the viewer is logged in, they see their own avatar on another person's profile page. Both avatar_url and the alt attribute should reference user, the profile owner.

@deadlyjack
Copy link
Copy Markdown
Member Author

@greptile_apps review the changes

Comment thread server/entities/user.js
Comment thread server/apis/oauth.js Outdated
@deadlyjack
Copy link
Copy Markdown
Member Author

@greptile_apps review the changes

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 16, 2026

Want your agent to iterate on Greptile's feedback? Try greploops.

Comment on lines +97 to +101
<div className='socials' data-show-sensitive-info={String(shouldShowSensitiveInfo)}>
{user.website && (
<a href={user.website} target='_blank' rel='noopener'>
<span className='icon earth' />
<span className='label'>{user.website}</span>
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.

P1 Broken link when user stores a full LinkedIn URL

The link is built as `https://linkedin.com${user.linkedin}`, expecting a stored value of /in/johndoe. If a user enters the full URL (https://linkedin.com/in/johndoe) in the edit form — which has no client- or server-side validation — the rendered href becomes https://linkedin.comhttps://linkedin.com/in/johndoe, producing a broken link on their public profile with no error surfaced to them.

@deadlyjack deadlyjack merged commit 15aa19e into main May 16, 2026
@deadlyjack deadlyjack deleted the ajit/feat-oauth branch May 16, 2026 21:46
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