Enhancement/error hints #1060

Onokaev · 2021-08-17T10:37:22Z

Overview

Solves the following authentication errors: null_or_empty_id_token,authorization_code_missing_from_server_response, no_tokens_found, invalid_request, user_login_error,nonce_mismatch_error, login_progress_error, interaction_in_progress,
'interaction_required, invalid_grant, endpoints_resolution_error, monitor_window_timeout.

Shows the user hints in case of common user mistakes like cancelling authentication popups, more information when scope consent is blocked and general hints on what to do when an authentication error occurs.

Demo

Notice the tip displayed after a user cancels the popup

Error and hint when a user cancels the auth consent on the auth pop-up window

Notes

Users might still get the authentication errors when signing in, but will not be blocked. Retrying a sign-in for the second time succeeds.

Testing Instructions

-Try signing in then cancel the window pop-up. A hint will be displayed on a messagebar on what to do.

-Sign in with your account and cancel the consent process or close the pop-up window. A hint will be displayed on what to do.

github-actions · 2021-08-17T10:41:53Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://jolly-sand-0ac78c710-1060.centralus.azurestaticapps.net

thewahome · 2021-08-17T13:18:01Z

src/app/views/authentication/Authentication.tsx

-          Message: `Authentication failed: ${errorCode ? errorCode.replace('_', ' ') : ''}`,
-        });
+          Message: `Authentication failed: ${
+            errorCode ? errorCode.replace('_', ' ') : ''


This will replace only the first occurrence of the _ character. Try doing it like this

Suggested change

errorCode ? errorCode.replace('_', ' ') : ''

errorCode ? errorCode.replace(/_/g, ' '): ''

Yes. Let me fix that

thewahome · 2021-08-17T13:26:13Z

src/app/views/authentication/Authentication.tsx

+  const deleteHomeAccountId = () : void => {
+    localStorage.removeItem(HOME_ACCOUNT_KEY);
  }


This method has already been declared in the modules\authentication\AuthenticationWrapper.ts file as a private method. Changing it to a public function enables access to it from outside of the class

Okay. Let me fix this also

thewahome · 2021-08-17T13:28:36Z

src/app/views/authentication/Authentication.tsx

-        <FormattedMessage id={`Signing you ${loginInProgress ? 'in' : 'out'}...`} />
-      </Label>}
-    </div>;
+  const getErrorAndHint = (errorCode: string): string => {


A function should do one thing only. Consider breaking down the functionalities here so that the getErrorAndHint() function gets the error with the hint (if a hint exists) and clearing the cache happens in an appropriately named function that is called elsewhere

I agree. Let me break it down.

thewahome · 2021-08-17T13:33:15Z

src/modules/authentication/AuthenticationWrapper.ts

+    for(const errorItem of AuthErrorList){
+      if( errorItem.trim() === errorCode.trim() ){
+        return true;
+      }
+    }
+    return false;


This could give the same result

Suggested change

for(const errorItem of AuthErrorList){

if( errorItem.trim() === errorCode.trim() ){

return true;

}

}

return false;

return AuthErrorList.includes(errorCode.trim());

Yeah. I've seen it. Shorter and cleaner. Let me fix this also

thewahome · 2021-08-17T13:45:23Z

src/modules/authentication/authUtils.ts

@@ -20,6 +20,6 @@ export function getLoginType(): LoginType {
 * -js/blob/9274fac6d100a6300eb2faa4c94aa2431b1ca4b0/lib/msal-browser/src/utils/BrowserUtils.ts#L49
 */
 export function getCurrentUri(): string {
-  const currentUrl = window.location.href.split('?')[0].split('#')[0];
-  return currentUrl.toLowerCase();
+  const currentUrl = window.location.href.split('?')[0].split('#')[0] + 'blank.html';


question: what is the effect of adding the blank.html to the URL?

https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-browser/docs/errors.md#monitor_window_timeout

We will no longer be getting monitor_window_timeout errors according to the msal documentation. I could use your input on this

thewahome · 2021-08-17T13:50:28Z

src/modules/authentication/AuthenticationWrapper.ts

+        if(this.checkIfAuthResultError(errorCode) && !this.consentingToNewScopes){
+        this.clearCache();
+        this.deleteHomeAccountId();
+        window.sessionStorage.clear();
+        this.eraseInteractionInProgressCookie();
+        const result = await msalApplication.loginPopup(popUpRequest);
+        this.storeHomeAccountId(result.account!);
+        return result;
+      }


nitpick: Check the indentation in this section

suggestion: There is a chance for a situation where the user will be in a popup loop since the error doesn't self resolve if the app tries to re-request a login popup. It could be safer to throw the error after clearing the cache since the user will get a hint about what went wrong

That's a good idea. I missed that. I'll throw the error after clearing the cache.

thewahome · 2021-08-17T13:56:25Z

src/app/views/authentication/Authentication.tsx

+        window.sessionStorage.clear();
+      }
+    }
+    return errorCode.replace('_', ' ') + '      ' + translateMessage('Tip') + ':  ' + errorMessageHint;


This should return the tip message if the error message hint exists

Let me add a check for that also. Nice observation

thewahome · 2021-08-17T13:58:13Z

src/app/services/actions/permissions-action-creator.ts

@@ -115,7 +115,7 @@ export function consentToScopes(scopes: string[]): Function {
      dispatch(
        setQueryResponseStatus({
          statusText: translateMessage('Scope consent failed'),
-          status: errorCode,
+          status: errorCode + ' ' + translateMessage('Tip') + ': ' + translateMessage('Access to permission denied'),


question: Is this the only situation that consenting to permissions fails?

They are two situations. The first one is an authentication error (access_denied) when a user is signing in to GE for the first time : If the user's tenant admin has restricted consent to client applications, they'll get the access denied error (Authentication failed: access_denied)

The second one is when a user is signed in to GE but they cannot consent to certain permissions because the tenant admin has blocked that. For this case, the user gets a 'Scope consent failed: access_denied

Is there a different scenario I have left out?

github-actions · 2021-08-19T13:56:42Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://jolly-sand-0ac78c710-1060.centralus.azurestaticapps.net

github-actions · 2021-08-23T13:37:28Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://jolly-sand-0ac78c710-1060.centralus.azurestaticapps.net

github-actions · 2021-08-23T13:38:54Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://jolly-sand-0ac78c710-1060.centralus.azurestaticapps.net

thewahome · 2021-08-23T15:01:39Z

package.json

@@ -6,7 +6,7 @@
    "@azure/msal-browser": "2.12.0",
    "@babel/core": "7.12.13",
    "@babel/eslint-parser": "7.12.13",
-    "@fluentui/react": "8.28.0",
+    "@fluentui/react": "^8.28.0",


Let's keep this one at a constant.. So we can remove ^

thewahome · 2021-08-23T15:04:08Z

src/types/status.ts

@@ -6,4 +6,5 @@ export interface IStatus {
  status: number;
  statusText: string;
  duration?: number;
+  hint?: string;


github-actions · 2021-08-23T19:29:50Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://jolly-sand-0ac78c710-1060.centralus.azurestaticapps.net

github-actions · 2021-08-26T06:04:00Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://jolly-sand-0ac78c710-1060.centralus.azurestaticapps.net

github-actions · 2021-08-26T09:03:49Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://jolly-sand-0ac78c710-1060.centralus.azurestaticapps.net

github-actions · 2021-08-27T08:14:10Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://jolly-sand-0ac78c710-1060.centralus.azurestaticapps.net

* Feature: Highlights the previously selected query (#1006) * Task: upgrade packages flagged by dependabot (#1054) * Task: Fluent upgrade (#1056) * Fix: ProfileType persistence (#1074) * Enhancement: error hints (#1060) * Bug: Persisting request body (#1055) * Fix: Sample queries UI bugs (#1084) * Fix: Disable aria-required children rule (#1080) * Task: Fix failing profile test (#1086) * Enhancement: permissions radio buttons change (#1087) * Task: add English file transfer automation (#1075)

Onokaev added 7 commits August 12, 2021 10:28

initial commit on auth error hints

6180027

added more errors to error library

3d144ea

fixed auth errors and added error tips

18ada30

updating local branch with dev

66820c1

Merge branch 'dev' into enhancement/error-hints

bb131aa

removed interaction_status from cookies

8cc2ded

added return type to createCookie

e15e689

Onokaev requested review from adhiambovivian, ddyett, ElinorW, millicentachieng and thewahome as code owners August 17, 2021 10:37

thewahome reviewed Aug 17, 2021

View reviewed changes

removed uri extension and added consent error tips

d1c3f84

Onokaev requested a review from thewahome August 19, 2021 15:04

Onokaev added 2 commits August 23, 2021 16:32

error hints now appear below error-code

f8bcc2f

fixed formating errors

5e11faf

thewahome reviewed Aug 23, 2021

View reviewed changes

maintained fluentui at 8.28.0

ed012a6

Onokaev added 2 commits August 25, 2021 09:30

adjustment to hints wording

0b8589e

corrected hinting language

44bc916

thewahome linked an issue Aug 26, 2021 that may be closed by this pull request

Authentication failed - interaction in progress #1039

Closed

code refactors in error hinting

9e93661

thewahome approved these changes Aug 26, 2021

View reviewed changes

Merge branch 'dev' into enhancement/error-hints

979c023

Onokaev merged commit ce8439a into dev Aug 27, 2021

Onokaev mentioned this pull request Sep 2, 2021

Add a warning if User Consent has been blocked #152

Closed

thewahome deleted the enhancement/error-hints branch September 9, 2021 07:58

Onokaev mentioned this pull request May 23, 2022

Drive down top errors impacting Graph Explorer #1070

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhancement/error hints #1060

Enhancement/error hints #1060

Onokaev commented Aug 17, 2021 •

edited

Loading

github-actions bot commented Aug 17, 2021

thewahome Aug 17, 2021

Onokaev Aug 17, 2021

thewahome Aug 17, 2021

Onokaev Aug 17, 2021

thewahome Aug 17, 2021

Onokaev Aug 17, 2021

thewahome Aug 17, 2021

Onokaev Aug 17, 2021

thewahome Aug 17, 2021

Onokaev Aug 17, 2021

thewahome Aug 17, 2021

Onokaev Aug 17, 2021

thewahome Aug 17, 2021

Onokaev Aug 17, 2021

thewahome Aug 17, 2021

Onokaev Aug 17, 2021

github-actions bot commented Aug 19, 2021

github-actions bot commented Aug 23, 2021

github-actions bot commented Aug 23, 2021

thewahome Aug 23, 2021

thewahome Aug 23, 2021

github-actions bot commented Aug 23, 2021

github-actions bot commented Aug 26, 2021

github-actions bot commented Aug 26, 2021

github-actions bot commented Aug 27, 2021

	errorCode ? errorCode.replace('_', ' ') : ''
	errorCode ? errorCode.replace(/_/g, ' '): ''

Enhancement/error hints #1060

Enhancement/error hints #1060

Conversation

Onokaev commented Aug 17, 2021 • edited Loading

Overview

Demo

Notes

Testing Instructions

github-actions bot commented Aug 17, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 19, 2021

github-actions bot commented Aug 23, 2021

github-actions bot commented Aug 23, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 23, 2021

github-actions bot commented Aug 26, 2021

github-actions bot commented Aug 26, 2021

github-actions bot commented Aug 27, 2021

Onokaev commented Aug 17, 2021 •

edited

Loading