Skip to content

Commit

Permalink
Use the user ID instead of a random token UID for GoogleDriveCredential
Browse files Browse the repository at this point in the history
fixes a bug which allowed credential duplicates
  • Loading branch information
phil1995 committed Oct 13, 2021
1 parent 7ae0767 commit 15fcc6d
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ public class GoogleDriveCloudProvider: CloudProvider {
self.identifierCache = try GoogleDriveIdentifierCache()
self.runningTickets = [GTLRServiceTicket]()
self.runningFetchers = [GTMSessionFetcher]()
setupDriveService(credential: credential, useBackgroundSession: useBackgroundSession)
try setupDriveService(credential: credential, useBackgroundSession: useBackgroundSession)
}

private func setupDriveService(credential: GoogleDriveCredential, useBackgroundSession: Bool) {
private func setupDriveService(credential: GoogleDriveCredential, useBackgroundSession: Bool) throws {
driveService.serviceUploadChunkSize = GoogleDriveCloudProvider.maximumUploadFetcherChunkSize
driveService.isRetryEnabled = true
driveService.retryBlock = { _, suggestedWillRetry, fetchError in
Expand All @@ -55,7 +55,7 @@ public class GoogleDriveCloudProvider: CloudProvider {
configuration.sharedContainerIdentifier = GoogleDriveSetup.constants.sharedContainerIdentifier
}
let bundleId = Bundle.main.bundleIdentifier ?? ""
configuration = URLSessionConfiguration.background(withIdentifier: "Crytomator-GoogleDriveSession-\(credential.tokenUID)-\(bundleId)")
configuration = URLSessionConfiguration.background(withIdentifier: "Crytomator-GoogleDriveSession-\(try credential.getAccountID())-\(bundleId)")
configuration.sharedContainerIdentifier = GoogleDriveSetup.constants.sharedContainerIdentifier
} else {
configuration = URLSessionConfiguration.default
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,30 +15,34 @@ import Promises
public enum GoogleDriveCredentialError: Error {
case notAuthenticated
case noUsername
case noAccountID
}

public class GoogleDriveCredential {
let keychainItemPrefix = "GoogleDriveAuth"
private let keychainItemName: String
public let tokenUID: String
public var authorization: GTMAppAuthFetcherAuthorization?
public let driveService: GTLRDriveService
public var isAuthorized: Bool {
authorization?.canAuthorize() ?? false
}

public init(tokenUID: String = UUID().uuidString) {
self.tokenUID = tokenUID
self.keychainItemName = keychainItemPrefix + "-" + tokenUID
self.authorization = GTMAppAuthFetcherAuthorization(fromKeychainForName: keychainItemName)
var authorization: GTMAppAuthFetcherAuthorization?
private static let keychainItemPrefix = "GoogleDriveAuth"

public init(userID: String? = nil) {
if let userID = userID {
let keychainItemName = GoogleDriveCredential.getKeychainName(forUserID: userID)
self.authorization = GTMAppAuthFetcherAuthorization(fromKeychainForName: keychainItemName)
} else {
self.authorization = nil
}
self.driveService = GTLRDriveService()
driveService.authorizer = authorization
}

public func save(authState: OIDAuthState) {
authorization = GTMAppAuthFetcherAuthorization(authState: authState)
driveService.authorizer = authorization
if let authorization = authorization {
if let authorization = authorization, let userID = authorization.userID {
let keychainItemName = GoogleDriveCredential.getKeychainName(forUserID: userID)
GTMAppAuthFetcherAuthorization.save(authorization, toKeychainForName: keychainItemName)
}
}
Expand All @@ -53,10 +57,27 @@ public class GoogleDriveCredential {
return userEmail
}

public func getAccountID() throws -> String {
guard isAuthorized else {
throw GoogleDriveCredentialError.notAuthenticated
}
guard let userID = authorization?.userID else {
throw GoogleDriveCredentialError.noAccountID
}
return userID
}

public func deauthenticate() {
GTMAppAuthFetcherAuthorization.removeFromKeychain(forName: keychainItemName)
if let authorization = authorization, let userID = authorization.userID {
let keychainItemName = GoogleDriveCredential.getKeychainName(forUserID: userID)
GTMAppAuthFetcherAuthorization.removeFromKeychain(forName: keychainItemName)
}
driveService.fetcherService.resetSession()
authorization = nil
driveService.authorizer = nil
}

private static func getKeychainName(forUserID userID: String) -> String {
return keychainItemPrefix + userID
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class GoogleDriveAuthenticatorMock {
let tokenParameters = ["refresh_token": refreshToken as NSString]
let tokenResponse = OIDTokenResponse(request: tokenRequest, parameters: tokenParameters)
let authState = OIDAuthState(authorizationResponse: authResponse, tokenResponse: tokenResponse)
let credential = GoogleDriveCredential(tokenUID: tokenUID)
let credential = GoogleDriveCredential(userID: tokenUID)
credential.save(authState: authState)
return credential
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@
//

import XCTest

#if canImport(CryptomatorCloudAccessCore)
@testable import CryptomatorCloudAccessCore
#else
@testable import CryptomatorCloudAccess
#endif
class GoogleDriveAuthenticatorMockTests: XCTestCase {
/**
It is necessary to call another function than canAuthorize, because it returns true as soon as any refreshToken is set and does not check it online for correctness before.
Expand Down

0 comments on commit 15fcc6d

Please sign in to comment.