Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposed implementation of RequestDelegate for authentication #18

Closed
wants to merge 23 commits into from

Conversation

simorgh3196
Copy link
Contributor

Hello :)

I've created a proposal for an additional feature and would appreciate your feedback.

Introduction

Provides an implementation of APIClientDelegate specifically for authenticating requests.

Motivation

Alamofire provides an implementation called AuthenticationInterceptor for authentication. (docs)
AuthenticationInterceptor takes care of state management when sending concurrent requests.
Therefore, users only need to implement each type that conforms to the AuthenticationCredential protocol and the Authenticator protocol.

I would like to see an implementation of AuthenticationInterceptor that manages authentication in Get as well.
(With an implementation based on async/await and actor, of course.)

Implementation

Authenticator

Types adopting the Authenticator protocol will load or update Credential and apply the Credential to URLRequest.

AuthenticationInterceptor

The AuthenticationInterceptor class provides authentication for requests using exclusive control. It relies on an Authenticator type to handle the actual URLRequest authentication and Credential refresh.

AuthenticationInterceptor uses actor State, for exclusion control, and can apply and refresh authentication in order even for parallel requests.

Sample Usage

  1. Implement a class that adapt to the Authenticator protocol.
public class SampleAuthenticator: Authenticator {
    public typealias Credential = Token

    public var tokenStore: TokenStore
    public let client: APIClient

    public init(tokenStore: TokenStore, clientToRefreshCredential: APIClient) {
        self.tokenStore = tokenStore
        self.client = clientToRefreshCredential
    }

    public func credential() async throws -> Token {
        if let token = tokenStore.token, token.expiresDate < Date() {
            return token
        }

        // If there is no token, generate a token.
        let token: Token = try await client.send(.post("/token")).value
        tokenStore.save(token)
        return token
    }

    public func apply(_ credential: Token, to request: inout URLRequest) async throws {
        request.setValue(authorizationHeaderValue(for: credential), forHTTPHeaderField: "Authorization")
    }

    public func refresh(_ credential: Credential) async throws -> Credential {
        let token: Token = try await client.send(.put("/token", body: ["refresh_token": credential.refreshToken])).value
        tokenStore.save(token)
        return token
    }

    public func didRequest(_: URLRequest, failDueToAuthenticationError error: Error) -> Bool {
        if case .unacceptableStatusCode(let status) = (error as? APIError), status == 401 {
            return true
        }
        return false
    }

    public func isRequest(_ request: URLRequest, authenticatedWith credential: Token) -> Bool {
        request.value(forHTTPHeaderField: "Authorization") == authorizationHeaderValue(for: credential)
    }

    private func authorizationHeaderValue(for token: Token) -> String {
        "token \(token.accessToken)"
    }
}
  1. Set AuthenticationInterceptor with SampleAuthenticator as APIClientDelegate
let authenticator = SampleAuthenticator(tokenStore: TokenStore(),
                                        clientToRefreshCredential: APIClient(host: "example.com"))

let apiClient = APIClient(host: "example.com") {
  $0.delegate = AuthenticationInterceptor(authenticator: authenticator)
}

Impact on existing packages

Breaking Changes

Changed method shouldClientRetry(_ client: APIClient, withError error: Error) async throws -> Bool to shouldClientRetry(_ client: APIClient, for request: URLRequest, with error: Error) async throws -> Bool in APIClientDelegate because URLRequest was needed to manage retries for parallel requests.

Other Changes

In order to pass the URLRequest actually sent to APIClientDelegate.shouldClientRetry(_:for:with:), APIClientDelegate.client(_:willSendRequest:) is now called from APIClient.send(_:) to call it from APIClient.send(_:) instead of APIClient.actuallySend(_:).

@thanosbellos
Copy link

I am trying to implement your pr and i have a question. How can we still implement our own version of

public func shouldClientRetry(_ client: APIClient, for request: URLRequest, with error: Error) async throws -> Bool

of the APIClientDelegate?

You have already implemented 2 of the 3 APIClientDelegate methods and the last one has a default implementation.

@kean
Copy link
Owner

kean commented Apr 3, 2022

Changed method shouldClientRetry(_ client: APIClient, withError error: Error) async throws -> Bool to shouldClientRetry(_ client: APIClient, for request: URLRequest, with error: Error) async throws -> Bool in APIClientDelegate because URLRequest was needed to manage retries for parallel requests.

This change was released separately in 0.6.0.

I'm not sure if Authenticator should be shipped as part of the main framework. Unlike Alamofire, the goal of Get is to allow the users to build these things based on their requirements, while keeping the main framework small.

@kean
Copy link
Owner

kean commented Apr 9, 2022

I suggested creating a separate authentication package and I'll happily mention it in the repo. What do you think @simorgh3196?

@kean
Copy link
Owner

kean commented Apr 27, 2022

I'm going to close this PR for now because there are no plans to bake any authentication mechanisms into the framework. The goal is to provide just enough APIs to build on top of the framework.

@kean kean closed this Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants