From 9ceaaf1bf8e0f72b0b0d5541d580c9c97031019b Mon Sep 17 00:00:00 2001 From: turtton Date: Mon, 14 Sep 2026 23:19:12 +0900 Subject: [PATCH] =?UTF-8?q?fix(booskiff-web):=20real=20=E3=83=AD=E3=82=B0?= =?UTF-8?q?=E3=82=A4=E3=83=B3=E6=88=90=E5=8A=9F=E5=BE=8C=E3=81=AB=20Hydra?= =?UTF-8?q?=20OAuth=20=E9=96=8B=E5=A7=8B=E3=81=BE=E3=81=A7=20UI=20?= =?UTF-8?q?=E5=B0=8E=E7=B7=9A=E3=82=92=E8=87=AA=E5=8B=95=E5=AE=8C=E7=B5=90?= =?UTF-8?q?=20(#20)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- apps/booskiff-web/README.md | 14 ++-- apps/booskiff-web/bff/auth-real.test.ts | 79 +++++++++++++++++++ apps/booskiff-web/bff/auth-real.ts | 4 +- apps/booskiff-web/e2e/real/tests/auth.spec.ts | 20 +++-- apps/booskiff-web/spago.lock | 3 +- apps/booskiff-web/spago.yaml | 1 + apps/booskiff-web/src/App/Api/Auth.purs | 8 +- apps/booskiff-web/src/Client/Update.purs | 17 +++- 8 files changed, 123 insertions(+), 23 deletions(-) create mode 100644 apps/booskiff-web/bff/auth-real.test.ts diff --git a/apps/booskiff-web/README.md b/apps/booskiff-web/README.md index 9535113..40266ac 100644 --- a/apps/booskiff-web/README.md +++ b/apps/booskiff-web/README.md @@ -109,13 +109,13 @@ NixOS では `PLAYWRIGHT_CHROMIUM_PATH=$(command -v chromium)` を指定でき 負の系は Hydra 同意拒否、state 不一致、実 token endpoint の不正 code 拒否を 検証します。署名不正等は既存 `bff/auth-oidc.test.ts` の担当です。 -**検出した既存不具合と暫定措置:** 実検証で [#17](https://github.com/ShuttlePub/shuttlepub-frontends/issues/17) -（UI が Kratos 認証後に OAuth を開始しない）と [#19](https://github.com/ShuttlePub/shuttlepub-frontends/issues/19) -（認証済み `/drive` のフルロードで resume DOM が破損する）を検出しました。認証コードは -変更せず、E2E は #17 に対して `/auth/oauth/start` へ明示遷移し、#19 に対して -callback の `return_to` を `/login` として SPA 遷移で回避しています（永続化は API で検証）。 -そのため、このスイートは途切れない UI ログイン導線と `/drive` 直接ロードの完成は保証しません。 -両 issue の修正後に workaround を外して再検証してください。 +**検出した既存不具合と暫定措置:** [#17](https://github.com/ShuttlePub/shuttlepub-frontends/issues/17) / +[#20](https://github.com/ShuttlePub/shuttlepub-frontends/issues/20) のログイン導線は修正済みです。 +ログイン成功後に BFF が `next: /auth/oauth/start?return_to=/login` を返し、UI が +トップレベル遷移で OAuth を開始するため、E2E の `/auth/oauth/start` 明示遷移は解除済みです。 +[#19](https://github.com/ShuttlePub/shuttlepub-frontends/issues/19)（認証済み `/drive` の +フルロード時の resume DOM 破損）は未修正のため、`return_to=/login` の SPA 経路は +BFF 側で維持し、永続化は引き続き API で検証します。 CI は mock/real を別 matrix job で実行し、両方 green です。 ## 構成 diff --git a/apps/booskiff-web/bff/auth-real.test.ts b/apps/booskiff-web/bff/auth-real.test.ts new file mode 100644 index 0000000..cf14ab3 --- /dev/null +++ b/apps/booskiff-web/bff/auth-real.test.ts @@ -0,0 +1,79 @@ +import "./test-setup.ts"; +import { afterAll, expect, test } from "bun:test"; +import { handleAuthRequest, type RealAuthConfig } from "./routes.ts"; +import { createTestSessionAdapter } from "./session-test-adapter.ts"; +import { TEST_COOKIE_SECRET_BASE64 } from "./test-setup.ts"; + +// ============================================================ +// Kratos ログイン用スタブ +// ============================================================ + +let credentialsAccepted = true; +const server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url); + if (req.method === "GET" && url.pathname === "/self-service/login/browser" && url.searchParams.get("refresh") === "true") { + return Response.json({ + id: "flow-1", ui: { nodes: [{ attributes: { name: "csrf_token", value: "csrf-token-1" } }] }, + }, { headers: { "Set-Cookie": "ory_kratos_flow=flow-cookie; Path=/; HttpOnly" } }); + } + if (req.method === "POST" && url.pathname === "/self-service/login" && url.searchParams.get("flow") === "flow-1") { + if (!credentialsAccepted) return Response.json({ + ui: { messages: [{ text: "The provided credentials are invalid" }] }, + }, { status: 400 }); + return Response.json({ + session: { identity: { traits: { email: "alice@example.test" } } }, + }, { headers: { "Set-Cookie": "ory_kratos_session=kratos-session-cookie; Path=/; HttpOnly" } }); + } + return new Response(null, { status: 404 }); + }, +}); +afterAll(() => server.stop(true)); +const config: RealAuthConfig = { + kind: "real", kratosPublicUrl: server.url.origin, hydraPublicUrl: "http://hydra.test", hydraClientId: "client", + hydraClientSecret: "secret", hydraRedirectUri: "http://localhost:3000/auth/callback", + hydraScopes: "openid", hydraAudience: "account", oauthStateTtlSeconds: 300, +}; +const adapter = createTestSessionAdapter({ + cookieSecretBase64: TEST_COOKIE_SECRET_BASE64, sessionCookieName: "booskiff_session", + isSecureOrigin: false, hydraPublicUrl: "http://hydra.test", hydraClientId: "client", hydraClientSecret: "secret", refreshSkewSeconds: 60, +}); + +function loginRequest(identifier: string, password: string): Request { + return new Request("http://localhost:3000/auth/login", { + method: "POST", + headers: { "Content-Type": "application/json", Origin: "http://localhost:3000" }, + body: JSON.stringify({ identifier, password }), + }); +} + +// ============================================================ +// real ログインの OAuth ハンドオフ +// ============================================================ + +test("returns OAuth next URL and Kratos cookies without an app session when login succeeds", async () => { + // Given: Kratos が credential を受理する。 + credentialsAccepted = true; + // When: ブラウザが BFF にログインする。 + const response = await handleAuthRequest(loginRequest("alice@example.test", "password"), { adapter, mode: config }); + // Then: OAuth 開始 URL と Kratos cookie のみを返す。 + expect(response?.status).toBe(200); + expect(await response?.json()).toEqual({ + authenticated: true, username: "alice@example.test", next: "/auth/oauth/start?return_to=/login", + }); + expect(response?.headers.getSetCookie().some((cookie) => cookie.startsWith("ory_kratos_session="))).toBe(true); + expect(response?.headers.getSetCookie().some((cookie) => cookie.startsWith("booskiff_session="))).toBe(false); +}); + +test("returns an error without next when Kratos rejects credentials", async () => { + // Given: Kratos が credential を拒否する。 + credentialsAccepted = false; + // When: ブラウザが BFF にログインする。 + const response = await handleAuthRequest(loginRequest("alice@example.test", "wrong-password"), { adapter, mode: config }); + // Then: OAuth に進まず、認証エラーを返す。 + expect(response?.status).toBe(401); + const body: unknown = await response?.json(); + expect(body).toEqual({ error: "The provided credentials are invalid" }); + expect(body).not.toHaveProperty("next"); +}); diff --git a/apps/booskiff-web/bff/auth-real.ts b/apps/booskiff-web/bff/auth-real.ts index 742439e..b6bd808 100644 --- a/apps/booskiff-web/bff/auth-real.ts +++ b/apps/booskiff-web/bff/auth-real.ts @@ -111,7 +111,9 @@ export async function realLogin(req: Request, config: RealAuthConfig, adapter: S const headers = new Headers({ "Content-Type": "application/json" }); jar.applyToResponse(headers); - return new Response(JSON.stringify({ authenticated: true, username: email }), { status: 200, headers }); + // XHR は 302 で cross-origin Hydra chain を進められないため、トップレベル遷移用 URL を渡す。 + // return_to=/login は #19 の SPA 経路回避策。修正後に /drive へ戻すことを再検討する。 + return new Response(JSON.stringify({ authenticated: true, username: email, next: "/auth/oauth/start?return_to=/login" }), { status: 200, headers }); } catch (err) { console.error("Kratos login error:", err); return Response.json({ error: "Authentication service unavailable" }, { status: 502 }); diff --git a/apps/booskiff-web/e2e/real/tests/auth.spec.ts b/apps/booskiff-web/e2e/real/tests/auth.spec.ts index 1824402..1b5f5a1 100644 --- a/apps/booskiff-web/e2e/real/tests/auth.spec.ts +++ b/apps/booskiff-web/e2e/real/tests/auth.spec.ts @@ -5,16 +5,22 @@ async function enterCredentials(page: Page): Promise { await page.goto("/login"); await page.getByTestId("login-identifier").fill("testuser@example.com"); await page.getByTestId("login-password").fill("testuser"); + // The login form is hydrated client-side; a click fired before hydration is + // inert, so retry click+wait until the login XHR actually fires. await expect(async () => { await page.getByTestId("login-submit").click(); - await expect(page).toHaveURL(/\/drive$/, { timeout: 5_000 }); + await page.waitForResponse( + (response) => new URL(response.url()).pathname === "/auth/login" && response.request().method() === "POST", + { timeout: 5_000 }, + ); }).toPass({ timeout: 20_000 }); - // The current UI omits this real-mode navigation; tracked separately, not patched here. - // return_to=/login: a direct full load of /drive after the OAuth callback corrupts - // the resumed DOM (duplicated folder section, dead handlers); landing on /login and - // letting the session check navigate client-side follows the healthy SPA path. - await page.goto("/auth/oauth/start?return_to=/login"); - await expect(page.getByRole("button", { name: "Allow", exact: true })).toBeVisible(); + // Real-mode login success hands off to /auth/oauth/start automatically (the + // BFF `next` URL), so the Hydra consent screen arrives through the UI flow + // alone. return_to=/login is applied BFF-side: a direct full load of /drive + // after the OAuth callback corrupts the resumed DOM (issue #19), so landing + // on /login and letting the session check navigate client-side follows the + // healthy SPA path. + await expect(page.getByRole("button", { name: "Allow", exact: true })).toBeVisible({ timeout: 30_000 }); } async function expectSignedOut(page: Page): Promise { diff --git a/apps/booskiff-web/spago.lock b/apps/booskiff-web/spago.lock index 80e4c51..f507676 100644 --- a/apps/booskiff-web/spago.lock +++ b/apps/booskiff-web/spago.lock @@ -30,7 +30,8 @@ "shuttlepub-ui", "strings", "tuples", - "web-events" + "web-events", + "web-html" ] }, "test": { diff --git a/apps/booskiff-web/spago.yaml b/apps/booskiff-web/spago.yaml index fade807..6611f82 100644 --- a/apps/booskiff-web/spago.yaml +++ b/apps/booskiff-web/spago.yaml @@ -34,6 +34,7 @@ package: - strings - tuples - web-events + - web-html test: main: Test.Main dependencies: [] diff --git a/apps/booskiff-web/src/App/Api/Auth.purs b/apps/booskiff-web/src/App/Api/Auth.purs index f663f2a..1746467 100644 --- a/apps/booskiff-web/src/App/Api/Auth.purs +++ b/apps/booskiff-web/src/App/Api/Auth.purs @@ -4,8 +4,9 @@ import Prelude import App.Api.Client as Api import Data.Argonaut.Decode.Class (class DecodeJson, decodeJson) -import Data.Argonaut.Decode.Combinators ((.:)) +import Data.Argonaut.Decode.Combinators ((.:), (.:?)) import Data.Either (Either) +import Data.Maybe (Maybe) import Effect.Aff (Aff) -- | POST /auth/login body (identifier = email or username) @@ -15,14 +16,15 @@ type LoginRequest = } -- | Login response from BFF -newtype LoginResponse = LoginResponse { authenticated :: Boolean, username :: String } +newtype LoginResponse = LoginResponse { authenticated :: Boolean, username :: String, next :: Maybe String } instance DecodeJson LoginResponse where decodeJson json = do obj <- decodeJson json authenticated <- obj .: "authenticated" username <- obj .: "username" - pure (LoginResponse { authenticated, username }) + next <- obj .:? "next" + pure (LoginResponse { authenticated, username, next }) -- | Session response from BFF (GET /auth/session) newtype SessionResponse = SessionResponse { authenticated :: Boolean, username :: String } diff --git a/apps/booskiff-web/src/Client/Update.purs b/apps/booskiff-web/src/Client/Update.purs index bea177b..c5e5d8b 100644 --- a/apps/booskiff-web/src/Client/Update.purs +++ b/apps/booskiff-web/src/Client/Update.purs @@ -26,6 +26,9 @@ import Flame (Update, noMessages) import Foreign (unsafeToForeign) import Routing.Duplex (print) import Routing.PushState (PushStateInterface) +import Web.HTML (window) +import Web.HTML.Location as Location +import Web.HTML.Window (location) -- | File input the FFI upload flow reads from (see Client.Upload and the -- | Drive view owned by the follow-up UX task). @@ -311,10 +314,16 @@ checkSessionAff = do submitLoginAff :: String -> String -> Aff (Maybe Message) submitLoginAff identifier password = do result <- Auth.login { identifier, password } - pure $ Just $ case result of - Right (LoginResponse r) | r.authenticated -> SessionChecked (Just r.username) - Right _ -> LoginFailed "Login failed: invalid credentials" - Left err -> LoginFailed err + case result of + Right (LoginResponse r) | r.authenticated -> case r.next of + Just url -> do + -- Real mode hands off to /auth/oauth/start via full-page navigation: + -- XHR cannot drive the cross-origin Hydra redirect chain. + liftEffect (window >>= location >>= Location.assign url) + pure Nothing + Nothing -> pure $ Just $ SessionChecked (Just r.username) + Right _ -> pure $ Just $ LoginFailed "Login failed: invalid credentials" + Left err -> pure $ Just $ LoginFailed err logoutAff :: Aff (Maybe Message) logoutAff = do