FusionAuth / fusionauth-issues

FusionAuth issue submission project
https://fusionauth.io
90 stars 12 forks source link

Throwing Exception within Lambda Exits Login Flow #1707

Open matt1hathcock opened 2 years ago

matt1hathcock commented 2 years ago

Throwing Exception within Reconcile Lambda Exits Login Flow

Description

I configured the Google IdP with a working client id and secret to succesfully login users via Google.

User login was sucessful.

I then added some javascript to the Google Reconcile Lambda to throw an exception.

The lambda was then added to the Google IdP.

Upon attempting to login again the login failed.

The error message is shown as pictured.

Screen Shot 2022-04-28 at 4 49 07 PM

Related

mooreds commented 2 years ago

This is either a doc bug (we should document this behavior) or a product bug (we should catch thrown exceptions from a lambda). I vote for the latter, as I think it is more consistent to have only webhooks be able to intervene in the login flow, but either option works.

robotdan commented 2 years ago

What was the expected behavior in this case? Currently if you throw an exception in the lambda, it will fail the login request, this is by design.

mooreds commented 2 years ago

Excellent, then we should document this.

The alternative is that we don't allow thrown exceptions to stop the login process. But if this is working as designed, I'll just document it.

robotdan commented 2 years ago

I would consider this a dev time issue. I would hesitate to mask the exception because it may affect data integrity depending upon what is happening in the lambda, and what the user of FusionAuth expects the user to look like after a login is complete.

matt1hathcock commented 2 years ago

Need to add some notes here as this was done with a reconcile lambda.

@robotdan This behavior is not the same in the JWT populate lambda as the user is still authenticated and redirected to the application

mooreds commented 2 months ago

Per https://github.com/FusionAuth/fusionauth-site/pull/1670 we should not document this behavior for now.