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

✨ Update authentication scheme #51

Merged
merged 2 commits into from
Dec 3, 2020

Conversation

mnlevy1981
Copy link
Contributor

Rather than rely on connect_kwargs['password'] to send password to Connection, use a lightweight handler that is basically a lightweight version of the example in https://stackoverflow.com/a/43943658. This has the added bonus of the password prompt reflecting what you get from ssh... e.g. Casper requests Token_Response: and if you have your ssh keys configured to avoid requiring a password then you should no longer get any prompt at all (rather than a prompt that is not actually passed to the host).

Fixes #49

users can specify --tfa-2-pass if remote machine requires users to type in two
passwords (e.g. google authenticate)
Removed password from connect_kwargs and now use auth_interactive_dumb() for
all interactive prompts... so I removed the --tfa-2-pass option and now any
BadAuthenticationType exceptions (whether missing password or missing password
and 2FA) will prompt users for input. Note that these prompts will always go
through getpass, so even things like username will not be mirrored on stdout.
@mnlevy1981
Copy link
Contributor Author

I'd vote for a squash-merge on this one as an API change I made in 85a05b5 was undone in 2134078 and we don't need to carry those poor decisions around :)

Copy link
Member

@andersy005 andersy005 left a comment

Choose a reason for hiding this comment

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

This looks great 👍... I'm going to wait for @rmg55's confirmation that this is working properly before merging...

@rmg55
Copy link

rmg55 commented Dec 3, 2020

This solved the authentication issue I raised in #49 . Thanks of bunch @andersy005 and @mnlevy1981

@andersy005
Copy link
Member

Many thanks @mnlevy1981 & @rmg55! Merging this....

@andersy005 andersy005 changed the title Update authentication scheme ✨ Update authentication scheme Dec 3, 2020
@andersy005 andersy005 merged commit a11177e into ncar-xdev:master Dec 3, 2020
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.

2nd Factor Auth. - Google Authenticator
3 participants