Network [#61] 소셜 로그인 API 수정 및 AccessToken 관리

[Heyjooo]

👻 PULL REQUEST

💻 작업한 내용

• 소셜 로그인 API 수정
• AccessToken 관리

💡 참고사항

• KeychainWrapper를 설정하여 accessToken과 refreshToken을 관리합니다.
  

  Don-tBe-iOS/DontBe-iOS/DontBe-iOS/Network/Foundation/KeychainWrapper.swift

  Lines 8 to 110 in bf6c8dd

  	import Foundation
  	import Security
  	class KeychainWrapper {
  	static let serviceName = "com.SOPT33.DontBe-iOS"
  	// 토큰을 Keychain에 저장하는 함수
  	// - parameter token: 저장할 토큰
  	// - parameter key: Keychain에 저장될 키
  	// - parameter access: 추가적인 접근 제어 설정 (기본값은 nil)
  	static func saveToken(_ token: String, forKey key: String, withAccess access: SecAccessControl? = nil) {
  	// 해당 키에 대한 토큰이 이미 존재하는지 확인
  	if let existingToken = loadToken(forKey: key) {
  	// 토큰이 이미 존재하면 업데이트 또는 필요에 따라 처리
  	if existingToken != token {
  	// 토큰이 다르면 업데이트
  	updateToken(token, forKey: key)
  	}
  	return
  	}
  	// 토큰이 존재하지 않으면 저장 진행
  	if let data = token.data(using: .utf8) {
  	var query: [String: Any] = [
  	kSecClass as String: kSecClassGenericPassword,
  	kSecAttrService as String: serviceName,
  	kSecAttrAccount as String: key,
  	kSecValueData as String: data,
  	kSecAttrAccessible as String: kSecAttrAccessibleWhenUnlockedThisDeviceOnly
  	]
  	if let accessControl = access {
  	query[kSecAttrAccessControl as String] = accessControl
  	}
  	let status = SecItemAdd(query as CFDictionary, nil)
  	if status != errSecSuccess {
  	print("토큰을 Keychain에 저장하는 데 실패했습니다. 에러 코드: \(status)")
  	}
  	}
  	}
  	// Keychain에서 특정 키에 대한 토큰을 불러오는 함수
  	// - parameter key: 불러올 토큰의 키
  	// - returns: 키에 대한 토큰이 존재하면 해당 토큰을 반환, 그렇지 않으면 nil 반환
  	static func loadToken(forKey key: String) -> String? {
  	let query: [String: Any] = [
  	kSecClass as String: kSecClassGenericPassword,
  	kSecAttrService as String: serviceName,
  	kSecAttrAccount as String: key,
  	kSecReturnData as String: kCFBooleanTrue!,
  	kSecMatchLimit as String: kSecMatchLimitOne
  	]
  	var data: AnyObject?
  	let status = SecItemCopyMatching(query as CFDictionary, &data)
  	if status == errSecSuccess, let retrievedData = data as? Data {
  	return String(data: retrievedData, encoding: .utf8)
  	} else {
  	print("Keychain에서 토큰을 불러오는 데 실패했습니다.")
  	return nil
  	}
  	}
  	// Keychain에서 특정 키에 대한 토큰을 삭제하는 함수
  	// - parameter key: 삭제할 토큰의 키
  	static func deleteToken(forKey key: String) {
  	let query: [String: Any] = [
  	kSecClass as String: kSecClassGenericPassword,
  	kSecAttrService as String: serviceName,
  	kSecAttrAccount as String: key
  	]
  	let status = SecItemDelete(query as CFDictionary)
  	if status != errSecSuccess {
  	print("Keychain에서 토큰을 삭제하는 데 실패했습니다.")
  	}
  	}
  	// Keychain에 저장된 특정 키에 대한 토큰을 업데이트하는 함수
  	// - parameter token: 업데이트할 토큰
  	// - parameter key: 업데이트할 토큰의 키
  	private static func updateToken(_ token: String, forKey key: String) {
  	if let data = token.data(using: .utf8) {
  	let query: [String: Any] = [
  	kSecClass as String: kSecClassGenericPassword,
  	kSecAttrService as String: serviceName,
  	kSecAttrAccount as String: key
  	]
  	let attributes: [String: Any] = [
  	kSecValueData as String: data
  	]
  	let status = SecItemUpdate(query as CFDictionary, attributes as CFDictionary)
  	if status != errSecSuccess {
  	print("Keychain에서 토큰을 업데이트하는 데 실패했습니다. 에러 코드: \(status)")
  	}
  	}
  	}
  	}

📟 관련 이슈

• Resolved: [Feat] 소셜 로그인 API 수정 및 AccessToken 관리  #61

[github-actions]

아래는 코드 패치입니다. 해당 코드를 간단히 검토해 드리겠습니다. 버그의 위험성 및 개선 제안을 환영합니다.

• 다음과 같은 파일들이 추가되었습니다:

  • "HttpMethod.swift" 파일을 Sources 그룹에 추가하였습니다.
  • "NetworkServiceType.swift" 파일을 Sources 그룹에 추가하였습니다.
  • "KeychainWrapper.swift" 파일을 Sources 그룹에 추가하였습니다.

• 다음과 같은 파일 이름이 변경되었습니다:

  • "SocialLoginService.swift" 파일이 "NetworkService.swift"로 변경되었습니다.

위 내용을 토대로 코드가 올바르게 동작하는지 확인해야 합니다.

[github-actions]

이 코드 패치에 대해 간단한 코드 리뷰를 도와드리겠습니다. 어떤 버그나 위험이 있는지 및 개선 제안을 알려드리겠습니다.

1. 첫번째 변경 사항:

   • networkProvider에 대한 의존성 주입 시 변경이 이루어짐.
   • 기존 코드: SocialLoginService()
   • 변경된 코드: NetworkService()
   • 개선 제안: NetworkService()는 어떤 서비스를 정확히 제공하는지 알 수 없으므로, 보다 구체적인 이름을 지정하는 것이 좋습니다. 예를 들어 SocialNetworkService()와 같은 이름을 사용할 수 있습니다.

2. 두 번째 변경 사항:

   • 여전히 rootViewController에 대한 설정 변경
   • 개선 제안: 첫 번째 조건문의 else 구문과 세 번째 조건문의 else 구문이 동일한 코드를 가지고 있습니다. 중복 코드를 피하기 위해 이 두 구문을 별도의 함수로 추출하고, 필요한 경우 해당 함수를 호출하여 중복을 제거하는 것이 좋습니다.

3. 오류 가능성: loadUserData()의 사용

   • loadUserData() 함수의 반환 값을 반복해서 검사하여 로드하고 있습니다.
   • loadUserData()가 매번 데이터를 로드하면 성능 저하의 원인이 될 수 있으므로, 이를 개선할 수 있는 방법을 고려해보세요. 예를 들어, 데이터를 한 번 로드한 후 전역 변수에 저장하여 불필요한 반복 로드를 피하는 것이 좋습니다.

4. 의존성 주입:

   • 의존성 주입(Dependency Injection)은 좋은 설계 원칙입니다. 하지만 현재 코드에서는 SceneDelegate 클래스에서 직접 의존성을 주입하고 있어, 클래스가 비대해지고 단일 책임 원칙(Single Responsibility Principle)에 어긋날 수 있습니다. 가능하다면, 의존성 주입 프레임워크나 컨테이너를 사용하여 의존성 관리를 효율적으로 할 수 있습니다.

이러한 변경 사항과 개선 제안을 고려하여 코드를 검토하고 수정하시면 됩니다. 감사합니다!

[github-actions]

@@ -48,8 +48,8 @@ enum StringLiterals {
}

enum Onboarding {

• static let placeHolder = "한문장으로 소개를 남겨주세요!"
• static let information = "설정한 사진, 닉네임, 한줄소개는 설정에서 변경 가능해요!\n작성한 한 줄 소개는 작성한 게시글로 업로드 돼요."

• static let placeHolder = "한 문장으로 소개를 남겨주세요!"
• static let information = "설정한 닉네임, 한줄소개는 설정에서 변경 가능해요!\n작성한 한 줄 소개는 작성한 게시글로 업로드 돼요."
  }

enum Button {

위 코드 패치에 대해 짧은 코드 리뷰를 도와드리겠습니다. 버그 위험과/또는 개선 제안을 환영합니다:
@@ -48,8 +48,8 @@ enum StringLiterals {
}

enum Onboarding {

• static let placeHolder = "한문장으로 소개를 남겨주세요!"
• static let information = "설정한 사진, 닉네임, 한줄소개는 설정에서 변경 가능해요!\n작성한 한 줄 소개는 작성한 게시글로 업로드 돼요."

• static let placeHolder = "한 문장으로 소개를 남겨주세요!"
• static let information = "설정한 닉네임, 한줄소개는 설정에서 변경 가능해요!\n작성한 한 줄 소개는 작성한 게시글로 업로드 돼요."
  }

enum Button {

이 코드 패치의 문제나 개선할 점은 없는 것으로 보입니다. 단지 한 문구에 띄어쓰기를 추가하고 있습니다. 처음 코드가 한문장("한문장으로 소개를 남겨주세요!")으로 작성돼 있는 반면 패치된 코드에서 띄어쓰기가 추가돼 "한 문장으로 소개를 남겨주세요!"로 변경됩니다. 또한, Onboarding 열거형 내 information 값을 간소화하고 사진을 닉네임으로 수정해 주었습니다. 이 변경 사항은 규모가 크게 영향을 미치지 않는 수정으로 보입니다.

[github-actions]

해당 코드는 "NetworkServiceType"라는 프로토콜을 정의하고 있습니다. 이 프로토콜은 다양한 네트워크 작업을 수행하기 위한 메서드를 선언하고 있습니다.

주요 기능:

• donMakeRequest: HttpRequest를 생성하는 메서드입니다. HttpMethod(요청의 종류), baseURL, accessToken, body(전송할 데이터), pathVariables(경로 변수) 등을 인자로 받습니다. URLRequest를 반환합니다.
• donNetwork: 네트워크 요청을 수행하고 서버로부터 응답을 받아와 처리하는 메서드입니다. async/await를 사용하여 비동기적으로 동작합니다. HttpMethod, baseURL, accessToken, body, pathVariables를 인자로 받으며, 응답 데이터를 Decodable 타입으로 변환하여 반환합니다. 오류가 발생할 경우 throw 됩니다.

개선 사항:

• 현재 코드에서 알려진 버그 또는 개선 사항은 없어 보입니다.
• 하지만 코드 전체를 볼 때 사용되는 함수나 객체들의 내용이 완전하지 않아 해당 파일만으로는 전체 시스템의 동작을 파악하기 어렵습니다.
• 코드 리뷰 및 테스트 과정을 거쳐 전체 시스템 기능 및 성능 면에서 최적화 여지가 있는지 확인하는 것이 좋습니다.

[github-actions]

해당 코드 패치는 HttpMethod라는 열거형을 정의하고, HTTP 메서드를 나타내는 값을 반환하는 method 속성을 가지고 있습니다. 각각의 HTTP 메서드 (GET, POST, DELETE, PATCH) 에 대한 case가 정의되어 있으며, 해당 case에 따라 적절한 문자열 값을 반환합니다.

이 코드 패치는 보안상 이슈나 버그의 위험이 없으며, 간단하면서도 명확하게 작성된 것으로 보입니다. 작업하는 프로젝트에서 필요한 HTTP 메서드 상수들을 정의하고 사용할 수 있는 유용한 기능을 제공합니다.

[github-actions]

이 코드는 KeychainWrapper 클래스를 포함하는 파일의 코드 패치입니다. 이 클래스는 토큰을 Keychain에 저장하고 불러오며 삭제 및 업데이트하는 기능을 제공합니다.

여기 몇 가지 개선점과 버그 위험성이 있습니다:

• 토큰을 저장할 때, 이미 해당 키에 대한 토큰이 존재하는지 확인하고 있습니다. 그러나없는 경우 저장 진행 중에 이미 동일한 키에 다른 스레드에서 저장하는 경우에도 업데이트가 발생할 수 있습니다.
• loadToken 메서드에서 토큰을 불러올 때, 오류 발생 시 nil을 반환하고 에러메시지만 출력합니다. 일부 클라이언트 코드에서 이 에러를 처리해야 할 수 있으므로, nil 대신 에러를 던지는 것이 더 좋은 방법일 수 있습니다.
• 인스턴스화될 필요가 없는 정적 유틸리티 함수들은 static으로 선언되어 있습니다. 그러나 클래스 자체를 인스턴스화하지 않아도 되므로 클래스 말고 구조체(struct)로 변경하여 더욱 명확하게 표현할 수 있습니다.
• 에러 메시지 출력은 콘솔에 출력되도록 되어 있지만, 실제 애플리케이션의 콘솔에서 확인할 수 없습니다. 대신 로깅 라이브러리를 사용하거나 에러를 전달하고 호출하는 코드 쪽에서 오류를 처리할 수 있도록 수정하는 것이 좋습니다.

이러한 개선점과 주의사항을 고려하여 코드에서 버그나 위험성은 발견되지 않았습니다.

[github-actions]

코드 리뷰에 대한 답변:

1. DonMakeRequest 메서드에서 urlComponents.queryItems를 반복할 때마다 재할당하고 있습니다. 이렇게 하면 마지막에 설정된 값만 남고, 이전 값들은 덮어씌워집니다. 따라서 모든 path 변수를 포함하도록 수정해야 합니다.
2. 예외 처리 부분에서 catch 절에서 에러를 다시 throw하는 코드가 있습니다. 이는 필요하지 않으므로 제거해도 됩니다.
3. URLRequest의 header 설정 시, key-value 쌍을 순회하며 추가하고 있습니다. 하지만 자체적으로 request.allHTTPHeaderFields를 사용하여 한 번에 헤더를 추가하는 것이 더 간결합니다.
4. 응답 코드 검사 시, 범위를 200..<300으로 설정하였는데 요청이 성공했을 때만 해당 범위에 속한다고 가정합니다. 만약 일부 요청에 대한 성공 응답 코드가 다른 범위에 속한다면, 상황에 맞게 조정해야 합니다.
5. dump(request)는 디버깅 목적으로 사용되는데, 실제로 릴리즈 버전에서는 제거하는 것이 좋습니다.

추가적인 개선 사항:

1. URLComponents를 사용할 때 baseURL을 직접 지정하고 있습니다. baseURL을 URLComponents의 host 속성이나 scheme 속성으로 설정하는 것이 확장성과 유지 보수에 더 좋습니다.
2. donMakeRequest와 donNetwork 메서드에서 body 파라미터를 Encodable로 받고 있습니다. 이는 모든 요청에 body가 필요한 경우에만 사용하는 것이 좋습니다. 불필요한 경우, Optional로 설정하거나 다른 메서드를 추가하여 body를 받지 않도록 할 수 있습니다.
3. donNetwork 메서드에서 T? 타입으로 응답을 반환하고 있습니다. 응답이 항상 존재해야 하는 경우, 옵셔널이 아니라 강제 언래핑을 제거하면 안전합니다.

위의 피드백은 코드 리뷰를 위한 것이며, 실제 상황과 요구사항에 맞게 적용해야합니다.

[github-actions]

I'm sorry, but I cannot predict the future as my knowledge is based on information available up until September 2021.

[github-actions]

이 코드 패치에서는 UserInfo 구조체에 isFirstUser라는 속성이 추가되었습니다. 코드 리뷰를 진행하면서 발견된 버그 위험과 개선 제안은 다음과 같습니다:

1. 변수 이름: 현재 isFirstUser 변수의 이름은 이해하기 어려울 수 있습니다. 사용자가 최초로 앱을 사용하는지 여부를 나타내는 불리언 값을 의미하는 것으로 추측됩니다. 변수 이름을 보다 명확하고 이해하기 쉽게 변경하는 것이 좋습니다. 예를 들어, isInitialUser와 같은 이름을 고려해 볼 수 있습니다.

2. isJoinedApp 변수: 현재 코드에서 isJoinedApp 변수가 어떤 목적으로 사용되는지 확실하지 않습니다. 변수 이름을 통해 의도가 명확히 전달되어야 합니다. 변수가 필요하지 않거나 목적이 명확하게 정의될 수 있도록 변경할 수 있는지 확인해 보세요.

3. 멤버 변수의 접근 제어: 현재 모든 변수는 기본적으로 public으로 선언되었습니다. 변수의 공개 범위가 필요한 만큼만 되도록 private, fileprivate 또는 internal로 수정해야 합니다. 데이터 캡슐화 및 코드 유지보수성을 향상시키기 위해 적절한 접근 제어를 사용하세요.

4. 코드 주석: 코드에는 아직 주석이 없습니다. 각 변수의 역할과 용도를 설명하는 주석을 추가하는 것이 좋습니다.

위의 지적 사항을 고려하여 코드를 개선하면 더 명확하고 유지보수하기 쉬운 코드가 될 수 있습니다.

[github-actions]

위 코드 패치를 간단히 검토해 드리겠습니다. 버그나 개선 제안이 있다면 언제든 환영합니다:

오류 위험:

• 현재 코드에서는 오류 또는 버그의 특정 징후를 찾지 못했습니다.

개선 제안:

• 105번째 줄에 있는 saveUserData 메서드 호출에서 isFirstUser 변수가 추가되었습니다. 처음 사용자인지 여부를 나타내는 변수입니다.
• isFirstUser 변수 이름을 명확하게 지정하여 코드 가독성을 향상시킬 수 있습니다. 예를 들어 isNewUser와 같이 변경할 수 있습니다.

이외에는 적절한 수정 사항을 찾지 못했습니다.

[boogios]

P3
네트워크 서비스 너무 잘만드셨네요!!! 고생하셨습니다~!!

[boogios]

P3
donNetwork 너무 좋네요!

[boogios]

P3
토큰 처리 수고하셨습니다!!! 깔끔하네요~!

[github-actions]

이 코드 패치는 다음과 같은 변경 사항을 가지고 있습니다.

1. Onboarding 열거형의 placeHolder 값이 "한문장으로 소개를 남겨주세요!"에서 "한 문장으로 소개를 남겨주세요!"로 변경되었습니다.
2. Onboarding 열거형의 information 값이 "설정한 사진, 닉네임, 한줄소개는 설정에서 변경 가능해요!\n작성한 한 줄 소개는 작성한 게시글로 업로드 돼요."에서 "설정한 닉네임, 한 줄 소개는 설정에서 변경 가능해요!\n작성한 한 줄 소개는 작성한 게시글로 업로드 돼요."로 변경되었습니다.

이 코드 패치에 대한 버그 리스크나 개선 제안을 주지 못하고 있습니다. 해당 코드 조각만 보여주셔서 더 자세한 리뷰를 제공하기 어렵습니다.