Skip to content

feat(web): add login page, cookie session auth, and logout - #578

Closed
ligonfei wants to merge 4 commits into
agegr:mainfrom
ligonfei:feat/web-login-page
Closed

feat(web): add login page, cookie session auth, and logout#578
ligonfei wants to merge 4 commits into
agegr:mainfrom
ligonfei:feat/web-login-page

Conversation

@ligonfei

@ligonfei ligonfei commented Aug 22, 2026

Copy link
Copy Markdown

Summary

Adds a web login page and cookie-based session authentication for PI_WEB_PASSWORD protection, while keeping full backward compatibility with HTTP Basic Auth.

Changes

  • Login page (/login): Dedicated login interface with light/dark theme and English/Chinese (zh-CN) localization. Redirects back to the requested path after authentication.
  • Cookie session authentication: Adds HMAC-SHA256 signed session cookie support in lib/web-auth.ts. proxy.ts handles both session cookies (browser) and Basic Auth (API/CLI).
  • Auth endpoints:
    • POST /api/auth/web/login: Validates credentials and sets HttpOnly session cookie.
    • POST /api/auth/web/logout: Clears session cookie.
  • UI integration: Adds logout button in settings when password protection is enabled.
  • Tests: Adds unit tests for session signing, validation, expiration, and hybrid authentication.

Verification

  • Ran unit tests: npm test (588 tests passing).
  • Ran build: npm run build passed cleanly.

Copilot AI lite review requested due to automatic review settings August 22, 2026 04:22
@ligonfei ligonfei changed the title feat(web): add web login page, cookie-based session auth, and logout support feat(web): add login page, cookie session auth, and logout Aug 22, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved redirect-security, authentication compatibility, endpoint-contract, redirect-preservation, and accessibility findings remain.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds localized web login/logout flows with signed cookie sessions while retaining Basic Auth support.

Changes:

  • Adds hybrid cookie and Basic Auth middleware.
  • Adds login UI, session/logout endpoints, and logout controls.
  • Adds authentication tests and translations.
File summaries
File Description
proxy.ts Hybrid authentication, redirects, and route protection.
package-lock.json Updates dependency metadata.
lib/web-auth.ts Implements signed session tokens.
lib/web-auth.test.mjs Tests authentication behavior.
lib/i18n/messages/zh-CN.ts Adds Chinese authentication strings.
lib/i18n/messages/en.ts Adds English authentication strings.
components/AppShell.tsx Adds authentication status and logout controls.
app/login/page.tsx Adds the localized login form.
app/api/auth/web/session/route.ts Handles session status and login.
app/api/auth/web/logout/route.ts Clears session cookies.
Review details

Suppressed comments (4)

app/api/auth/web/session/route.ts:49

  • The login form collects and sends a username, but this handler ignores it and accepts any username when the password matches. That diverges from the documented Basic Auth contract, where the username is fixed to pi, and lets users create cookie sessions with credentials that Basic Auth would reject. Validate body.username against PI_WEB_AUTH_USERNAME or remove the username field from the web flow.
  let body: { password?: string } = {};
  try {
    body = await request.json();
  } catch {
    return NextResponse.json(
      { error: "Invalid JSON payload" },
      { status: 400 },
    );
  }

  const inputPassword = typeof body.password === "string" ? body.password : "";
  if (inputPassword !== password) {
    return NextResponse.json({ error: "Invalid password" }, { status: 401 });

app/api/auth/web/session/route.ts:48

  • request.json() can return JSON null; the TypeScript annotation does not validate the runtime value, so body.password throws for that payload and turns a malformed login request into a 500. Guard the parsed value before reading its fields.
  const inputPassword = typeof body.password === "string" ? body.password : "";
  if (inputPassword !== password) {

app/api/auth/web/session/route.ts:49

  • The unauthenticated login path compares the configured secret with !==, unlike the existing Basic Auth verifier's constant-time comparison. Repeated remote login attempts can observe string-comparison timing; reuse the constant-time credential verifier (while validating the username) instead of comparing the password directly.
  const inputPassword = typeof body.password === "string" ? body.password : "";
  if (inputPassword !== password) {
    return NextResponse.json({ error: "Invalid password" }, { status: 401 });

app/login/page.tsx:462

  • This error is inserted asynchronously without a live-region or alert role, so screen-reader users are not notified when a login attempt fails. Mark the error container as role="alert" (or provide an equivalent live region).
              {error && (
                <div
  • Files reviewed: 9/10 changed files
  • Comments generated: 6
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +29 to +35
export async function POST(request: NextRequest) {
const password = process.env.PI_WEB_PASSWORD;
const passwordRequired = isWebPasswordEnabled(password);

if (!passwordRequired) {
return NextResponse.json({ success: true });
}
Comment on lines +20 to +25
const cookieToken = request.cookies.get(PI_WEB_SESSION_COOKIE)?.value;
const authenticated = isValidSessionToken(cookieToken, password);

return NextResponse.json({
authRequired: true,
authenticated,
Comment thread app/login/page.tsx Outdated
{/* 显隐密码切换按钮 */}
<button
type="button"
tabIndex={-1}
Comment thread proxy.ts Outdated
Comment on lines +80 to +86
const redirectParam = request.nextUrl.searchParams.get("redirect");
const redirectUrl =
redirectParam &&
redirectParam.startsWith("/") &&
!redirectParam.startsWith("//")
? new URL(redirectParam, request.url)
: new URL("/", request.url);
Comment thread proxy.ts Outdated
Comment on lines +72 to +74
if (pathname && pathname !== "/") {
loginUrl.searchParams.set("redirect", pathname + request.nextUrl.search);
}
Comment thread proxy.ts
Comment on lines +62 to +65
status: 401,
headers: {
"Cache-Control": "no-store",
},
@ligonfei

Copy link
Copy Markdown
Author

@copilot review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Critical redirect validation issues and a moderate whitespace-password handling issue remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (8)

Previously missed (1) — in code that hasn't changed since the last review.

app/login/page.tsx:52

  • isWebPasswordEnabled treats any non-empty string as a valid password, including whitespace-only values, but this trims the entered value and returns before submitting it. The submit button uses the same trimmed predicate, so a whitespace-only PI_WEB_PASSWORD works with Basic Auth but cannot be used through the new login page; test emptiness without trimming in both places.
      if (!password.trim()) return;

app/api/auth/web/logout/route.ts:24

  • This only expires pi_web_session, but the proxy still accepts a browser's cached Authorization: Basic ... header. A user who entered through the legacy Basic Auth flow therefore remains authenticated and is immediately redirected back home when this handler navigates to /login, so the new logout control does not actually log them out. Either hide this control for Basic-authenticated sessions or add browser-specific state that makes the proxy stop honoring the cached Basic credentials after logout.
  const response = NextResponse.json({ success: true });
  response.cookies.set({
    name: PI_WEB_SESSION_COOKIE,
    value: "",
    httpOnly: true,
    secure: isHttps,
    sameSite: "lax",
    path: "/",
    maxAge: 0,
  });

app/login/page.tsx:6

  • useEffect is imported but never referenced in this new page. Remove the unused import so the page remains lint-clean.
  useCallback,

components/AppShell.tsx:85

  • Because proxy.ts accepts a valid Authorization header as authenticated, a browser using the backward-compatible Basic Auth flow remains authenticated after this clears the cookie: the subsequent /login navigation is immediately redirected back to /. Hide or disable this control for Basic-auth sessions, or otherwise provide an auth flow that can actually terminate that browser authentication state.
      await fetch("/api/auth/web/logout", { method: "POST" });
    } catch {}
    window.location.href = "/login";

components/AppShell.tsx:85

  • Using href leaves the authenticated application entry in browser history and may allow it to be restored from the back/forward cache after the cookie is cleared, exposing the previously rendered session to the next person using the browser. Replace the current entry when leaving after logout.
    window.location.href = "/login";

lib/web-auth.ts:93

  • > leaves a token valid for the whole second in which Date.now() / 1000 equals expiresAt, so it remains usable after its declared expiration (and a zero-age token can be accepted briefly). Use >= so the signed expiry and cookie max-age have the same boundary.
  if (Math.floor(Date.now() / 1000) > expiresAt) return false;

proxy.ts:61

  • The added tests cover the pure token helpers, but not the new proxy/route integration: page redirects, API 401 responses, Basic-versus-cookie acceptance, or cookie set/clear attributes. Since these paths define the authentication boundary, add focused integration tests before relying on the helper tests alone.
  const authorization = request.headers.get("authorization");
  const cookieToken = request.cookies.get(PI_WEB_SESSION_COOKIE)?.value;
  const isAuthenticated = isValidWebAuth(authorization, cookieToken, password);

proxy.ts:104

  • Because this matcher now covers all non-static paths, it also intercepts Next's development HMR endpoint (/_next/webpack-hmr). With PI_WEB_PASSWORD enabled, the unauthenticated login page receives the /login redirect instead of the HMR stream, so edits to the new login page do not hot-reload during npm run dev; exclude this internal endpoint from the matcher.
    "/((?!_next/static|_next/image|favicon.ico|manifest.webmanifest|icons/|offline.html|sw.js).*)",
  • Files reviewed: 10/11 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread app/login/page.tsx
Comment on lines +14 to +30
function getSafeRedirectUrl(): string {
try {
const urlParams = new URLSearchParams(window.location.search);
const redirect = urlParams.get("redirect");
if (
redirect &&
redirect.startsWith("/") &&
!redirect.startsWith("//") &&
!redirect.includes("\\")
) {
return redirect;
}
} catch {
// 忽略异常
}
return "/";
}
Comment thread proxy.ts Outdated
Comment on lines +12 to +20
function isSafeInternalPath(path: string | null | undefined): boolean {
if (!path || typeof path !== "string") return false;
return (
path.startsWith("/") &&
!path.startsWith("//") &&
!path.includes("\\") &&
!path.includes("\0")
);
}
@huangyxi

Copy link
Copy Markdown
Contributor

This is related to #347.
And I don't think introducing a username is a good idea when there's typically only a single user.

@agegr

agegr commented Aug 25, 2026

Copy link
Copy Markdown
Owner

Thanks for the contribution. The existing Basic Auth already covers our current authentication needs, and adding a separate login/session flow introduces more security and maintenance complexity than we need right now. I’m going to close this PR for now.

@agegr agegr closed this Aug 25, 2026
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.

4 participants