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

Add callback URL to authorization URL, not request token URL #373

Closed
wants to merge 1 commit into from

Conversation

tschmitz
Copy link

It looks like the callback URL is currently being added as a parameter on the request token URL, but it should really be part of the authorization URL. This change adds the callback to the authorization URL.

It looks like the callback URL is currently being added as a parameter on the request token URL, but it should really be part of the authorization URL. This change adds the callback to the authorization URL.
@tschmitz tschmitz changed the title Add callback URL to authorization URL, not request token Add callback URL to authorization URL, not request token URL May 13, 2017
@phimage
Copy link
Member

phimage commented May 14, 2017

could you need why you are doing this?
a bug?
If yes could you create it with all information needed.

postOAuthRequestToken already send the callbackurl
maybe your provider don t retain it
anyway you cannot commit a different behaviour without putting a boolean for configurate this special behaviour, because many website could reject this parameter here

@tschmitz
Copy link
Author

tschmitz commented May 19, 2017

I believe this is a bug. I need to include a callback URL when sending the user to the authentication page. However, when OAuth1Swift constructs the authorization URL, it's only using the value in authorizeURL without appending the callback URL I passed when calling authorize().

I expect to be able to do the following:

oauthSwift = OAuth1Swift(consumerKey: "CONSUMER_KEY",
		                         consumerSecret: "CONSUMER_SECRET",
		                         requestTokenUrl: "https://mydomain.com/oauth/request_token",
		                         authorizeUrl: "https://mydomain.com/oauth/authorize",
		                         accessTokenUrl: "https://mydomain.com/oauth/access_token")

let callbackURL = URL(string: "myapp://oauth-callback/myservice")!
oauthSwift.authorize(withCallbackURL: callbackURL, success: { (credential, response, parameters) in
    // Handle success
}) { (error) in
    // Handle failure
}

When I do that, I expect the URL called in the authorization step (step 2) to be:
https://mydomain.com/oauth/authorize?oauth_token=TOKEN&oauth_callback= myapp://oauth-callback/myservice

However, the current functionality is that the generated URL is:
https://mydomain.com/oauth/authorize?oauth_token=TOKEN

@garyhooper
Copy link

I have the same requirement as @tschmitz for TripIt. Use auth URL should be of the form:
https://m.tripit.com/oauth/authorize?oauth_token=<oauth_token>&oauth_callback=<oauth_callback> but oauth1swift.authorize only produces: https://m.tripit.com/oauth/authorize?oauth_token=<oauth_token>

Is there a work-around?

Thanks.

@phimage
Copy link
Member

phimage commented May 20, 2017

please refer to the RFC

https://tools.ietf.org/html/rfc5849
https://oauth.net/core/1.0a/#anchor44

For me accepting here the callback url, is a security issue
The callback url has been previously send to the server and checked/confirmed

The only thing I can accept is a boolean to configure oauth swift to add this functionality
and the default value is false

@garyhooper
Copy link

That would work for me.

@phimage
Copy link
Member

phimage commented May 20, 2017

Original author could update its PR or you can make a new one

@tschmitz
Copy link
Author

That would work for me as well. (Sorry for the slow response - been out of town for a few days.) I'd rather defer to you on the best way to implement rather than updating this PR.

@phimage
Copy link
Member

phimage commented Jun 4, 2017

76c7873
addCallbackURLToAuthorizeURL added to OAuth1Swift
set it to true

@phimage phimage closed this Jun 4, 2017
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