search

ueberauth / ueberauth_vk

vk.com OAuth2 Strategy for Überauth.
https://hex.pm/packages/ueberauth_vk
MIT License
19 stars 12 forks source link

Addition of the state breaks functionality #21

Closed JustMikey closed 6 years ago

JustMikey commented 6 years ago

So the pattern matching to handle state was added here but the problem is that it may not be there and then it simple doesn't pattern match and we get the missing code error. State should be extracted without breaking the pattern match.

sobolevn commented 6 years ago

Oh, I see. @JustMikey can you provide a fix?

This feature was initially implemented by @Virviil so you can consult him for any questions.

Virviil commented 6 years ago

@JustMikey Did you check the functionality? It seems to me, that even when you don't provide state it's still passed as empty parameter like this. (empty state with state= So, it can be parsed properly with Plug.

sobolevn commented 6 years ago

Just got an email from a person who was not able to use the new version without state. This is a confirmed bug. @Virviil any ideas how to deal with it?

And could please help me to figure out good testing strategy that both version (with and without state) work?

Virviil commented 6 years ago

@sobolevn Basically saying I changed cassettes directly, instead of running the code with my token. So, I think that the best testing strategy will be run it with real VK requests. I can try to do it myself - but then I'll ask you to make testing.md file with description how to add tokens, which tests to run and so one.

The fastest fix is to add one more handle_callback! implementation without state in pattern matching. Better way will be to test different VK responses and develop suitable code.

sobolevn commented 6 years ago

Here's a pull request with a failing test. https://github.com/sobolevn/ueberauth_vk/pull/22