From 84ce6f29b2ee2192b57d8ff7c36af3378c696653 Mon Sep 17 00:00:00 2001 From: Guilherme Souza Date: Wed, 6 Nov 2024 06:55:16 -0300 Subject: [PATCH] fix(auth): incorrect error when error occurs during PKCE flow (#592) --- Sources/Auth/AuthClient.swift | 61 ++++++++++++++++++--------- Tests/AuthTests/AuthClientTests.swift | 53 +++++++++++++++++++---- 2 files changed, 85 insertions(+), 29 deletions(-) diff --git a/Sources/Auth/AuthClient.swift b/Sources/Auth/AuthClient.swift index 1cec6346..2ca227c1 100644 --- a/Sources/Auth/AuthClient.swift +++ b/Sources/Auth/AuthClient.swift @@ -756,33 +756,32 @@ public final class AuthClient: Sendable { /// Gets the session data from a OAuth2 callback URL. @discardableResult public func session(from url: URL) async throws -> Session { - logger?.debug("received \(url)") + logger?.debug("Received URL: \(url)") let params = extractParams(from: url) - if configuration.flowType == .implicit, !isImplicitGrantFlow(params: params) { - throw AuthError.implicitGrantRedirect(message: "Not a valid implicit grant flow url: \(url)") - } - - if configuration.flowType == .pkce, !isPKCEFlow(params: params) { - throw AuthError.pkceGrantCodeExchange(message: "Not a valid PKCE flow url: \(url)") - } - - if isPKCEFlow(params: params) { - guard let code = params["code"] else { - throw AuthError.pkceGrantCodeExchange(message: "No code detected.") + switch configuration.flowType { + case .implicit: + guard isImplicitGrantFlow(params: params) else { + throw AuthError.implicitGrantRedirect( + message: "Not a valid implicit grant flow URL: \(url)") } + return try await handleImplicitGrantFlow(params: params) - let session = try await exchangeCodeForSession(authCode: code) - return session + case .pkce: + guard isPKCEFlow(params: params) else { + throw AuthError.pkceGrantCodeExchange(message: "Not a valid PKCE flow URL: \(url)") + } + return try await handlePKCEFlow(params: params) } + } - if params["error"] != nil || params["error_description"] != nil || params["error_code"] != nil { - throw AuthError.pkceGrantCodeExchange( - message: params["error_description"] ?? "Error in URL with unspecified error_description.", - error: params["error"] ?? "unspecified_error", - code: params["error_code"] ?? "unspecified_code" - ) + private func handleImplicitGrantFlow(params: [String: String]) async throws -> Session { + precondition(configuration.flowType == .implicit, "Method only allowed for implicit flow.") + + if let errorDescription = params["error_description"] { + throw AuthError.implicitGrantRedirect( + message: errorDescription.replacingOccurrences(of: "+", with: " ")) } guard @@ -827,6 +826,25 @@ public final class AuthClient: Sendable { return session } + private func handlePKCEFlow(params: [String: String]) async throws -> Session { + precondition(configuration.flowType == .pkce, "Method only allowed for PKCE flow.") + + if params["error"] != nil || params["error_description"] != nil || params["error_code"] != nil { + throw AuthError.pkceGrantCodeExchange( + message: params["error_description"]?.replacingOccurrences(of: "+", with: " ") + ?? "Error in URL with unspecified error_description.", + error: params["error"] ?? "unspecified_error", + code: params["error_code"] ?? "unspecified_code" + ) + } + + guard let code = params["code"] else { + throw AuthError.pkceGrantCodeExchange(message: "No code detected.") + } + + return try await exchangeCodeForSession(authCode: code) + } + /// Sets the session data from the current session. If the current session is expired, setSession /// will take care of refreshing it to obtain a new session. /// @@ -1304,7 +1322,8 @@ public final class AuthClient: Sendable { private func isPKCEFlow(params: [String: String]) -> Bool { let currentCodeVerifier = codeVerifierStorage.get() - return params["code"] != nil && currentCodeVerifier != nil + return params["code"] != nil || params["error_description"] != nil || params["error"] != nil + || params["error_code"] != nil && currentCodeVerifier != nil } private func getURLForProvider( diff --git a/Tests/AuthTests/AuthClientTests.swift b/Tests/AuthTests/AuthClientTests.swift index 715fcab9..17905696 100644 --- a/Tests/AuthTests/AuthClientTests.swift +++ b/Tests/AuthTests/AuthClientTests.swift @@ -5,14 +5,15 @@ // Created by Guilherme Souza on 23/10/23. // -@testable import Auth import ConcurrencyExtras import CustomDump -@testable import Helpers import InlineSnapshotTesting import TestHelpers import XCTest +@testable import Auth +@testable import Helpers + #if canImport(FoundationNetworking) import FoundationNetworking #endif @@ -126,7 +127,9 @@ final class AuthClientTests: XCTestCase { message: "", errorCode: .unknown, underlyingData: Data(), - underlyingResponse: HTTPURLResponse(url: URL(string: "http://localhost")!, statusCode: 404, httpVersion: nil, headerFields: nil)! + underlyingResponse: HTTPURLResponse( + url: URL(string: "http://localhost")!, statusCode: 404, httpVersion: nil, + headerFields: nil)! ) } @@ -157,7 +160,9 @@ final class AuthClientTests: XCTestCase { message: "", errorCode: .invalidCredentials, underlyingData: Data(), - underlyingResponse: HTTPURLResponse(url: URL(string: "http://localhost")!, statusCode: 401, httpVersion: nil, headerFields: nil)! + underlyingResponse: HTTPURLResponse( + url: URL(string: "http://localhost")!, statusCode: 401, httpVersion: nil, + headerFields: nil)! ) } @@ -188,7 +193,9 @@ final class AuthClientTests: XCTestCase { message: "", errorCode: .invalidCredentials, underlyingData: Data(), - underlyingResponse: HTTPURLResponse(url: URL(string: "http://localhost")!, statusCode: 403, httpVersion: nil, headerFields: nil)! + underlyingResponse: HTTPURLResponse( + url: URL(string: "http://localhost")!, statusCode: 403, httpVersion: nil, + headerFields: nil)! ) } @@ -277,13 +284,17 @@ final class AuthClientTests: XCTestCase { response, OAuthResponse( provider: .github, - url: URL(string: "https://github.com/login/oauth/authorize?client_id=1234&redirect_to=com.supabase.swift-examples://&redirect_uri=http://127.0.0.1:54321/auth/v1/callback&response_type=code&scope=user:email&skip_http_redirect=true&state=jwt")! + url: URL( + string: + "https://github.com/login/oauth/authorize?client_id=1234&redirect_to=com.supabase.swift-examples://&redirect_uri=http://127.0.0.1:54321/auth/v1/callback&response_type=code&scope=user:email&skip_http_redirect=true&state=jwt" + )! ) ) } func testLinkIdentity() async throws { - let url = "https://github.com/login/oauth/authorize?client_id=1234&redirect_to=com.supabase.swift-examples://&redirect_uri=http://127.0.0.1:54321/auth/v1/callback&response_type=code&scope=user:email&skip_http_redirect=true&state=jwt" + let url = + "https://github.com/login/oauth/authorize?client_id=1234&redirect_to=com.supabase.swift-examples://&redirect_uri=http://127.0.0.1:54321/auth/v1/callback&response_type=code&scope=user:email&skip_http_redirect=true&state=jwt" let sut = makeSUT { _ in .stub( """ @@ -312,7 +323,8 @@ final class AuthClientTests: XCTestCase { fromFileName: "list-users-response", headers: [ "X-Total-Count": "669", - "Link": "; rel=\"next\", ; rel=\"last\"", + "Link": + "; rel=\"next\", ; rel=\"last\"", ] ) } @@ -340,6 +352,31 @@ final class AuthClientTests: XCTestCase { XCTAssertEqual(response.lastPage, 14) } + func testSessionFromURL_withError() async throws { + sut = makeSUT() + + Dependencies[sut.clientID].codeVerifierStorage.set("code-verifier") + + let url = URL( + string: + "https://my.redirect.com?error=server_error&error_code=422&error_description=Identity+is+already+linked+to+another+user#error=server_error&error_code=422&error_description=Identity+is+already+linked+to+another+user" + )! + + do { + try await sut.session(from: url) + XCTFail("Expect failure") + } catch { + expectNoDifference( + error as? AuthError, + AuthError.pkceGrantCodeExchange( + message: "Identity is already linked to another user", + error: "server_error", + code: "422" + ) + ) + } + } + private func makeSUT( fetch: ((URLRequest) async throws -> HTTPResponse)? = nil ) -> AuthClient {