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

How to program the refresh token logic when using Swift generated code? #5462

Closed
Ricardo1980 opened this issue Feb 27, 2020 · 11 comments
Closed

Comments

@Ricardo1980
Copy link

Ricardo1980 commented Feb 27, 2020

Description

Hello,

I'm using openapi-generator, version 4.2.3, exactly this one:
openapi-generator generate -g swift5 -i my_openapi.yaml -o ./swift5 --library alamofire

The reason I want to use Alamofire is because my api (created by a workmate), uses access token and refresh token, and the access token expires every hour so the refresh token has to be used to get a new one, while the ongoing requests are paused until I get a new valid access token. Basically, if I get 401 in any request, I try to download the access token using the refresh token. If unauthorized, I logout the user, otherwise, I continue with the original request. That logic is very easy to implement using alamofire.

Without this generated code, I do something like:

awsManager = AFManager.getManagerWith(UtilityMethods.setupConfigurationWithTimeout())
let jwtHandler = JWTHandler()
awsManager.adapter = jwtHandler
awsManager.retrier = jwtHandler

In the adapter, I add the access token for all requests, except signup request.
In retrier, I add the refresh token logic.

I already did that in other app using Alamofire but a custom networking layer, not code autogenerated.

https://github.com/Alamofire/Alamofire/blob/master/Documentation/AdvancedUsage.md#requestadapter
https://github.com/Alamofire/Alamofire/blob/master/Documentation/AdvancedUsage.md#requestretrier
for the retrier I used code similar to this one, and working perfectly
https://stackoverflow.com/questions/42079180/understanding-alamofire-oauth-example

Researching the generated code, I see:

    /**
     May be overridden by a subclass if you want to control the session
     configuration.
     */
    open func createSessionManager() -> Alamofire.SessionManager

I guess I have to subclass AlamofireRequestBuilder and overwrite createSessionManager.
Perhaps I also have to subclass AlamofireRequestBuilderFactory and then pass this new instance to OpenAPIClientAPI.requestBuilderFactory

So, the question is, how can I attach the refresh token logic I explained to the generated code without modifying it? Is that possible? Any other suggestion?
How do you usually manage the refresh token logic when using autogenerated code?

Thanks for suggestions.

EDIT:
I can do something like this and inject it using
OpenAPIClientAPI.requestBuilderFactory = CustomAlamofireRequestBuilderFactory()

class CustomAlamofireRequestBuilderFactory: RequestBuilderFactory {
    func getNonDecodableBuilder<T>() -> RequestBuilder<T>.Type {
        return CustomAlamofireRequestBuilder<T>.self
    }

    func getBuilder<T: Decodable>() -> RequestBuilder<T>.Type {
        return AlamofireDecodableRequestBuilder<T>.self
    }
}

But AlamofireDecodableRequestBuilder is a subclass of AlamofireRequestBuilder and I cannot change that. I tried to copy/paste the entire AlamofireDecodableRequestBuilder class (creating CustomAlamofireDecodableRequestBuilder), changing the base class from AlamofireRequestBuilder to CustomAlamofireRequestBuilder, but it does not work because "credential" and "managerStore" are innacesible.

However, if I edit manually the autogenerated code and I use something like:

    open func createSessionManager() -> Alamofire.SessionManager {
        let configuration = URLSessionConfiguration.default
        configuration.httpAdditionalHeaders = buildHeaders()

        let sessionManager = Alamofire.SessionManager(configuration: configuration)
        let jwtHandler = JWTHandler()
        sessionManager.adapter = jwtHandler
        sessionManager.retrier = jwtHandler
        return sessionManager
    }

It works. But this is a crappy solution. It means I have to edit the generated code all the time is generated.

Any idea? How should I fix this?

openapi-generator version

4.2.3

OpenAPI declaration file content or url

No issues with this, the code generated works perfectly.

Command line used for generation

openapi-generator generate -g swift5 -i my_openapi.yaml -o ./swift5 --library alamofire

Related issues/PRs

I see several of them but regarding other languages, nothing about swift.

Suggest a fix/enhancement

Explained in the description.

@wing328
Copy link
Member

wing328 commented Feb 28, 2020

cc @jgavris (2017/07) @ehyche (2017/08) @Edubits (2017/09) @jaz-ah (2017/09) @4brunu (2019/11)

@4brunu
Copy link
Contributor

4brunu commented Feb 28, 2020

Hey,
you don't need to change the generated code.
First you need to create those classes

class CustomAlamofireRequestBuilderFactory: RequestBuilderFactory {
    func getNonDecodableBuilder<T>() -> RequestBuilder<T>.Type {
        return CustomAlamofireRequestBuilder<T>.self
    }

    func getBuilder<T: Decodable>() -> RequestBuilder<T>.Type {
        return CustomAlamofireDecodableRequestBuilder<T>.self
    }
}

class CustomAlamofireDecodableRequestBuilder<T: Decodable>: AlamofireDecodableRequestBuilder<T> {
    override func createSessionManager() -> SessionManager {
        let sessionManager = super.createSessionManager()

        let jwtHandler = JWTHandler()
        sessionManager.adapter = jwtHandler
        sessionManager.retrier = jwtHandler

        return sessionManager
    }
}

class CustomAlamofireRequestBuilder<T>: AlamofireRequestBuilder<T> {
    override func createSessionManager() -> SessionManager {
        let sessionManager = super.createSessionManager()

        let jwtHandler = JWTHandler()
        sessionManager.adapter = jwtHandler
        sessionManager.retrier = jwtHandler

        return sessionManager
    }
}

Then you assign the CustomAlamofireRequestBuilderFactory to the property requestBuilderFactory in the APIs.swift file.
PetStoreAPI.requestBuilderFactory = CustomAlamofireRequestBuilderFactory()
The name PetStoreAPI.requestBuilderFactory will change depending on your project name.

Hope this helps.

@4brunu
Copy link
Contributor

4brunu commented Feb 28, 2020

This is also possible if someone is using URLSession.
Here is an example how to do it.

class CustomURLSessionRequestBuilderFactory: RequestBuilderFactory {
    func getNonDecodableBuilder<T>() -> RequestBuilder<T>.Type {
        return CustomURLSessionRequestBuilder<T>.self
    }

    func getBuilder<T: Decodable>() -> RequestBuilder<T>.Type {
        return CustomURLSessionDecodableRequestBuilder<T>.self
    }
}

class CustomURLSessionDecodableRequestBuilder<T: Decodable>: URLSessionDecodableRequestBuilder<T> {
    required init(method: String, URLString: String, parameters: [String: Any]?, isBody: Bool, headers: [String: String] = [:]) {
        super.init(method: method, URLString: URLString, parameters: parameters, isBody: isBody, headers: headers)

        taskCompletionShouldRetry = { _, response, _, shouldRetry in
            if let response = response as? HTTPURLResponse, response.statusCode == 401 {
                // If this is a 401 unauthorized, we should refresh, update the headers in {ProjectName}Api.customHeaders with the new token and retry the request
                TokenRefresher.refreshToken {
                    shouldRetry(true)
                }
            } else {
                // If the error is not 401 unauthorized, we don't need to retry the request
                shouldRetry(false)
            }
        }
    }
}

class CustomURLSessionRequestBuilder<T>: URLSessionRequestBuilder<T> {
    required init(method: String, URLString: String, parameters: [String: Any]?, isBody: Bool, headers: [String: String] = [:]) {
        super.init(method: method, URLString: URLString, parameters: parameters, isBody: isBody, headers: headers)

        taskCompletionShouldRetry = { _, response, _, shouldRetry in
            if let response = response as? HTTPURLResponse, response.statusCode == 401 {
                // If this is a 401 unauthorized, we should refresh, update the headers in {ProjectName}Api.customHeaders with the new token and retry the request
                TokenRefresher.refreshToken {
                    shouldRetry(true)
                }
            } else {
                // If the error is not 401 unauthorized, we don't need to retry the request
                shouldRetry(false)
            }
        }
    }
}

Then you assign the CustomURLSessionRequestBuilderFactory to the property requestBuilderFactory in the APIs.swift file.
PetStoreAPI.requestBuilderFactory = CustomURLSessionRequestBuilderFactory()
The name PetStoreAPI.requestBuilderFactory will change depending on your project name.

@Ricardo1980
Copy link
Author

Thanks a lot @4brunu
I was so close!
Now it is working without modifying the generated code.

Last question, what do you recommend me know if a request needs an access token or not?

Right now, in my prototype I have this:

extension JWTHandler: RequestAdapter {
    func adapt(_ urlRequest: URLRequest) throws -> URLRequest {
        var urlRequest = urlRequest

        if needsToken(urlRequest: urlRequest) {
            if let acccessToken = UserDefaults.standard.string(forKey: TestNetworkViewController.accessTokenKey) {
                print("---token is required and provided")
                urlRequest.setValue("Bearer \(acccessToken)", forHTTPHeaderField: "Authorization")
            } else {
                print("---token is required but we dont have it")
            }
        } else {
            print("----dont use token")
        }

        return urlRequest
    }

    private func needsToken(urlRequest: URLRequest) -> Bool {
        guard let url = urlRequest.url, let httpMethod = urlRequest.httpMethod else {
            return true
        }

        if url.absoluteString.contains("/directory/customers") || url.absoluteString.contains("/authentication/tokens"), httpMethod == "POST" {
            return false
        }
        return true
    }
}

Which is working, but it is crappy using those endpoints to know it.

In the open api specification file, I see:

     - BASIC:
       - type: http
       - name: BearerToken

But no mention in the generated code.
What approach do you recommend me to know if I have to insert the access token or not?

Thanks a lot.

@4brunu
Copy link
Contributor

4brunu commented Feb 28, 2020

Hum, sorry but I don't have a better solution for that.

@Ricardo1980
Copy link
Author

@4brunu
would be possible to attach a flag or something (using a URLRequest extension) in the generated code so I know when a request requires an access token, because that info is in the specification file?
What other suggestion do you suggest me to try to implement something in the code generator?
Thanks a lot.

@4brunu
Copy link
Contributor

4brunu commented Feb 28, 2020

I don't see how this can fit in the generator. :/
But wouldn't be easier if all the endpoints require authentication?
Or you can send the authentication headers always, even if the method doesn't require it.

@Ricardo1980
Copy link
Author

@4brunu
"But wouldn't be easier if all the endpoints require authentication?"
There must be at least one or 2 endpoints that don't require auth: signup and signin, for obvious reasons.
In fact, I already tried to ignore that and send the access token all the time (if available in the keychain), but if I do that in those endpoints, the backend replies with 401 unauthorized.
Somehow, that error makes sense.

@Ricardo1980
Copy link
Author

Well, the refresh token logic I implemented is working properly.
Only remains to improve the piece of code that decides whether an endpoint should include an access token or not.

lightman73 added a commit to lightman73/openapi-generator that referenced this issue Sep 28, 2020
Problem: custom headers could be overwritten between 
request retries following, e.g. a token refresh ( for example,
while implementing @4brunu refresh code :
OpenAPITools#5462 (comment) ).
A simple reordering of the modifiedRequest headers construction
solves the problem.
wing328 pushed a commit that referenced this issue Sep 28, 2020
…7527)

* fix: correct handling of customHeaders

Problem: custom headers could be overwritten between 
request retries following, e.g. a token refresh ( for example,
while implementing @4brunu refresh code :
#5462 (comment) ).
A simple reordering of the modifiedRequest headers construction
solves the problem.

* fix: correct PetStore samples implementation [PR#7527]

Co-authored-by: Franz Marini <[email protected]>
@hyunoosung
Copy link

hyunoosung commented Oct 16, 2020

Do you guys have an example project for this?

@JonnyBeeGod
Copy link

Well, the refresh token logic I implemented is working properly.
Only remains to improve the piece of code that decides whether an endpoint should include an access token or not.

@Ricardo1980 Sorry for digging this issue up. I am wondering whether you guys did find a better solution for doing this instead of implementing a hardcoded whitelist of requests in your code base?

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

No branches or pull requests

5 participants