[Add] #35 - Auth Endpoint, Token 관리 로직 및 타입 구현 #36
Conversation
WalkthroughEndpoint 및 요청 인코딩 재설계, 응답/오류 DTO 분리, 토큰 저장소·리프레셔 프로토콜과 Keychain 구현 추가, DefaultNetworkProvider를 actor로 전환해 401 발생 시 토큰 재발급·재시도 흐름을 도입했습니다. 일부 Mock 코드 삭제 및 빈 파일·.gitignore 변경이 포함됩니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant NetworkProvider as DefaultNetworkProvider
participant TokenStorage
participant TokenRefresher
participant Remote as Server
Client->>NetworkProvider: request(endpoint)
NetworkProvider->>NetworkProvider: buildRequest(endpoint)
NetworkProvider->>TokenStorage: fetch()
TokenStorage-->>NetworkProvider: AuthTokens
NetworkProvider->>Remote: HTTP request (Bearer)
Remote-->>NetworkProvider: 401 Unauthorized
NetworkProvider->>NetworkProvider: handleUnauthorized()
NetworkProvider->>TokenRefresher: refresh(provider: NetworkProvider)
TokenRefresher->>Remote: POST reissueToken
Remote-->>TokenRefresher: 200 + TokenPair
TokenRefresher-->>NetworkProvider: AuthTokens
NetworkProvider->>TokenStorage: store(tokens)
TokenStorage-->>NetworkProvider: success
NetworkProvider->>Remote: retry original request
Remote-->>NetworkProvider: 200 OK + data
NetworkProvider-->>Client: decoded response
sequenceDiagram
participant Client
participant NetworkProvider as DefaultNetworkProvider
participant KeychainStorage as KeychainTokenStorage
participant Keychain as iOS_Keychain
Client->>NetworkProvider: setTokenRefresher(refresher)
NetworkProvider-->>Client: ack
Client->>NetworkProvider: request(endpoint)
NetworkProvider->>KeychainStorage: fetch()
KeychainStorage->>Keychain: SecItemCopyMatching(query)
Keychain-->>KeychainStorage: CFTypeRef(data)
KeychainStorage-->>NetworkProvider: AuthTokens
NetworkProvider->>NetworkProvider: appendBearerToken(request)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 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
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 |
Remaked-Swain
added
Add ✚
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In @Neki-iOS/Core/Sources/Network/Sources/Base/Endpoint.swift:
- Around line 67-73: The multipart branch in Endpoint.swift currently accepts
only Data and ignores the provided boundary, risking header/body mismatch;
implement a MultipartBuilder that constructs multipart/form-data body using the
supplied boundary (wrapping fields/files with --boundary, CRLFs, correct
Content-Disposition and Content-Type per part) and set request.httpBody to that
generated Data while also setting the request's "Content-Type" to
"multipart/form-data; boundary=\(boundary)". Also update
DefaultNetworkProvider.appendRefreshToken to avoid unconditionally overwriting
Content-Type: when authorizationType == .reissue, preserve an existing
"Content-Type" header (especially multipart/form-data) or only set Content-Type
if it's not already present to prevent clobbering the multipart boundary header.
In @Neki-iOS/Core/Sources/Network/Sources/DefaultNetworkProvider.swift:
- Around line 118-120: The catch clause currently uses pattern `catch is
EncodingError` but references an undefined `error`; change the clause to bind
the thrown error (e.g., `catch let error as EncodingError`) so you can call
`error.localizedDescription` and rethrow the bound error; update the catch block
in DefaultNetworkProvider (the EncodingError catch) to bind the error variable
and use that bound name when logging and rethrowing.
- Around line 132-140: handleUnauthorized currently performs
performTokenRefresh() but never returns Data; after a successful token refresh
you must retry the original request and return its Data. Update
handleUnauthorized to call performTokenRefresh(), then re-invoke the existing
request execution path (e.g., call the function that executes an Endpoint such
as request(endpoint:retryCount:) or the internal performRequest method) with an
updated retryCount and return the resulting Data; keep the existing branch that
calls throwIfServerError(data:) and tokenStorage.delete() when reissue is not
allowed, and ensure errors from the retry propagate correctly.
In @Neki-iOS/Core/Sources/Network/Sources/KeychainTokenStorage.swift:
- Around line 12-14: KeychainTokenStorage declares JSONEncoder/JSONDecoder
properties while conforming to Sendable, which is unsafe because
JSONEncoder/JSONDecoder are not Sendable; fix by removing the encoder and
decoder stored properties from KeychainTokenStorage and instead create
JSONEncoder() and JSONDecoder() locally inside each method that needs them (or
alternately mark those properties nonisolated(unsafe) if you deliberately accept
the risk), updating all uses in methods like the save/load functions to use the
locally created encoder/decoder instances.
- Around line 43-48: The guard in checkStatus(_:which:) is correct (it returns
on noErr), but replace the debug print with structured logging: remove
print(errorMessage) and use a Logger (e.g., os.Logger or SwiftLog) to log a
clear message including the function parameter, the OSStatus value, and the
SecCopyErrorMessageString-derived errorMessage before throwing; keep the
existing throw behavior (throw errSecItemNotFound ? .notFound : .unknown) and
leave SecCopyErrorMessageString, TokenStorageError and the errSecItemNotFound
check unchanged.
In @Neki-iOS/Features/Archive/Sources/Data/Sources/Empty2.swift:
- Around line 1-6: 헤더 주석의 파일명이 실제 파일명과 일치하지 않습니다: 파일은 Empty2.swift인데 주석 상단에는
"Empty.swift"로 남아 있습니다; 파일 명과 헤더 주석을 일치시키도록 상단 주석의 파일명 문자열을 "Empty2.swift"로 수정하고
저자/생성일 등 다른 메타정보는 그대로 두거나 필요시 업데이트하여 일관성을 유지하세요.
In @Neki-iOS/Features/Auth/Sources/Data/Sources/AuthTokenRefresher.swift:
- Around line 10-21: Mark the type and its API as public so it satisfies the
public TokenRefresher protocol: change `struct AuthTokenRefresher` to `public
struct AuthTokenRefresher` and make its members public as well (e.g., `public
var destination: Endpoint { AuthEndpoint.reissueToken }` and `public func
refresh(provider: any NetworkProvider) async throws(NetworkError) ->
AuthTokens`). Keep the existing do-catch error conversion inside `refresh()` (do
not remove it) so generic thrown Errors from `NetworkProvider.request()` are
mapped to `NetworkError` as required.
In @Neki-iOS/Features/Auth/Sources/Data/Sources/DTOs/SocialLoginDTO.swift:
- Around line 10-16: SocialLoginDTO and its nested Request are missing public
access control so they’re inaccessible from other modules; update the
declaration of enum SocialLoginDTO and the nested struct Request to be public,
and also mark the idToken property as public if needed for external encoding
usage, and ensure the typealias Response (TokenPair) is public or that TokenPair
itself is public so external modules can use SocialLoginDTO.Response.
In @Neki-iOS/Features/Auth/Sources/Domain/Sources/Entities/User.swift:
- Around line 11-19: User and UserSession are public but their stored properties
(nickname, providerType, user, tokens) and the synthesized memberwise
initializers are internal, preventing usage from other modules; make these
properties and/or initializers public so external modules can read and
instantiate them. Update the User struct by marking nickname and providerType as
public (or add a public init on User), and update UserSession by marking user
and tokens as public (or add a public init on UserSession), ensuring
ProviderType and AuthTokens are also accessible publicly where needed.
🧹 Nitpick comments (10)
Neki-iOS/Features/Auth/Sources/Presentation/Empty.swift (1)
1-8: 빈 파일의 목적을 명확히 하거나 제거를 고려하세요.이 파일은 Foundation import만 포함하고 실제 구현은 없습니다. 파일명이 "Empty.swift"인 것으로 보아 의도적인 플레이스홀더로 보이지만, 그 목적이 명확하지 않습니다.
다음 중 하나를 권장합니다:
- Swift Package Manager나 Xcode 모듈 구조상 필요한 경우: 파일 상단에 그 이유를 설명하는 주석을 추가
- 향후 구현 예정인 경우:
// TODO: Implement Auth presentation layer등의 주석 추가- 불필요한 경우: 파일 제거
다음 스크립트로 Presentation 레이어에 다른 구현 파일이 있는지 확인할 수 있습니다:
#!/bin/bash # Presentation 레이어의 다른 Swift 파일 확인 fd -e swift . "Neki-iOS/Features/Auth/Sources/Presentation"Neki-iOS/Features/Auth/Sources/Data/Sources/DTOs/UserInfoDTO.swift (1)
10-19: ProviderType enum 사용 검토 권장현재
providerType이String으로 선언되어 있으나, Domain 레이어의User엔티티에서는ProviderTypeenum을 사용하고 있습니다. 데이터 매핑 시 type-safe하게 변환하려면,ProviderType으로 선언하고 JSON 문자열에서 enum으로 변환하는 커스텀 디코딩 로직을 추가하는 것을 고려해 보세요.enum UserInfoDTO { struct Response: Decodable { let nickname: String let providerType: ProviderType // String 대신 enum 사용 enum CodingKeys: String, CodingKey { case providerType case nickname = "name" } } }다만,
ProviderTypeenum이 현재Decodable을 구현하지 않으므로, 다음과 같이 커스텀 디코딩이 필요합니다:init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) nickname = try container.decode(String.self, forKey: .nickname) let providerTypeString = try container.decode(String.self, forKey: .providerType) // providerTypeString을 ProviderType enum으로 변환 }Neki-iOS/Core/Sources/Network/Sources/Base/BaseResponseDTO.swift (1)
25-29:BaseFailedResponseDTO에도 동일한 접근 제어자 불일치
BaseFailedResponseDTO도publicstruct이지만 프로퍼티들이internal입니다.NetworkError.swift에서dto.message에 접근하는 것으로 보아, 모듈 간 일관성을 위해 프로퍼티를public으로 선언하는 것이 좋습니다.♻️ 수정 제안
public struct BaseFailedResponseDTO: Decodable { - let status: Int - let message: String - let timestamp: String + public let status: Int + public let message: String + public let timestamp: String }Neki-iOS/Features/Auth/Sources/Data/Sources/AuthEndpoint.swift (1)
10-20: TODO: Endpoint 프로토콜 구현 필요
AuthEndpoint가 정의되어 있지만Endpoint프로토콜 채택이 아직 구현되지 않았습니다.AuthTokenRefresher에서 이 endpoint를 사용하려면 구현이 필요합니다.API 명세서가 확정되면
Endpoint프로토콜 구현을 도와드릴 수 있습니다. 새 이슈를 생성하여 추적할까요?#!/bin/bash # AuthEndpoint가 Endpoint 프로토콜을 채택하는 다른 endpoint 파일 참조 확인 fd -e swift Endpoint.swift --exec cat {} \; 2>/dev/null | head -50 # 다른 Endpoint 구현체 패턴 확인 ast-grep --pattern 'enum $_: Endpoint { $$$ }'Neki-iOS/Core/Sources/Network/Sources/Base/TokenRefresher.swift (1)
10-14: 프로토콜 설계가 적절합니다.
Sendable준수와 typed throws를 사용한 프로토콜 정의가 actor 기반NetworkProvider와 잘 어울립니다.destination프로퍼티를 통해 refresh endpoint를 유연하게 지정할 수 있습니다.참고:
Foundationimport는 이 파일에서 사용되지 않으므로 제거해도 됩니다.Neki-iOS/Core/Sources/Network/Sources/Base/TokenStorage.swift (1)
10-16: 프로토콜 설계가 깔끔합니다.CRUD 스타일의 토큰 저장소 인터페이스가 명확하게 정의되어 있습니다. 다만
Querytypealias가 프로토콜 메서드 시그니처에서 직접 사용되지 않고 구현체에서만 사용됩니다. 이 typealias를 구현체(KeychainTokenStorage)로 이동하거나, 프로토콜에서 제거하는 것을 고려해 보세요.Neki-iOS/Features/Auth/Sources/Domain/Sources/Entities/AuthTokens.swift (1)
17-17: TODO 코멘트 확인만료 시각(
expiredDate) 프로퍼티 추가에 대한 TODO가 있습니다. 토큰 만료 시간 관리가 구현되면 proactive한 토큰 갱신이 가능해져 401 응답 전에 미리 갱신할 수 있습니다.이 기능 구현을 위한 이슈를 생성해 드릴까요?
Neki-iOS/Core/Sources/Network/Sources/KeychainTokenStorage.swift (3)
46-47:print()대신Logger사용 권장다른 파일에서는
Logger.network를 사용하고 있으므로, 일관성을 위해 여기서도Logger를 사용하세요.♻️ 수정 제안
- // TODO: Logging 필요 - print(errorMessage) + Logger.network.error("Keychain error in \(function): \(errorMessage)")
51-55: 디코딩 실패 시 오류 정보 손실
try?를 사용하여 디코딩 오류를 무시하고 있어, 디버깅 시 실제 원인 파악이 어려울 수 있습니다.♻️ 수정 제안
func convert(_ ref: CFTypeRef?) throws(TokenStorageError) -> AuthTokens { guard let ref = ref else { throw .notFound } - guard let data = ref as? Data, let tokens = try? decoder.decode(AuthTokens.self, from: data) else { throw .conversionFailed } + guard let data = ref as? Data else { throw .conversionFailed } + do { + return try decoder.decode(AuthTokens.self, from: data) + } catch { + Logger.network.error("Token decode error: \(error.localizedDescription)") + throw .conversionFailed + } - return tokens }
79-79: 불필요한 nil 초기화
var dataTypeRef: CFTypeRef? = nil에서= nil은 불필요합니다. Optional 변수는 기본값이nil입니다.♻️ 수정 제안
-var dataTypeRef: CFTypeRef? = nil +var dataTypeRef: CFTypeRef?
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
Neki-iOS.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolvedNeki-iOS/Core/Sources/Network/Sources/Base/BaseResponseDTO.swiftNeki-iOS/Core/Sources/Network/Sources/Base/Endpoint.swiftNeki-iOS/Core/Sources/Network/Sources/Base/NetworkError.swiftNeki-iOS/Core/Sources/Network/Sources/Base/TokenRefresher.swiftNeki-iOS/Core/Sources/Network/Sources/Base/TokenStorage.swiftNeki-iOS/Core/Sources/Network/Sources/DefaultNetworkProvider.swiftNeki-iOS/Core/Sources/Network/Sources/KeychainTokenStorage.swiftNeki-iOS/Features/Archive/Sources/Data/Sources/Empty2.swiftNeki-iOS/Features/Archive/Sources/Data/Sources/Network/Mock/DTO/Request/MockRequestDTO.swiftNeki-iOS/Features/Archive/Sources/Data/Sources/Network/Mock/DTO/Response/MockResponseDTO.swiftNeki-iOS/Features/Archive/Sources/Data/Sources/Network/Mock/MockAPI.swiftNeki-iOS/Features/Archive/Sources/Data/Sources/Network/Mock/MockService.swiftNeki-iOS/Features/Archive/Sources/Data/Sources/Repositories/DefaultMockRepository.swiftNeki-iOS/Features/Auth/Sources/Data/Sources/AuthEndpoint.swiftNeki-iOS/Features/Auth/Sources/Data/Sources/AuthTokenRefresher.swiftNeki-iOS/Features/Auth/Sources/Data/Sources/DTOs/ReissueTokenDTO.swiftNeki-iOS/Features/Auth/Sources/Data/Sources/DTOs/SocialLoginDTO.swiftNeki-iOS/Features/Auth/Sources/Data/Sources/DTOs/TokenPair.swiftNeki-iOS/Features/Auth/Sources/Data/Sources/DTOs/UserInfoDTO.swiftNeki-iOS/Features/Auth/Sources/Domain/Sources/Entities/AuthTokens.swiftNeki-iOS/Features/Auth/Sources/Domain/Sources/Entities/ProviderType.swiftNeki-iOS/Features/Auth/Sources/Domain/Sources/Entities/User.swiftNeki-iOS/Features/Auth/Sources/Presentation/Empty.swift
💤 Files with no reviewable changes (5)
- Neki-iOS/Features/Archive/Sources/Data/Sources/Network/Mock/DTO/Response/MockResponseDTO.swift
- Neki-iOS/Features/Archive/Sources/Data/Sources/Network/Mock/DTO/Request/MockRequestDTO.swift
- Neki-iOS/Features/Archive/Sources/Data/Sources/Network/Mock/MockService.swift
- Neki-iOS/Features/Archive/Sources/Data/Sources/Repositories/DefaultMockRepository.swift
- Neki-iOS/Features/Archive/Sources/Data/Sources/Network/Mock/MockAPI.swift
🧰 Additional context used
🧬 Code graph analysis (5)
Neki-iOS/Core/Sources/Network/Sources/Base/TokenStorage.swift (1)
Neki-iOS/Core/Sources/Network/Sources/KeychainTokenStorage.swift (4)
store(110-119)fetch(121-126)delete(100-103)delete(128-131)
Neki-iOS/Core/Sources/Network/Sources/Base/TokenRefresher.swift (1)
Neki-iOS/Features/Auth/Sources/Data/Sources/AuthTokenRefresher.swift (1)
refresh(13-21)
Neki-iOS/Core/Sources/Network/Sources/DefaultNetworkProvider.swift (3)
Neki-iOS/Core/Sources/Network/Sources/Base/Endpoint.swift (1)
asURLRequest(46-78)Neki-iOS/Core/Sources/Network/Sources/KeychainTokenStorage.swift (2)
fetch(121-126)store(110-119)Neki-iOS/Features/Auth/Sources/Data/Sources/AuthTokenRefresher.swift (1)
refresh(13-21)
Neki-iOS/Core/Sources/Network/Sources/Base/Endpoint.swift (1)
Neki-iOS/Core/Sources/Network/Sources/DefaultNetworkProvider.swift (2)
request(36-38)request(44-47)
Neki-iOS/Core/Sources/Network/Sources/KeychainTokenStorage.swift (1)
Neki-iOS/Core/Sources/Network/Sources/DefaultNetworkProvider.swift (1)
decode(177-184)
🪛 SwiftLint (0.57.0)
Neki-iOS/Features/Auth/Sources/Data/Sources/AuthEndpoint.swift
[Warning] 17-17: TODOs should be resolved (API 명세서 확인하여 구현)
(todo)
Neki-iOS/Features/Auth/Sources/Domain/Sources/Entities/AuthTokens.swift
[Warning] 17-17: TODOs should be resolved (만료시각 정해지면 expiredDate 프로퍼티 추가)
(todo)
Neki-iOS/Core/Sources/Network/Sources/KeychainTokenStorage.swift
[Warning] 79-79: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
[Warning] 46-46: TODOs should be resolved (Logging 필요)
(todo)
🔇 Additional comments (13)
Neki-iOS.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved (1)
1-141: LGTM!
originHash변경은 Swift Package Manager가 패키지를 다시 resolve할 때 자동으로 발생하는 예상된 변경입니다. 실제 의존성 버전이나 구조에는 변경이 없으며, 이 파일은 자동 생성되므로 문제없습니다.Neki-iOS/Core/Sources/Network/Sources/Base/BaseResponseDTO.swift (1)
10-21: 프로퍼티 접근 제어자 확인 필요
BaseSucceededResponseDTO는public으로 선언되어 있지만 내부 프로퍼티들(status,message,isSuccess,data)은internal(기본값)입니다. 외부 모듈에서 이 DTO를 사용할 때 프로퍼티에 접근할 수 없습니다.다른 모듈에서 이 DTO의 프로퍼티에 접근해야 하는지 확인해 주세요. 필요하다면 프로퍼티에
public접근 제어자를 추가하세요.♻️ 수정 제안
public struct BaseSucceededResponseDTO<T: Decodable>: Decodable { - let status: Int - let message: String - let isSuccess: Bool - let data: T? + public let status: Int + public let message: String + public let isSuccess: Bool + public let data: T? enum CodingKeys: String, CodingKey { case message, data case status = "resultCode" case isSuccess = "success" } }Neki-iOS/Features/Auth/Sources/Data/Sources/DTOs/ReissueTokenDTO.swift (1)
10-16: LGTM!enum을 namespace로 활용하여 Request/Response를 그룹화한 깔끔한 DTO 구조입니다.
Response를TokenPair의 typealias로 선언한 것도 재사용성 측면에서 좋습니다.Neki-iOS/Features/Auth/Sources/Data/Sources/DTOs/TokenPair.swift (1)
10-13: LGTM!토큰 쌍을 담는 간단한 데이터 컨테이너로 적절합니다.
테스트 작성 시 비교가 필요하다면
Equatable프로토콜 채택을 고려해 보세요.struct TokenPair: Decodable, Equatable { let accessToken: String let refreshToken: String }Neki-iOS/Features/Auth/Sources/Domain/Sources/Entities/ProviderType.swift (1)
10-16: LGTM!OAuth 공급자 종류를 명확하게 정의한 enum입니다.
testcase가 프로덕션 코드에 포함되어 있는데, 테스트 전용이라면#if DEBUG조건부 컴파일이나 별도의 테스트 타겟으로 분리하는 것을 고려해 보세요. 의도된 설계라면 무시해도 됩니다.Neki-iOS/Core/Sources/Network/Sources/Base/NetworkError.swift (1)
11-11:apiError케이스 개선이 적절합니다.
String대신BaseFailedResponseDTO를 사용하여 서버 오류 응답의 전체 정보를 보존할 수 있게 되었습니다.errorDescription에서dto.message를 추출하는 방식도 적절합니다.Also applies to: 25-25
Neki-iOS/Core/Sources/Network/Sources/Base/TokenStorage.swift (1)
18-22: 에러 케이스 확장 고려현재 에러 케이스는 기본적인 시나리오를 커버합니다. 향후 Keychain 관련 보안 오류(예: 인증 실패, 접근 거부 등)를 세분화하려면 추가 케이스가 필요할 수 있습니다.
Neki-iOS/Core/Sources/Network/Sources/Base/Endpoint.swift (1)
10-21: 인증 타입 설계가 적절합니다.
reissue케이스에 대한@Important문서화가 잘 되어 있어, 특수 용도임을 명확히 알 수 있습니다.Neki-iOS/Core/Sources/Network/Sources/KeychainTokenStorage.swift (1)
109-118:store()로직 검증
read(query)가 성공하지만nil을 반환하는 경우(실제로는checkStatus에서 예외가 발생하므로 불가능),guard let _ = try read(query) else { return }에서 토큰을 저장하지 않고 반환합니다. 현재 구현에서는read()가 성공하면 항상 non-nil을 반환하므로 문제없지만, 의도를 명확히 하기 위해 주석을 추가하거나 로직을 단순화하는 것을 고려하세요.Neki-iOS/Core/Sources/Network/Sources/DefaultNetworkProvider.swift (3)
11-13: Actor 기반 설계 승인
actor로 전환하여refreshTask와 같은 가변 상태를 안전하게 보호하고, 동시 요청 시 토큰 갱신 중복을 방지하는 설계가 적절합니다.
162-165: 서버 에러 파싱 로직 승인
BaseFailedResponseDTO디코딩 실패 시 조용히 무시하고 계속 진행하는 것은 적절합니다. 서버가 예상 형식의 에러 응답을 보내지 않을 수 있기 때문입니다.
142-155: Refresh Task 캐싱 패턴 검토동시 401 응답 시 중복 갱신을 방지하기 위해
refreshTask를 캐싱하는 패턴은 좋습니다. Actor isolation이 적용되며,refresher.refresh(provider: self)를 통해 전달된 actor에서provider.request()를 호출할 때 실제로 재진입이 발생할 수 있습니다.다행히 코드가 이를 안전하게 처리합니다:
refreshTask확인(line 143)으로 이미 진행 중인 갱신을 기다립니다handleUnauthorized의 조건 검사(endpoint.authorizationType != .reissue && retryCount > 0)가 무한 루프를 방지합니다- Swift actor의 재진입 허용으로 인한 데드락은 없습니다
현재 구현은 충분히 안전하며 상태 변경 순서도 적절합니다.
Neki-iOS/Features/Auth/Sources/Domain/Sources/Entities/AuthTokens.swift (1)
14-15: 프로퍼티 접근 수준은 현재 구조에서 문제 없음
accessToken과refreshToken이internal접근 수준(접근 제어자 미명시)이지만,AuthTokens와DefaultNetworkProvider는 모두 동일한Neki-iOS타겟 내에 있습니다. 단일 타겟 구조에서는internal접근 수준으로 충분하며, 같은 모듈 내에서 자유롭게 접근 가능합니다. 따라서 현재 코드는 정상적으로 작동합니다.
| case .multipart(let boundary): | ||
| // TODO: MultipartBuilder 구현.. | ||
| if let data = body as? Data { | ||
| request.httpBody = data | ||
| } else { | ||
| throw NetworkError.requestEncodingError | ||
| } |
There was a problem hiding this comment.
Multipart 구현 미완성 및 잠재적 문제
현재 multipart 처리는 body가 이미 Data 타입인 경우만 허용합니다. 그러나 boundary 파라미터가 실제 body 생성에 사용되지 않아, 헤더에 설정된 boundary와 body의 boundary가 불일치할 수 있습니다.
또한 DefaultNetworkProvider.appendRefreshToken에서 Content-Type을 다시 설정하므로, authorizationType이 .reissue인 경우 여기서 설정한 값이 덮어씌워질 수 있습니다.
🤖 Prompt for AI Agents
In @Neki-iOS/Core/Sources/Network/Sources/Base/Endpoint.swift around lines 67 -
73, The multipart branch in Endpoint.swift currently accepts only Data and
ignores the provided boundary, risking header/body mismatch; implement a
MultipartBuilder that constructs multipart/form-data body using the supplied
boundary (wrapping fields/files with --boundary, CRLFs, correct
Content-Disposition and Content-Type per part) and set request.httpBody to that
generated Data while also setting the request's "Content-Type" to
"multipart/form-data; boundary=\(boundary)". Also update
DefaultNetworkProvider.appendRefreshToken to avoid unconditionally overwriting
Content-Type: when authorizationType == .reissue, preserve an existing
"Content-Type" header (especially multipart/form-data) or only set Content-Type
if it's not already present to prevent clobbering the multipart boundary header.
There was a problem hiding this comment.
@Remaked-Swain 확인해 주셔서 감사합니다! 추가로 도움이 필요하시면 언제든지 말씀해 주세요. 😊
Neki-iOS/Core/Sources/Network/Sources/DefaultNetworkProvider.swift
Outdated
Show resolved
Hide resolved
| final class KeychainTokenStorage: Sendable { | ||
| private let encoder: JSONEncoder | ||
| private let decoder: JSONDecoder |
There was a problem hiding this comment.
Sendable 준수 문제
KeychainTokenStorage가 Sendable을 준수하지만, JSONEncoder와 JSONDecoder는 기본적으로 Sendable이 아닙니다. 이로 인해 동시성 환경에서 데이터 레이스가 발생할 수 있습니다.
🔧 수정 제안
nonisolated(unsafe)를 사용하거나, 메서드 내에서 인코더/디코더를 생성하는 방식을 고려하세요:
-final class KeychainTokenStorage: Sendable {
- private let encoder: JSONEncoder
- private let decoder: JSONDecoder
+final class KeychainTokenStorage: Sendable {
+ private nonisolated(unsafe) let encoder: JSONEncoder
+ private nonisolated(unsafe) let decoder: JSONDecoder또는 매번 새 인스턴스를 생성:
final class KeychainTokenStorage: Sendable {
// encoder/decoder 프로퍼티 제거
// 메서드 내에서 JSONEncoder(), JSONDecoder() 직접 생성
}🤖 Prompt for AI Agents
In @Neki-iOS/Core/Sources/Network/Sources/KeychainTokenStorage.swift around
lines 12 - 14, KeychainTokenStorage declares JSONEncoder/JSONDecoder properties
while conforming to Sendable, which is unsafe because JSONEncoder/JSONDecoder
are not Sendable; fix by removing the encoder and decoder stored properties from
KeychainTokenStorage and instead create JSONEncoder() and JSONDecoder() locally
inside each method that needs them (or alternately mark those properties
nonisolated(unsafe) if you deliberately accept the risk), updating all uses in
methods like the save/load functions to use the locally created encoder/decoder
instances.
| func checkStatus(_ status: OSStatus, which function: String) throws(TokenStorageError) { | ||
| guard status != noErr else { return } | ||
| let errorMessage = SecCopyErrorMessageString(status, nil) as? String ?? "\(TokenStorageError.unknown)" | ||
| // TODO: Logging 필요 | ||
| print(errorMessage) | ||
| throw status == errSecItemNotFound ? .notFound : .unknown |
There was a problem hiding this comment.
조건 로직 오류 (Critical)
guard status != noErr else { return }은 status가 noErr일 때(성공) 반환하고, 오류일 때 계속 진행합니다. 그러나 이후 로직은 오류 메시지를 출력하고 예외를 던지므로, 현재 동작이 의도한 것인지 확인이 필요합니다.
만약 성공 시 early return이 의도라면, 조건이 반대로 되어 있을 수 있습니다:
🐛 수정 제안
func checkStatus(_ status: OSStatus, which function: String) throws(TokenStorageError) {
- guard status != noErr else { return }
+ guard status == noErr else {
+ let errorMessage = SecCopyErrorMessageString(status, nil) as? String ?? "\(TokenStorageError.unknown)"
+ Logger.network.error("Keychain error in \(function): \(errorMessage)")
+ throw status == errSecItemNotFound ? .notFound : .unknown
+ }
- let errorMessage = SecCopyErrorMessageString(status, nil) as? String ?? "\(TokenStorageError.unknown)"
- // TODO: Logging 필요
- print(errorMessage)
- throw status == errSecItemNotFound ? .notFound : .unknown
}죄송합니다, 다시 확인해보니 guard status != noErr else { return }은 status == noErr일 때 return하므로 정상입니다. 그러나 print() 대신 Logger를 사용하는 것이 좋습니다.
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 46-46: TODOs should be resolved (Logging 필요)
(todo)
🤖 Prompt for AI Agents
In @Neki-iOS/Core/Sources/Network/Sources/KeychainTokenStorage.swift around
lines 43 - 48, The guard in checkStatus(_:which:) is correct (it returns on
noErr), but replace the debug print with structured logging: remove
print(errorMessage) and use a Logger (e.g., os.Logger or SwiftLog) to log a
clear message including the function parameter, the OSStatus value, and the
SecCopyErrorMessageString-derived errorMessage before throwing; keep the
existing throw behavior (throw errSecItemNotFound ? .notFound : .unknown) and
leave SecCopyErrorMessageString, TokenStorageError and the errSecItemNotFound
check unchanged.
| // | ||
| // Empty.swift | ||
| // Neki-iOS | ||
| // | ||
| // Created by SwainYun on 1/13/26. | ||
| // |
There was a problem hiding this comment.
헤더 주석의 파일명이 실제 파일명과 일치하지 않습니다.
파일명은 Empty2.swift이지만 헤더 주석에는 Empty.swift로 되어 있습니다. 복사-붙여넣기 과정에서 발생한 것으로 보이니 수정해 주세요.
🔧 수정 제안
//
-// Empty.swift
+// Empty2.swift
// Neki-iOS
//
// Created by SwainYun on 1/13/26.
//📝 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.
| // | |
| // Empty.swift | |
| // Neki-iOS | |
| // | |
| // Created by SwainYun on 1/13/26. | |
| // | |
| // | |
| // Empty2.swift | |
| // Neki-iOS | |
| // | |
| // Created by SwainYun on 1/13/26. | |
| // |
🤖 Prompt for AI Agents
In @Neki-iOS/Features/Archive/Sources/Data/Sources/Empty2.swift around lines 1 -
6, 헤더 주석의 파일명이 실제 파일명과 일치하지 않습니다: 파일은 Empty2.swift인데 주석 상단에는 "Empty.swift"로 남아
있습니다; 파일 명과 헤더 주석을 일치시키도록 상단 주석의 파일명 문자열을 "Empty2.swift"로 수정하고 저자/생성일 등 다른 메타정보는
그대로 두거나 필요시 업데이트하여 일관성을 유지하세요.
| struct AuthTokenRefresher: TokenRefresher { | ||
| var destination: Endpoint { AuthEndpoint.reissueToken } | ||
|
|
||
| func refresh(provider: any NetworkProvider) async throws(NetworkError) -> AuthTokens { | ||
| do { | ||
| let tokens: AuthTokens = try await provider.request(endpoint: destination) | ||
| return tokens | ||
| } catch { | ||
| guard let error = error as? NetworkError else { throw .unknownError } | ||
| throw error | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find NetworkProvider definition
rg "protocol NetworkProvider" -A 15Repository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 498
🏁 Script executed:
# Find request method signature
rg "func request.*endpoint.*async throws" -A 5Repository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 1614
🏁 Script executed:
# Find TokenRefresher protocol
rg "protocol TokenRefresher" -A 10Repository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 567
🏁 Script executed:
# Check file structure and imports
fd "AuthTokenRefresher.swift" -x cat -n {}Repository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 803
🏁 Script executed:
# Check for module declaration and access modifiers in the file
fd "AuthTokenRefresher.swift" -x head -30 {}Repository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 649
🏁 Script executed:
# Check DefaultNetworkProvider.processRequest to see what errors it actually throws
rg "func processRequest" -A 20Repository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 2376
🏁 Script executed:
# Check if there are other TokenRefresher implementations
rg "TokenRefresher" -B 2 -A 10Repository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 5817
🏁 Script executed:
# Check what errors are thrown in the network layer
rg "throw.*NetworkError" -B 2 -A 2Repository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 5913
public 접근 제어자를 추가하세요.
struct AuthTokenRefresher는 public protocol TokenRefresher를 구현하므로, 다른 모듈에서 사용할 수 있도록 public 접근 제어자를 명시해야 합니다.
다만 현재의 do-catch 에러 처리는 제거할 수 없습니다. NetworkProvider.request()는 일반 throws(타입이 지정되지 않은)를 사용하고 refresh() 메서드는 throws(NetworkError)(타입이 지정된)을 요구하므로, 제네릭 Error를 NetworkError로 변환하기 위해 do-catch 블록이 필수입니다. 이를 제거하면 컴파일 오류가 발생합니다.
-struct AuthTokenRefresher: TokenRefresher {
+public struct AuthTokenRefresher: TokenRefresher {🤖 Prompt for AI Agents
In @Neki-iOS/Features/Auth/Sources/Data/Sources/AuthTokenRefresher.swift around
lines 10 - 21, Mark the type and its API as public so it satisfies the public
TokenRefresher protocol: change `struct AuthTokenRefresher` to `public struct
AuthTokenRefresher` and make its members public as well (e.g., `public var
destination: Endpoint { AuthEndpoint.reissueToken }` and `public func
refresh(provider: any NetworkProvider) async throws(NetworkError) ->
AuthTokens`). Keep the existing do-catch error conversion inside `refresh()` (do
not remove it) so generic thrown Errors from `NetworkProvider.request()` are
mapped to `NetworkError` as required.
| enum SocialLoginDTO { | ||
| struct Request: Encodable { | ||
| let idToken: String | ||
| } | ||
|
|
||
| typealias Response = TokenPair | ||
| } |
There was a problem hiding this comment.
접근 제어자가 누락되었습니다.
SocialLoginDTO enum과 내부 Request struct에 public 접근 제어자가 없습니다. 다른 모듈에서 이 타입을 사용해야 한다면 접근이 불가능합니다.
🔧 수정 제안
-enum SocialLoginDTO {
- struct Request: Encodable {
+public enum SocialLoginDTO {
+ public struct Request: Encodable {
let idToken: String
}
- typealias Response = TokenPair
+ public typealias Response = TokenPair
}📝 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.
| enum SocialLoginDTO { | |
| struct Request: Encodable { | |
| let idToken: String | |
| } | |
| typealias Response = TokenPair | |
| } | |
| public enum SocialLoginDTO { | |
| public struct Request: Encodable { | |
| let idToken: String | |
| } | |
| public typealias Response = TokenPair | |
| } |
🤖 Prompt for AI Agents
In @Neki-iOS/Features/Auth/Sources/Data/Sources/DTOs/SocialLoginDTO.swift around
lines 10 - 16, SocialLoginDTO and its nested Request are missing public access
control so they’re inaccessible from other modules; update the declaration of
enum SocialLoginDTO and the nested struct Request to be public, and also mark
the idToken property as public if needed for external encoding usage, and ensure
the typealias Response (TokenPair) is public or that TokenPair itself is public
so external modules can use SocialLoginDTO.Response.
| public struct User { | ||
| let nickname: String | ||
| let providerType: ProviderType | ||
| } | ||
|
|
||
| /// 로그인된 사용자의 연결 정보 | ||
| public struct UserSession { | ||
| let user: User | ||
| let tokens: AuthTokens |
There was a problem hiding this comment.
프로퍼티와 이니셜라이저 접근 제어자를 확인해주세요.
User와 UserSession struct가 public으로 선언되었지만, 프로퍼티들은 internal 접근 수준입니다. 다른 모듈에서 이 타입들을 사용할 경우:
- 프로퍼티에 접근할 수 없음
- 자동 생성된 memberwise initializer가
internal이므로 인스턴스 생성 불가
🔧 수정 제안
/// 사용자 정보
public struct User {
- let nickname: String
- let providerType: ProviderType
+ public let nickname: String
+ public let providerType: ProviderType
+
+ public init(nickname: String, providerType: ProviderType) {
+ self.nickname = nickname
+ self.providerType = providerType
+ }
}
/// 로그인된 사용자의 연결 정보
public struct UserSession {
- let user: User
- let tokens: AuthTokens
+ public let user: User
+ public let tokens: AuthTokens
+
+ public init(user: User, tokens: AuthTokens) {
+ self.user = user
+ self.tokens = tokens
+ }
}🤖 Prompt for AI Agents
In @Neki-iOS/Features/Auth/Sources/Domain/Sources/Entities/User.swift around
lines 11 - 19, User and UserSession are public but their stored properties
(nickname, providerType, user, tokens) and the synthesized memberwise
initializers are internal, preventing usage from other modules; make these
properties and/or initializers public so external modules can read and
instantiate them. Update the User struct by marking nickname and providerType as
public (or add a public init on User), and update UserSession by marking user
and tokens as public (or add a public init on UserSession), ensuring
ProviderType and AuthTokens are also accessible publicly where needed.
OneTen19
left a comment
OneTen19
left a comment
There was a problem hiding this comment.
수고많으셨습니다!!
증말 속도가 엄청 빠르시네요..
| let status: Int | ||
| let message: String | ||
| let isSuccess: Bool | ||
| let data: T? | ||
|
|
||
| enum CodingKeys: String, CodingKey { | ||
| case message, data | ||
| case status = "resultCode" | ||
| case isSuccess = "success" | ||
| } | ||
| } |
There was a problem hiding this comment.
p3
아직 API 연결한 게 없어서 변수명 자체를 바꿔도 상관없었을 것 같긴한데 코딩키를 활용하셨군여
There was a problem hiding this comment.
resultCode-status는 크게 상관이 없지만 success 필드는 false도 들어올 수 있는 Bool 타입이라서 isSuccess라는 Swift식 네이밍을 선택했습니다.
| public func setTokenRefresher(_ refresher: TokenRefresher) { tokenRefresher = refresher } | ||
|
|
There was a problem hiding this comment.
p3
tokenRefresher는 init 시점에 받지 않는 이유가 있나요?? 굳이 필요 없을 때 사용할 이유가 없어서인가??
There was a problem hiding this comment.
Constructor Injection이 더 깔끔할 수 있을 것 같네요. 어차피 네트워킹 모델은 하나라서 무조건 주입하게 되어있으니까요. 리팩해보겠습니다.
| func handleUnauthorized(data: Data, endpoint: Endpoint, retryCount: Int) async throws -> Data { | ||
| if endpoint.authorizationType == .reissue || retryCount <= .zero { | ||
| try throwIfServerError(data: data) | ||
| try? tokenStorage.delete() | ||
| throw NetworkError.unauthorizedError |
There was a problem hiding this comment.
p3
request 시점에 processRequest를 호출하며 retry 횟수(1회)를 주입중.
-> processRequest에서 응답 실패일 경우 handleFailure로 전달
-> handleFailure에서 401에러일 경우 handelUnauthorized로 전달
-> handelUnauthorized 에서 retryCount 사용
이 상황인데 주입연쇄가 꽤 길어보이는데 request 시점부터 retry 횟수를 전달하는 이유가 있을까요?
There was a problem hiding this comment.
연쇄의 첫 부분인 processRequest에서 재시도 횟수를 설정하도록 했습니다.
이유는 재시도 과정에서 내부적으로 processRequest를 재귀 호출하기 떄문입니다.
재귀 호출 시점에 재시도 횟수를 차감한 뒤에 다시 연쇄가 돕니다.
| func handleFailure(statusCode: Int, data: Data, endpoint: Endpoint, retryCount: Int) async throws -> Data { | ||
| guard statusCode == 401 else { | ||
| try throwIfServerError(data: data) | ||
| throw mapError(statusCode: statusCode) | ||
| } | ||
|
|
||
| return try await handleUnauthorized(data: data, endpoint: endpoint, retryCount: retryCount) |
There was a problem hiding this comment.
p1
코드를 쭉 확인해봤을때 실질적으로 retry 인자가 사용되는 곳이 handleUnauthorized인데, 정작 retryCount를 조절하는 로직이 안보이는 것 같아요!! 뭔가 제 느낌에는 이 위치에서 retryCount를 1씩 줄여주던가 해야하지 않나 싶은데 한 번 확인부탁드려요!
There was a problem hiding this comment.
func handleUnauthorized(data: Data, endpoint: Endpoint, retryCount: Int) async throws -> Data {
if endpoint.authorizationType == .reissue || retryCount <= .zero {
try throwIfServerError(data: data)
try? tokenStorage.delete()
throw NetworkError.unauthorizedError
}
try await performTokenRefresh()
return try await processRequest(endpoint: endpoint, retryCount: retryCount - 1)
}정확합니다. 음주 코딩 때문인지 재귀호출 자체를 까먹었네요.
performTokenRefresh()로 토큰 갱신 후 문제가 없다면 processRequest를 재귀호출하게 됩니다.
| case .bearer: try appendBearerToken(to: &request) | ||
| case .reissue: try appendRefreshToken(to: &request) | ||
| } | ||
|
|
||
| return request | ||
| } | ||
|
|
||
| func appendBearerToken(to request: inout URLRequest) throws { |
There was a problem hiding this comment.
p3
inout 키워드를 써서 원본 주소값에 접근해 직접 수정하는 방식으로 구현하셨군요!!
저는 개인적으로 반환값을 사용하는 방식을 주로 써서 새롭게 느껴지네요.
private extension DefaultNetworkProvider {
func buildRequest(for endpoint: Endpoint) async throws -> URLRequest {
var request = try endpoint.asURLRequest()
switch endpoint.authorizationType {
case .none: break
case .bearer: request = try appendBearerToken(to: request)
case .reissue: request = try appendRefreshToken(to: request)
}
return request
}
func appendBearerToken(to request: URLRequest) throws -> URLRequest {
var newRequest = request
do {
let tokens = try tokenStorage.fetch()
newRequest.setValue("Bearer \(tokens.accessToken)", forHTTPHeaderField: "Authorization")
return newRequest
} catch ... {
throw ...
}
}
}
저는 함수의 반환값을 -> 을 통해 바로 확인할 수 있다는 게 가독성적으로 좋아보여서 이런식으로 많이 짰었거든요!!
이 방식을 사용하시는 이유가 있으실까요?? 장점이라던지??
There was a problem hiding this comment.
재귀 방식이어서 자연스레 inout 매개변수로 선언한 것 같은데, 질문 주셔서 다시 한 번 이유를 생각해보았습니다.
inout 방식
- 장점: 분기를 처리할 때 원본 URLRequest를 복사하지 않고 직접 변경합니다.
- 단점: 해당 함수가 다른 스코프에 있는 URLRequest를 다루는 사이드이펙트가 존재합니다.
명시적 반환 방식
- 장점: 원본을 건드리지 않고 CICO하여 함수형 프로그래밍 스타일을 유지해, 말씀하신 것처럼 데이터 흐름이 보기 쉽습니다.
- 단점: 호출 시
request = ...처럼 복사본에 대한 재할당 코드를 작성해야 합니다.
현재 재귀호출은 많아봤자 1회(retryCount == 1)이므로 값타입 복사에 대한 리소스 낭비 걱정보다 가독성을 챙기는 트레이드오프가 더 좋은 가치인 것 같습니다.
따라서 '명시적 반환 방식'으로 심볼 변경했습니다.
There was a problem hiding this comment.
근데 일부러 1번만 재귀 호출하시는 건가요?? 막 3회정도는 할 줄...
There was a problem hiding this comment.
그건 이제 서버와의 협의가 필요한 사항인 것 같아서리....
실질적으로 토큰 재발급 후 재요청에서만 1회 카운트 쓰이고, 웬만한 요청에서는 단일 요청, 단일 실패일거라 에러핸들링에 더 집중하면 될 것 같습니다.
| // TODO: Logging 필요 | ||
| print(errorMessage) |
There was a problem hiding this comment.
p3
음?? Logger를 바로 사용하지 않은 이유가 있나요??
There was a problem hiding this comment.
Logger에 String 집어넣으면 되는데, 왜인지 모르게 컴파일 에러가 나서 프린트문으로 대체하고 TODO 작성해두었습니다.
There was a problem hiding this comment.
아 이제보니 깃 추적을 위한 파일인가보군여
.gitkeep 을 쓰는 걸 추천드립니다!
| enum UserInfoDTO { | ||
| struct Response: Decodable { | ||
| let nickname: String | ||
| let providerType: String | ||
|
|
||
| enum CodingKeys: String, CodingKey { | ||
| case providerType | ||
| case nickname = "name" | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
p3
enum으로 한 번 감싸는 이유가 있을까요???
저는 보통 DTO를 만들때 DTO/Response 폴더와 DTO/Request 폴더를 분리해두고
UserInfoResponseDTO 랑 UserInfoRequestDTO를 따로 만들긴 했어요
[DTO/Response]
struct UserInfoResponseDTO: Decodable {
let nickname: String
let providerType: String
enum CodingKeys: String, CodingKey {
case providerType
case nickname = "name"
}
}
[DTO/Request]
struct UserInfoRequestDTO: Decodable {
let nickname: String
let providerType: String
enum CodingKeys: String, CodingKey {
case providerType
case nickname = "name"
}
}
요런 느낌으로?
There was a problem hiding this comment.
이전에 제가 하는 선언방식ㅇ과 같습니다.
| enum ReissueTokenDTO { | ||
| struct Request: Encodable { | ||
| let refreshToken: String | ||
| } | ||
|
|
||
| typealias Response = TokenPair | ||
| } |
There was a problem hiding this comment.
오.. enum으로 만드니까 내부에서 response, request 를 다 지니고 관리할 수 있구나
제 방식에 비해서 파일개수랑 이름명 측면에서 더 좋아보이네요.. 이제 나도 이렇게 해야지
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
Neki-iOS/Core/Sources/Network/Sources/DefaultNetworkProvider.swift (3)
30-31:tokenRefresher를 생성자 주입으로 변경하는 것을 권장합니다.이전 리뷰에서 논의된 것처럼,
setTokenRefresher를 호출하기 전에request가 호출되면performTokenRefresh()에서tokenRefresher가nil이어서unauthorizedError가 발생합니다. 생성자 주입 방식이 더 안전합니다.♻️ 생성자 주입 방식 제안
public init( session: URLSessionProtocol = URLSession.shared, tokenStorage: TokenStorage, + tokenRefresher: TokenRefresher? = nil, decoder: JSONDecoder = JSONDecoder() ) { self.session = session self.tokenStorage = tokenStorage + self.tokenRefresher = tokenRefresher self.decoder = decoder } - -public func setTokenRefresher(_ refresher: TokenRefresher) { tokenRefresher = refresher }
174-182: 추가 HTTP 상태 코드 매핑 고려현재 매핑에서 일반적인 HTTP 오류 코드 일부가 누락되어 있습니다. 필요에 따라 추가를 고려해 주세요:
403: Forbidden (권한 없음)408: Request Timeout429: Too Many Requests (Rate Limiting)♻️ 확장된 상태 코드 매핑 제안
func mapError(statusCode: Int) -> NetworkError { switch statusCode { case 400: return .badRequestError case 401: return .unauthorizedError + case 403: return .forbiddenError // NetworkError에 추가 필요 case 404: return .notFound + case 429: return .rateLimitError // NetworkError에 추가 필요 case 500..<600: return .internalServerError default: return .unknownError } }
197-218: 로그에 민감한 정보 노출 주의
requestLog와responseLog가 전체 헤더와 바디를 로깅합니다. 프로덕션 환경에서는Authorization헤더, 토큰, 비밀번호 등 민감한 정보가 로그에 노출될 수 있습니다.개발 빌드에서만 상세 로깅을 활성화하거나, 민감한 필드를 마스킹하는 것을 고려해 주세요.
♻️ 조건부 로깅 예시
func requestLog(_ request: URLRequest) { `#if` DEBUG Logger.network.debug("➡️ [REQUEST] \(request.httpMethod ?? "") \(request.url?.absoluteString ?? "")") if let headers = request.allHTTPHeaderFields { // Authorization 헤더 마스킹 var sanitizedHeaders = headers if sanitizedHeaders["Authorization"] != nil { sanitizedHeaders["Authorization"] = "[REDACTED]" } Logger.network.debug("🧾 Headers: \(sanitizedHeaders.description)") } // ... body logging `#endif` }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.gitignoreNeki-iOS/Core/Sources/Network/Sources/DefaultNetworkProvider.swift
🧰 Additional context used
🧬 Code graph analysis (1)
Neki-iOS/Core/Sources/Network/Sources/DefaultNetworkProvider.swift (3)
Neki-iOS/Core/Sources/Network/Sources/Base/Endpoint.swift (1)
asURLRequest(46-78)Neki-iOS/Core/Sources/Network/Sources/KeychainTokenStorage.swift (1)
fetch(121-126)Neki-iOS/Features/Auth/Sources/Data/Sources/AuthTokenRefresher.swift (1)
refresh(13-21)
🔇 Additional comments (4)
.gitignore (1)
135-137:.xcconfig파일 제외 처리 승인AppSecret 관련 설정 파일을 버전 관리에서 제외하는 것은 보안상 올바른 접근입니다. 현재 git에 추적 중인
.xcconfig파일이 없으므로 추가 정리 작업은 필요하지 않습니다.선택사항으로 개발자들이 로컬 환경을 설정할 때 참조할 수 있도록
.xcconfig.example같은 템플릿 파일을 추가하는 것을 고려해보세요.Neki-iOS/Core/Sources/Network/Sources/DefaultNetworkProvider.swift (3)
36-47: LGTM!Public API가 깔끔하게 분리되어 있고,
processRequest로 위임하는 구조가 명확합니다.
53-80: LGTM!요청 처리 흐름이 명확하게 분리되어 있습니다:
buildRequest→session.data→ 상태 코드 검증 →handleFailure. 역할별 분리가 잘 되어 있습니다.
149-162: LGTM!Actor 모델을 활용하여
refreshTask캐싱으로 동시 토큰 갱신 요청을 안전하게 처리합니다. Actor의 직렬화 특성 덕분에 race condition이 방지됩니다.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| func appendRefreshToken(to request: URLRequest) throws -> URLRequest { | ||
| var request = request | ||
|
|
||
| do { | ||
| let tokens = try tokenStorage.fetch() | ||
| let body = ["refreshToken": tokens.refreshToken] | ||
| request.httpBody = try JSONEncoder().encode(body) | ||
| request.setValue("application/json", forHTTPHeaderField: "Content-Type") | ||
| return request | ||
| } catch TokenStorageError.notFound { | ||
| throw NetworkError.unauthorizedError | ||
| } catch let error as EncodingError { | ||
| Logger.network.error("❌ Encoding Error: \(error.localizedDescription)") | ||
| throw error | ||
| } catch { | ||
| Logger.network.error("❌ Token Fetch Error: \(error.localizedDescription)") | ||
| throw error | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's explore the repository structure
git ls-files | grep -E "(Endpoint|Network)" | head -20Repository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 777
🏁 Script executed:
# Find the Endpoint definition and asURLRequest implementation
fd Endpoint --type fRepository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 195
🏁 Script executed:
# Look for reissue endpoint definitions
rg "authorizationType.*reissue" -A 5 -B 5 --type swiftRepository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 1213
🏁 Script executed:
# Examine the Endpoint definition to understand body handling
rg "struct Endpoint|enum Endpoint" -A 30 --type swiftRepository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 57
🏁 Script executed:
# Let's examine the Endpoint.swift to understand body handling
cat -n Neki-iOS/Core/Sources/Network/Sources/Base/Endpoint.swiftRepository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 2672
🏁 Script executed:
# Examine the AuthEndpoint.swift to see reissue endpoint definitions
cat -n Neki-iOS/Features/Auth/Sources/Data/Sources/AuthEndpoint.swiftRepository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 501
🏁 Script executed:
# Look at the full DefaultNetworkProvider.swift to see the context
wc -l Neki-iOS/Core/Sources/Network/Sources/DefaultNetworkProvider.swiftRepository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 145
🏁 Script executed:
# Let's read the DefaultNetworkProvider.swift around the lines mentioned in the review
sed -n '85,135p' Neki-iOS/Core/Sources/Network/Sources/DefaultNetworkProvider.swiftRepository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 1889
🏁 Script executed:
# Let's see the buildRequest method and its full context
sed -n '1,150p' Neki-iOS/Core/Sources/Network/Sources/DefaultNetworkProvider.swiftRepository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 5127
🏁 Script executed:
# Search for all Endpoint implementations to see if any use reissue
rg "extension.*Endpoint.*Endpoint\|enum.*Endpoint\|struct.*Endpoint" -A 20 --type swiftRepository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 57
🏁 Script executed:
# Search for authorizationType = .reissue usage
rg "authorizationType.*\.reissue\|\.reissue.*authorizationType" -B 2 -A 2 --type swiftRepository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 57
🏁 Script executed:
# Check if there's any documentation in comments about reissue endpoint requirements
rg "reissue|Reissue" -B 3 -A 3 --type swiftRepository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 4490
🏁 Script executed:
# Check the AuthTokenRefresher to see how reissueToken is used
cat -n Neki-iOS/Features/Auth/Sources/Data/Sources/AuthTokenRefresher.swiftRepository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 803
🏁 Script executed:
# Check ReissueTokenDTO to understand the request structure
cat -n Neki-iOS/Features/Auth/Sources/Data/Sources/DTOs/ReissueTokenDTO.swiftRepository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 426
🏁 Script executed:
# Look for any TODOs or comments about AuthEndpoint implementation
rg "AuthEndpoint|Endpoint.*Endpoint" -B 5 -A 10 --type swiftRepository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 3217
🏁 Script executed:
# Final check: are there any other implementations of Endpoint protocol in the codebase
rg "extension.*:\s*Endpoint\|struct.*:\s*Endpoint\|enum.*:\s*Endpoint" --type swiftRepository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 57
appendRefreshToken에서 httpBody 덮어쓰기 관련 설계 정의 필요
appendRefreshToken이 request.httpBody를 새로운 JSON으로 무조건 덮어씁니다. 현재 AuthEndpoint는 Endpoint 프로토콜을 구현하지 않아 실제 문제가 없지만, 구현 시점에 endpoint.body가 설정되어 있다면 그 값이 손실됩니다.
authorizationType == .reissue인 엔드포인트는 항상 body가 nil이어야 한다는 규칙을 명시적으로 문서화하거나, Endpoint.asURLRequest()에서 설정된 기존 body와의 병합 방식을 고려해 주세요.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@Neki-iOS/Core/Sources/Network/Sources/Base/Multipart.swift`:
- Around line 27-45: MultipartFormField is declared public but its stored
properties name and value are internal, preventing instantiation outside the
module; make the properties public or add a public initializer so external code
can create instances. Locate the public struct MultipartFormField and either
mark its properties as public (public let name: String; public let value: Any)
or add a public init(name:value:) initializer that assigns to the existing
properties and keep any existing access levels intact; ensure access levels are
consistent so the public type is constructible from other modules.
- Around line 48-57: The struct MultipartFile is public but its stored
properties (name, data, fileName, mimeType) are internal, preventing callers
outside the module from constructing instances; make each property public and
add an explicit public initializer (public init(name:data:fileName:mimeType:))
so external code can create MultipartFile instances while keeping the existing
public func append(to:) unchanged.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Neki-iOS/Core/Sources/Network/Sources/Base/Endpoint.swiftNeki-iOS/Core/Sources/Network/Sources/Base/Multipart.swift
🧰 Additional context used
🧬 Code graph analysis (1)
Neki-iOS/Core/Sources/Network/Sources/Base/Endpoint.swift (2)
Neki-iOS/Core/Sources/Network/Sources/DefaultNetworkProvider.swift (2)
request(36-38)request(44-47)Neki-iOS/Core/Sources/Network/Sources/Base/Multipart.swift (6)
append(11-14)append(31-44)append(54-56)append(66-72)append(75-82)finalize(85-89)
🪛 SwiftLint (0.57.0)
Neki-iOS/Core/Sources/Network/Sources/Base/Multipart.swift
[Warning] 39-39: TODOs should be resolved (Date 등 다양한 타입 지원이 필요하면 이곳에 케이스...)
(todo)
🔇 Additional comments (7)
Neki-iOS/Core/Sources/Network/Sources/Base/Endpoint.swift (4)
10-21: LGTM!인증 타입에 대한 문서화가 잘 되어있고,
.reissue케이스의 특수한 용도에 대한 경고도 적절합니다.
23-28: LGTM!Content-Type 종류를 열거형으로 명확하게 정의했습니다.
multipart케이스에 boundary를 associated value로 포함한 설계가 적절합니다.
30-41: LGTM!Endpoint 프로토콜이 인증, 콘텐츠 타입, 쿼리 파라미터, 멀티파트 아이템 등 필요한 요소들을 명확하게 정의하고 있습니다.
50-105: LGTM!
asURLRequest()구현이 세 가지 콘텐츠 타입(JSON, multipart, raw)을 잘 처리하고 있습니다. URL 구성, 쿼리 파라미터 매핑, 바디 인코딩 로직이 올바르게 구현되어 있습니다.Neki-iOS/Core/Sources/Network/Sources/Base/Multipart.swift (3)
10-15: LGTM!UTF-8 변환 실패에 대한 적절한 에러 처리가 포함되어 있습니다.
17-24: LGTM!
MultipartItem프로토콜과MultipartError열거형이 간결하고 명확하게 정의되어 있습니다.
59-89: LGTM!
MultipartItemBuilder가 올바르게 구현되어 있습니다. boundary 기반의 multipart 형식이 RFC 2046 표준에 맞게 CRLF와 헤더를 적절히 처리하고 있으며, public initializer가 제공되어 모듈 외부에서도 사용 가능합니다.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| public struct MultipartFormField: MultipartItem { | ||
| let name: String | ||
| let value: Any | ||
|
|
||
| public func append(to builder: inout MultipartItemBuilder) throws { | ||
| let stringValue: String | ||
|
|
||
| switch value { | ||
| case let string as String: stringValue = string | ||
| case let int as Int: stringValue = String(int) | ||
| case let bool as Bool: stringValue = bool ? "true" : "false" | ||
| case let double as Double: stringValue = String(double) | ||
| // TODO: Date 등 다양한 타입 지원이 필요하면 이곳에 케이스 추가 | ||
| default: throw MultipartError.unsupportedValueType | ||
| } | ||
|
|
||
| try builder.append(value: stringValue, name: name) | ||
| } | ||
| } |
There was a problem hiding this comment.
public struct의 프로퍼티에 public 접근자가 누락되었습니다.
MultipartFormField가 public struct로 선언되어 있지만, name과 value 프로퍼티가 internal(기본값)로 되어 있어 모듈 외부에서 인스턴스를 생성할 수 없습니다. 외부에서 사용하려면 public initializer 또는 public 프로퍼티가 필요합니다.
🔧 수정 제안
/// 단순한 키-값 파라미터 멀티파트 아이템
public struct MultipartFormField: MultipartItem {
- let name: String
- let value: Any
+ public let name: String
+ public let value: Any
+
+ public init(name: String, value: Any) {
+ self.name = name
+ self.value = value
+ }
public func append(to builder: inout MultipartItemBuilder) throws {🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 39-39: TODOs should be resolved (Date 등 다양한 타입 지원이 필요하면 이곳에 케이스...)
(todo)
🤖 Prompt for AI Agents
In `@Neki-iOS/Core/Sources/Network/Sources/Base/Multipart.swift` around lines 27 -
45, MultipartFormField is declared public but its stored properties name and
value are internal, preventing instantiation outside the module; make the
properties public or add a public initializer so external code can create
instances. Locate the public struct MultipartFormField and either mark its
properties as public (public let name: String; public let value: Any) or add a
public init(name:value:) initializer that assigns to the existing properties and
keep any existing access levels intact; ensure access levels are consistent so
the public type is constructible from other modules.
| public struct MultipartFile: MultipartItem { | ||
| let name: String | ||
| let data: Data | ||
| let fileName: String | ||
| let mimeType: String | ||
|
|
||
| public func append(to builder: inout MultipartItemBuilder) throws { | ||
| try builder.append(data: data, name: name, fileName: fileName, mimeType: mimeType) | ||
| } | ||
| } |
There was a problem hiding this comment.
동일한 접근자 누락 문제
MultipartFile도 public struct이지만 프로퍼티들이 internal로 되어 있어 모듈 외부에서 인스턴스를 생성할 수 없습니다.
🔧 수정 제안
/// 파일 데이터 멀티파트 아이템
public struct MultipartFile: MultipartItem {
- let name: String
- let data: Data
- let fileName: String
- let mimeType: String
+ public let name: String
+ public let data: Data
+ public let fileName: String
+ public let mimeType: String
+
+ public init(name: String, data: Data, fileName: String, mimeType: String) {
+ self.name = name
+ self.data = data
+ self.fileName = fileName
+ self.mimeType = mimeType
+ }
public func append(to builder: inout MultipartItemBuilder) throws {🤖 Prompt for AI Agents
In `@Neki-iOS/Core/Sources/Network/Sources/Base/Multipart.swift` around lines 48 -
57, The struct MultipartFile is public but its stored properties (name, data,
fileName, mimeType) are internal, preventing callers outside the module from
constructing instances; make each property public and add an explicit public
initializer (public init(name:data:fileName:mimeType:)) so external code can
create MultipartFile instances while keeping the existing public func
append(to:) unchanged.
OneTen19
left a comment
OneTen19
left a comment
There was a problem hiding this comment.
수고하셨씁니더
서버측에서 기본 응답 통일시켜주신다고 했으니 고거만 한 번 받아서 수정하면 머지해도 될 듯!
🌴 작업한 브랜치
add/#35-auth-endpoint✅ 작업한 내용
개요
Network Layer의 구조적 안정성 확보와 동시성 문제 해결, 그리고 SOLID 원칙 준수라는 목적을 가지고 작업을 진행했습니다.
이를 기반하여 Auth-Feature의 도메인 모델, 토큰 관리 로직을 구상해보았습니다.
주요 변경사항
KeychainTokenStorage 구현
TokenStorage를 선언하고 Keychain 기반의KeychainTokenStroage를 구현했습니다.Endpoint 역할 변경
HTTPContentType열거형에 케이스를 추가하고 URLRequest를 빌드할 때 분기 로직을 작성해주면 됩니다.토큰 재발급 및 재시도 로직 구현
TokenRefresher,NetworkProvider의 도움을 받아 구현했습니다.이제 기본서버응답이 두 가지로 관리됩니다. (기본성공응답, 기본실패응답)
❗️PR Point
📟 관련 이슈
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선사항
정리
✏️ Tip: You can customize this high-level summary in your review settings.