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

Add support for users with long passwords and 2FA enabled #1496

Closed
MaybeNetwork opened this issue May 6, 2020 · 24 comments
Closed

Add support for users with long passwords and 2FA enabled #1496

MaybeNetwork opened this issue May 6, 2020 · 24 comments
Labels
Auto-closed - Stale Automatically closed due to being stale for too long Feature New feature or request Stale Issue or pull request has been inactive for 20 days
Milestone

Comments

@MaybeNetwork
Copy link
Contributor

Describe the solution you'd like
Reddit's suggested solution of passing the 2FA token to /api/v1/access_token as part of the value of the password parameter, i.e 'password' = password+':'+token, doesn't work for users with long passwords because of what appears to be a bug on their end. This isn't a big deal, because instead of using Reddit's suggested method, it also turns out that you can pass the 2FA token to /api/v1/access_token in a separate otp parameter—see below. PRAW or prawcore could be tweaked to handle 2FA tokens if users input their original otp secret at initialization,

Describe alternatives you've considered
Update documentation to inform users with long passwords and 2FA enabled that they're SOL.

Additional context
Someone on /r/redditdev claimed that you couldn't use a password flow with 2FA if the password was >= 72 characters long, and it's true. Setting the password to password:token fails for large passwords, with or without PRAW. Reddit passwords can be up to 500kb long, but bcrypt only cares about the first 72 characters, so I would guess that Reddit has a bug somewhere in their password validation process.

I used the network inspector while logging into new Reddit, and noticed that the password form had a field for the 2FA token named 'otp'. On a whim, I added 'otp':'xxxxxx' to the POST data of the usual non-PRAW oauth login with Python:

import pyotp
import requests

otp_secret = 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
otp_token = pyotp.TOTP(otp_secret).now()
client_auth = requests.auth.HTTPBasicAuth('client_id','client_secret')
post_data = {
	"grant_type": "password",
	"username": username,
	"password": password,
	"otp": otp_token
}
headers = {"User-Agent": "xxxxxxxxxxxxxxxxxxxxxxxx"}
response = requests.post(
	"https://www.reddit.com/api/v1/access_token",
	auth=client_auth,
	data=post_data,
	headers=headers
)

...and it worked. response.json() returns the usual set of

{'access_token': 'xxxxxxxx-xxxxxxxxxxxxxxxxxxxxxxxxxxx', 'token_type': 'bearer', 'expires_in': 3600, 'scope': '*'}.

So sure enough, 'otp' is a valid parameter for /api/v1/access_token when using a password with 2FA. If a user passed in the otp_secret at initialization, when it comes time to fetch or refresh bearer tokens, PRAW could then just calculate the 2FA token with pyotp.TOTP(otp_secret).now() (or with an equivalent function) and add that to the rest of the data in the request.

@PythonCoderAS
Copy link
Contributor

PythonCoderAS commented May 6, 2020

That would require giving the 2FA secret to PRAW, and it's only available once. I mean it would just be easier to use a refresh token. Maybe a better method would be to just ask for the 2fa token itself, so that instead of appending it to the password, you give it as a separate parameter. Using the secret will also require another dependency, so I'm not too fond of the idea.

@PythonCoderAS PythonCoderAS added the Feature New feature or request label May 6, 2020
@PythonCoderAS PythonCoderAS added this to the PRAW 7.1 milestone May 6, 2020
@jarhill0
Copy link
Contributor

jarhill0 commented May 6, 2020

Thanks for the investigation, @MaybeNetwork! I think that PRAW and prawcore should certainly use this otp parameter. It could also be nice to add support for generating otp codes on the fly from the secret as you mention. However, as @PythonCoderAS says, calculating an otp code may require another dependency, and consistent authentication can just as easily be obtained with a refresh token.

@PythonCoderAS
Copy link
Contributor

I think a better idea may be to include a note on how a user could generate it on their own. Maybe if the otp param was used, we could send a different exception that they could use and call another method (maybe Reddit.resupply_otp?) to get another token.

@PythonCoderAS
Copy link
Contributor

Of course, all of this depends on prawcore being updated.

@MaybeNetwork
Copy link
Contributor Author

With the linked changes on my forks of praw and prawcore, and assuming that pyotp is on your system. you can run something like

import praw
reddit = praw.Reddit(
    client_id=my_client_id,
    client_secret=my_client_secret,
    user_agent=my_user_agent,
    username='MaybeNetwork',
    password=my_password,
    otp_secret='xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
)

The other usual flows should work too.

It's understandable if you don't want to add things that require extra dependencies, since a user with 2FA could just use refresh tokens. Adding a line in the documentation about the Reddit 2FA login bug would also work.

@PythonCoderAS
Copy link
Contributor

I think it would just be easier to use the OTP parameter to add a 1-time token. We could mention using pyotp in order to generate this token.

@PythonCoderAS
Copy link
Contributor

Another option is to supply a function that will return the 2FA code, so that once could supply input and then every hour it would prompt for a code.

@jarhill0
Copy link
Contributor

jarhill0 commented May 7, 2020

Another option is to supply a function that will return the 2FA code, so that once could supply input and then every hour it would prompt for a code.

I think you're onto something. This could be an elegant solution. Usage would look like:

import praw
import pyotp 

reddit = praw.Reddit(
    client_id=my_client_id,
    client_secret=my_client_secret,
    user_agent=my_user_agent,
    username='MaybeNetwork',
    password=my_password,
    otp_generator=lambda: pyotp.TOTP(otp_secret).now()
)

That makes it easier for users without requiring PRAW to add another dependency for an edge case.

@PythonCoderAS
Copy link
Contributor

I can adapt the code he has to just take this function.

@MaybeNetwork
Copy link
Contributor Author

There is a missing closing ) in the documentation about authentication.

print(reddit.auth.url(["identity"], "...", implicit=True)

should be

print(reddit.auth.url(["identity"], "...", implicit=True))

It could be added if that page is updated with instructions about 2FA.

@PythonCoderAS
Copy link
Contributor

Thanks for noting!

@bboe
Copy link
Member

bboe commented May 7, 2020

Another option is to supply a function that will return the 2FA code, so that once could supply input and then every hour it would prompt for a code.

I really like that idea. I'd suggest starting with said callback in prawcore to ensure it works.

@PythonCoderAS PythonCoderAS modified the milestones: PRAW 7.1, PRAW 7.1.1 Jun 23, 2020
@LilSpazJoekp LilSpazJoekp modified the milestones: PRAW 7.1.1, PRAW 8.0 Feb 18, 2021
@github-actions
Copy link

This issue is stale because it has been open for 20 days with no activity. Remove the Stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale Issue or pull request has been inactive for 20 days label May 20, 2021
@MaybeNetwork
Copy link
Contributor Author

I can close this issue with a PR soon.

@github-actions github-actions bot removed the Stale Issue or pull request has been inactive for 20 days label May 21, 2021
@github-actions
Copy link

This issue is stale because it has been open for 20 days with no activity. Remove the Stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale Issue or pull request has been inactive for 20 days label Jun 10, 2021
@LilSpazJoekp LilSpazJoekp removed the Stale Issue or pull request has been inactive for 20 days label Jun 10, 2021
@github-actions
Copy link

This issue is stale because it has been open for 20 days with no activity. Remove the Stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale Issue or pull request has been inactive for 20 days label Jun 30, 2021
@MaybeNetwork
Copy link
Contributor Author

Commenting just so that this can be closed with a link instead of being auto-closed

@github-actions github-actions bot removed the Stale Issue or pull request has been inactive for 20 days label Jul 10, 2021
@github-actions
Copy link

This issue is stale because it has been open for 20 days with no activity. Remove the Stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale Issue or pull request has been inactive for 20 days label Jul 30, 2021
@LilSpazJoekp LilSpazJoekp removed the Stale Issue or pull request has been inactive for 20 days label Jul 30, 2021
@github-actions
Copy link

This issue is stale because it has been open for 20 days with no activity. Remove the Stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale Issue or pull request has been inactive for 20 days label Aug 19, 2021
@github-actions
Copy link

This issue was closed because it has been stale for 10 days with no activity.

@github-actions github-actions bot added the Auto-closed - Stale Automatically closed due to being stale for too long label Aug 29, 2021
@bboe
Copy link
Member

bboe commented Aug 29, 2021

Commenting just so that this can be closed with a link instead of being auto-closed

What did you mean by that?

@bboe bboe reopened this Aug 29, 2021
@github-actions github-actions bot removed Auto-closed - Stale Automatically closed due to being stale for too long Stale Issue or pull request has been inactive for 20 days labels Aug 29, 2021
@LilSpazJoekp
Copy link
Member

What did you mean by that?

He means linking this issue to a PR with Closes #1496 or manually linking it.

@github-actions
Copy link

github-actions bot commented Oct 1, 2021

This issue is stale because it has been open for 20 days with no activity. Remove the Stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale Issue or pull request has been inactive for 20 days label Oct 1, 2021
@github-actions
Copy link

This issue was closed because it has been stale for 10 days with no activity.

@github-actions github-actions bot added the Auto-closed - Stale Automatically closed due to being stale for too long label Oct 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auto-closed - Stale Automatically closed due to being stale for too long Feature New feature or request Stale Issue or pull request has been inactive for 20 days
Projects
None yet
Development

No branches or pull requests

5 participants