Skip to content

Add OTP function#97

Closed
PythonCoderAS wants to merge 4 commits intopraw-dev:masterfrom
PythonCoderAS:otp-func
Closed

Add OTP function#97
PythonCoderAS wants to merge 4 commits intopraw-dev:masterfrom
PythonCoderAS:otp-func

Conversation

@PythonCoderAS
Copy link
Contributor

Per praw-dev/praw#1496 this will allow OTPs (2fa codes) to be supplied hourly instead of being appended to the password.

Copy link
Contributor

@jarhill0 jarhill0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is surprisingly simple to implement. Can you also add tests and update the changelog?

Also, I'd like to hear @bboe's thoughts on this interface. I laid out the reasons why I like it in my comment.

Co-authored-by: Joey RH <jarhill0@users.noreply.github.com>
@PythonCoderAS
Copy link
Contributor Author

This is surprisingly simple to implement. Can you also add tests and update the changelog?

Also, I'd like to hear @bboe's thoughts on this interface. I laid out the reasons why I like it in my comment.

I can't currently test it because I don't have access to my 2FA authenticator device.

Copy link
Member

@bboe bboe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I too am really surprised at how easy adding this support turned out to be. That's awesome. I just have one comment on naming, otherwise, this is awesome!

@MaybeNetwork
Copy link
Contributor

I think that this needs a corresponding change in PRAW to work, similar to the one in my first suggestion. Without one, when you try to authenticate, PRAW executes the following block

if self.config.username and self.config.password:
            script_authorizer = ScriptAuthorizer(
                authenticator, self.config.username, self.config.password
            )
            self._core = self._authorized_core = session(script_authorizer)

which doesn't use the otp_function.

@PythonCoderAS
Copy link
Contributor Author

I think that this needs a corresponding change in PRAW to work, similar to the one in my first suggestion. Without one, when you try to authenticate, PRAW executes the following block

if self.config.username and self.config.password:
            script_authorizer = ScriptAuthorizer(
                authenticator, self.config.username, self.config.password
            )
            self._core = self._authorized_core = session(script_authorizer)

which doesn't use the otp_function.

I'm going to make a PR for that once this gets through first.

@MaybeNetwork
Copy link
Contributor

I can write and run the tests for this, and then add the corresponding feature to PRAW. If it's possible, add me as contributor to this PR. Otherwise I can make a separate PR.

@LilSpazJoekp
Copy link
Member

You'll have to open a PR.

I would cherry-pick the commits from this branch and then add your own commits after that.

@bboe bboe closed this May 21, 2021
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

Successfully merging this pull request may close these issues.

5 participants