Add two factor callback #111
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bboe
reviewed
May 26, 2021
tests/conftest.py
Outdated
| @@ -31,6 +31,11 @@ def b64_string(input_string): | |||
| return b64encode(input_string.encode("utf-8")).decode("utf-8") | |||
|
|
|||
|
|
|||
| def test_two_factor_callback(): | |||
There was a problem hiding this comment.
Since this isn't a test case can we remove the test_ prefix please?
tests/test_authorizer.py
Outdated
| @@ -340,6 +341,60 @@ def test_refresh(self): | |||
| self.assertEqual(set(["*"]), authorizer.scopes) | |||
| self.assertTrue(authorizer.is_valid()) | |||
|
|
|||
| # def test_refresh_with__2fa(self): | |||
There was a problem hiding this comment.
I don't think this example is necessary. If you want to add some information in the documentation about running tests with 2FA that'd be great, but not necessary for this PR.
There was a problem hiding this comment.
Ok. I included it just in case you wanted to verify that the feature works, since the other tests don't really prove it.
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This feature allows script authorizations to supply an optional 'otp' parameter when authenticating. This is accomplished by adding a callback named
two_factor_callbackto the methodScriptAuthorizer.refresh.See 97 and praw's 1496 for previous discussions of this.I have added some support for running other tests with 2fa enabled. There is now a
test_two_factor_callbackfunction in conftest, which is used in script auth tests to generate an OTP. By default, it returns None, which allows all tests to pass. This PR also adds two tests that show what successful and unsuccessful authorization attempts look like. I have included a third test in test_authorizer,test_refresh_with__2fa, that is commented out and does not have a corresponding cassette. If you changetest_two_factor_callbackto something that will generate your real otp code, you can use this test to produce the same output astest_refresh_with__valid_otp. You don't need to install a separate library to run the test if you have already have a 2fa authenticator device, program, or app. Just change the definition of test_two_factor_callback fromreturn Nonetoreturn input("OTP: "), run the test with pytest -s, and enter the code when prompted.There are some issues with running tests with 2fa enabled. The uri for the initial authorization in all previous tests run with a password flow looks like
but if you use 2fa, this changes to
This means that all tests which match uris will fail if you change the test_two_factor_callback to anything other than something that returns None. This could be dealt with by adding before_record and before_playback callbacks to the Betamax config, but I don't think it is worth doing that right now. Also, the rate limit for the password flow doesn't really allow you to record multiple tests at once. If you try, you will get the same
{'error': 'invalid_grant'}message if you're still in timeout that you get when you supply the wrong credentials. The other flows aren't affected.Other notes: if you don't have 2fa on your account, you can authorize with the otp parameter present as long as it is set to the empty string. If you make it anything other than the empty string, you'll get an invalid grant.