From f1b4e884ee56193b6fe0d6a68a0b85259a44fa5d Mon Sep 17 00:00:00 2001 From: Jeffrey Chen Date: Fri, 22 Nov 2024 11:17:51 -0800 Subject: [PATCH 1/9] Wrap re-login error with suggestion message --- cli/azd/pkg/auth/errors.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/cli/azd/pkg/auth/errors.go b/cli/azd/pkg/auth/errors.go index e774e460198..0c5a21f1004 100644 --- a/cli/azd/pkg/auth/errors.go +++ b/cli/azd/pkg/auth/errors.go @@ -11,6 +11,7 @@ import ( "slices" msal "github.com/AzureAD/microsoft-authentication-library-for-go/apps/errors" + "github.com/azure/azure-dev/cli/azd/internal" "github.com/azure/azure-dev/cli/azd/pkg/cloud" ) @@ -27,6 +28,8 @@ type ReLoginRequiredError struct { // The scenario in which the login is required scenario string + + description string } // newReLoginRequiredError returns an error if the response indicates that the user needs to reauthenticate. @@ -47,13 +50,15 @@ func newReLoginRequiredError( "interaction_required": err := ReLoginRequiredError{} err.init(response, scopes, cloud) - return &err, true + errWithSuggestion := err.ErrorWithSuggestion() + return &errWithSuggestion, true } return nil, false } func (e *ReLoginRequiredError) init(response *AadErrorResponse, scopes []string, cloud *cloud.Cloud) { + e.description = response.ErrorDescription e.scenario = "reauthentication required" e.loginCmd = "azd auth login" if !matchesLoginScopes(scopes, cloud) { // if matching default login scopes, no scopes need to be specified @@ -68,7 +73,14 @@ func (e *ReLoginRequiredError) init(response *AadErrorResponse, scopes []string, } func (e *ReLoginRequiredError) Error() string { - return fmt.Sprintf("%s, run `%s` to log in", e.scenario, e.loginCmd) + return e.description +} + +func (e *ReLoginRequiredError) ErrorWithSuggestion() internal.ErrorWithSuggestion { + return internal.ErrorWithSuggestion { + Err: e, + Suggestion: fmt.Sprintf("Suggestion: %s, run `%s` to acquire a new token", e.scenario, e.loginCmd), + } } // matchesLoginScopes checks if the elements contained in the slice match the scopes acquired during login. From a353337ffab8b23b4547f2b2b7aeb6ff69f6f0e5 Mon Sep 17 00:00:00 2001 From: Jeffrey Chen Date: Fri, 22 Nov 2024 11:40:24 -0800 Subject: [PATCH 2/9] Add unit test --- cli/azd/pkg/auth/errors_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/cli/azd/pkg/auth/errors_test.go b/cli/azd/pkg/auth/errors_test.go index d0f77e2ab81..dd7daa3d593 100644 --- a/cli/azd/pkg/auth/errors_test.go +++ b/cli/azd/pkg/auth/errors_test.go @@ -100,3 +100,21 @@ func TestReLoginRequired(t *testing.T) { }) } } + +func TestReLoginRequiredDescription(t *testing.T) { + tests := []struct { + name string + resp *AadErrorResponse + want string + }{ + {"invalid_grant", &AadErrorResponse{Error: "invalid_grant", ErrorDescription: "AADSTS50005"}, "AADSTS50005"}, + {"interaction_required", &AadErrorResponse{Error: "interaction_required", ErrorDescription: "AADSTS50076"}, "AADSTS50076"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err, _ := newReLoginRequiredError(tt.resp, LoginScopes(cloud.AzurePublic()), cloud.AzurePublic()) + got := err.Error() + require.Equal(t, tt.want, got) + }) + } +} From 5a43124ad30bc22a5a6c15f4907bbc8d4ea4c9fe Mon Sep 17 00:00:00 2001 From: Jeffrey Chen Date: Fri, 22 Nov 2024 14:03:30 -0800 Subject: [PATCH 3/9] Fix lint warnings --- cli/azd/pkg/auth/errors.go | 4 ++-- cli/azd/pkg/auth/errors_test.go | 18 ++++++++++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/cli/azd/pkg/auth/errors.go b/cli/azd/pkg/auth/errors.go index 0c5a21f1004..654b7d3e659 100644 --- a/cli/azd/pkg/auth/errors.go +++ b/cli/azd/pkg/auth/errors.go @@ -77,8 +77,8 @@ func (e *ReLoginRequiredError) Error() string { } func (e *ReLoginRequiredError) ErrorWithSuggestion() internal.ErrorWithSuggestion { - return internal.ErrorWithSuggestion { - Err: e, + return internal.ErrorWithSuggestion{ + Err: e, Suggestion: fmt.Sprintf("Suggestion: %s, run `%s` to acquire a new token", e.scenario, e.loginCmd), } } diff --git a/cli/azd/pkg/auth/errors_test.go b/cli/azd/pkg/auth/errors_test.go index dd7daa3d593..b28f4e47fe8 100644 --- a/cli/azd/pkg/auth/errors_test.go +++ b/cli/azd/pkg/auth/errors_test.go @@ -107,8 +107,22 @@ func TestReLoginRequiredDescription(t *testing.T) { resp *AadErrorResponse want string }{ - {"invalid_grant", &AadErrorResponse{Error: "invalid_grant", ErrorDescription: "AADSTS50005"}, "AADSTS50005"}, - {"interaction_required", &AadErrorResponse{Error: "interaction_required", ErrorDescription: "AADSTS50076"}, "AADSTS50076"}, + { + "invalid_grant", + &AadErrorResponse{ + Error: "invalid_grant", + ErrorDescription: "AADSTS50005", + }, + "AADSTS50005", + }, + { + "interaction_required", + &AadErrorResponse{ + Error: "interaction_required", + ErrorDescription: "AADSTS50076", + }, + "AADSTS50076", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 409bbe5676bba5db92290ad964c7b9a7168b157c Mon Sep 17 00:00:00 2001 From: Jeffrey Chen Date: Mon, 25 Nov 2024 14:29:56 -0800 Subject: [PATCH 4/9] Create ErrorWithSuggestion inline --- cli/azd/pkg/auth/errors.go | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/cli/azd/pkg/auth/errors.go b/cli/azd/pkg/auth/errors.go index 654b7d3e659..2a749b4da5f 100644 --- a/cli/azd/pkg/auth/errors.go +++ b/cli/azd/pkg/auth/errors.go @@ -50,8 +50,10 @@ func newReLoginRequiredError( "interaction_required": err := ReLoginRequiredError{} err.init(response, scopes, cloud) - errWithSuggestion := err.ErrorWithSuggestion() - return &errWithSuggestion, true + return &internal.ErrorWithSuggestion{ + Err: &err, + Suggestion: fmt.Sprintf("Suggestion: %s, run `%s` to acquire a new token", err.scenario, err.loginCmd), + }, true } return nil, false @@ -76,13 +78,6 @@ func (e *ReLoginRequiredError) Error() string { return e.description } -func (e *ReLoginRequiredError) ErrorWithSuggestion() internal.ErrorWithSuggestion { - return internal.ErrorWithSuggestion{ - Err: e, - Suggestion: fmt.Sprintf("Suggestion: %s, run `%s` to acquire a new token", e.scenario, e.loginCmd), - } -} - // matchesLoginScopes checks if the elements contained in the slice match the scopes acquired during login. func matchesLoginScopes(scopes []string, cloud *cloud.Cloud) bool { for _, scope := range scopes { From 259dbbc82affeba769829a9a2c9a1323d5c14225 Mon Sep 17 00:00:00 2001 From: Jeffrey Chen Date: Tue, 26 Nov 2024 15:24:34 -0800 Subject: [PATCH 5/9] Specify `--use-device-code=false` and include link to docs --- cli/azd/pkg/auth/errors.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/cli/azd/pkg/auth/errors.go b/cli/azd/pkg/auth/errors.go index 2a749b4da5f..75c60dc0b1e 100644 --- a/cli/azd/pkg/auth/errors.go +++ b/cli/azd/pkg/auth/errors.go @@ -30,6 +30,8 @@ type ReLoginRequiredError struct { scenario string description string + + helpLink string } // newReLoginRequiredError returns an error if the response indicates that the user needs to reauthenticate. @@ -50,9 +52,13 @@ func newReLoginRequiredError( "interaction_required": err := ReLoginRequiredError{} err.init(response, scopes, cloud) + suggestion := fmt.Sprintf("Suggestion: %s, run `%s` to acquire a new token.", err.scenario, err.loginCmd) + if err.helpLink != "" { + suggestion += fmt.Sprintf(" See %s for more info.", err.helpLink) + } return &internal.ErrorWithSuggestion{ Err: &err, - Suggestion: fmt.Sprintf("Suggestion: %s, run `%s` to acquire a new token", err.scenario, err.loginCmd), + Suggestion: suggestion, }, true } @@ -69,9 +75,17 @@ func (e *ReLoginRequiredError) init(response *AadErrorResponse, scopes []string, } } + // BadTokenDueToSignInFrequency - The refresh token has expired or is invalid due to sign-in frequency checks by Conditional Access. if slices.Contains(response.ErrorCodes, 70043) { e.scenario = "login expired" } + + // DevicePolicyError - User tried to sign in to a device from a platform not currently supported through Conditional Access policy + if slices.Contains((response.ErrorCodes), 50005) { + e.loginCmd += " --use-device-code=false" + // TODO: Use aka.ms short link + e.helpLink = "https://learn.microsoft.com/azure/developer/azure-developer-cli/troubleshoot#azd-pipeline-config-failure-due-to-conditional-access-policy" + } } func (e *ReLoginRequiredError) Error() string { From e973ea3c61f16743909661be3b7d5701e45befbc Mon Sep 17 00:00:00 2001 From: Jeffrey Chen Date: Tue, 26 Nov 2024 15:36:38 -0800 Subject: [PATCH 6/9] Address lint warnings --- cli/azd/pkg/auth/errors.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cli/azd/pkg/auth/errors.go b/cli/azd/pkg/auth/errors.go index 75c60dc0b1e..71c67c82d4b 100644 --- a/cli/azd/pkg/auth/errors.go +++ b/cli/azd/pkg/auth/errors.go @@ -75,15 +75,16 @@ func (e *ReLoginRequiredError) init(response *AadErrorResponse, scopes []string, } } - // BadTokenDueToSignInFrequency - The refresh token has expired or is invalid due to sign-in frequency checks by Conditional Access. + // The refresh token has expired or is invalid due to sign-in frequency checks by Conditional Access. if slices.Contains(response.ErrorCodes, 70043) { e.scenario = "login expired" } - // DevicePolicyError - User tried to sign in to a device from a platform not currently supported through Conditional Access policy + // User tried to sign in to a device from a platform not currently supported through Conditional Access policy if slices.Contains((response.ErrorCodes), 50005) { e.loginCmd += " --use-device-code=false" // TODO: Use aka.ms short link + //nolint:lll e.helpLink = "https://learn.microsoft.com/azure/developer/azure-developer-cli/troubleshoot#azd-pipeline-config-failure-due-to-conditional-access-policy" } } From d1bfb33fb59fd2454397d25ae57db123d0957929 Mon Sep 17 00:00:00 2001 From: Jeffrey Chen Date: Mon, 2 Dec 2024 13:46:28 -0800 Subject: [PATCH 7/9] Address feedback --- cli/azd/pkg/auth/errors.go | 8 ++++---- cli/azd/pkg/auth/errors_test.go | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cli/azd/pkg/auth/errors.go b/cli/azd/pkg/auth/errors.go index 71c67c82d4b..1154c89ac6a 100644 --- a/cli/azd/pkg/auth/errors.go +++ b/cli/azd/pkg/auth/errors.go @@ -29,7 +29,7 @@ type ReLoginRequiredError struct { // The scenario in which the login is required scenario string - description string + errText string helpLink string } @@ -66,7 +66,7 @@ func newReLoginRequiredError( } func (e *ReLoginRequiredError) init(response *AadErrorResponse, scopes []string, cloud *cloud.Cloud) { - e.description = response.ErrorDescription + e.errText = response.ErrorDescription e.scenario = "reauthentication required" e.loginCmd = "azd auth login" if !matchesLoginScopes(scopes, cloud) { // if matching default login scopes, no scopes need to be specified @@ -81,7 +81,7 @@ func (e *ReLoginRequiredError) init(response *AadErrorResponse, scopes []string, } // User tried to sign in to a device from a platform not currently supported through Conditional Access policy - if slices.Contains((response.ErrorCodes), 50005) { + if slices.Contains(response.ErrorCodes, 50005) { e.loginCmd += " --use-device-code=false" // TODO: Use aka.ms short link //nolint:lll @@ -90,7 +90,7 @@ func (e *ReLoginRequiredError) init(response *AadErrorResponse, scopes []string, } func (e *ReLoginRequiredError) Error() string { - return e.description + return e.errText } // matchesLoginScopes checks if the elements contained in the slice match the scopes acquired during login. diff --git a/cli/azd/pkg/auth/errors_test.go b/cli/azd/pkg/auth/errors_test.go index b28f4e47fe8..5c1256923bd 100644 --- a/cli/azd/pkg/auth/errors_test.go +++ b/cli/azd/pkg/auth/errors_test.go @@ -101,7 +101,7 @@ func TestReLoginRequired(t *testing.T) { } } -func TestReLoginRequiredDescription(t *testing.T) { +func TestReLoginRequiredError(t *testing.T) { tests := []struct { name string resp *AadErrorResponse @@ -111,17 +111,17 @@ func TestReLoginRequiredDescription(t *testing.T) { "invalid_grant", &AadErrorResponse{ Error: "invalid_grant", - ErrorDescription: "AADSTS50005", + ErrorDescription: "description 1", }, - "AADSTS50005", + "description 1", }, { "interaction_required", &AadErrorResponse{ Error: "interaction_required", - ErrorDescription: "AADSTS50076", + ErrorDescription: "description 2", }, - "AADSTS50076", + "description 2", }, } for _, tt := range tests { From 318176579009d12e3fbb722c71e933e5633481ab Mon Sep 17 00:00:00 2001 From: Jeffrey Chen Date: Fri, 6 Dec 2024 10:06:16 -0800 Subject: [PATCH 8/9] Update to use aka.ms short link --- cli/azd/pkg/auth/errors.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cli/azd/pkg/auth/errors.go b/cli/azd/pkg/auth/errors.go index 1154c89ac6a..b1945e69d9c 100644 --- a/cli/azd/pkg/auth/errors.go +++ b/cli/azd/pkg/auth/errors.go @@ -80,12 +80,11 @@ func (e *ReLoginRequiredError) init(response *AadErrorResponse, scopes []string, e.scenario = "login expired" } - // User tried to sign in to a device from a platform not currently supported through Conditional Access policy + // In a Codespaces environment, `azd auth login` defaults to device code flow, which can cause issues getting tokens if the + // the Entra tenant has Conditional Access Policies set. if slices.Contains(response.ErrorCodes, 50005) { e.loginCmd += " --use-device-code=false" - // TODO: Use aka.ms short link - //nolint:lll - e.helpLink = "https://learn.microsoft.com/azure/developer/azure-developer-cli/troubleshoot#azd-pipeline-config-failure-due-to-conditional-access-policy" + e.helpLink = "https://aka.ms/azd/troubleshoot/conditional-access-policy" } } From dbcc331420ba0b32ac339291f17cbd1b43136eb4 Mon Sep 17 00:00:00 2001 From: Jeffrey Chen Date: Fri, 6 Dec 2024 10:19:45 -0800 Subject: [PATCH 9/9] Fix lint warning --- cli/azd/pkg/auth/errors.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/azd/pkg/auth/errors.go b/cli/azd/pkg/auth/errors.go index b1945e69d9c..5961d3b6ebe 100644 --- a/cli/azd/pkg/auth/errors.go +++ b/cli/azd/pkg/auth/errors.go @@ -80,8 +80,8 @@ func (e *ReLoginRequiredError) init(response *AadErrorResponse, scopes []string, e.scenario = "login expired" } - // In a Codespaces environment, `azd auth login` defaults to device code flow, which can cause issues getting tokens if the - // the Entra tenant has Conditional Access Policies set. + // In a Codespaces environment, `azd auth login` defaults to device code flow, which can cause issues + // getting tokens if the Entra tenant has Conditional Access Policies set. if slices.Contains(response.ErrorCodes, 50005) { e.loginCmd += " --use-device-code=false" e.helpLink = "https://aka.ms/azd/troubleshoot/conditional-access-policy"