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

feat: asks user for OTP in CLI #636

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

timbru31
Copy link
Contributor

@timbru31 timbru31 commented Apr 1, 2021

feat: sends a notification if OTP input is required
feat: tries to set the "Do not ask for OTP on this device" flag

This closes #617

This is the notification the bot sends on OTP titles:
otp

@timbru31
Copy link
Contributor Author

timbru31 commented Apr 1, 2021

I'll look into the build failure, seems like my local formatting is off.

@DakkJaniels
Copy link
Collaborator

Yeah, requires blackd format

@timbru31
Copy link
Contributor Author

timbru31 commented Apr 1, 2021

Fixed the formatting.

@Spareo
Copy link

Spareo commented Apr 3, 2021

I just tested this with my setup and I run into an issue because I have another screen before the OTP screen asking which form of OTP I want to use

Screen Shot 2021-04-03 at 2 00 08 PM

Since the OTP box is unavailable, the flow fails with
2021-04-03 14:00:10|0.7.0.dev2|ERROR| OTP entry box did not exist, please fill in OTP manually...

@timbru31
Copy link
Contributor Author

timbru31 commented Apr 3, 2021

I’ve feared this is a possible intermediate screen. I’ll see if I can reproduce this with my second account.

the question remains though: what should the bot choose?

@Spareo
Copy link

Spareo commented Apr 3, 2021

I’ve feared this is a possible intermediate screen. I’ll see if I can reproduce this with my second account.

the question remains though: what should the bot choose?

@timbru31 if the solution proposed in this PR is to allow the user to input an OTP value from the CLI, then I think its safe to assume that the user should be using a 2FA app which can generate an OTP. The OTP from the 2FA app should be input into the console for the bot to use.

I think adding support for the other options is unnecessary and more nice-to-have than useful. I captured screenshots of the elements that need to be selected by the bot

The radio selection fields
Screen Shot 2021-04-03 at 4 52 30 PM

The button to click after selecting the radio button
Screen Shot 2021-04-03 at 4 52 47 PM

@timbru31
Copy link
Contributor Author

timbru31 commented Apr 3, 2021

Thanks! I’ll integrate this intermediate screen

@timbru31 timbru31 marked this pull request as draft April 3, 2021 22:05
@Spareo
Copy link

Spareo commented Apr 3, 2021

Cool, ill wait for the PR update and try it out

@timbru31
Copy link
Contributor Author

@Spareo can you give my new commits a second try?

feat: sends a notification if OTP input is required
feat: tries to set the "Do not ask for OTP on this device" flag

This closes Hari-Nagarajan#617
@Spareo
Copy link

Spareo commented Apr 17, 2021

@timbru31 just tried it out, worked great! Worked in headless as well. Thank you!

@timbru31
Copy link
Contributor Author

Nice. Marking as ready then 👍👊

@timbru31 timbru31 marked this pull request as ready for review April 17, 2021 23:17
Copy link
Collaborator

@DakkJaniels DakkJaniels left a comment

Choose a reason for hiding this comment

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

So one concern I have is that if the OTP is entered in the terminal, people are going to get confused about why it doesn't continue if they enter in their OTP in the browser and it doesn't continue.
Should we enable this only if running headless? Thoughts?

@timbru31
Copy link
Contributor Author

This might be a change in the current flow, but the e-mail and password are entered in the CLI, too.

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.

Feature Request: 2FA support for headless mode
3 participants