feat(web): config-driven login auth options - #177
Conversation
266ea7d to
03624d2
Compare
hojinzs
left a comment
There was a problem hiding this comment.
리뷰 요약 — Issue #167 요구사항 검증
요구사항 반영 (issue #167)
전 항목 반영 확인, diff와 1:1 대조 완료:
- Auth-config adapter —
api.getAuthConfig()(packages/web/src/lib/api.ts), 로그인 라우트에서 로드 전까지 컨트롤을 아예 렌더링하지 않음 (showAuthControls = !authConfigQuery.isLoading) → 폼 깜빡임 없음 - 3가지 상태 — local+OIDC(폼+divider+SSO), OIDC-only(폼 없음), local-only(기존 UI 무변경, e2e로 회귀 방지) 모두 구현·테스트됨
- Config fetch 실패 폴백 —
authConfigQuery.isError일 때 local 폼으로 폴백 + 인라인 경고 문구, 백엔드 전체 장애 시 lockout 방지 - SSO 진입점 —
fetch가 아닌 순수<a href>전체 페이지 네비게이션(getOidcStartHref),callbackUrl보존 확인 - 에러 표면 —
getOidcLoginErrorMessage()가oidc_failed/oidc_forbidden만 화이트리스트 매핑, 그 외 문자열은null→ 원본 에러 노출 없음. 백엔드(packages/api/src/routes/auth.ts,lib/auth/oidc.ts)의 실제 리다이렉트 경로(/login?error=oidc_failed,/login?error=oidc_forbidden)와 프런트 매핑이 정확히 일치 /api/auth/config계약 — 백엔드{ local, oidc: { enabled, label } }응답과AuthConfig타입이 정확히 일치 (WP2 계약 그대로 소비)- 리다이렉트 가드(
router.tsx:45)·로그아웃 경로 비변경 확인
과잉설계 여부: LoginForm/SsoButton 분리는 조건부 렌더링 가독성을 위한 최소 리팩터로 범위 내. 임의의 별도 결정사항 없음.
Smoke Test
Preview 환경(evidence-browser-pr-177.dev0.daniworks.kr)은 이 세션의 네트워크 egress 정책상 접근 불가(EGRESS_BLOCKED)하여, PR 브랜치를 별도 워크트리에 체크아웃해 로컬/e2e로 대체 수행:
npm -w @evidence-browser/web run test→ 29 passednpm -w @evidence-browser/web run typecheck→ cleannpm run build(전체 워크스페이스) → 성공npx playwright test e2e/login-auth-config.spec.ts(실제 빌드 산출물 + 실행 중인 API 서버 대상) → 7/7 passed (loading 상태, local-only 무변경, local+OIDC, OIDC-only, SSO 네비게이션+callbackUrl, 안전한 에러 문구, config 실패 폴백)- CI의 9개 체크(build/lint/test×4/coverage/changeset/e2e) 전부 success 확인
코드 품질 리뷰
- P2 —
.changeset/quiet-login-sso.md가@evidence-browser/api를 대상으로 하지만 실제 변경분은 전부packages/web/**. 인라인 코멘트 참고 (릴리스 changelog 부정확 위험, 기능에는 영향 없음) - 그 외 P1/P3/nit 없음.
SsoButton의 스타일(text-[13px], hover 토큰 등)은 기존Button/Card 컴포넌트 규격과 정확히 일치 —docs/DESIGN_GUIDE.md위반 없음
이전 리뷰 이력
없음 (이번이 첫 리뷰 라운드)
최종 의견: Approve
기능/보안/설계 상 블로커 없음. P2 changeset 대상 패키지 수정은 병합 전이든 후속 커밋이든 작성자 판단에 맡김.
Generated by Claude Code
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| "@evidence-browser/api": minor | |||
There was a problem hiding this comment.
P2 — changeset targets the wrong package.
This changeset bumps @evidence-browser/api": minor, but every file this PR touches is under packages/web/** (router.tsx, lib/api.ts, lib/types.ts, tests, e2e spec). packages/api has no changes here — the /api/auth/config route itself already shipped in #175 (bright-rivers-serve.md, correctly scoped to @evidence-browser/api).
As written, changeset version will bump @evidence-browser/api with an empty diff and give @evidence-browser/web — the package that actually changed — no version bump or CHANGELOG entry. Should be:
---
"@evidence-browser/web": minor
---Non-blocking for functionality, but worth a one-line fix before/at merge so the release changelog stays accurate.
Generated by Claude Code
There was a problem hiding this comment.
Fixed in 89d85e9: the changeset now targets @evidence-browser/web and uses patch, matching the issue's changeset:patch label.
Issues — Closed #167
TL;DR
/api/auth/configbefore rendering auth controls, so deployments can expose local login, OIDC SSO, or both.<a href>to/api/auth/oidc/startwith the currentcallbackUrlpreserved./login?error=oidc_failed|oidc_forbiddennow shows safe user-facing copy instead of raw error detail.@evidence-browser/web: patchfor issue [E10·WP3] Login page: /api/auth/config-driven SSO button and conditional local form #167.변경 지점 다이어그램
/api/auth/config→api.getAuthConfig()→LoginPagequery → conditional local form / divider / SSO link/login?error=...→validateSearch→ safe OIDC error mapper → existing destructive text stylingpackages/web/**change →.changeset/quiet-login-sso.md→@evidence-browser/webpatch release entry여기부터 보세요
packages/web/src/router.tsx— login search parsing, config query, conditional form/SSO rendering, OIDC-only helper copy, OIDC error copy.packages/web/src/lib/api.tsandpackages/web/src/lib/types.ts— auth config adapter and type.packages/web/e2e/login-auth-config.spec.ts— Playwright coverage for loading, local-only, local+OIDC, OIDC-only, SSO navigation, safe error copy, and config failure fallback..changeset/quiet-login-sso.md— patch changeset for the web login behavior.Evidence
npm run lint— passnpm test— passnpm run build— passnpm -w @evidence-browser/web run e2e -- e2e/login-auth-config.spec.ts— pass, 7 tests89d85e91ca3de2b2f94ff018e9795a79ee3f3035— pass: build, changeset, coverage, e2e, lint, test (api), test (cli), test (shared), test (web)위험 & 롤백
/api/auth/config; fallback keeps local login visible on config fetch failure./api/auth/oidc/startroute being configured; the UI performs a normal navigation rather than fetch-based flow control.변경 파일
.changeset/quiet-login-sso.mdpackages/web/src/lib/api.tspackages/web/src/lib/types.tspackages/web/src/router.tsxpackages/web/src/lib/api.test.tspackages/web/src/router.test.tsxpackages/web/e2e/login-auth-config.spec.ts머지 후/사람 확인