traPtitech / traPtitech/Checkin

権限が広がるときにセッション ID を入れ替えず、古いセッションも無効にしていない

Open
#73 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
TypeScript
Stars
0
Forks
0
Avg merge
3d 5h
Merged PRs (30d)
7

Description

症状

packages/api/src/auth/session.tsattachUserToSession は、sessionsuser_id を UPDATE するだけで、セッション ID は変えない。クッキーに入るトークンは id_hash 列(トークンの SHA-256)で引かれるが、packagesapps の中に id_hash を UPDATE する経路は 1 つも無い。id_hash に対応する idHash が現れるのは packages/api/src/auth/session.tspackages/db/src/schema.ts だけで、前者では INSERT の値、resolveSession のローカル変数、SELECT と DELETE と UPDATE の where にしか現れず、set には現れない(数え方: git grep -n 'idHash' -- packages apps)。

テストを除くと、この関数を呼ぶのは apps/web/server/routes/verify-email/confirm.get.ts だけである(数え方: git grep -n 'attachUserToSession' -- packages appspackages/api/src/auth/session.ts の定義と packages/api/src/auth/identity.test.ts を除くと、confirm.get.ts の import 行と呼び出し行だけが残る)。呼ばれる条件は、クッキーのセッションが存在し、そのセッションが traQ の ID を持ち、user_id を持たず、resolveUserForEmailVerifyconflict を返していない場合である。この条件を満たすセッションは、呼び出しの前は packages/api/src/auth/context.tsrequireUserUNAUTHORIZED を投げる状態であり、呼び出しの後は requireUser を通る状態になる。requireUsersession.userIdsession.mailHash の両方を必要とし、resolveSessionusers を LEFT JOIN して mailHash を補うためである。つまり、セッションの権限が広がるのに、クッキーに入っているトークンはそのまま据え置かれる。そのセッションから到達できるプロシージャが増える。増える範囲は packages/api/src/orpc.tsuserProc を使ったプロシージャで、2026-09-18 時点では packages/api/src/membership/router.tsissueInvoice である(数え方: git grep -n 'userProc' -- packages/api/src)。

セッション ID が据え置かれるのは attachUserToSession を通る経路だけである。テストを除くと createSession を呼ぶのは 3 箇所で、apps/web/server/routes/login/callback.get.ts(traQ OAuth)、apps/web/server/routes/verify-email/confirm.get.ts(上記の条件を満たさない側の分岐)、apps/web/server/routes/dev/login.get.ts である(数え方: git grep -n 'createSession' -- packages appspackages/api/src/auth/session.ts の定義と packages/api/src/auth/identity.test.ts を除くと、この 3 ファイルの import 行と呼び出し行だけが残る)。いずれも新しいトークンを発行してクッキーを置き換えるので、セッション ID は入れ替わる。

このうち dev/login.get.ts は、import.meta.dev が真で、かつ環境変数 CHECKIN_DEV_LOGIN1 でなければ 404 を返す開発専用の経路である。同ファイルの doc comment は、import.meta.dev がビルド時にリテラルの false に置き換わるので本番のバンドルでは 404 に短絡すると書いている。この短絡そのものは実行して確かめていない。この経路をここに数えているのは、セッション ID を据え置くのが attachUserToSession だけであることを示すためである。下の「取りうる手」が対象とする経路からは外す。本番で到達しない前提だからである。

あわせて、新しいセッションを発行する経路は、それまで使っていたセッションのレコードを消していない。テストを除くと、destroySession を呼ぶのは apps/web/server/routes/logout.post.ts だけである(数え方: git grep -n 'destroySession' -- packages appspackages/api/src/auth/session.ts の定義と packages/api/src/auth/identity.test.ts を除くと、logout.post.ts の import 行と呼び出し行だけが残る)。上に挙げた createSession の 3 箇所はどれも、クッキーに入っていた前のトークンを無効にしない。そのトークンは sessions.expires_at に達するまで有効なままである。packages/db/src/schema.tssessionsid_hash に一意制約を持ち、expires_at を持ち、他に有効・無効を表す列を持たない。resolveSession はレコードが存在して expires_at が未来であれば SessionIdentity を返す。

ここまでに挙げたコードの事実は、2026-09-18 に main、HEAD=74ee63c の作業ツリーで確かめた。

OWASP Session Management Cheat Sheet(https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html 、2026-09-18 に参照)の「Renew the Session ID After Any Privilege Level Change」の節は、セッション内で権限の水準が変わったらセッション ID を必ず再発行しなければならないこと、未認証(匿名)の状態から認証済みの状態へ変わる認証処理がその最も一般的な場面であることを述べている。古いセッション ID を破棄しなければならないという要求は、同じ節で For all sensitive pages of the web application という条件の下に置かれているので、無条件の要求としては引かない。同じ節は、再発行が必須である理由を session fixation attacks の防止だと書いている。IPA の「安全なウェブサイトの作り方 - 1.4 セッション管理の不備」(https://www.ipa.go.jp/security/vuln/websecurity/session-management.html 、2026-09-19 に参照)が「セッションIDの固定化(Session Fixation)」と呼んでいるものである。攻撃者が被害者のセッション ID を盗むのではなく、攻撃者が用意した ID を被害者のブラウザーに設定させ、被害者がログインして権限が広がった後もその ID が使い続けられることを利用する攻撃である。この攻撃が成立するには、攻撃者が先にクッキー __Host-checkin_session を被害者のブラウザーへ書き込めていなければならない__Host- 接頭辞が課す条件は、draft-ietf-httpbis-rfc6265bis-22 の「The "__Host-" Prefix」の節(https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis-22 、2026-09-19 に参照)のとおり Secure 属性・値が /Path 属性・Domain 属性が無いこと の 3 つで、HttpOnly はこの条件に含まれない。サーバー側は buildRequestContext がこのクッキーを getCookie で読むだけなので、書き込まれたクッキーに HttpOnly が付いているかは検査していない。セッション ID を再発行しないことだけで成立するものではない。再発行は、その書き込みができてしまった場合に権限の広がりが引き継がれないようにするための防御である。

今の実装は、権限が広がる attachUserToSession の経路でセッション ID を再発行しておらず、セッションを新しく発行する経路でも古いセッションを破棄していない。

誰が何を誤るか

attachUserToSession の doc comment は「Attach an isct user to an EXISTING session (keeping the same token/cookie)」と書いている。この関数を読む人は、トークンを据え置くことが意図した設計であると読み、セッション ID を入れ替える必要がないと判断する。実際には、この関数が呼ばれる瞬間がセッションの権限が広がる瞬間であり、セッション ID の再発行が求められるのはまさにその瞬間である。

セッションを捨てる経路がログアウトしかないことも同様に誤りやすい。ログインし直しても、それ以前に発行されたトークンは期限まで有効なまま残る。セッションの一覧や失効の手段を作る人が、新しいセッションの発行が古いセッションの無効化を伴うと読むと、実際とは違う前提で作ることになる。

取りうる手

  • attachUserToSession を、user_id の UPDATE と同時に id_hash を新しいトークンのハッシュへ入れ替える形にし、新しいトークンを返す。呼び出し側はそれをクッキーに設定する。sessionsid_hash に一意制約があるので、新しいトークンのハッシュが既存の行と衝突した場合の扱いを決める必要がある。
  • attachUserToSession を使わず、その経路でも新しいセッションを発行し、直前のセッションを destroySession で消す。createSession の引数 NewSessiontraqIdisAdminuserId の 3 つを受け取り、packages/db/src/schema.tssessions にも is_admin 列があるので、デュアル・アイデンティティを持つセッションはこの形でも作れる。ただし引き継ぐ値を 3 つとも渡さないと権限が落ちることがある。会計担当のセッションで isAdmin を渡し忘れると、createSession の既定が false なので requireAdmin が通らなくなる。これは trustForwardAuth が偽のときに限る。真のときは apps/web/server/utils/auth.tsbuildRequestContextapplyForwardedIdentity を通し、そこで isAdminaccountantTraqIds.includes(forwardedTraqId) として計算し直されるので、セッションの is_admin 列の値は使われない。attachUserToSession を残すかどうかは、この案を採るかで決まる。
  • セッションを新しく発行するすべての経路で、クッキーに入っていた前のトークンを destroySession で消す。上の 2 つのどちらを採る場合でも要る。

#74 と併せて採る場合の注意: #74 の「取りうる手」の 1 つ目は、CSRF トークンをセッション ID を含む値の HMAC にする案である。両方を採ると、セッション ID を入れ替える側が CSRF トークンの発行し直しも行う必要がある。行わないと、入れ替えの直後は古いセッション ID で作られた CSRF トークンが送られるので、検査が通らなくなる。先に紐付けだけを入れて、後からこの issue の入れ替えを足すときに起きやすい。

直さずに残した理由

技術選定の見直しで認証・セッションの基盤の作り直しを決めており、その結論は #72 にある。セッション ID の再発行と古いセッションの破棄は、セッションの発行・保存・失効の仕組みそのものに手を入れる変更なので、その作り直しと同じ範囲に入る。基盤が入れ替わる前にこの 3 つの関数だけを書き換えると、入れ替えの時点で同じ箇所をもう一度書き換えることになるため、単独では直さずに残した。

ただし、基盤を better-auth に差し替えるだけでは、この issue の症状は解消しない。1.7 系のドキュメントの一覧 https://better-auth.com/docs/llms.txt が挙げる 175 ページ(2026-09-19 に取得)には、権限の水準が変わったときにセッション識別子を再発行する動作の記述が無く、セッションの token を入れ替える手段の記述も無い。そのうちの「Session Management」(https://better-auth.com/docs/concepts/session-management 、2026-09-19 に参照)は、追加フィールドを更新する updateSession について次のように書いている。

Core session fields (token, userId, expiresAt, createdAt, updatedAt, ipAddress, userAgent) cannot be updated through this endpoint. Only custom additional fields are allowed.

セッション ID が据え置かれる側が解消するかどうかは、移行の設計でこの段階をどう表すかで決まる。ここでいう段階とは、traQ のセッションに isct のメール確認を後から結び付ける段階、すなわち今 attachUserToSession が担っている段階である。

  • 新しいサインインとして表す場合。マジックリンクのサインインを通すと better-auth が新しいセッションを作るので、識別子が据え置かれる側は起きなくなる。
  • 同じセッションの追加フィールドの更新として表す場合。上の引用のとおり token は更新できないので、症状がそのまま残る

古いセッションを破棄しない側は、どちらを採っても自動では直らない。失効させる手段は用意されている(revokeSessionrevokeOtherSessionsrevokeSessionsadmin プラグインの revokeUserSession)が、サインインのときにそれが自動で走るとは、公式ドキュメントのどのページにも書かれていない。自動の失効として書かれているものは、いずれもサインイン以外の契機に結び付いている。changePasswordrevokeOtherSessions: true を渡したパスワードの変更、revokeSessionsOnPasswordReset を有効にしたパスワードの再設定、admin プラグインの banUser による利用の停止、magicLink プラグインと emailOTP プラグインがメールを確認していない既存アカウントを検証した場合、multiSession プラグインでのサインアウトである(数え方: 上記の一覧が挙げる 175 ページの .md を取得し、取得先で grep -ril revoke が挙げるファイルの該当行を読む)。ここに挙げたのは公式ドキュメントの記述であって、実際に動かして確かめてはいない。

したがって、この issue は基盤の差し替えによって自動的に閉じるものではない。移行の設計で、上の段階をどちらの形で表すかを決め、そのうえで古いセッションの破棄を明示的に書くかどうかを決める必要がある。

スコープ外/関連

  • 認証・セッションの基盤をどう作るかの結論は #72。上の「直さずに残した理由」のとおり、その結論を採ってもこの issue の症状は自動では解消しない。この issue は、移行の設計で決める必要がある事項と、その結論を採らない場合に個別に直す対象の両方を記録するものである。
  • #16 は認証サブシステムを実装するための issue である。この issue が扱うのは、実装済みの attachUserToSessioncreateSessiondestroySession が現に行っている振る舞いなので、別の事項である。
  • 認可とレート制限は #15。この issue は誰に何を許すかではなく、セッション ID をいつ入れ替えるかを扱う。
  • #74 は CSRF トークンが何に紐付いているかの issue である。別の事項だが、両方を採る場合は実装が絡む。上の「取りうる手」の末尾に書いた。

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read packages/api/src/auth/session.ts, packages/api/src/auth/context.ts, and the callers in apps/web/server/routes/verify-email/confirm.get.ts, login/callback.get.ts, and logout.post.ts. Review #72 before choosing an implementation, since the issue identifies a session-foundation redesign as related work. Done means the migration or retained implementation explicitly handles session-ID renewal on privilege changes, old-session invalidation, and any CSRF-token coordination with #74.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
authentication, backend, security
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
28/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.