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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add two-factor authentication support #5

Merged
merged 2 commits into from Nov 25, 2014

Conversation

remixz
Copy link
Contributor

@remixz remixz commented Nov 24, 2014

Fixes #4.

This was a bit more work than I expected! 馃槅 To support 2FA without having to enter the 2FA auth code every time, I had to refactor the login from using Basic auth to token-based auth (also needed for the stream view... more on that in a sec). Overall, I think this is better though, since storing someone's password in plaintext isn't too safe.

I had to refactor the stream view to use an API route, since the Atom feeds only support Basic auth, which doesn't work with 2FA nicely. The API route (/users/:user/received_events) doesn't include the user-friendly text description, so I had to manually construct it. This does remove one dependency on xml2js though!

I did my best to follow your code style. Let me know what you think!

2FA Preview

@IonicaBizau
Copy link
Owner

That's really really nice! 馃憤

However the thing you removed atom feeds request worries me a little. That's what I wanted: not compute manually all these messages.

Are you sure that Atom feeds only support Basic auth?


Please create contributors array in package.json and add yourself there. 馃槃

@remixz
Copy link
Contributor Author

remixz commented Nov 25, 2014

Unfortunately, to get all of the info possible, yeah. https://developer.github.com/v3/activity/feeds/ Specifically:

Note: Private feeds are only returned when authenticating via Basic Auth since current feed URIs use the older, non revokable auth tokens.

So it would be possible to get the non-private feed without any auth, but this won't show anything from private repos, which would make the stream less useful. I'm not a fan of having the messages manually generated either, but it's the best solution I could think of.

Also, added the contributors field!

@IonicaBizau
Copy link
Owner

I have just tested it. Really nice improvement. 馃憤

IonicaBizau added a commit that referenced this pull request Nov 25, 2014
Add two-factor authentication support
@IonicaBizau IonicaBizau merged commit 8022653 into IonicaBizau:master Nov 25, 2014
@IonicaBizau
Copy link
Owner

https://github.com/<myUsername>.private.atom?token=<token> provides an XML response including the private events. Having the token with correct scopes, we can fetch it, convert it to JSON and use it.

@remixz
Copy link
Contributor Author

remixz commented Dec 1, 2014

Oh, that's sweet! I wish I had saw that before, haha. I can implement that if you want, unless you want to take it.

@IonicaBizau
Copy link
Owner

@remixz If you like, you can do it. I'm a little bit busy right now. If not, I will probably do it this week. 馃槃

@remixz
Copy link
Contributor Author

remixz commented Dec 4, 2014

Sorry for my delayed response here, been busy with school!

Anyway, I took a look at the token parameter, and I couldn't get it to work with an oAuth token. Based on the feed link that's on the GitHub homepage that uses the token param, it's using some sort of Base64 token (decoding it just gives me random characters though), so I'm assuming it's based on basic authentication. I think it's a dead end, unfortunately. :\

@IonicaBizau
Copy link
Owner

NP, I will try to do it, maybe today. 馃槃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Two-Factor Auth
2 participants