-
Notifications
You must be signed in to change notification settings - Fork 130
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
Twitter sign in and authorization methods #21
Conversation
Missed the vcr fixture for the request_token test, but the Exvcr is not detecting the custom cassette for Also, cool thing these custom cassettes, I can use one to make the access_token method testable :) |
Alright, it seems I thought that might be a bug, so I updated the dependencies. Turned out it wasn't, so I made the custom fixture take regex urls and fixed it. All tests are passing but hackney is blowing up for some reason. Could it be the updated dependencies? Will investigate further (help or input is welcome). |
Yay, all tests passing =] Looks like there was a problem with SSL and 17.3, something in the coveralls script was blowing up due to that. Thank goodness there was a clue in |
|
||
## Reference | ||
https://dev.twitter.com/web/sign-in/implementing | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following APIs corresponds to the newly added methods (correct?) If so, can we have them as "Reference" too?
https://dev.twitter.com/oauth/reference/post/oauth/request_token
https://dev.twitter.com/oauth/reference/post/oauth/access_token
https://dev.twitter.com/oauth/reference/get/oauth/authorize
https://dev.twitter.com/oauth/reference/get/oauth/authenticate
Thanks for the PR! I put some comments in the code, I appreciate if you could check. I'm thinking it's good to have example sequence in somewhere (readme or wiki). I could confirm the minimal behavior in the following, but if you have other example sequence (ex. callback case), I appreciate if you could share them! token = ExTwitter.request_token()
{:ok, authorize_url} = ExTwitter.authorize_url(token.oauth_token)
IO.puts authorize_url
# Open browser and authorize to retrieve verifier (pincode)
pincode = "999999"
access_token = ExTwitter.access_token(pincode, token.oauth_token)
Extwitter.configure(
consumer_key: System.get_env("TWITTER_CONSUMER_KEY"),
consumer_secret: System.get_env("TWITTER_CONSUMER_SECRET"),
access_token: access_token.oauth_token,
access_token_secret: access_token.oauth_token_secret
)
ExTwitter.user_timeline |
… Changed access_token to make specs more uniform.
33554c0
to
730921e
Compare
Hey parroty, check out the changes. Tuples, maps and structs were still kinda weird for me when I did this. Thanks for pointing out that specs thing =] Any more comments? |
Twitter sign in and authorization methods
Thanks for the extensive work! |
I've used the code created by @arjan referenced on Issue #14, adapted it to the extwitter structure and added some tests where things are testable.
As @arjan said, authorization is difficult to test with automation as it requires user interation on the twitter website, so I left the twitter
access_token
method commented in the tests.This is my first Elixir PR so I may have made something silly in the few lines of code that were my own. As so I'm open to any suggestions, code or commit cleanups, etc.
I'm also OK with the idea that authorization/authentication/access token logistics could be better off in a separate module in another repo. It's plausible that people doing just twitter sign-in don't necessarily want to add a complete twitter API to their projects.
Oh yeah, I also updated mix.lock to make it build on ERL 18.