Skip to content

refactor(join): decide the approved-member redirect on the server - #66

Merged
yufoxda merged 12 commits into
developfrom
refactor/join-redirect-server-side
Jul 27, 2026
Merged

refactor(join): decide the approved-member redirect on the server#66
yufoxda merged 12 commits into
developfrom
refactor/join-redirect-server-side

Conversation

@yufoxda

@yufoxda yufoxda commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

⚠️ スタックPRです。ベースは security/close-dev-auth-bypass(#65)。マージ順は #61 → #62 → #63 → #64 → #65 → 本PR。

レビューで指摘した useEffect によるリダイレクトをサーバー側へ移します。

何が問題だったか

useEffect(() => {
  if (existingMember?.memberStatus === 'active') router.replace('/me')
}, [existingMember?.memberStatus, router])

画面遷移自体は正当な副作用なので「典型的なアンチパターン」ではありません。ただし判断材料はレンダリング前に確定しているため、Effect に置く必要がありませんでした。結果として承認済み会員には「承認済みです。会員ページへ移動しています…」が一瞬描画されてから遷移する、いわゆるフラッシュが出ていました。

変更内容

/join をサーバーコンポーネント化し、描画前に判定します。

  • サーバーで会員情報を1回読み、active なら redirect('/me')
  • それ以外は従来のフォームをクライアントコンポーネント(JoinPageClient)として描画
  • 取得失敗時はクライアントに委ねます。クライアント側には既に再試行UIがあるため、APIの不調でページ全体がエラー境界に置き換わることを避けています
  • redirect() は例外で制御を移す仕様なので、try/catch の外に置いています

既存のサーバーコンポーネント(/event, /event/[id])と同じ cookies() + Cookie ヘッダー転送の方式に揃えました。

結果

入部フローから Effect が完全になくなりました。 フォームの初期値は元々 key による再マウントで解決されており(existing values initialize a keyed form without an effect というテストがあります)、今回のリダイレクト移行で残る1件も解消しました。

検証

対象 結果
frontend テスト 25 pass、tsc --noEmit クリーン、lint エラー0、本番ビルド成功

ビルド出力で /join が ○(静的)から ƒ(サーバーレンダリング)へ変わることを確認しています。

検証の限界(正直に)

ブラウザでの実動作確認はしていません。このブランチのマイグレーション(app_accounts 等)が実DBに未適用のため、このチェックアウトからアプリを起動しても DB 層で失敗します。確認したのは型・テスト・lint・本番ビルドと、ルートがサーバーレンダリングに変わったことまでです。

🤖 Generated with Claude Code

@yufoxda
yufoxda force-pushed the security/close-dev-auth-bypass branch from e8b03cf to 357fc37 Compare July 27, 2026 07:31
@yufoxda
yufoxda force-pushed the refactor/join-redirect-server-side branch from a2a762f to b5b1d32 Compare July 27, 2026 07:31
@yufoxda
yufoxda force-pushed the security/close-dev-auth-bypass branch from 357fc37 to 0e479e8 Compare July 27, 2026 07:41
@yufoxda
yufoxda force-pushed the refactor/join-redirect-server-side branch from b5b1d32 to 55211eb Compare July 27, 2026 07:41
yufoxda and others added 6 commits July 27, 2026 16:45
@yufoxda @claude
refactor(community): give the provider port vendor-neutral types
c18ece5 
The port was generic in name only. Its methods returned DiscordGuildMembership,
DiscordMessage and DiscordReactionUser, and every identifier was validated
against the Discord snowflake format, so the identifier regex reached callers
that have no reason to know what a snowflake is. Replacing the provider would
have meant editing the interface and both api_v0 services rather than swapping
an adapter.

The port now speaks CommunityRole, CommunityMembership, CommunityMessage,
CommunityReactionUser and CommunityAccountProfile, treats identifiers as opaque
strings, and drops the Discord message length limit. Discord's snowflake
format, its 2000-character limit, and the global_name field it returns are
refinements applied in discord/schema.ts, which is the only layer that issues
those values. The provider-specific field name is mapped to displayName at the
adapter boundary, matching the provider_display_name column it is stored in.

Behaviour is unchanged: the adapter still rejects a malformed provider response
and a guild member response for another user, both of which depend on the
snowflake assertion that moved.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@yufoxda @claude
fix(message): stop repeating private member fields in reaction badges
f2650c9 
The reaction summary returned the complete member record — student ID, student
email, emergency contact, insurance and allergy details — inside every
reaction's user list as well as in `members`. A member who reacted with three
emoji had those fields serialised four times, so the private data on the wire
grew with the number of reactions rather than the number of members.

The badges only ever rendered names: the client maps that list through
getDisplayName and reads nothing else from it. They now carry a
ReactionParticipant with the identity and name fields, while `members` keeps
the full record the admin table and its CSV export need, including the
emergency contact and allergy details an organiser relies on.

Adds a regression test asserting no private field appears in the badge payload.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@yufoxda @claude
refactor(db): move the authentication store into its own schema
a7e6ef8 
Better Auth's tables move to app_auth, and every reference that crossed between
them and the domain is removed, so the authentication store can be lifted into
its own database without touching a domain table. Moving the tables alone would
not have achieved that: a foreign key cannot span databases, and three of them
crossed this boundary, one of which only surfaced when the new assertion ran.

The domain now keeps its own account record in public.app_accounts, holding the
membership link and the application role. role has to live on this side because
the membership trigger authorizes against it, and a trigger cannot read another
database. user_id is stored as a value rather than a foreign key; the reviewer
and community identity references become snapshots too, matching
member_status_history, which already recorded its actor that way.

Losing those foreign keys costs the guarantee that a reviewed user cannot be
deleted. That is the price of the move, and it is now stated in the schema
comments and asserted in the pgTAP suite rather than left implicit.

The auth middleware is the only place that reads both sides. It resolves the
subject from the authentication store and the account from the domain in two
separate queries, so a future split turns the first into a remote call and
leaves the second untouched. The admin views that display an account email do
the same thing in batch instead of joining.

A replay test asserts the tables sit in app_auth, that "user" carries no domain
column, and that no foreign key crosses the boundary in either direction.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@yufoxda @claude
fix(db): assert the reviewer snapshot instead of the dropped foreign key
6bc0b0f 
The RLS suite still required a reviewed account to be undeletable, which was
the guarantee the dropped foreign key provided. That key crossed into the
authentication store, so the assertion now states what replaced it: the delete
succeeds and the review record keeps the reviewer it was written with.

Caught by the pgTAP job, which the membership work added and which is the only
thing that executes these suites.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@yufoxda @claude
fix(auth): repair a missing domain account at sign-in
2e77823 
Sign-in now requires a public.app_accounts row, which the Better Auth create
hook provisions. If that hook ever fails, or a subject is seeded outside the
flow, the account exists in the authentication store with nothing on the domain
side, and the JWT endpoint and the auth middleware both reject it. The user is
then locked out permanently with no path back except editing the database.

The sign-in path now inserts the row when it is absent, which costs one
statement per sign-in and makes the missing row self-correcting.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@yufoxda @claude
security: close the development authentication bypass on a deployment
23ab54b 
Both Workers skip token verification entirely when NODE_ENV is 'development',
serving every request as DEV_USER_ID. Production was safe only because the
variable happened to be unset, so a single misconfigured variable would have
opened the whole API, including the admin endpoints if DEV_USER_ID named an
admin.

The bypass now also requires the request to have arrived on a local hostname.
NODE_ENV is an ordinary variable a deployment can carry by mistake; the host a
request actually reached is not, so the two must agree. Both wrangler configs
additionally state NODE_ENV=production, which .dev.vars overrides locally.

Regression tests assert that a deployed hostname carrying NODE_ENV=development
still returns 401 without touching the database, and that local development
keeps working.

The frontend middleware keys on NODE_ENV alone, which is correct there because
Next inlines it at build time; the reasoning is recorded next to it so it is
not mistaken for the same gap.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@yufoxda
yufoxda force-pushed the security/close-dev-auth-bypass branch from 0e479e8 to 23ab54b Compare July 27, 2026 07:46
@yufoxda @claude
refactor(join): decide the approved-member redirect on the server
81a0561 
The join page sent approved members to /me from an effect, so it first painted
the application form and then navigated away from it. The status it keys on is
already known before rendering, so the decision does not belong in an effect.

The route is now a server component that reads the member once, redirects when
the application is active, and otherwise renders the form as a client
component. A lookup failure falls through to the client, which already renders
and retries its own errors, so a slow or failing API cannot replace the page
with an error boundary.

This leaves no effect in the join flow: the form was already keyed rather than
synchronised through one.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@yufoxda
yufoxda force-pushed the refactor/join-redirect-server-side branch from 55211eb to 81a0561 Compare July 27, 2026 07:46
@yufoxda
yufoxda changed the base branch from security/close-dev-auth-bypass to develop July 27, 2026 12:53
@yufoxda
yufoxda merged commit 285ac1b into develop Jul 27, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@yufoxda