fix(auth): restore sign-in recovery and safe return paths - #969
fix(auth): restore sign-in recovery and safe return paths#969seonghobae wants to merge 7 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (10)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughOIDC 반환 URL의 안전성 검증과 콜백 파라미터 정제를 강화했습니다. 인증 실패와 토큰 누락을 로그인 재시도 화면으로 통합했습니다. 관련 테스트, Storybook 기록, ADR 및 기술 기준 문서를 갱신했습니다. ChangesOIDC 로그인 복구
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to Failed sign-ins now offer a safe retry while preserving sanitized in-app destinations and avoiding exposure of callback error details. The implemented recovery and return-path behavior has focused coverage, with no remaining concrete merge-blocking risk. Sequence Diagram(s)sequenceDiagram
participant App
participant StatusNotice
participant restoreOidcReturnUrl
participant signinRedirect
App->>StatusNotice: 인증 실패 상태 표시
StatusNotice->>App: Log in 액션 실행
App->>restoreOidcReturnUrl: 정제된 목적지 복원
App->>signinRedirect: 복원된 목적지로 OIDC 재시도
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 5 files. (5 skipped: 5 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
|
DESIGN ASSURANCE — exact head The retry/return-path repair has substantive current-head evidence: frontend and backend suites and CodeQL completed successfully, and the patch preserves the authoritative OIDC destination while removing callback artifacts. I found no new causal defect in that flow. The material sign-in surface is still not merge-ready under the current acceptance matrix. Owner acceptance: retain domain/auth truth outside presentation state; supply ko/en/ja/zh/vi/es/de/fr screen-scoped versioned resources; prove retry/loading/error/offline/permission and successful-auth transitions in a real browser at desktop/intermediate/mobile widths with pointer, keyboard, visible focus, screen-reader role/name/state, reduced-motion and destination persistence/reload; then obtain terminal exact-head security/review gates. I left the dedicated writer’s source and refs unchanged. |
A failed sign-in previously replaced the page with the authentication error and removed the login action. This change keeps a bounded retry notice in the existing login card, preserves the intended post destination, removes callback artifacts from stored/restored/shared paths, and rejects paths that URL parsing resolves to another origin. The login card now includes padding in its width so its edges fit narrow screens.
Reuses StatusNotice, current translated strings, the shared return-path helper, and the existing OIDC redirect. ADR 0109/0220 and the dated gap baseline record the behavior and owner boundaries. Product-owned login/signup/recovery forms still require released Keyverse contracts.
Validation on exact head
583157c5d007d4903a78b2e30c0caac224f6460e:34073159430are terminal GREEN: frontend lint/full tests/production build/Storybook and the PostgreSQL-backed full backend suite passed on the exact head.34073140374and Security34073140368are terminal GREEN on the exact head.34073140303is terminal failure/fail-closed; it is not relabeled as source GREEN.Material UI delivery is therefore not complete. Intentionality/content fit are supported by the existing login-card composition and bounded retry copy; narrow-screen and keyboard recovery have browser evidence. Eight-locale DB-ledger delivery, full permission/error/recovery browser coverage against released Keyverse contracts, production authentication, and final independent evidence remain outstanding.
Keep Ready for Review but do not merge. Do not substitute hosted Tests/SAST/Security success, local browser evidence, historical review, synthetic status, or no-op head churn for the missing required CodeQL verdict and independent approval. No deployment, all-page p95 claim, or protected release is claimed.