OAuthSwift / OAuthSwiftAlamofire

Utility method to sign Alamofire request
MIT License
104 stars 38 forks source link

refresh_token shouldn't send Authorization header #12

Closed neoneye closed 6 years ago

neoneye commented 6 years ago

Thank you for a great framework.

I think there is a problem with OAuthSwift2RequestAdapter.refreshTokens(). Currently an Authorization header is inserted in the refresh_token request.

However none of the RFC examples show an Authorization header.

This is causing me problems. The server rejects my attempt at refreshing, because the access_token is expired. I have made a workaround, but I would very much prefer that this got fixed in OAuthSwiftAlamofire.

Here are examples of the refresh_token request: https://tools.ietf.org/html/rfc6749#page-17 https://tools.ietf.org/html/rfc6749#page-47

The RFC doesn't mention if the Authorization header is optional in the refresh_token request. I'm unsure if it's allowed. I think the Authorization header shouldn't be there. Forgive me if I'm wrong about this.

I have made 2 workarounds.

Current behavior

Here the request has an Authorization header.

Request

:method: POST
:scheme: https
:path: /oauth
:authority: sandbox.alamofire.org
accept: */*
content-type: application/x-www-form-urlencoded; charset=utf-8
accept-encoding: br, gzip, deflate
authorization: Bearer xTKx8LNvDCmS570GiqmqVGMuciv
user-agent: Install/1 CFNetwork/887 Darwin/16.7.0
accept-language: en-us
content-length: 141

grant_type=refresh_token&refresh_token=tGzv3JOkF0XG5Qx2TlKWIA&client_secret=CLIENT_SECRET&client_id=CLIENT_ID

Response

:status: 401
content-type: text/plain; charset=utf-8
x-content-type-options: nosniff
content-length: 20
date: Thu, 24 Dec 1984 23:59:59 GMT

Authorization error

New behavior with workaround

Here the request is without the Authorization header.

Request

:method: POST
:scheme: https
:path: /oauth
:authority: sandbox.alamofire.org
accept: */*
content-type: application/x-www-form-urlencoded; charset=utf-8
accept-encoding: br, gzip, deflate
user-agent: Install/1 CFNetwork/887 Darwin/16.7.0
content-length: 141
accept-language: en-us

grant_type=refresh_token&refresh_token=tGzv3JOkF0XG5Qx2TlKWIA&client_secret=CLIENT_SECRET&client_id=CLIENT_ID

Response

:status: 200
content-type: application/json
content-length: 177
date: Thu, 24 Dec 1984 23:59:59 GMT

{"access_token":"2YotnFZFEjr1zCsicMWpAA","expires_in":3600,"refresh_token":"tGvz3JOkFX0G5Q2xTlKWIA"}

Workaround A

private func refreshTokens(completion: @escaping RefreshCompletion) {
    guard !isRefreshing else { return }

    isRefreshing = true

    print("reset access_token")
    oauth2Swift.client.credential.oauthToken = ""
    oauth2Swift.client.credential.oauthTokenExpiresAt = nil

    oauth2Swift.renewAccessToken(
        withRefreshToken: oauth2Swift.client.credential.oauthRefreshToken,
        success: { [weak self] (credential, response, parameters) in
            guard let strongSelf = self else { return }
            completion(true)
            strongSelf.isRefreshing = false
        }, failure: { [weak self] (error) in
            guard let strongSelf = self else { return }
            completion(false)
            strongSelf.isRefreshing = false
        }
    )
}

Workaround B

class OAuth2SwiftWorkaround: OAuth2Swift {
    override func renewAccessToken(withRefreshToken refreshToken: String, parameters: OAuthSwift.Parameters?, headers: OAuthSwift.Headers?, success: @escaping OAuthSwift.TokenSuccessHandler, failure: OAuthSwift.FailureHandler?) -> OAuthSwiftRequestHandle? {

        print("reset access_token")
        client.credential.oauthToken = ""
        client.credential.oauthTokenExpiresAt = nil

        return super.renewAccessToken(withRefreshToken: refreshToken, parameters: parameters, headers: headers, success: success, failure: failure)
    }
}
Germandrummer92 commented 6 years ago

@neoneye If you take a look at https://tools.ietf.org/html/rfc6749#section-6, it does require an authorization header in the rfc IF the client was issued client credentials (which means, you HAVE a client_secret and not only a client_id, correct me if I'm wrong). So I think this is the correct behavior by oauthSwift.

This is the original wording in the rfc for refreshing an access token:

If the client type is confidential or the client was issued client credentials (or assigned other authentication requirements), the client MUST authenticate with the authorization server

neoneye commented 6 years ago

@Germandrummer92 ok, thank you for answering this. I have forwarded your answer to my backend developer friend.