warrant
warrant copied to clipboard
Authentication failed not handling nested challenges
Utilizing the develop branch commit a53cb082b6a07f32ddc5c9cacff6f5dfe926f170
Using this code which seems to be entirely correct doesn't seem to authenticate correctly
config = ce.get_config('Cognito', args.configfile)
u = Cognito(config['pool_id'],config['client_id'], username=args.username)
response = u.authenticate(args.password)
2017-04-11 12:24:16 DEBUG parsers.parse Response body:
{"ChallengeName":"NEW_PASSWORD_REQUIRED","ChallengeParameters":{"requiredAttributes":"[]","userAttributes":"{\"email_verified\":\"true\",\"email\":\"<ommitted email>\"}"},"Session":"<sessiong string ommitted>"}
2017-04-11 12:24:16 DEBUG hooks._emit Event needs-retry.cognito-idp.RespondToAuthChallenge: calling handler <botocore.retryhandler.RetryHandler object at 0x7fc01d531d90>
2017-04-11 12:24:16 DEBUG retryhandler.__call__ No retry needed.
Traceback (most recent call last):
File "bin/auth-cognito", line 37, in <module>
main()
File "bin/auth-cognito", line 33, in main
response = u.authenticate(args.password)
File "~/.local/lib/python2.7/site-packages/warrant/__init__.py", line 213, in authenticate
self.id_token = tokens['AuthenticationResult']['IdToken']
KeyError: 'AuthenticationResult'
It doesn't look like there is support for Challenges of NEW_PASSWORD_REQUIRED and it failed to raise like it should https://github.com/capless/warrant/blob/develop/warrant/aws_srp.py#L187
Looks like it assumes there will only be one response challenge. We'll need to add the ability to complete the challenge with an additional input (the new password).
Yeah I also just realized my initial diagnosis in where I thought the problem was was wrong. And you've hit the nail on the head for the conclusion I just came to.
The handling in #15 isn't perfect for NEW_PASSWORD_REQUIRED I'd like there to be a module global which is a callable and returns back the password. Allows users to specify a system to get the password rather than using a TTY is present.
I like the way #25 handles the authentication, it raises by default and then lets the user come up with their own way of getting the password which keeps in line with a web usage model.
Based on your suggested workflow @bjinwright I'm working on this aspect. Not sure if any one else is.
However admin_get_user looks like it requires developer credentials, credentials which we won't have. How would we get the user status without being able to login. I'm assuming a web framework which and the user provides credentials which calls warrant which calls cognito. Those are the only credentials in the framework. Can you expound on how your proposed workflow executes that?
Thanks for the feedback that makes sense
So with that in mind we may need to implement the remaining work flows based on challenge types rather than user status. What do you think?
For historical purpose I'm still going to put my work for user statuses:
UNCONFIRMED - User has been created but not confirmed. ConfirmSignUp
CONFIRMED - User has been confirmed. Normal workflow
ARCHIVED - User is no longer active. Not sure there is a workflow here.
COMPROMISED - User is disabled due to a potential security threat. Not sure there is a workflow here.
UNKNOWN - User status is not known. Not sure there is a workflow here.
RESET_REQUIRED - User has issued a reset password request but not reset it. ConfirmForgotPassword, possibly RespondToAuthChallenge
FORCE_CHANGE_PASSWORD - User has not authenticated for the first time and must change password RespondToAuthChallenge
The current state:
-
Warrant raises
ForceChangePasswordExceptionfor users which are required to change their passwords after successful first login (NEW_PASSWORD_REQUIREDchallenge). -
new_password_challengemethod is not documented -
Warrant raises
NotImplementedErrorwhen a challenge is not supported. -
Warrant doesn't support
SMS_MFA,CUSTOM_CHALLENGE,DEVICE_SRP_AUTHDEVICE_PASSWORD_VERIFIERchallenges.
The first post says about the problem with the NEW_PASSWORD_REQUIRED challenge only and this issue is fixed. The title has more broad meaning, it implies that warrant should support all challenges.
@bjinwright The decision of closing the issue depends on what to take into account: the title or the first post.