refactor(auth): move auth server internals under features/auth, rename session guards module by KATO-Hiro · Pull Request #3872 · AtCoder-NoviSteps/AtCoderNoviSteps

KATO-Hiro · 2026-07-18T10:05:11Z

Summary by CodeRabbit

改善
- ログイン、ログアウト、ユーザー登録などの認証処理を整理しました。
- 認証関連のアーキテクチャガイドとルール文書を更新しました。

sample_data.ts was a leftover placeholder predating real DB-backed task/answer fetching; nothing referenced it besides a commented-out import.

…e session guards module Consolidates src/lib/server/{auth,session,password,random}.ts into src/features/auth/server/ to keep auth-domain code colocated per the feature-scoped structure. Renames services/session.ts to session_guards.ts to reflect its actual responsibility (guard/redirect helpers) and disambiguate from the new server/session.ts.

… document server/ convention Completes the src/lib/server -> src/features/auth/server migration by fixing the remaining consumers (hooks, app.d.ts, seed, auth routes, credentials service). Also documents the feature-scoped server/ subdirectory convention in architecture.md.

coderabbitai · 2026-07-18T10:05:56Z

📝 Walkthrough

Walkthrough

認証のサーバー実装を src/features/auth に集約し、パスワード・乱数・セッション処理とセッションガードを追加・検証した。各ルート、型定義、seed、ドキュメントの参照先も更新した。

Changes

認証サーバー基盤

Layer / File(s)	Summary
パスワード・乱数・セッション実装とテスト `src/features/auth/server/*`	Lucia v2 互換のパスワードハッシュ、NFKC 正規化、定数時間比較、乱数文字列生成、セッション期限・削除処理を実装し、境界条件とエラー処理をテストした。

セッションガード

Layer / File(s)	Summary
ログイン必須処理の追加とルート切り替え `src/features/auth/services/session_guards.ts`, `src/features/auth/services/session_guards.test.ts`, `src/routes/*`	`ensureSessionOrRedirect` と `getLoggedInUser` を追加し、未認証時のログインリダイレクトを各ページサーバーから利用する構成に変更した。

認証参照先の切り替え

Layer / File(s)	Summary
型・フック・認証フローの import 更新 `src/app.d.ts`, `src/hooks.server.ts`, `src/routes/(auth)/`, `src/features/auth/services/credentials.`, `prisma/seed.ts`	認証リクエスト、セッション、パスワード処理の参照先を `$features/auth` 系へ変更した。

配置規約と不要記述

Layer / File(s)	Summary
server 配置規約と補助記述の整理 `.claude/rules/auth.md`, `docs/guides/architecture.md`, `src/lib/components/TagListForEdit.svelte`	`features/{feature}/server/` の配置規約と `src/lib/server/` の構成例を更新し、不要なコメントアウト import を削除した。

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

AtCoder-NoviSteps/AtCoderNoviSteps#3864: 同じ Lucia から自前認証への移行として、認証コアと credential フローの features/auth 集約に直接関連する。
AtCoder-NoviSteps/AtCoderNoviSteps#3492: getLoggedInUser とログインリダイレクトを用いた認証ガードの実装変更に関連する。

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	認証関連のサーバ実装を features/auth に集約し、参照先も更新されており、#3870 の目的に合致しています。
Out of Scope Changes check	✅ Passed	削除された sample_data と関連 import の整理も、明記されたリファクタ範囲内です。
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	認証サーバ内部の移設と session_guards へのリネームを簡潔に要約しており、変更内容と一致しています。

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch #3870

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.}

coderabbitai

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (1)

.claude/rules/auth.md (1)
38-43: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

サービス層の責務と記載を一致させてください。

src/features/auth/services/ 配下の関数が redirect() / error() を実行する設計は、サービスを framework-agnostic な値/null返却に限定する規約と矛盾します。ガードは判定結果を返し、リダイレクトや HTTP エラー変換は各 +page.server.ts / +server.ts で行う形に、実装とドキュメントを揃えてください。

As per coding guidelines and path instructions, services must return values or null and must not call redirect() or error().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/rules/auth.md around lines 38 - 43, Update the authentication
service functions described in the auth documentation, including
getLoggedInUser, ensureSessionOrRedirect, validateAdminAccess, and
validateAdminAccessForApi, so they remain framework-agnostic and return users,
authorization results, or null instead of calling redirect() or error(). Move
redirect and HTTP error translation into the corresponding +page.server.ts and
+server.ts callers, and revise the documented contracts to match.
Sources: Coding guidelines, Path instructions

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/features/auth/services/session_guards.ts`:
- Around line 24-49: Change getLoggedInUser in
src/features/auth/services/session_guards.ts (lines 24-49) to return
App.Locals['user'] | null without accepting url or redirecting, and remove
ensureSessionOrRedirect; update
src/features/auth/services/session_guards.test.ts (line 19) to remove its test
and cover the null result. In src/routes/problems/[slug]/+page.server.ts (lines
14 and 27), src/routes/users/edit/+page.server.ts (line 19), and
src/routes/workbooks/edit/[slug]/+page.server.ts (lines 24 and 82), add explicit
post-getLoggedInUser redirects for missing users with the required
buildLoginPath import. In src/routes/workbooks/create/+page.server.ts (lines
18-20), replace ensureSessionOrRedirect with getLoggedInUser, redirecting when
the returned author is null.

---

Outside diff comments:
In @.claude/rules/auth.md:
- Around line 38-43: Update the authentication service functions described in
the auth documentation, including getLoggedInUser, ensureSessionOrRedirect,
validateAdminAccess, and validateAdminAccessForApi, so they remain
framework-agnostic and return users, authorization results, or null instead of
calling redirect() or error(). Move redirect and HTTP error translation into the
corresponding +page.server.ts and +server.ts callers, and revise the documented
contracts to match.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7ed40603-c80b-44b9-a784-f1edef606d3d

📥 Commits

Reviewing files that changed from the base of the PR and between 04d51da and 2f6d4dc.

📒 Files selected for processing (29)

.claude/rules/auth.md
docs/guides/architecture.md
prisma/seed.ts
src/app.d.ts
src/features/auth/server/auth.test.ts
src/features/auth/server/auth.ts
src/features/auth/server/password.test.ts
src/features/auth/server/password.ts
src/features/auth/server/random.test.ts
src/features/auth/server/random.ts
src/features/auth/server/session.test.ts
src/features/auth/server/session.ts
src/features/auth/services/credentials.test.ts
src/features/auth/services/credentials.ts
src/features/auth/services/session_guards.test.ts
src/features/auth/services/session_guards.ts
src/hooks.server.ts
src/lib/components/TagListForEdit.svelte
src/lib/server/sample_data.ts
src/routes/(auth)/login/+page.server.ts
src/routes/(auth)/logout/+page.server.ts
src/routes/(auth)/signup/+page.server.ts
src/routes/problems/[slug]/+page.server.ts
src/routes/users/[username]/+page.server.ts
src/routes/users/edit/+page.server.ts
src/routes/workbooks/+page.server.ts
src/routes/workbooks/[slug]/+page.server.ts
src/routes/workbooks/create/+page.server.ts
src/routes/workbooks/edit/[slug]/+page.server.ts

💤 Files with no reviewable changes (2)

src/lib/server/sample_data.ts
src/lib/components/TagListForEdit.svelte

coderabbitai

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (1)

.claude/rules/auth.md (1)
38-43: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

サービス層の責務と記載を一致させてください。

src/features/auth/services/ 配下の関数が redirect() / error() を実行する設計は、サービスを framework-agnostic な値/null返却に限定する規約と矛盾します。ガードは判定結果を返し、リダイレクトや HTTP エラー変換は各 +page.server.ts / +server.ts で行う形に、実装とドキュメントを揃えてください。

As per coding guidelines and path instructions, services must return values or null and must not call redirect() or error().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/rules/auth.md around lines 38 - 43, Update the authentication
service functions described in the auth documentation, including
getLoggedInUser, ensureSessionOrRedirect, validateAdminAccess, and
validateAdminAccessForApi, so they remain framework-agnostic and return users,
authorization results, or null instead of calling redirect() or error(). Move
redirect and HTTP error translation into the corresponding +page.server.ts and
+server.ts callers, and revise the documented contracts to match.
Sources: Coding guidelines, Path instructions

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/features/auth/services/session_guards.ts`:
- Around line 24-49: Change getLoggedInUser in
src/features/auth/services/session_guards.ts (lines 24-49) to return
App.Locals['user'] | null without accepting url or redirecting, and remove
ensureSessionOrRedirect; update
src/features/auth/services/session_guards.test.ts (line 19) to remove its test
and cover the null result. In src/routes/problems/[slug]/+page.server.ts (lines
14 and 27), src/routes/users/edit/+page.server.ts (line 19), and
src/routes/workbooks/edit/[slug]/+page.server.ts (lines 24 and 82), add explicit
post-getLoggedInUser redirects for missing users with the required
buildLoginPath import. In src/routes/workbooks/create/+page.server.ts (lines
18-20), replace ensureSessionOrRedirect with getLoggedInUser, redirecting when
the returned author is null.

---

Outside diff comments:
In @.claude/rules/auth.md:
- Around line 38-43: Update the authentication service functions described in
the auth documentation, including getLoggedInUser, ensureSessionOrRedirect,
validateAdminAccess, and validateAdminAccessForApi, so they remain
framework-agnostic and return users, authorization results, or null instead of
calling redirect() or error(). Move redirect and HTTP error translation into the
corresponding +page.server.ts and +server.ts callers, and revise the documented
contracts to match.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7ed40603-c80b-44b9-a784-f1edef606d3d

📥 Commits

Reviewing files that changed from the base of the PR and between 04d51da and 2f6d4dc.

📒 Files selected for processing (29)

.claude/rules/auth.md
docs/guides/architecture.md
prisma/seed.ts
src/app.d.ts
src/features/auth/server/auth.test.ts
src/features/auth/server/auth.ts
src/features/auth/server/password.test.ts
src/features/auth/server/password.ts
src/features/auth/server/random.test.ts
src/features/auth/server/random.ts
src/features/auth/server/session.test.ts
src/features/auth/server/session.ts
src/features/auth/services/credentials.test.ts
src/features/auth/services/credentials.ts
src/features/auth/services/session_guards.test.ts
src/features/auth/services/session_guards.ts
src/hooks.server.ts
src/lib/components/TagListForEdit.svelte
src/lib/server/sample_data.ts
src/routes/(auth)/login/+page.server.ts
src/routes/(auth)/logout/+page.server.ts
src/routes/(auth)/signup/+page.server.ts
src/routes/problems/[slug]/+page.server.ts
src/routes/users/[username]/+page.server.ts
src/routes/users/edit/+page.server.ts
src/routes/workbooks/+page.server.ts
src/routes/workbooks/[slug]/+page.server.ts
src/routes/workbooks/create/+page.server.ts
src/routes/workbooks/edit/[slug]/+page.server.ts

💤 Files with no reviewable changes (2)

src/lib/server/sample_data.ts
src/lib/components/TagListForEdit.svelte

🛑 Comments failed to post (1)

src/features/auth/services/session_guards.ts (1)
24-49: 🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy lift

サービスのガイドライン違反および未ログインアクセスの破損。

session_guards.ts 内で redirect() を呼び出していることは、「サービス層では error() や redirect() を呼ばない（HTTPエラー変換はルートハンドラで行う）」というコーディングガイドラインに直接違反しています。
さらに、getLoggedInUser が一律でリダイレクトを投げるようになったことで、これまで null を許容して未ログインユーザーにコンテンツを表示していたページ（/workbooks、/workbooks/[slug]、/users/[username] など）にアクセスできなくなる致命的なバグを引き起こしています。

共有の根本原因を解決するために、サービス層では null を返すようにし、セッション必須のルートハンドラ側で明示的にリダイレクトを行ってください。

src/features/auth/services/session_guards.ts#L24-L49: getLoggedInUser を App.Locals['user'] | null を返すように変更し（引数 url および内部の redirect を削除）、ガイドライン違反となる ensureSessionOrRedirect は削除してください。

src/features/auth/services/session_guards.test.ts#L19-L19: 実装の変更に合わせて、ensureSessionOrRedirect のテストを削除し、getLoggedInUser が null を返すケースのテストに修正してください。

src/routes/problems/[slug]/+page.server.ts#L14-L14: セッション必須の処理であるため、getLoggedInUser の直後に if (!loggedInUser) redirect(TEMPORARY_REDIRECT, buildLoginPath(url)); を追加してください（buildLoginPath の import が必要です）。

src/routes/problems/[slug]/+page.server.ts#L27-L27: 同上（action内）。

src/routes/users/edit/+page.server.ts#L19-L19: セッション必須のため、getLoggedInUser の直後に if (!loggedInUser) redirect(TEMPORARY_REDIRECT, buildLoginPath(url)); を追加してください（要import）。

src/routes/workbooks/create/+page.server.ts#L18-L20: ensureSessionOrRedirect を削除し、代わりに const author = await getLoggedInUser(locals); if (!author) redirect(TEMPORARY_REDIRECT, buildLoginPath(url)); としてください（要import）。

src/routes/workbooks/edit/[slug]/+page.server.ts#L24-L24: 編集画面にはセッションが必要なため、getLoggedInUser の直後に if (!loggedInUser) redirect(TEMPORARY_REDIRECT, buildLoginPath(url)); を追加してください（要import）。

src/routes/workbooks/edit/[slug]/+page.server.ts#L82-L82: 同上（action内）。

📍 Affects 6 files

src/features/auth/services/session_guards.ts#L24-L49 (this comment)

src/features/auth/services/session_guards.test.ts#L19-L19

src/routes/problems/[slug]/+page.server.ts#L14-L14

src/routes/problems/[slug]/+page.server.ts#L27-L27

src/routes/users/edit/+page.server.ts#L19-L19

src/routes/workbooks/create/+page.server.ts#L18-L20

src/routes/workbooks/edit/[slug]/+page.server.ts#L24-L24

src/routes/workbooks/edit/[slug]/+page.server.ts#L82-L82
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/features/auth/services/session_guards.ts` around lines 24 - 49, Change
getLoggedInUser in src/features/auth/services/session_guards.ts (lines 24-49) to
return App.Locals['user'] | null without accepting url or redirecting, and
remove ensureSessionOrRedirect; update
src/features/auth/services/session_guards.test.ts (line 19) to remove its test
and cover the null result. In src/routes/problems/[slug]/+page.server.ts (lines
14 and 27), src/routes/users/edit/+page.server.ts (line 19), and
src/routes/workbooks/edit/[slug]/+page.server.ts (lines 24 and 82), add explicit
post-getLoggedInUser redirects for missing users with the required
buildLoginPath import. In src/routes/workbooks/create/+page.server.ts (lines
18-20), replace ensureSessionOrRedirect with getLoggedInUser, redirecting when
the returned author is null.
Source: Coding guidelines

KATO-Hiro

LGTM

KATO-Hiro added 5 commits July 18, 2026 05:56

chore: remove unused sample_data and its dangling commented-out import

5008ac5

sample_data.ts was a leftover placeholder predating real DB-backed task/answer fetching; nothing referenced it besides a commented-out import.

docs(dev-notes): add auth-feature-migration plan

96fb196

docs(dev-notes): remove auth-feature-migration plan

2f6d4dc

KATO-Hiro temporarily deployed to preview July 18, 2026 10:06 — with GitHub Actions Inactive

coderabbitai Bot reviewed Jul 18, 2026

View reviewed changes

KATO-Hiro commented Jul 19, 2026

View reviewed changes

KATO-Hiro merged commit cb0dbd3 into staging Jul 19, 2026
3 checks passed

KATO-Hiro deleted the #3870 branch July 19, 2026 05:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

refactor(auth): move auth server internals under features/auth, rename session guards module#3872

refactor(auth): move auth server internals under features/auth, rename session guards module#3872
KATO-Hiro merged 5 commits into
stagingfrom
#3870

KATO-Hiro commented Jul 18, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 18, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot left a comment

Uh oh!

KATO-Hiro left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

KATO-Hiro commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

KATO-Hiro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

KATO-Hiro commented Jul 18, 2026 •

edited

Loading

coderabbitai Bot commented Jul 18, 2026 •

edited

Loading