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

How to handle 2FA authentication? #11

Open
gr2m opened this issue Dec 9, 2017 · 9 comments
Open

How to handle 2FA authentication? #11

gr2m opened this issue Dec 9, 2017 · 9 comments

Comments

@gr2m
Copy link
Contributor

gr2m commented Dec 9, 2017

context: octokit/octokit.js#669

I’d love to hear the other teams’ opinions / thoughts on this @octokit/rb @octokit/net

@ryangribble
Copy link
Contributor

What's the context where basic auth is being used on a 2fa enabled account?

Given the primary use is for automated systems they wouldn't have access to a rolling 2fa code they could provide anyway would they?

Using a personal access token is the way I would normally do this rather than basic username/password

@gr2m
Copy link
Contributor Author

gr2m commented Dec 9, 2017

A usecase where it could be problematic is an app where users provide username/password, so authentication is basic. Users with 2FA enabled won’t be able to use it at all I guess?

Maybe we should discourage the usage of basic authentication altogether, as it won’t be supported with GraphQL anyway, as far as I can tell?

@kytrinyx
Copy link

A usecase where it could be problematic is an app where users provide username/password, so authentication is basic.

I think we should strongly discourage this. The only time I think it would be ok to use basic auth is if you're writing a one-off script for yourself... but even then I would encourage people to just get themselves a personal access token.

it won’t be supported with GraphQL anyway, as far as I can tell?

Ah. I'm double-checking that, but if so it makes even more sense to simply discourage the use of basic auth altogether.

@kytrinyx
Copy link

It looks like currently Basic Auth works with GraphQL, but it's undocumented, and I'm not sure that we have decided whether or not this is something that will be supported going forward.

@colinrymer
Copy link

I'd love to see 2FA supported - the existing v3 REST docs specifically describe using Basic Auth to generate personal access tokens programmatically[0] for non-website applications[1], which we want to do so that we can keep users inside a CLI tool we're building and not require that they switch to their browser, generate the token, and then copy/paste it into the tool.


0 - https://developer.github.com/v3/oauth_authorizations/#create-a-new-authorization
1 - "Note that OAuth2 tokens can be acquired programmatically, for applications that are not websites." - https://developer.github.com/v3/#authentication

@kytrinyx
Copy link

@colinrymer That's a good use case. (I had forgotten that) I use this REST endpoint fairly regularly with curl to create personal tokens.

Just to clarify: is this discussion about how to support 2FA from within the client wrappers?

If I were to try to use this from within Octokit.rb I would go straight to the client.post method.

@colinrymer
Copy link

colinrymer commented Feb 23, 2018

@kytrinyx Not sure about the other wrappers, but this discussion seems to be started because of a request for 2FA support in the JS client (that's how I ended up here 😁), as that client provides an authenticate function that supports basic auth credentials as arguments but doesn't include 2FA support, so what could be one simple function call becomes more complex. I have an example of that here - see lines 15-17.

@kytrinyx
Copy link

@colinrymer Ok, that makes sense, thanks.

My gut instinct is that we should have a way to support it with a single call. I don't mind if that means dropping down to some lower-level call in the library, as long as we document it.

@gr2m
Copy link
Contributor Author

gr2m commented Feb 24, 2018

@colinrymer for the JavaScript Octokit specifically I'd keep the discussion at https://github.com/octokit/rest.js/issues/669. I'll address the issue as part of the next milestone.

As a general learning, the conceptional problem with the existing client.authenticate from the JavaScript REST Octokit is that it is synchronous. The 2FA authentication requires a on-time request to generate a token which should be used after, so it needs to be asynchronous.

See also my follow up at https://github.com/octokit/rest.js/issues/669#issuecomment-368184029

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

No branches or pull requests

4 participants