uber / uber-ios-sdk

Uber iOS SDK (beta)
https://developer.uber.com/docs
MIT License
376 stars 125 forks source link

Creating AccessToken from string does not populate relevant properties #187

Closed aspyre closed 6 years ago

aspyre commented 7 years ago

I'm using custom authorisation with the Uber iOS SDK, and having trouble creating the AccessToken in my iOS code. This is the response I get from my server with what appears to be a valid access token:

{
"access_token":"token here",
"expires_in":2592000,
"token_type":"Bearer",
"scope":"all_trips request",
"refresh_token":"refresh token here",
"last_authenticated":0
}

I then pass this to the AccessToken initialiser, like so:

let jsonString = //response from server as above
let accessToken = AccessToken(tokenString: jsonString)

My access token is created (ie. non-nil), but none of the relevant property are populated.

accessToken //non-nil
accessToken.expirationDate //nil
accessToken.refreshToken //nil
accessToken.tokenString //populated with the full text of jsonString

Digging through the AccessToken.swift source file of the Uber project, I find this:

@objc public init(tokenString: String) {
        super.init()
        self.tokenString = tokenString
}

This means refreshToken and expiration date will never be populated.

It also means that API attempts fail, as the API is expecting the tokenString field to contain the actual access_token text, not the full text.

I have verified that if I initialise the AccessToken like so, then API requests succeed (at least until the access token expires).

let accessTokenText = //access_token text only
let accessToken = AccessToken(tokenString: accessTokenText)
aspyre commented 7 years ago

Related Stack Overflow question (created by me): https://stackoverflow.com/questions/46397270/creating-accesstoken-from-tokenstring-with-uber-ios-sdk-failing

edjiang commented 7 years ago

Hey @aspyre,

The tokenString is meant to be just the access token itself, as you observed. If you want to parse the JSON itself, I would suggest using the fact that the model conforms to the Decodable protocol, and pass in your JSON through that method.

let decoder = JSONDecoder()
decoder.dateDecodingStrategy = .secondsSince1970
let accessToken = try? decoder.decode(AccessToken.self, from: jsonData)
// If you need to convert a string to data, use String.data(using: .utf8)!

Looking at this question, I think it'll be a good idea to make it easier to initialize this object from JSON. I'll leave this issue open to address this.

aspyre commented 7 years ago

Thanks @edjiang for the reply, makes sense and not sure why I didn't connect that I can just use the JSONDecoder. In previous versions of the UberRides SDK the token was created like so, which did in fact populate all the properties as expected:

AccessTokenFactory.createAccessTokenFromJSONString(jsonString)

So, I think I was expecting something similar here. I agree it would be a good idea to make it easier when initializing from JSON, as I expect this would be the primary use case.

aspyre commented 7 years ago

To follow up on this, I believe there is still some work to be done. Two issues:

  1. The response from https://login.uber.com/oauth/v2/token contains the key "expires_in", but the CodingKey in AccessToken.swift is "expiration_date". So at the moment I have to manually update the response data to use this key instead.
  2. "expires_in" is not a date at all, but number of seconds until the access token expires. So again, this requires manually updating the response from https://login.uber.com/oauth/v2/token

Of course, these are not particularly complicated, but it does creating an AccessToken object from https://login.uber.com/oauth/v2/token not the trivial task that it used to be.

edjiang commented 7 years ago

Thanks for reporting this. Definitely looks like some warts in here. I didn't realize that the implementation did #1, so I recommended that strategy in error. I'll get these cleaned up for you and fixed. :)

aspyre commented 7 years ago

Thanks for the follow up!

edjiang commented 6 years ago

@aspyre should be fixed in 0.8; let me know how it goes! https://github.com/uber/rides-ios-sdk/releases/tag/v0.8.0

aspyre commented 6 years ago

@edjiang thanks for the note, and the fix! Looking good so far.