ArranJacques / laravel-aws-cognito-auth

An authentication driver for Laravel for authenticating users in AWS Cognito User Pools.
Other
74 stars 26 forks source link

New password required challenge #2

Closed zoul0813 closed 7 years ago

zoul0813 commented 7 years ago

Seems my users are failing to authenticate because of NEW_PASSWORD_REQUIRES challenge response being sent back. I dug into the code and noticed its throwing a Failed event due to this challenge response. Was wondering if adding some custom events to handle these challenges might be possible? Right now I'm looking at forking the project and adding a generic Challenge event that I can listen for and respond too... for example, redirecting the user to a "change password" form.

Dayjo commented 7 years ago

Sounds like a good plan @zoul0813 . Feel free to create a pull request!

ArranJacques commented 7 years ago

Yes this is a good idea.

Originally I wanted to keep the authentication as close to native Laravel's as possible and not have to have users wrap their Auth::attemp() calls in try catch statements, which is why it currently catches all errors thrown by the API.

Realistically this isn't going to work though so I'll take a look at implementing a solution for handling errors with custom events and handlers.

Will see if I can get this up in the next day or two.

Thanks.

zoul0813 commented 7 years ago

Thanks Arran - We'd need support to handle exceptions like PasswordResetRequiredException, UserNotConfirmedException as well as the various Challenge's like NEW_PASSWORD_REQUIRED and SMS_MFA.

If you think you can have something up and working in the next day or so, I'll hold off on my end and see what you come up with.

zoul0813 commented 7 years ago

Arran, any update on this? If you've got delays on your end then I'll just fork the project and see what I can come up with ... but I didn't want to do that and wind up having us go in different directions. Thanks.

ArranJacques commented 7 years ago

Hi.

Sorry, we got busy in the office and this had to take a back seat. I've got some free time tomorrow so I'm going to take a look at it then. Will keep you posted.

Thanks

ArranJacques commented 7 years ago

I've added in 3 different ways of handling failed authentication attempts so hopefully one of them will work for your particular use case. You can read about how they work here https://github.com/pallant/laravel-aws-cognito-auth#handling-failed-authentication.

If they don't work for your use case then let me know and I'll take another look.

Thanks

zoul0813 commented 7 years ago

I made a couple changes to my fork. I created a handleError($response, $handler) and getErrorHandler($handler) method on AwsCognitoIdentityGuard, as well as added a $errorHandler property. I also updated the __construct() to look at $this->config['errors']['handler'] to determine if the developer has a default handler in the config. As well as added support for calling an event handler class

use Pallant\LaravelAwsCognitoAuth\AuthAttempt;

class AwsCognitoHandler {
  public function handle(AuthAttempt $response) {
    // TODO: handle error response
  }
}

The class can be used by adding something like this to config/aws-cognito-auth.php

    'errors' => [
        'handler' => App\Listeners\AwsCognitoHandler::class,
    ],

Setting up the default error handler means I can have the AwsCognitoHandler class handle($response) method take care of all my custom logic, and also means I don't have to add extra params to my Auth::* calls ... allowing me to possibly use other packages which make generic Auth:: calls, as well as swapping cognito out with something else later on (or switching to cognito from existing projects without updating Auth:: calls).

I've created a pull request with these changes.

Ref #3

ArranJacques commented 7 years ago

Good idea. I've merged in your pull request, but have made a few changes. Specifically:

use Pallant\LaravelAwsCognitoAuth\AuthAttemptException;

class AwsCognitoHandler
{
    public function handle(AuthAttemptException $e)
    {
        $response = $e->getResponse();
        // Handle error...
    }
}

I also updated the README documentation to include instructions on using a default error handler and custom error handling class.

zoul0813 commented 7 years ago

Sounds good - thanks.

zoul0813 commented 7 years ago

Closing issue