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

Fix issue WP-API/OAuth1#59, OAuth callback isn't called #84

Closed
wants to merge 1 commit into from

Conversation

sblaz
Copy link

@sblaz sblaz commented Sep 14, 2015

No description provided.

@coderkevin
Copy link

I had an issue with this. I had to do a rawurldecode() on the callback before saving it to make this work.
However, I agree that this needs to be stored between the request and authorization, otherwise the callback isn't redirected to after successful authorization.

@sblaz
Copy link
Author

sblaz commented Nov 4, 2015

Hmm, indeed, I think this patch applied by itself is insufficient to get the callback working. I also had some issues with double-encoding of the oauth_callback parameter value. Applied #65 and then removed the two rawurlencode() calls in join_with_equals_sign() and was good to go. The url decoding stuff all seems to happen earlier in normalize_parameters()

@coderkevin
Copy link

In my case, normalize_parameters() is really messing up my already partially encoded parameters, so by removing normalize_parameters() instead, I was able to keep the rawurldecode() calls.

See: #91

I have created a PR which merges your changes with AlexC's and removes normalize_parameters() in favor of encoding them in the join_with_equals_sign() function: #92

@rmccue
Copy link
Member

rmccue commented Nov 22, 2015

Thanks for the PR!

This is already applied in a separate spot; is this not working for you?

@rmccue
Copy link
Member

rmccue commented Dec 7, 2015

Closing out; this should be fixed, and in any case, this PR no longer applies as a lot of the code has been rewritten. Thanks anyway!

@rmccue rmccue closed this Dec 7, 2015
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.

3 participants