Merge pull request #193 from peat-psuwit/error-codes

Improve error messages for Azure AD error codes

internal/aad/aad.go

@@ -17,9 +17,11 @@ import (
 const (
 	endpoint = "https://login.microsoftonline.com"
 
-	invalidCredCode = 50126
-	requiresMFACode = 50076
-	noSuchUserCode  = 50034
+	invalidCredCode    = 50126
+	requiresMFACode    = 50076
+	noSuchUserCode     = 50034
+	noConsentCode      = 65001
+	noClientSecretCode = 7000218
 )
 
 var (
@@ -86,8 +88,25 @@ func (auth AAD) Authenticate(ctx context.Context, cfg config.AAD, username, pass
 				logger.Debug(ctx, "Authentication successful even if requiring MFA")
 				return nil
 			}
+			if errcode == noConsentCode {
+				logger.Err(ctx, "Azure AD application requires consent, either from tenant, or from user. "+
+					"If you're a tenant's administrator, go to: %s/adminconsent?client_id=%s",
+					authority, cfg.AppID)
+				return ErrDeny
+			}
+			if errcode == noClientSecretCode {
+				logger.Err(ctx, "Azure AD application requires enabling 'Allow public client flows'. "+
+					"https://learn.microsoft.com/en-us/azure/active-directory/develop/scenario-desktop-app-registration#redirect-uris")
+				return ErrDeny
+			}
 		}
 		logger.Err(ctx, "Unknown error code(s) from server: %v", addErrWithCodes.ErrorCodes)
+
+		logger.Debug(ctx, "For more information about the error code(s), see:")
+		for _, errcode := range addErrWithCodes.ErrorCodes {
+			logger.Debug(ctx, "- Error code %d: https://login.microsoftonline.com/error?code=%d", errcode, errcode)
+		}
+
 		return ErrDeny
 	}
 

internal/aad/aad_test.go

@@ -24,6 +24,8 @@ func TestAuthenticate(t *testing.T) {
 
 		// error cases
 		"can't connect to authority": {appID: "connection failed", wantErr: aad.ErrNoNetwork},
+		"public client disallowed":   {appID: "public client disallowed", wantErr: aad.ErrDeny},
+		"no tenant-wide consent":     {appID: "no tenant-wide consent", wantErr: aad.ErrDeny},
 		"unreadable server response": {username: "unreadable server response", wantErr: aad.ErrDeny},
 		"invalid server response":    {username: "invalid server response", wantErr: aad.ErrDeny},
 		"invalid credentials":        {username: "invalid credentials", wantErr: aad.ErrDeny},

internal/aad/msal_mock.go

@@ -21,17 +21,31 @@ func NewWithMockClient() AAD {
 
 func publicNewMockClient(clientID string, _ ...public.Option) (publicClient, error) {
 	var forceOffline bool
+	var publicClientDisallowed bool
+	var noTenantWideConsent bool
+
 	switch clientID {
 	case "connection failed":
 		return publicClientMock{}, errors.New("connection failed")
 	case "force offline":
 		forceOffline = true
+	case "public client disallowed":
+		publicClientDisallowed = true
+	case "no tenant-wide consent":
+		noTenantWideConsent = true
 	}
-	return publicClientMock{forceOffline: forceOffline}, nil
+
+	return publicClientMock{
+		forceOffline:           forceOffline,
+		publicClientDisallowed: publicClientDisallowed,
+		noTenantWideConsent:    noTenantWideConsent,
+	}, nil
 }
 
 type publicClientMock struct {
-	forceOffline bool
+	forceOffline           bool
+	publicClientDisallowed bool
+	noTenantWideConsent    bool
 }
 
 func (m publicClientMock) AcquireTokenByUsernamePassword(_ context.Context, _ []string, username string, _ string, _ ...public.AcquireByUsernamePasswordOption) (public.AuthResult, error) {
@@ -44,6 +58,16 @@ func (m publicClientMock) AcquireTokenByUsernamePassword(_ context.Context, _ []
 		return r, fmt.Errorf("Offline")
 	}
 
+	if m.publicClientDisallowed {
+		callErr.Resp.Body = io.NopCloser(strings.NewReader(fmt.Sprintf("{\"error_codes\": [%d]}", noClientSecretCode)))
+		return r, callErr
+	}
+
+	if m.noTenantWideConsent {
+		callErr.Resp.Body = io.NopCloser(strings.NewReader(fmt.Sprintf("{\"error_codes\": [%d]}", noConsentCode)))
+		return r, callErr
+	}
+
 	switch username {
 	case "success@domain.com":
 	case "success@otherdomain.com":