[Feat] #77 - 인증 흐름 및 프론트 구조 정리#96
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR restructures the frontend codebase from a feature-based architecture (views/widgets/features/entities) to a screen-centric layering model with dedicated API and business-logic layers. It adds backend authentication integration with Google OAuth profile fetching, creates new route modules for login and home pages, updates environment configuration for production Firebase Hosting with API rewrites, and refactors shared UI components and Storybook stories throughout. Changes
Sequence DiagramsequenceDiagram
participant User
participant Browser as Browser/React
participant Firebase as Firebase Auth
participant GoogleAPI as Google People API
participant Backend as Backend API
User->>Browser: Click "Sign in with Google"
activate Browser
Browser->>Firebase: signInWithPopup(GoogleAuthProvider)
activate Firebase
Firebase->>GoogleAPI: OAuth flow
GoogleAPI-->>Firebase: accessToken + credential
Firefox-->>Browser: credential.accessToken
deactivate Firebase
Browser->>GoogleAPI: fetchGoogleProfile(accessToken)
activate GoogleAPI
GoogleAPI-->>Browser: name, organization, metadata
deactivate GoogleAPI
Note over Browser: Infer role from org text<br/>("Student" / "Professor" / error)
Browser->>Firebase: getIdToken(user)
activate Firebase
Firebase-->>Browser: idToken
deactivate Firebase
Browser->>Backend: loginWithBackend(idToken, profile)
activate Backend
Backend-->>Browser: BackendUser {uid, role, ...}
deactivate Backend
Note over Browser: Session created<br/>Redirect to /home
Browser-->>User: Logged in
deactivate Browser
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
4 tasks
sebeeeen
force-pushed
the
feat/auth-flow-frontend-cleanup
branch
from
bf4c955 to
f782f79
Compare
sebeeeen
changed the title
로그인 실패 정리, 역할 추론 실패 처리, 정적 자산과 잠금파일을 정리했습니다. Confidence: high Scope-risk: moderate Tested: install; lint; tsc; build; storybook build; 정적 화면 확인 Not-tested: 실제 OAuth 팝업 로그인
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.env.production (1)
3-8: ⚡ Quick winPrefer placeholders for production Firebase values in tracked env files.
Line 3–Line 8 currently commit concrete production identifiers/API key. Even for
NEXT_PUBLIC_*, keeping live values in VCS weakens rotation hygiene and increases blast radius if project restrictions drift.Please confirm this key is locked down in Google Cloud (HTTP referrer restrictions + API allowlist), and consider injecting real values via CI/CD secrets instead of committing them.
Suggested change
-NEXT_PUBLIC_FIREBASE_API_KEY=AIzaSyABxF3nO6fVtgk-RQEwE5eYK5GKEt8diEI -NEXT_PUBLIC_FIREBASE_AUTH_DOMAIN=ajou-project-cafd9.firebaseapp.com -NEXT_PUBLIC_FIREBASE_PROJECT_ID=ajou-project-cafd9 -NEXT_PUBLIC_FIREBASE_STORAGE_BUCKET=ajou-project-cafd9.firebasestorage.app -NEXT_PUBLIC_FIREBASE_MESSAGING_SENDER_ID=926871086070 -NEXT_PUBLIC_FIREBASE_APP_ID=1:926871086070:web:9a01bad8170cb172f15dd7 +NEXT_PUBLIC_FIREBASE_API_KEY= +NEXT_PUBLIC_FIREBASE_AUTH_DOMAIN= +NEXT_PUBLIC_FIREBASE_PROJECT_ID= +NEXT_PUBLIC_FIREBASE_STORAGE_BUCKET= +NEXT_PUBLIC_FIREBASE_MESSAGING_SENDER_ID= +NEXT_PUBLIC_FIREBASE_APP_ID=🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.production around lines 3 - 8, Replace the concrete Firebase production values in the tracked .env.production with placeholder entries (e.g. NEXT_PUBLIC_FIREBASE_API_KEY=__FIREBASE_API_KEY__) and remove any committed real secrets; specifically update NEXT_PUBLIC_FIREBASE_API_KEY, NEXT_PUBLIC_FIREBASE_AUTH_DOMAIN, NEXT_PUBLIC_FIREBASE_PROJECT_ID, NEXT_PUBLIC_FIREBASE_STORAGE_BUCKET, NEXT_PUBLIC_FIREBASE_MESSAGING_SENDER_ID, and NEXT_PUBLIC_FIREBASE_APP_ID to placeholder strings and configure your CI/CD pipeline to inject the real values from secret store at deploy time; after replacing, ensure the real API key is rotated/locked down (HTTP referrers + API allowlist) outside the repo and add a README note or comment indicating CI secret injection for these NEXT_PUBLIC_* variables.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/CONTRIBUTING.md`:
- Around line 165-167: The fenced code block containing the path string "app →
screens → lib → api → shared" lacks a language tag; update the opening fence
from ``` to ```text so the block reads ```text followed by the path and the
closing ``` to satisfy markdown lint (MD040).
In `@src/lib/auth/auth-service.ts`:
- Around line 74-82: After a successful signInWithEmailAndPassword in the
authService email login path, failures during credential.user.getIdToken or
createSession can leave the Firebase auth session active while backend session
creation fails; update the flow in the function that calls
signInWithEmailAndPassword / credential.user.getIdToken / createSession to catch
errors from getIdToken or createSession and perform cleanup by calling
signOut(auth) (or otherwise clearing the Firebase client session) before
rethrowing the error so no orphaned Firebase session remains; reference the
symbols signInWithEmailAndPassword, credential.user.getIdToken, createSession
and signOut to locate where to add the try/catch and cleanup logic.
In `@src/screens/login/company-login-form.tsx`:
- Around line 67-72: The CTA anchor currently uses href="#signup" which points
to a non-existent in-page anchor; update the element that contains the text "기업
계정이 없으신가요? 회원가입으로 이동하세요." to instead navigate to the real signup flow—either
replace href="#signup" with the correct signup route (e.g., "/signup") or
convert the anchor into a client-side navigation (e.g., using your router's Link
component or onClick handler like navigateToSignup) so the link actually opens
the signup page; ensure you update the <a ... href="#signup"> instance
accordingly.
---
Nitpick comments:
In @.env.production:
- Around line 3-8: Replace the concrete Firebase production values in the
tracked .env.production with placeholder entries (e.g.
NEXT_PUBLIC_FIREBASE_API_KEY=__FIREBASE_API_KEY__) and remove any committed real
secrets; specifically update NEXT_PUBLIC_FIREBASE_API_KEY,
NEXT_PUBLIC_FIREBASE_AUTH_DOMAIN, NEXT_PUBLIC_FIREBASE_PROJECT_ID,
NEXT_PUBLIC_FIREBASE_STORAGE_BUCKET, NEXT_PUBLIC_FIREBASE_MESSAGING_SENDER_ID,
and NEXT_PUBLIC_FIREBASE_APP_ID to placeholder strings and configure your CI/CD
pipeline to inject the real values from secret store at deploy time; after
replacing, ensure the real API key is rotated/locked down (HTTP referrers + API
allowlist) outside the repo and add a README note or comment indicating CI
secret injection for these NEXT_PUBLIC_* variables.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cea9d84d-b291-4b12-81fa-aca36b546a86
⛔ Files ignored due to path filters (4)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpublic/favicon.icois excluded by!**/*.icopublic/vite.svgis excluded by!**/*.svgsrc/assets/react.svgis excluded by!**/*.svg
📒 Files selected for processing (48)
.env.env.example.env.production.gitignoredocs/CONTRIBUTING.mdfirebase.jsonsrc/App.csssrc/api/auth/auth-api.tssrc/api/auth/google-profile-api.tssrc/api/auth/index.tssrc/app/(auth)/login/page.tsxsrc/app/home/page.tsxsrc/app/layout.tsxsrc/entities/index.tssrc/entities/user/index.tssrc/features/auth/index.tssrc/features/index.tssrc/lib/auth/auth-error-message.tssrc/lib/auth/auth-service.tssrc/lib/auth/index.tssrc/screens/home/home.stories.tsxsrc/screens/home/home.tsxsrc/screens/home/index.tssrc/screens/index.tssrc/screens/login/company-login-form.tsxsrc/screens/login/index.tsxsrc/screens/login/login.module.csssrc/screens/login/student-login-panel.tsxsrc/shared/ui/avatars/avatars.stories.tsxsrc/shared/ui/checkbox/checkbox.stories.tsxsrc/shared/ui/empty-states/empty-states.stories.tsxsrc/shared/ui/empty-states/empty-states.tsxsrc/shared/ui/file-uploader/file-uploader.stories.tsxsrc/shared/ui/form/form.stories.tsxsrc/shared/ui/index.tssrc/shared/ui/input/index.tssrc/shared/ui/input/index.tsxsrc/shared/ui/input/input.stories.tsxsrc/shared/ui/input/input.tsxsrc/shared/ui/lists/lists.stories.tsxsrc/shared/ui/modal/modal.stories.tsxsrc/shared/ui/modal/modal.tsxsrc/views/home/home.stories.tsxsrc/views/home/home.tsxsrc/views/login/index.tsxsrc/widgets/index.tstsconfig.app.jsontsconfig.json
💤 Files with no reviewable changes (11)
- src/widgets/index.ts
- src/features/index.ts
- src/App.css
- src/entities/index.ts
- src/shared/ui/input/index.tsx
- .env
- src/features/auth/index.ts
- src/entities/user/index.ts
- src/views/home/home.tsx
- src/views/home/home.stories.tsx
- src/views/login/index.tsx
Comment on lines
165
to
167
| ``` | ||
| app → views → widgets → features → entities → shared | ||
| app → screens → lib → api → shared | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language to the fenced code block to satisfy markdown lint.
The fence at Line 165 should declare a language (e.g., text) to avoid MD040 warnings.
Suggested fix
-```
+```text
app → screens → lib → api → shared</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 165-165: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @docs/CONTRIBUTING.md around lines 165 - 167, The fenced code block
containing the path string "app → screens → lib → api → shared" lacks a language
tag; update the opening fence from totext so the block reads text followed by the path and the closing to satisfy markdown lint (MD040).
</details>
<!-- fingerprinting:phantom:poseidon:hawk:6cf36a1a-0c8b-4fc2-93ba-4e43bb0f31d4 -->
<!-- d98c2f50 -->
<!-- This is an auto-generated comment by CodeRabbit -->
Comment on lines
+74
to
+82
| const credential = await signInWithEmailAndPassword(auth, email, password); | ||
| const idToken = await credential.user.getIdToken(); | ||
|
|
||
| return createSession(idToken, { | ||
| role: "COMPANY", | ||
| name: credential.user.displayName ?? credential.user.email ?? email, | ||
| department: "", | ||
| }); | ||
| }; |
Contributor
There was a problem hiding this comment.
Email login path misses cleanup on post-signin intermediate failure.
If Line 75 fails after Firebase auth succeeds, the Firebase session may remain while backend session creation never completes.
Proposed fix
export const signInWithEmail = async (
email: string,
password: string,
): Promise<SessionResponse> => {
- const credential = await signInWithEmailAndPassword(auth, email, password);
- const idToken = await credential.user.getIdToken();
-
- return createSession(idToken, {
- role: "COMPANY",
- name: credential.user.displayName ?? credential.user.email ?? email,
- department: "",
- });
+ const credential = await signInWithEmailAndPassword(auth, email, password);
+
+ try {
+ const idToken = await credential.user.getIdToken();
+ return await createSession(idToken, {
+ role: "COMPANY",
+ name: credential.user.displayName ?? credential.user.email ?? email,
+ department: "",
+ });
+ } catch (error) {
+ await firebaseSignOut(auth).catch(() => undefined);
+ throw error;
+ }
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/auth/auth-service.ts` around lines 74 - 82, After a successful
signInWithEmailAndPassword in the authService email login path, failures during
credential.user.getIdToken or createSession can leave the Firebase auth session
active while backend session creation fails; update the flow in the function
that calls signInWithEmailAndPassword / credential.user.getIdToken /
createSession to catch errors from getIdToken or createSession and perform
cleanup by calling signOut(auth) (or otherwise clearing the Firebase client
session) before rethrowing the error so no orphaned Firebase session remains;
reference the symbols signInWithEmailAndPassword, credential.user.getIdToken,
createSession and signOut to locate where to add the try/catch and cleanup
logic.
Comment on lines
+67
to
+72
| <a | ||
| href="#signup" | ||
| className="mt-3 text-center text-[16px] font-medium leading-[150%] tracking-[-0.025em] text-[#000000] hover:text-[var(--color-primary-800)]" | ||
| > | ||
| 기업 계정이 없으신가요? 회원가입으로 이동하세요. | ||
| </a> |
Contributor
There was a problem hiding this comment.
Signup CTA points to a dead in-page anchor.
Line 68 uses href="#signup", but this login screen does not render a matching id="signup" target, so the CTA won’t take users to signup flow.
Proposed fix
- <a
- href="#signup"
+ <a
+ href="/signup"
className="mt-3 text-center text-[16px] font-medium leading-[150%] tracking-[-0.025em] text-[`#000000`] hover:text-[var(--color-primary-800)]"
>
기업 계정이 없으신가요? 회원가입으로 이동하세요.
</a>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <a | |
| href="#signup" | |
| className="mt-3 text-center text-[16px] font-medium leading-[150%] tracking-[-0.025em] text-[#000000] hover:text-[var(--color-primary-800)]" | |
| > | |
| 기업 계정이 없으신가요? 회원가입으로 이동하세요. | |
| </a> | |
| <a | |
| href="/signup" | |
| className="mt-3 text-center text-[16px] font-medium leading-[150%] tracking-[-0.025em] text-[`#000000`] hover:text-[var(--color-primary-800)]" | |
| > | |
| 기업 계정이 없으신가요? 회원가입으로 이동하세요. | |
| </a> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/screens/login/company-login-form.tsx` around lines 67 - 72, The CTA
anchor currently uses href="#signup" which points to a non-existent in-page
anchor; update the element that contains the text "기업 계정이 없으신가요? 회원가입으로 이동하세요."
to instead navigate to the real signup flow—either replace href="#signup" with
the correct signup route (e.g., "/signup") or convert the anchor into a
client-side navigation (e.g., using your router's Link component or onClick
handler like navigateToSignup) so the link actually opens the signup page;
ensure you update the <a ... href="#signup"> instance accordingly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
변경 사항
src/api/auth,src/lib/auth,src/screens/login으로 정리해 route 파일은 얇게 유지했습니다./api/**rewrite와 로컬/배포 환경 변수 흐름을 정리했습니다.dashboard대신home라우트로 이동하도록 정리했습니다.검증
CI=true pnpm install --frozen-lockfilepnpm lintpnpm exec tsc --noEmitpnpm buildpnpm exec storybook build --quiet/login/,/home/,/favicon.ico정적 응답 200 및 Chrome headless 화면 확인참고
.env.production은 기존 추적 파일이며 이번 커밋에는 env 변경이 없습니다.Summary by CodeRabbit
New Features
Documentation