[Feat] 소셜 로그인 운영 설정 연결 - #112
Conversation
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (18)
📝 WalkthroughWalkthroughThe PR connects concrete social-login release settings, adds strict four-route policy validation, introduces structured Apple and social-login failure reporting, expands flow tests, and adds automated configuration checks plus release-readiness documentation. ChangesSocial login release readiness
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Sequence Diagram(s)sequenceDiagram
participant SocialLoginCoordinator
participant AuthRemoteDataSource
participant AccountSessionStore
participant FailureReporter
SocialLoginCoordinator->>AuthRemoteDataSource: Submit provider credentials
AuthRemoteDataSource-->>SocialLoginCoordinator: Return credentials or remote error
SocialLoginCoordinator->>FailureReporter: Report remote exchange diagnostic
SocialLoginCoordinator->>AccountSessionStore: Establish account session
AccountSessionStore-->>SocialLoginCoordinator: Return success or storage error
SocialLoginCoordinator->>FailureReporter: Report session storage diagnostic
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
2c5425b to
ee508a5
Compare
Google·Kakao 공개 설정과 callback, Apple capability, 정책 URL, 실패 진단을 운영 경계에 고정한다. Constraint: 클라이언트에는 공개 식별자만 포함하고 provider secret과 서버 자격 증명은 저장하지 않는다 Rejected: 이전 stacked parent 이력을 유지 | #110 squash merge 뒤 중복 diff와 충돌 상태를 남긴다 Confidence: high Scope-risk: moderate Directive: OAuth 서버 설정과 secret은 iOS 저장소 밖에서 관리하고 provider 계약 변경 시 실제 기기 E2E를 다시 수행한다 Tested: original #112 tree equivalence, social login release configuration gate, iPhone functional gate, SwiftData boundary, git diff check Not-tested: fresh GitHub CI and post-merge main checks External-verified: server redeploy 후 실제 iPhone 로그인 및 재실행 세션 유지 사용자 확인
64b2c3d to
dcc178e
Compare
Google·Kakao 공개 설정과 callback, Apple capability, 정책 URL, 실패 진단을 운영 경계에 고정한다. Constraint: 클라이언트에는 공개 식별자만 포함하고 provider secret과 서버 자격 증명은 저장하지 않는다 Rejected: 이전 stacked parent 이력을 유지 | #110 squash merge 뒤 중복 diff와 충돌 상태를 남긴다 Confidence: high Scope-risk: moderate Directive: OAuth 서버 설정과 secret은 iOS 저장소 밖에서 관리하고 provider 계약 변경 시 실제 기기 E2E를 다시 수행한다 Tested: original #112 tree equivalence, social login release configuration gate, iPhone functional gate, SwiftData boundary, git diff check Not-tested: fresh GitHub CI and post-merge main checks External-verified: server redeploy 후 실제 iPhone 로그인 및 재실행 세션 유지 사용자 확인
dcc178e to
97b9895
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Moru/Moru/Features/Profile/SocialLoginCoordinator.swift (1)
130-143: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
SocialLoginFailureLoggerdrops all diagnostics outside DEBUG.The elaborate sanitization in
SocialLoginFailureDiagnostic(safe server code, safe provider name) exists precisely to make failure info safe to log, butreport(_:)is gated by#if DEBUG, so Release/TestFlight builds get zero telemetry for social login failures — exactly the real-device failures this PR defers to post-release verification.If there isn't a separate production logging/analytics path already covering this, consider logging unconditionally since the message is already sanitized.
♻️ Possible fix
func report(_ diagnostic: SocialLoginFailureDiagnostic) { - `#if` DEBUG logger.error( "Social login failed: \(diagnostic.logMessage, privacy: .public)" ) - `#endif` }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Moru/Moru/Features/Profile/SocialLoginCoordinator.swift` around lines 130 - 143, Update SocialLoginFailureLogger.report(_:) to emit the sanitized SocialLoginFailureDiagnostic.logMessage unconditionally, removing the DEBUG-only compilation guard so Release and TestFlight builds retain social-login failure telemetry.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Moru/Moru/Features/AccountEntry/AccountEntryView.swift`:
- Around line 502-537: Update the accessibilitySortPriority values on the
policyLink entries in AccountEntryView so they do not collide with
continueWithoutLoginButton or kakaoButton. Assign unique fractional priorities
that place mainIdentifier after the guest button and privacyIdentifier after
mainIdentifier, while preserving the existing termsIdentifier and
supportIdentifier order and the canonical voiceOverOrder.
In `@Moru/Moru/Features/Profile/AppleAuthorizationAdapter.swift`:
- Around line 60-74: Update AppleAuthorizationFailureLogger.report(_:) to emit
the sanitized logger.error message in all build configurations by removing the
DEBUG-only compilation guard, while preserving the existing logMessage and
public privacy setting.
---
Nitpick comments:
In `@Moru/Moru/Features/Profile/SocialLoginCoordinator.swift`:
- Around line 130-143: Update SocialLoginFailureLogger.report(_:) to emit the
sanitized SocialLoginFailureDiagnostic.logMessage unconditionally, removing the
DEBUG-only compilation guard so Release and TestFlight builds retain
social-login failure telemetry.
🪄 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 Plus
Run ID: d890d8d9-58e9-418d-bfda-725691a3740c
📒 Files selected for processing (18)
Moru/Info.plistMoru/Moru.xcodeproj/project.pbxprojMoru/Moru/Features/AccountEntry/AccountEntryView.swiftMoru/Moru/Features/Profile/AppleAuthorizationAdapter.swiftMoru/Moru/Features/Profile/AppleLoginReadiness.swiftMoru/Moru/Features/Profile/SocialLoginCoordinator.swiftMoru/MoruTests/AppleAccountLinkingTests.swiftMoru/MoruTests/GoogleAccountLinkingTests.swiftMoru/MoruTests/OptionalLoginEntryTests.swiftMoru/MoruTests/OptionalLoginEntryVisualTests.swiftMoru/MoruTests/SocialLoginFoundationTests.swiftMoru/docs/AppleLoginReadiness.mdMoru/docs/GoogleLoginReadiness.mdMoru/docs/KakaoLoginReadiness.mdMoru/docs/OptionalLoginEntryReadiness.mdMoru/docs/SocialLoginReleaseConfiguration.mdScripts/check-social-login-release-config.shmydocs/moru-ios-v2-social-login-context.md
| policyLink( | ||
| title: "MORU 홈", | ||
| url: policyConfiguration.mainURL, | ||
| identifier: AccountEntryAccessibility.mainIdentifier | ||
| ) | ||
| .accessibilitySortPriority(3) | ||
|
|
||
| policyLink( | ||
| title: "개인정보처리방침", | ||
| url: policyConfiguration.privacyPolicyURL, | ||
| identifier: AccountEntryAccessibility.privacyIdentifier | ||
| ) | ||
| .accessibilitySortPriority(1) | ||
| .accessibilitySortPriority(2) | ||
| } | ||
|
|
||
| HStack(spacing: MoruPilotSpacing.sixteen) { | ||
| policyLink( | ||
| title: "이용약관", | ||
| url: policyConfiguration.termsOfServiceURL, | ||
| identifier: AccountEntryAccessibility.termsIdentifier | ||
| ) | ||
| .accessibilitySortPriority(1) | ||
|
|
||
| policyLink( | ||
| title: "고객지원", | ||
| url: policyConfiguration.supportURL, | ||
| identifier: AccountEntryAccessibility.supportIdentifier | ||
| ) | ||
| .accessibilitySortPriority(0) | ||
| } | ||
|
|
||
| if policyConfiguration.privacyPolicyURL == nil | ||
| || policyConfiguration.termsOfServiceURL == nil { | ||
| Text("공개 정책 URL이 준비되지 않아 현재 빌드에서는 링크를 열 수 없어요.") | ||
| if !policyConfiguration.isReady { | ||
| Text( | ||
| "공개 웹 URL이 준비되지 않아 현재 빌드에서는 " | ||
| + "링크를 열 수 없어요." | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
VoiceOver order regression: duplicate accessibilitySortPriority values.
mainIdentifier link gets priority 3, colliding with the pre-existing kakaoButton priority 3 (line 369); privacyIdentifier link gets priority 2, colliding with the pre-existing continueWithoutLoginButton priority 2 (line 257). Since priority values are compared globally (not per-nested-container) within the single .accessibilityElement(children: .contain) root, main(3) will always be announced before guest(2) — the actual VoiceOver order becomes ... kakao → main → guest → privacy ..., which contradicts the documented/tested canonical order ... kakao → guest → main → privacy ... in AccountEntryAccessibility.voiceOverOrder. testAccessibilityIdentifiersAreUniqueAndVoiceOverOrderIsStable only checks the static array, not this runtime wiring, so it doesn't catch the mismatch.
Renumber to eliminate collisions and preserve the intended order:
♿️ Proposed fix
HStack(spacing: MoruPilotSpacing.sixteen) {
policyLink(
title: "MORU 홈",
url: policyConfiguration.mainURL,
identifier: AccountEntryAccessibility.mainIdentifier
)
- .accessibilitySortPriority(3)
+ .accessibilitySortPriority(1.75)
policyLink(
title: "개인정보처리방침",
url: policyConfiguration.privacyPolicyURL,
identifier: AccountEntryAccessibility.privacyIdentifier
)
- .accessibilitySortPriority(2)
+ .accessibilitySortPriority(1.5)
}
HStack(spacing: MoruPilotSpacing.sixteen) {
policyLink(
title: "이용약관",
url: policyConfiguration.termsOfServiceURL,
identifier: AccountEntryAccessibility.termsIdentifier
)
- .accessibilitySortPriority(1)
+ .accessibilitySortPriority(1.25)
policyLink(
title: "고객지원",
url: policyConfiguration.supportURL,
identifier: AccountEntryAccessibility.supportIdentifier
)
- .accessibilitySortPriority(0)
+ .accessibilitySortPriority(1)
}(continueWithoutLoginButton stays at 2, kakaoButton stays at 3 — the fractional values above just slot the policy links strictly between them without colliding with either.)
📝 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.
| policyLink( | |
| title: "MORU 홈", | |
| url: policyConfiguration.mainURL, | |
| identifier: AccountEntryAccessibility.mainIdentifier | |
| ) | |
| .accessibilitySortPriority(3) | |
| policyLink( | |
| title: "개인정보처리방침", | |
| url: policyConfiguration.privacyPolicyURL, | |
| identifier: AccountEntryAccessibility.privacyIdentifier | |
| ) | |
| .accessibilitySortPriority(1) | |
| .accessibilitySortPriority(2) | |
| } | |
| HStack(spacing: MoruPilotSpacing.sixteen) { | |
| policyLink( | |
| title: "이용약관", | |
| url: policyConfiguration.termsOfServiceURL, | |
| identifier: AccountEntryAccessibility.termsIdentifier | |
| ) | |
| .accessibilitySortPriority(1) | |
| policyLink( | |
| title: "고객지원", | |
| url: policyConfiguration.supportURL, | |
| identifier: AccountEntryAccessibility.supportIdentifier | |
| ) | |
| .accessibilitySortPriority(0) | |
| } | |
| if policyConfiguration.privacyPolicyURL == nil | |
| || policyConfiguration.termsOfServiceURL == nil { | |
| Text("공개 정책 URL이 준비되지 않아 현재 빌드에서는 링크를 열 수 없어요.") | |
| if !policyConfiguration.isReady { | |
| Text( | |
| "공개 웹 URL이 준비되지 않아 현재 빌드에서는 " | |
| + "링크를 열 수 없어요." | |
| ) | |
| policyLink( | |
| title: "MORU 홈", | |
| url: policyConfiguration.mainURL, | |
| identifier: AccountEntryAccessibility.mainIdentifier | |
| ) | |
| .accessibilitySortPriority(1.75) | |
| policyLink( | |
| title: "개인정보처리방침", | |
| url: policyConfiguration.privacyPolicyURL, | |
| identifier: AccountEntryAccessibility.privacyIdentifier | |
| ) | |
| .accessibilitySortPriority(1.5) | |
| } | |
| HStack(spacing: MoruPilotSpacing.sixteen) { | |
| policyLink( | |
| title: "이용약관", | |
| url: policyConfiguration.termsOfServiceURL, | |
| identifier: AccountEntryAccessibility.termsIdentifier | |
| ) | |
| .accessibilitySortPriority(1.25) | |
| policyLink( | |
| title: "고객지원", | |
| url: policyConfiguration.supportURL, | |
| identifier: AccountEntryAccessibility.supportIdentifier | |
| ) | |
| .accessibilitySortPriority(1) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Moru/Moru/Features/AccountEntry/AccountEntryView.swift` around lines 502 -
537, Update the accessibilitySortPriority values on the policyLink entries in
AccountEntryView so they do not collide with continueWithoutLoginButton or
kakaoButton. Assign unique fractional priorities that place mainIdentifier after
the guest button and privacyIdentifier after mainIdentifier, while preserving
the existing termsIdentifier and supportIdentifier order and the canonical
voiceOverOrder.
| nonisolated struct AppleAuthorizationFailureLogger: | ||
| AppleAuthorizationFailureReporting { | ||
| private let logger = Logger( | ||
| subsystem: Bundle.main.bundleIdentifier ?? "com.teammoru.Moru", | ||
| category: "AppleAuthorization" | ||
| ) | ||
|
|
||
| func report(_ reason: AppleAuthorizationFailureReason) { | ||
| #if DEBUG | ||
| logger.error( | ||
| "Apple authorization failed: \(reason.logMessage, privacy: .public)" | ||
| ) | ||
| #endif | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate target file =="
git ls-files | grep -F 'AppleAuthorizationAdapter.swift' || true
echo "== Target file excerpt =="
if [ -f "Moru/Moru/Features/Profile/AppleAuthorizationAdapter.swift" ]; then
cat -n "Moru/Moru/Features/Profile/AppleAuthorizationAdapter.swift" | sed -n '1,120p'
fi
echo "== Search logMessage / failure reason usages =="
rg -n "AppleAuthorizationFailureReporting|AppleAuthorizationFailureLogger|logMessage|report\\(" -S . --glob '*.swift' | head -200
echo "== Search DEBUG guard logging patterns around reported APIs =="
rg -n "`#if` DEBUG|Logger\\(|\\.error\\(|report\\(" -S Moru/Moru --glob '*.swift' | head -200Repository: Team-Moru/moru-iOS
Length of output: 11915
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Apple Authorization failure call sites =="
cat -n "Moru/Moru/Features/Profile/AppleAuthorizationAdapter.swift" | sed -n '118,240p'
cat -n "Moru/Moru/Features/Profile/AppleLoginReadiness.swift" | sed -n '80,145p'
echo "== Existing failure reporter defaults =="
cat -n "Moru/Moru/Features/Profile/SocialLoginCoordinator.swift" | sed -n '120,155p'
echo "== Project build configs mentioning DEBUG/RELEASE/Xcframework =="
grep -RInE -i "DEBUG|RELEASE|Default Configuration|build configuration|xcconfig|enableUserInteraction|archive" . \
--include='*.xcodeproj' --include='*.xcworkspace' --include='*.plist' --include='*.xcconfig' --include='*.swift' \
| head -200
echo "== README/docs mentions of diagnostics/logs/production QA =="
grep -RInE -i "qa|real device|qa note|credential|authorization|diagn|log|production|release|sysdiagnose|console" . \
--include='*.md' --include='*.txt' --include='*.plist' --include='*.swift' \
| head -300Repository: Team-Moru/moru-iOS
Length of output: 45496
Log Apple authorization failures outside DEBUG-only guards.
failed(_:) only calls failureReporter.report(...), and AppleAuthorizationFailureLogger.report(_:) compiles the logger.error(...) call out of Release builds because it is wrapped in #if DEBUG. Since logMessage is already sanitized and uses privacy: .public, these structured Apple auth failures won’t appear in Release/TestFlight/App Store logs; log them unconditionally or through a Release-visible OS log channel.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Moru/Moru/Features/Profile/AppleAuthorizationAdapter.swift` around lines 60 -
74, Update AppleAuthorizationFailureLogger.report(_:) to emit the sanitized
logger.error message in all build configurations by removing the DEBUG-only
compilation guard, while preserving the existing logMessage and public privacy
setting.
1 participant
✨ PR 유형
🛠️ 작업내용
team-moru.github.io의 HTTPS exact route만 허용합니다.vrVBDLEy0UmqlLVfxnUcY9, node2644:2751로 manifest에 고정했습니다.검증:
📋 추후 진행 상황
2644:2751의 before/after/overlay를 완료합니다. 현재 pixel-match 완료로 주장하지 않습니다.📌 리뷰 포인트
✅ Checklist
Refs #111
Summary by CodeRabbit
New Features
Bug Fixes
Documentation