[BugBash] MSA - Add exception handler for access denied #5485
Conversation
shishirx34
requested review from
joelverhagen,
loic-sharma,
ryuyu,
anangaur,
chenriksson,
cristinamanum,
skofman1 and
scottbommarito
| @@ -33,6 +35,9 @@ public static class AuthenticationType | |||
| public static readonly string V2CommonTenant = "common"; | |||
| public static readonly string Authority = "https://login.microsoftonline.com/{0}/v2.0"; | |||
|
|
|||
| private string redirectUrl; | |||
There was a problem hiding this comment.
authenticator instance is per request?
There was a problem hiding this comment.
Ah! good catch, it is a SingleInstance. this won't work.
There was a problem hiding this comment.
Can we ignore the redirectUrl in the error case, and just redirect to home with the info message?
There was a problem hiding this comment.
I found another way. This is cleaner.
| if (notification.Exception.Message == ACCESS_DENIED) | ||
| { | ||
| notification.HandleResponse(); | ||
| notification.Response.Redirect(redirectUrl); |
There was a problem hiding this comment.
For confirm transform by the org admin, it makes sense to redirect so the user sees the transform failed error page if the AAD linking failed or was cancelled.
For UIAuthorize actions that don't support deprecated password auth, will the user still see the info message if the UIAuthorize redirects them to the homepage for the link/transform prompt?
There was a problem hiding this comment.
This will only be shown once when the permission was denied to access permissions, consumed from cookie and is removed so won't repeat upon further UIAuthorize redirects.
| .Unprotect(authenticationPropertiesEncodedString[1]); | ||
|
|
||
| notification.HandleResponse(); | ||
| notification.Response.Redirect(authenticationProperties.RedirectUri); |
There was a problem hiding this comment.
Is this guaranteed to be set, or do we need to default to redirecting to homepage?
There was a problem hiding this comment.
Its guaranteed to be set, see
The OpenIdConnect library throws exception upon failed authentication which resulted in 500 when access was denied by user on MSA/AAD auth. This fixes #5451 and shows ExternalLinkFailed error.