Skip to content

Conversation

@dethan3
Copy link
Contributor

@dethan3 dethan3 commented Dec 15, 2025

PR-27 PR-27 PR-27 Powered by Pull Request Badge

  • Redesign /me and /user/[id] pages with modern hero sections
  • Fix avatar clipping by moving avatars outside overflow containers
  • Use Bootstrap utility classes for responsive styling
  • Adjust activity/create editor to responsive column layout
  • Fix hydration error on /me page by using SSR props directly

Checklist:

  • Labels
  • Assignees
  • Reviewers

Summary by CodeRabbit

  • New Features

    • GitHub OAuth-based authentication with sign-in/sign-out functionality
    • New user profile page displaying account information and actions
    • Automatic session restoration on app startup
  • Documentation

    • Added environment configuration guide with GitHub OAuth setup instructions
  • Improvements

✏️ Tip: You can customize this high-level summary in your review settings.

- Redesign /me and /user/[id] pages with modern hero sections
- Fix avatar clipping by moving avatars outside overflow containers
- Use Bootstrap utility classes for responsive styling
- Adjust activity/create editor to responsive column layout
- Fix hydration error on /me page by using SSR props directly
@dethan3 dethan3 added the enhancement Some improvements label Dec 15, 2025
@coderabbitai
Copy link

coderabbitai bot commented Dec 15, 2025

📝 Walkthrough

Walkthrough

Implements OAuth authentication and user profile features, including server-side session validation, login page redirection, session restoration on app startup, enhanced sign-in/sign-out UI in UserBar, and new user profile pages. Updates activity creation layout and adds configuration documentation.

Changes

Cohort / File(s) Change Summary
OAuth & Authentication Configuration
.env.development, pages/api/core.ts
Introduces SKIP_OAUTH_PROXY flag for local development and runtime validation of GitHub OAuth credentials; configures proxy behavior based on environment
Session & Login Management
models/User/Session.ts, pages/_app.tsx, pages/login.tsx
Adds client-side 401/403 redirect logic in HTTP middleware; implements session restoration on app startup via initSession(); creates new server-side guarded login page with redirect handling
Authentication UI
components/User/UserBar.tsx
Adds sign-in button with GitHub icon for unauthenticated users; expands authenticated menu with profile, home, edit profile, and sign-out options; implements router-based login redirect
User Profile Pages
pages/me/index.tsx, pages/me/index.module.less, pages/user/[id].tsx, pages/user/[id].module.less
Implements profile page with session guard showing user details; updates user detail page with email display, hero section, avatar handling, and tabbed layout; adds related styling with gradient backgrounds and hover states
Layout & Styling Updates
pages/activity/create.tsx, pages/activity/create.module.less
Refactors activity creation from flat Card to responsive Bootstrap Container/Row/Col layout; removes positioning and margin offsets
Documentation
README.md, docs/TASKS.md
Adds environment configuration guide for OAuth and proxy settings; introduces comprehensive MVP task breakdown with milestones, acceptance criteria, and affected code areas

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Browser as Browser/<br/>Next.js Client
    participant App as App<br/>(_app.tsx)
    participant Login as Login Page
    participant OAuth as GitHub OAuth
    participant Server as Server
    participant Session as Session Store
    
    rect rgb(200, 220, 255)
    Note over Browser,Server: App Startup - Session Restoration
    User->>Browser: Visit app
    activate App
    App->>App: componentDidMount()
    App->>App: initSession() [if JWT exists]
    App->>Session: getProfile()
    Session->>Server: Fetch user profile
    alt Profile fetch succeeds
        Server-->>Session: User data
        Session-->>App: User loaded
        App->>App: Update session store
    else Profile fetch fails (401/403)
        Server-->>Session: Error
        Session-->>App: Error logged
        App->>Session: signOut()
        Session-->>Browser: Clear JWT
    end
    deactivate App
    end
    
    rect rgb(220, 255, 220)
    Note over User,Server: Login Flow - Unauthenticated User
    User->>Browser: Click sign-in button
    Browser->>Browser: Redirect to /login
    Browser->>Login: Load login page
    Login->>Server: sessionGuard validation
    alt No JWT present
        Server-->>Login: Proceed to login UI
        Login->>Browser: Render loading + spinner
    end
    User->>Login: Redirected from GitHub OAuth
    Login->>Login: jwtPayload present
    Login->>Session: Store user profile
    Login->>Browser: Redirect to target URL
    Browser->>Browser: Navigate (e.g., to /)
    end
    
    rect rgb(255, 240, 200)
    Note over Browser,Server: Auth Failure - 401/403 Redirect
    User->>Browser: Perform authenticated request
    Browser->>Server: API call (with Authorization)
    Server->>Server: Check auth
    alt Auth fails (401/403)
        Server-->>Browser: HTTPError 401/403
        Browser->>Browser: HTTP middleware catches error
        Browser->>Browser: Check if not server
        Browser->>Browser: Redirect to /login
    else Auth succeeds
        Server-->>Browser: Success response
        Browser->>Browser: Render content
    end
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • Session restoration and lifecycle logic (pages/_app.tsx, models/User/Session.ts): Verify proper error handling, redirect conditions, and middleware chain ordering on 401/403
  • OAuth configuration and validation (pages/api/core.ts): Ensure environment checks are correct and proxy logic aligns with SKIP_OAUTH_PROXY flag
  • Server-side session guarding (pages/login.tsx): Confirm sessionGuard integration, JWT extraction, and redirect parameter handling
  • UserBar authentication UI (components/User/UserBar.tsx): Verify router-based redirect URL computation and menu structure consistency
  • New profile pages (pages/me/index.tsx, pages/user/[id].tsx): Review session binding, null-safety, and prop handling

Possibly related PRs

Suggested labels

feature

Poem

🐰 A hop, a skip, through login's door,
Sessions stored and auth galore,
Profile pages bloom so bright,
OAuth flows from left to right,
Sign in, sign out—the journey's right!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: redesigning profile pages (/me and /user/[id]) and fixing the activity create layout, which are the primary focuses of the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/auth

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
pages/api/core.ts (1)

84-89: Harden JWT cookie with HttpOnly/Secure/SameSite flags

The JWT session token is currently set without any security flags:

res.setHeader('Set-Cookie', `JWT=${user.token}; Path=/`);

This makes the token readable from JavaScript and more exposed to XSS and CSRF‑style attacks. Add HttpOnly, Secure (in production), and SameSite flags:

-    res.setHeader('Set-Cookie', `JWT=${user.token}; Path=/`);
+    const isProd = process.env.NODE_ENV === 'production';
+    res.setHeader(
+      'Set-Cookie',
+      [
+        `JWT=${user.token}`,
+        'Path=/',
+        'HttpOnly',
+        isProd ? 'Secure' : '',
+        'SameSite=Lax',
+      ]
+        .filter(Boolean)
+        .join('; '),
+    );

This significantly reduces the blast radius if any XSS occurs and improves CSRF resistance for JWT‑backed requests.

🧹 Nitpick comments (4)
pages/login.tsx (1)

17-24: Consider hydration error prevention and error handling.

The redirect logic looks correct, but consider these improvements:

  1. Effect cleanup: If the component unmounts during redirect (e.g., user navigates away), the router.replace could throw. Consider adding cleanup or error handling.

  2. Session storage timing: Direct assignment on Line 19 (sessionStore.user = jwtPayload) happens before the redirect completes. If the target page immediately reads session state, ensure MobX reactivity has settled.

  3. No-op when jwtPayload is absent: The effect only runs when jwtPayload exists, which is correct. However, consider what happens if a user reaches this page without a valid session—should there be a fallback UI or redirect to home?

Consider adding error boundary or try-catch:

  useEffect(() => {
    if (jwtPayload) {
      sessionStore.user = jwtPayload;

      const targetUrl = typeof redirect === 'string' ? redirect : '/';
-     router.replace(targetUrl);
+     router.replace(targetUrl).catch(error => {
+       console.error('Redirect failed:', error);
+     });
    }
  }, [jwtPayload, redirect, router]);
pages/me/index.tsx (1)

48-68: Optional: consider factoring repeated avatar/action patterns into shared components

The avatar block and some of the action button patterns overlap conceptually with /user/[id].tsx. Extracting a small Avatar and/or ProfileActions component could reduce duplication and keep behavior (e.g., future avatar sizing or styling tweaks) consistent across pages.

Also applies to: 166-188

pages/user/[id].tsx (2)

75-89: GitHub link assumes name is a username; consider using an explicit field or guarding

The GitHub button currently links to:

href={`https://github.com/${name}`}

This only works reliably if name is guaranteed to be the GitHub handle. In many user models, name is a display name, which can contain spaces or non‑ASCII characters and won’t map cleanly to a GitHub profile URL.

Consider either:

  • Using an explicit githubLogin (or similar) field from User, if available, or
  • Guarding the link so it’s only active when you have a valid GitHub handle, e.g.:
-  <a
-    href={`https://github.com/${name}`}
+  {name && (
+    <a
+      href={`https://github.com/${encodeURIComponent(name)}`}
       target="_blank"
       rel="noreferrer"
       ...
-  >
-    <Icon name="github" />
-  </a>
+    >
+      <Icon name="github" />
+    </a>
+  )}

This avoids obviously broken or misleading links for users whose name isn’t their GitHub username.


90-120: Optional: clarify placeholder social buttons to avoid implying wired accounts

The QQ/WeChat/Weibo icons are rendered as styled <span> elements without any actual linking or behavior. Visually they look like active buttons, which can confuse users if they click and nothing happens.

If these are placeholders for future work, consider:

  • Reducing opacity / disabling pointer events, or
  • Hiding them until real profile data (e.g., qq, wechat, weibo) is available.

That keeps the UI honest about which socials are actually connected.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between af47000 and 85b7901.

📒 Files selected for processing (14)
  • .env.development (1 hunks)
  • README.md (1 hunks)
  • components/User/UserBar.tsx (2 hunks)
  • docs/TASKS.md (1 hunks)
  • models/User/Session.ts (1 hunks)
  • pages/_app.tsx (2 hunks)
  • pages/activity/create.module.less (0 hunks)
  • pages/activity/create.tsx (1 hunks)
  • pages/api/core.ts (1 hunks)
  • pages/login.tsx (1 hunks)
  • pages/me/index.module.less (1 hunks)
  • pages/me/index.tsx (1 hunks)
  • pages/user/[id].module.less (1 hunks)
  • pages/user/[id].tsx (2 hunks)
💤 Files with no reviewable changes (1)
  • pages/activity/create.module.less
🧰 Additional context used
🧬 Code graph analysis (5)
pages/me/index.tsx (5)
pages/activity/create.tsx (1)
  • getServerSideProps (14-14)
pages/login.tsx (1)
  • getServerSideProps (11-11)
pages/user/[id].tsx (1)
  • getServerSideProps (19-24)
pages/api/core.ts (1)
  • sessionGuard (115-115)
models/Base/Translation.ts (1)
  • I18nContext (44-44)
pages/login.tsx (4)
pages/activity/create.tsx (1)
  • getServerSideProps (14-14)
pages/me/index.tsx (1)
  • getServerSideProps (15-15)
pages/api/core.ts (1)
  • sessionGuard (115-115)
components/layout/PageHead.tsx (1)
  • PageHead (11-19)
components/User/UserBar.tsx (1)
components/layout/MainNavigation.tsx (1)
  • () => import('../User/UserBar') (10-10)
models/User/Session.ts (1)
configuration/index.ts (3)
  • API_HOST (10-10)
  • isServer (3-3)
  • globalThis (12-15)
pages/api/core.ts (3)
configuration/index.ts (1)
  • process (8-8)
models/Git.ts (1)
  • next (15-21)
next.config.ts (1)
  • beforeFiles (25-38)
🪛 dotenv-linter (4.0.0)
.env.development

[warning] 4-4: [SpaceCharacter] The line has spaces around equal sign

(SpaceCharacter)

🔇 Additional comments (8)
README.md (1)

29-57: LGTM! Clear and comprehensive setup documentation.

The environment configuration section provides excellent guidance for developers. The OAuth setup instructions are particularly helpful.

pages/activity/create.tsx (1)

33-45: LGTM! Well-structured responsive layout.

The Bootstrap grid implementation is clean and follows best practices. The responsive column sizing (lg=10, xl=8) provides appropriate width constraints for the form.

docs/TASKS.md (1)

1-191: LGTM! Excellent project planning documentation.

The task breakdown is comprehensive and well-structured, with clear acceptance criteria and scope definitions for each milestone. This will help maintain organized development flow.

pages/me/index.module.less (1)

1-18: LGTM! Clean styling module.

The styles are well-structured with appropriate transitions and theming. The gradient and hover effects enhance the UI.

pages/user/[id].module.less (1)

1-46: LGTM! Consistent styling with good interaction states.

The styling module provides good visual feedback with transitions and hover states. The use of :global selectors for Bootstrap classes is appropriate.

pages/me/index.tsx (1)

17-27: Session source and hydration behavior look sound

Using jwtPayload from getServerSideProps as the primary user source and mirroring it into sessionStore.user in useEffect, while resolving user = jwtPayload || sessionStore.user, avoids hydration mismatches and still keeps the MobX store in sync. The null fallback when user is missing is a reasonable guard given sessionGuard should already handle redirects.

components/User/UserBar.tsx (1)

14-18: Login redirect handling is correct locally; verify server-side validation

loginUrl correctly encodes router.asPath, and the UI cleanly distinguishes authenticated (dropdown) vs unauthenticated (GitHub sign‑in button) states. The only remaining concern is how /login consumes the redirect query:

  • If /login blindly redirects to redirect, a crafted URL could potentially be used for open redirects.
  • Ideally, the login handler should allow only internal paths and ignore or sanitize absolute URLs.

Given that logic lives outside this file, please confirm /pages/login.tsx validates redirect as an internal path before using it.

Also applies to: 25-49

pages/_app.tsx (1)

14-16: > Likely an incorrect or invalid review comment.

NEXT_PUBLIC_API_HOST = http://127.0.0.1:8080

# Skip OAuth proxy for local development (proxy server may be unavailable)
SKIP_OAUTH_PROXY = 1
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove spaces around the equal sign.

Environment variable assignments should not have spaces around the = operator, as some parsers may not handle them correctly.

Apply this diff:

-SKIP_OAUTH_PROXY = 1
+SKIP_OAUTH_PROXY=1
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
SKIP_OAUTH_PROXY = 1
SKIP_OAUTH_PROXY=1
🧰 Tools
🪛 dotenv-linter (4.0.0)

[warning] 4-4: [SpaceCharacter] The line has spaces around equal sign

(SpaceCharacter)

🤖 Prompt for AI Agents
.env.development lines 4-4: the environment variable assignment has spaces
around the '=' which can break some parsers; edit the line to remove the spaces
so it uses the canonical format SKIP_OAUTH_PROXY=1 (no spaces) and save the
file.

Comment on lines +15 to +33
.use(async ({ request }, next) => {
try {
return await next();
} catch (error) {
if (error instanceof HTTPError) {
const { status } = error.response;

if (status === 401 || status === 403) {
if (!isServer()) {
const currentPath = globalThis.location?.pathname || '/';
const loginUrl = `/login?redirect=${encodeURIComponent(currentPath)}`;

globalThis.location?.assign(loginUrl);
}
}
}
throw error;
}
});
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "Session.ts" | head -20

Repository: FreeCodeCamp-Chengdu/HOP

Length of output: 92


🏁 Script executed:

git ls-files | grep -E "(login|auth)" | grep -E "\.(ts|tsx|js)$" | head -20

Repository: FreeCodeCamp-Chengdu/HOP

Length of output: 83


🏁 Script executed:

fd -e ts -e tsx | grep -i session | head -20

Repository: FreeCodeCamp-Chengdu/HOP

Length of output: 90


🏁 Script executed:

cat -n models/User/Session.ts

Repository: FreeCodeCamp-Chengdu/HOP

Length of output: 3357


🏁 Script executed:

cat -n pages/login.tsx

Repository: FreeCodeCamp-Chengdu/HOP

Length of output: 1385


🏁 Script executed:

rg -n "sessionGuard" pages/api/core.ts pages/api/core.js -A 20

Repository: FreeCodeCamp-Chengdu/HOP

Length of output: 254


🏁 Script executed:

fd -e ts -e js | xargs grep -l "sessionGuard" | head -10

Repository: FreeCodeCamp-Chengdu/HOP

Length of output: 85


🏁 Script executed:

cat -n pages/api/core.ts

Repository: FreeCodeCamp-Chengdu/HOP

Length of output: 4172


🏁 Script executed:

rg -n "ownClient|client\." pages/login.tsx

Repository: FreeCodeCamp-Chengdu/HOP

Length of output: 50


🏁 Script executed:

rg -n "fetch\|axios\|ownClient" pages/login.tsx pages/api/core.ts | grep -E "(fetch|axios|ownClient)"

Repository: FreeCodeCamp-Chengdu/HOP

Length of output: 50


🏁 Script executed:

rg -n "location.assign|location.href" --type ts --type tsx | head -20

Repository: FreeCodeCamp-Chengdu/HOP

Length of output: 95


🏁 Script executed:

rg -n "401|403" models/User/Session.ts pages/login.tsx pages/api/core.ts

Repository: FreeCodeCamp-Chengdu/HOP

Length of output: 141


The error re-throw after redirect is unnecessary but has minimal impact.

Line 31 throws the error even after initiating the redirect on line 27. Since location.assign() immediately begins navigation, the error propagates to calling code briefly before the page unloads. While this could cause transient console errors, the page navigation will complete and the error becomes moot. Consider not re-throwing the error after a successful redirect initiation.

Regarding infinite redirects: The login page is protected by server-side middleware (sessionGuard) in getServerSideProps and does not make client-side authenticated API calls, so it will not trigger the 401/403 error handler. This redirect scenario is not a concern for the login page specifically.

🤖 Prompt for AI Agents
In models/User/Session.ts around lines 15 to 33, the catch block re-throws the
HTTPError even after initiating a client-side redirect via
globalThis.location.assign(), which causes transient console errors; modify the
handler so that when status is 401 or 403 and a redirect is initiated (i.e.,
when not isServer()), do not re-throw the error — instead return or swallow the
error flow after calling location.assign(); in all other cases (not redirected
or non-401/403 errors) keep the current behavior and re-throw the error so
callers still receive exceptions.

Comment on lines +92 to 103
const client_id = process.env.GITHUB_OAUTH_CLIENT_ID,
client_secret = process.env.GITHUB_OAUTH_CLIENT_SECRET;

if (!client_id || !client_secret) {
console.error(
'[OAuth Config Error] Missing required environment variables:\n' +
' - GITHUB_OAUTH_CLIENT_ID\n' +
' - GITHUB_OAUTH_CLIENT_SECRET\n' +
'Please configure them in .env.local or environment settings.',
);
}

Copy link

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

🏁 Script executed:

cat -n pages/api/core.ts | head -120

Repository: FreeCodeCamp-Chengdu/HOP

Length of output: 4172


🏁 Script executed:

rg -n "githubSigner|sessionGuard" --type ts

Repository: FreeCodeCamp-Chengdu/HOP

Length of output: 5000


🏁 Script executed:

# Check if there are any try-catch blocks or validation around githubSigner construction
grep -A 10 -B 5 "export const githubSigner" pages/api/core.ts

Repository: FreeCodeCamp-Chengdu/HOP

Length of output: 535


🏁 Script executed:

# Check if GITHUB_OAUTH_CLIENT_ID/SECRET are set anywhere in config or .env files
find . -type f \( -name ".env*" -o -name "configuration*" -o -name "config*" \) | head -20

Repository: FreeCodeCamp-Chengdu/HOP

Length of output: 107


🏁 Script executed:

cat .env .env.development 2>/dev/null | head -50

Repository: FreeCodeCamp-Chengdu/HOP

Length of output: 411


🏁 Script executed:

# Search for any actual validation or error handling around githubSigner usage before the routes
rg -n "client_id|client_secret|GITHUB_OAUTH" pages/ --type ts | grep -v node_modules

Repository: FreeCodeCamp-Chengdu/HOP

Length of output: 504


🏁 Script executed:

# Check if there are any tests or documentation about githubOAuth2 behavior
find . -name "*.test.*" -o -name "*.spec.*" | xargs grep -l "githubOAuth2\|githubSigner" 2>/dev/null | head -5

Repository: FreeCodeCamp-Chengdu/HOP

Length of output: 50


Add fail-fast validation for OAuth configuration

The environment variable check correctly logs an error when GITHUB_OAUTH_CLIENT_ID or GITHUB_OAUTH_CLIENT_SECRET are missing, but githubSigner is still constructed unconditionally with non-null assertions:

const client_id = process.env.GITHUB_OAUTH_CLIENT_ID,
  client_secret = process.env.GITHUB_OAUTH_CLIENT_SECRET;

if (!client_id || !client_secret) {
  console.error('[OAuth Config Error] Missing required environment variables...');
}

export const githubSigner = githubOAuth2({
  client_id: client_id!,
  client_secret: client_secret!,
  ...
});

Non-null assertions (!) only affect TypeScript types. At runtime, undefined will still be passed to githubOAuth2(), affecting all 40+ routes using sessionGuard (e.g., pages/login.tsx, pages/me/index.tsx, etc.), resulting in confusing OAuth failures instead of immediate feedback.

Replace the console.error with a throw:

if (!client_id || !client_secret) {
-  console.error('[OAuth Config Error] Missing required environment variables...');
+  throw new Error('[OAuth Config Error] Missing GITHUB_OAUTH_CLIENT_ID or GITHUB_OAUTH_CLIENT_SECRET');
}

Then remove the non-null assertions:

-  client_id: client_id!,
-  client_secret: client_secret!,
+  client_id,
+  client_secret,

This applies to lines 92–111.

🤖 Prompt for AI Agents
In pages/api/core.ts around lines 92–111 the code only logs missing
GITHUB_OAUTH_CLIENT_ID / GITHUB_OAUTH_CLIENT_SECRET but still constructs
githubSigner using non-null assertions, allowing undefined to be passed at
runtime; change the log to a fail-fast throw (e.g., throw new Error with the
same descriptive message) when either env var is missing, and then remove the
non-null assertions when passing client_id and client_secret to githubOAuth2 so
the code never constructs the signer with undefined values.

Copy link
Member

@TechQuery TechQuery left a comment

Choose a reason for hiding this comment

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

先合并主分支更新、安装最新依赖后,再改代码。

Comment on lines +95 to +102
if (!client_id || !client_secret) {
console.error(
'[OAuth Config Error] Missing required environment variables:\n' +
' - GITHUB_OAUTH_CLIENT_ID\n' +
' - GITHUB_OAUTH_CLIENT_SECRET\n' +
'Please configure them in .env.local or environment settings.',
);
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (!client_id || !client_secret) {
console.error(
'[OAuth Config Error] Missing required environment variables:\n' +
' - GITHUB_OAUTH_CLIENT_ID\n' +
' - GITHUB_OAUTH_CLIENT_SECRET\n' +
'Please configure them in .env.local or environment settings.',
);
}
if (!client_id || !client_secret) {
throw new ReferenceError(
`[OAuth Config Error] Missing required environment variables:
- GITHUB_OAUTH_CLIENT_ID
- GITHUB_OAUTH_CLIENT_SECRET
Please configure them in .env.local or environment settings.`,
);

Comment on lines -99 to +111
client_id,
client_secret,
rootBaseURL: useProxy ? `${ProxyBaseURL}/github.com/` : undefined,
client_id: client_id!,
client_secret: client_secret!,
Copy link
Member

Choose a reason for hiding this comment

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

前面 throw 了后面就一定存在,就不用改了。

Comment on lines +20 to +28
useEffect(() => {
if (jwtPayload) sessionStore.user = jwtPayload;
}, [jwtPayload]);

const user = jwtPayload || sessionStore.user;

if (!user) return null;

const { name, email, avatar, mobilePhone } = user;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
useEffect(() => {
if (jwtPayload) sessionStore.user = jwtPayload;
}, [jwtPayload]);
const user = jwtPayload || sessionStore.user;
if (!user) return null;
const { name, email, avatar, mobilePhone } = user;
useEffect(() => {
sessionStore.getProfile();
});
const { name, email, avatar, mobilePhone } = sessionStore.user || jwtPayload;

sessionGuard 中间件在登陆成功之前不会让当前页渲染。

Comment on lines +29 to 30
<Dropdown.Item href="/me">{t('profile')}</Dropdown.Item>
<Dropdown.Item href={`/user/${user.id}`}>{t('home_page')}</Dropdown.Item>
Copy link
Member

Choose a reason for hiding this comment

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

这两项功能重复,请直接在原有用户详情页上改。

Comment on lines +49 to +61

this.initSession();
}

async initSession() {
if (!JWT) return;

try {
await sessionStore.getProfile();
} catch (error) {
console.error('Session restore failed:', error);
sessionStore.signOut();
}
Copy link
Member

Choose a reason for hiding this comment

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

不要在每个页面上检测登录状态,会拖慢很多不需要登录态的公开页面。哪个页面需要,哪个页面自己检查登录态。

Copy link
Member

Choose a reason for hiding this comment

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

sessionGuard 中间件的设计就是 —— 基于 OAuth 登录机制,完全不需要自己写登录页。哪个页面需要登录态,哪个页面声明 sessionGuard 中间件,剩下的中间件自动处理,是 AOP(面向切面、”装饰模式“)编程思想在 Next.js SSR 的应用。

Comment on lines +41 to +42
# API Host
NEXT_PUBLIC_API_HOST=https://openhackathon-service.onrender.com
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# API Host
NEXT_PUBLIC_API_HOST=https://openhackathon-service.onrender.com

公开变量在 .env(已被 Git 托管)

3. Fill in:
- **Application name**: HOP Local Dev
- **Homepage URL**: `http://localhost:3000`
- **Authorization callback URL**: `http://localhost:3000/login`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- **Authorization callback URL**: `http://localhost:3000/login`
- **Authorization callback URL**: `http://localhost:3000`

写死子路径会让 sessionGuard 给任意页面提供 OAuth 登录的能力彻底废掉……

Comment on lines +15 to +33
.use(async ({ request }, next) => {
try {
return await next();
} catch (error) {
if (error instanceof HTTPError) {
const { status } = error.response;

if (status === 401 || status === 403) {
if (!isServer()) {
const currentPath = globalThis.location?.pathname || '/';
const loginUrl = `/login?redirect=${encodeURIComponent(currentPath)}`;

globalThis.location?.assign(loginUrl);
}
}
}
throw error;
}
});
Copy link
Member

Choose a reason for hiding this comment

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

  1. 只有 401 未登录状态才表示用户 JWT 失效,403 只是某个资源当前用户无权访问
  2. 所有客户端网络请求错误的兜底错误处理应在全局事件回调,网络中间件只处理纯数据逻辑,有何错误应由上层调用方判断,Axios 拦截器那种从底层截胡的思维要不得:

    HOP/pages/_app.tsx

    Lines 40 to 48 in b962bc8

    window.addEventListener('unhandledrejection', ({ reason }) => {
    const { message, response } = reason as HTTPError;
    const { statusText, body } = response || {};
    const tips = body?.message || statusText || message;
    if (tips) alert(tips);
    });
    }

Comment on lines +3 to +4
# Skip OAuth proxy for local development (proxy server may be unavailable)
SKIP_OAUTH_PROXY = 1
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Skip OAuth proxy for local development (proxy server may be unavailable)
SKIP_OAUTH_PROXY = 1

不要跳过代理,它是为了中国大多数电脑即使挂梯子也无法让 Node.js 稳定访问 GitHub OAuth 接口而架设的。只是从“闭源社”迁回时没部署我们自己的测试服,但现在我搞好了。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Some improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

现代化 HOP(黑客松开放平台)个人主页 UI 样式

3 participants