diff --git a/cli/azd/cmd/auth_login.go b/cli/azd/cmd/auth_login.go index 66459ea7012..9c9ca9cb99c 100644 --- a/cli/azd/cmd/auth_login.go +++ b/cli/azd/cmd/auth_login.go @@ -314,10 +314,12 @@ func (la *loginAction) Run(ctx context.Context) (*actions.ActionResult, error) { // We print any non-setup related errors to stderr. // We always return a zero exit code. token, err := la.verifyLoggedIn(ctx) - _, loginExpiryError := errors.AsType[*auth.ReLoginRequiredError](err) + // An *internal.ErrorWithSuggestion already carries actionable, user-facing guidance + // surfaced through the login result; avoid double-printing the raw error. + _, hasSuggestion := errors.AsType[*internal.ErrorWithSuggestion](err) if err != nil && !errors.Is(err, auth.ErrNoCurrentUser) && - !loginExpiryError { + !hasSuggestion { fmt.Fprintln(la.console.Handles().Stderr, err.Error()) } diff --git a/cli/azd/cmd/auth_status.go b/cli/azd/cmd/auth_status.go index b27dacc334a..07d33ef6197 100644 --- a/cli/azd/cmd/auth_status.go +++ b/cli/azd/cmd/auth_status.go @@ -81,10 +81,12 @@ func (a *authStatusAction) Run(ctx context.Context) (*actions.ActionResult, erro // get user account information details, err := a.authManager.LogInDetails(ctx) - _, loginExpiryError := errors.AsType[*auth.ReLoginRequiredError](err) + // An *internal.ErrorWithSuggestion already carries actionable, user-facing guidance + // surfaced through the status result; avoid double-printing the raw error. + _, hasSuggestion := errors.AsType[*internal.ErrorWithSuggestion](err) if err != nil { if !errors.Is(err, auth.ErrNoCurrentUser) && - !loginExpiryError { + !hasSuggestion { // print a useful message for unknown errors fmt.Fprintln(a.console.Handles().Stderr, err.Error()) } diff --git a/cli/azd/internal/cmd/errors.go b/cli/azd/internal/cmd/errors.go index 32fc26169c2..61ccb89a354 100644 --- a/cli/azd/internal/cmd/errors.go +++ b/cli/azd/internal/cmd/errors.go @@ -52,7 +52,11 @@ func MapError(err error, span tracing.Span) { errDetails = append(errDetails, fields.ErrCategory.String("auth")) } else if errWithSuggestion, ok := errors.AsType[*internal.ErrorWithSuggestion](err); ok { errCode = "error.suggestion" - span.SetAttributes(fields.ErrType.String(classifySuggestionType(errWithSuggestion.Unwrap()))) + innerErr := errWithSuggestion.Unwrap() + span.SetAttributes(fields.ErrType.String(classifySuggestionType(innerErr))) + if authFailedErr, ok := errors.AsType[*auth.AuthFailedError](innerErr); ok { + errDetails = append(errDetails, authFailedTelemetryDetails(authFailedErr)...) + } } else if respErr, ok := errors.AsType[*azcore.ResponseError](err); ok { serviceName := "other" statusCode := -1 @@ -173,18 +177,7 @@ func MapError(err error, span tracing.Span) { errDetails = append(errDetails, fields.ToolName.String(strings.Join(toolCheckErr.ToolNames, ","))) } } else if authFailedErr, ok := errors.AsType[*auth.AuthFailedError](err); ok { - errDetails = append(errDetails, fields.ServiceName.String("aad")) - if authFailedErr.Parsed != nil { - codes := make([]string, 0, len(authFailedErr.Parsed.ErrorCodes)) - for _, code := range authFailedErr.Parsed.ErrorCodes { - codes = append(codes, fmt.Sprintf("%d", code)) - } - serviceErr := strings.Join(codes, ",") - errDetails = append(errDetails, - fields.ServiceStatusCode.String(authFailedErr.Parsed.Error), - fields.ServiceErrorCode.String(serviceErr), - fields.ServiceCorrelationId.String(authFailedErr.Parsed.CorrelationId)) - } + errDetails = append(errDetails, authFailedTelemetryDetails(authFailedErr)...) errCode = "service.aad.failed" } else if errors.Is(err, auth.ErrNoCurrentUser) { errCode = "auth.not_logged_in" @@ -216,6 +209,24 @@ func MapError(err error, span tracing.Span) { span.SetStatus(codes.Error, errCode) } +func authFailedTelemetryDetails(authFailedErr *auth.AuthFailedError) []attribute.KeyValue { + errDetails := []attribute.KeyValue{fields.ServiceName.String("aad")} + if authFailedErr == nil || authFailedErr.Parsed == nil { + return errDetails + } + + codes := make([]string, 0, len(authFailedErr.Parsed.ErrorCodes)) + for _, code := range authFailedErr.Parsed.ErrorCodes { + codes = append(codes, fmt.Sprintf("%d", code)) + } + + return append(errDetails, + fields.ServiceStatusCode.String(authFailedErr.Parsed.Error), + fields.ServiceErrorCode.String(strings.Join(codes, ",")), + fields.ServiceCorrelationId.String(authFailedErr.Parsed.CorrelationId), + ) +} + // classifySentinel checks if the error matches a known sentinel // and returns the corresponding telemetry code, or "" if no match. func classifySentinel(err error) string { diff --git a/cli/azd/internal/cmd/errors_test.go b/cli/azd/internal/cmd/errors_test.go index a14112341be..5a3012ad723 100644 --- a/cli/azd/internal/cmd/errors_test.go +++ b/cli/azd/internal/cmd/errors_test.go @@ -211,6 +211,28 @@ func Test_MapError(t *testing.T) { fields.ErrorKey(fields.ErrCategory.Key).String("auth"), }, }, + { + name: "WithTokenProtectionBlockedAuthFailedError", + err: &internal.ErrorWithSuggestion{ + Err: &auth.AuthFailedError{ + Parsed: &auth.AadErrorResponse{ + Error: "invalid_grant", + ErrorCodes: []int{530084}, + CorrelationId: "blocked-token-protection", + }, + }, + }, + // AADSTS530084 surfaced via the suggestion wrapper still classifies as the generic + // AAD service failure for telemetry, and should preserve the wrapped AAD details. + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String("service.aad.failed"), + fields.ErrorKey(fields.ServiceName.Key).String("aad"), + fields.ErrorKey(fields.ServiceErrorCode.Key).String("530084"), + fields.ErrorKey(fields.ServiceStatusCode.Key).String("invalid_grant"), + fields.ErrorKey(fields.ServiceCorrelationId.Key).String("blocked-token-protection"), + }, + }, { name: "WithAzidentityAuthenticationFailedError", err: &azidentity.AuthenticationFailedError{}, diff --git a/cli/azd/internal/grpcserver/server.go b/cli/azd/internal/grpcserver/server.go index c19ca10e45c..592ab3dbf8f 100644 --- a/cli/azd/internal/grpcserver/server.go +++ b/cli/azd/internal/grpcserver/server.go @@ -15,6 +15,7 @@ import ( "github.com/azure/azure-dev/cli/azd/pkg/auth" "github.com/azure/azure-dev/cli/azd/pkg/azdext" "github.com/azure/azure-dev/cli/azd/pkg/extensions" + "google.golang.org/genproto/googleapis/rpc/errdetails" "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" @@ -268,31 +269,84 @@ func (s *authenticatedStream) Context() context.Context { // This ensures that helpful suggestions (like "run azd auth login") are preserved // when errors are transmitted over gRPC, where only the error message string is sent. // -// Auth-related errors (ReLoginRequiredError, ErrNoCurrentUser) are returned with -// codes.Unauthenticated so that extensions can detect auth failures via gRPC status code. +// Auth-related errors are returned with codes.Unauthenticated and a structured +// ErrorInfo reason so extensions can reliably classify them as auth failures +// while still distinguishing the remediation path. func wrapErrorWithSuggestion(err error) error { if err == nil { return nil } - _, loginErr := errors.AsType[*auth.ReLoginRequiredError](err) - isAuthErr := errors.Is(err, auth.ErrNoCurrentUser) || loginErr + isAuthErr := isAuthError(err) if suggestionErr, ok := errors.AsType[*internal.ErrorWithSuggestion](err); ok { msg := fmt.Sprintf("%s\n%s", err.Error(), suggestionErr.Suggestion) if isAuthErr { - return status.Error(codes.Unauthenticated, msg) + return grpcAuthStatus(err, msg) } return fmt.Errorf("%w\n%s", err, suggestionErr.Suggestion) } if isAuthErr { - return status.Error(codes.Unauthenticated, err.Error()) + return grpcAuthStatus(err, err.Error()) } return err } +// isAuthError reports whether err's chain contains a known auth-failure type that should be +// surfaced over gRPC as codes.Unauthenticated. +func isAuthError(err error) bool { + if errors.Is(err, auth.ErrNoCurrentUser) { + return true + } + if _, ok := errors.AsType[*auth.ReLoginRequiredError](err); ok { + return true + } + if _, ok := errors.AsType[*auth.AuthFailedError](err); ok { + return true + } + return false +} + +func grpcAuthStatus(err error, msg string) error { + st := status.New(codes.Unauthenticated, msg) + reason := grpcAuthReason(err) + if reason == "" { + return st.Err() + } + + withDetails, detailErr := st.WithDetails(&errdetails.ErrorInfo{ + Reason: reason, + Domain: azdext.AuthErrorDomain, + }) + if detailErr != nil { + return st.Err() + } + + return withDetails.Err() +} + +func grpcAuthReason(err error) string { + if errors.Is(err, auth.ErrNoCurrentUser) { + return azdext.AuthErrorReasonNotLoggedIn + } + + // Pass through the originating AAD error code (e.g., "AADSTS530084") when available. + // This preserves Entra's own semantics rather than redefining them on azd's side. + if authFailed, ok := errors.AsType[*auth.AuthFailedError](err); ok { + if authFailed.Parsed != nil && len(authFailed.Parsed.ErrorCodes) > 0 { + return fmt.Sprintf("AADSTS%d", authFailed.Parsed.ErrorCodes[0]) + } + } + + if _, ok := errors.AsType[*auth.ReLoginRequiredError](err); ok { + return azdext.AuthErrorReasonLoginRequired + } + + return "" +} + func generateSigningKey() ([]byte, error) { bytes := make([]byte, 32) // 256-bit HMAC signing key if _, err := rand.Read(bytes); err != nil { diff --git a/cli/azd/internal/grpcserver/server_coverage3_test.go b/cli/azd/internal/grpcserver/server_coverage3_test.go index e24854f98a3..f57cb915fbb 100644 --- a/cli/azd/internal/grpcserver/server_coverage3_test.go +++ b/cli/azd/internal/grpcserver/server_coverage3_test.go @@ -11,8 +11,10 @@ import ( "github.com/azure/azure-dev/cli/azd/internal" "github.com/azure/azure-dev/cli/azd/pkg/auth" + "github.com/azure/azure-dev/cli/azd/pkg/azdext" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "google.golang.org/genproto/googleapis/rpc/errdetails" "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" @@ -77,6 +79,33 @@ func TestWrapErrorWithSuggestion_ReLoginRequired(t *testing.T) { require.Equal(t, codes.Unauthenticated, st.Code()) } +func TestWrapErrorWithSuggestion_TokenProtectionBlocked(t *testing.T) { + t.Parallel() + authFailed := &auth.AuthFailedError{ + Parsed: &auth.AadErrorResponse{ + Error: "invalid_grant", + ErrorCodes: []int{530084}, + }, + } + // In production the wrapper is built by newActionableAuthError; mirror that shape here so + // wrapErrorWithSuggestion classifies the wrapped *AuthFailedError as an auth interaction. + err := fmt.Errorf("token protection: %w", &internal.ErrorWithSuggestion{ + Err: authFailed, + Suggestion: "Contact your IT administrator or request a policy exception.", + }) + + wrapped := wrapErrorWithSuggestion(err) + st, ok := status.FromError(wrapped) + require.True(t, ok) + require.Equal(t, codes.Unauthenticated, st.Code()) + details := st.Details() + require.Len(t, details, 1) + info, ok := details[0].(*errdetails.ErrorInfo) + require.True(t, ok) + require.Equal(t, azdext.AuthErrorDomain, info.Domain) + require.Equal(t, "AADSTS530084", info.Reason) +} + func TestWrapErrorWithSuggestion_AuthErrorWithSuggestion(t *testing.T) { t.Parallel() inner := auth.ErrNoCurrentUser diff --git a/cli/azd/internal/grpcserver/server_test.go b/cli/azd/internal/grpcserver/server_test.go index 8416d3a603b..8e3080abe9a 100644 --- a/cli/azd/internal/grpcserver/server_test.go +++ b/cli/azd/internal/grpcserver/server_test.go @@ -18,6 +18,7 @@ import ( "github.com/azure/azure-dev/cli/azd/pkg/azdext" "github.com/azure/azure-dev/cli/azd/pkg/extensions" "github.com/stretchr/testify/require" + "google.golang.org/genproto/googleapis/rpc/errdetails" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) @@ -237,6 +238,7 @@ func Test_wrapErrorWithSuggestion(t *testing.T) { wantContain string wantSameInstance bool wantGrpcCode codes.Code + wantAuthReason string }{ { name: "nil error returns nil", @@ -266,16 +268,18 @@ func Test_wrapErrorWithSuggestion(t *testing.T) { wantContain: "azd auth login", }, { - name: "ErrNoCurrentUser returns Unauthenticated", - err: auth.ErrNoCurrentUser, - wantContain: "not logged in", - wantGrpcCode: codes.Unauthenticated, + name: "ErrNoCurrentUser returns Unauthenticated", + err: auth.ErrNoCurrentUser, + wantContain: "not logged in", + wantGrpcCode: codes.Unauthenticated, + wantAuthReason: azdext.AuthErrorReasonNotLoggedIn, }, { - name: "wrapped ErrNoCurrentUser returns Unauthenticated", - err: fmt.Errorf("failed to list subscriptions: %w", auth.ErrNoCurrentUser), - wantContain: "not logged in", - wantGrpcCode: codes.Unauthenticated, + name: "wrapped ErrNoCurrentUser returns Unauthenticated", + err: fmt.Errorf("failed to list subscriptions: %w", auth.ErrNoCurrentUser), + wantContain: "not logged in", + wantGrpcCode: codes.Unauthenticated, + wantAuthReason: azdext.AuthErrorReasonNotLoggedIn, }, { name: "ReLoginRequiredError with suggestion returns Unauthenticated", @@ -283,8 +287,24 @@ func Test_wrapErrorWithSuggestion(t *testing.T) { Err: &auth.ReLoginRequiredError{}, Suggestion: "login expired, run `azd auth login` to acquire a new token.", }, - wantContain: "azd auth login", - wantGrpcCode: codes.Unauthenticated, + wantContain: "azd auth login", + wantGrpcCode: codes.Unauthenticated, + wantAuthReason: azdext.AuthErrorReasonLoginRequired, + }, + { + name: "TokenProtectionBlockedError with suggestion returns Unauthenticated", + err: &internal.ErrorWithSuggestion{ + Err: &auth.AuthFailedError{ + Parsed: &auth.AadErrorResponse{ + Error: "invalid_grant", + ErrorCodes: []int{530084}, + }, + }, + Suggestion: "Contact your IT administrator or request a policy exception.", + }, + wantContain: "policy exception", + wantGrpcCode: codes.Unauthenticated, + wantAuthReason: "AADSTS530084", }, } @@ -304,6 +324,14 @@ func Test_wrapErrorWithSuggestion(t *testing.T) { st, ok := status.FromError(result) require.True(t, ok, "expected gRPC status error") require.Equal(t, tt.wantGrpcCode, st.Code()) + if tt.wantAuthReason != "" { + details := st.Details() + require.Len(t, details, 1) + info, ok := details[0].(*errdetails.ErrorInfo) + require.True(t, ok, "expected ErrorInfo detail") + require.Equal(t, azdext.AuthErrorDomain, info.Domain) + require.Equal(t, tt.wantAuthReason, info.Reason) + } } }) } diff --git a/cli/azd/pkg/auth/azd_credential.go b/cli/azd/pkg/auth/azd_credential.go index 5a125fa0956..2bb9f647c80 100644 --- a/cli/azd/pkg/auth/azd_credential.go +++ b/cli/azd/pkg/auth/azd_credential.go @@ -75,7 +75,7 @@ func (c *azdCredential) GetToken(ctx context.Context, options policy.TokenReques c.cacheTracer.LogSnapshotOnce(failurePhase) if authFailed, ok := errors.AsType[*AuthFailedError](err); ok { - if loginErr, ok := newReLoginRequiredError(authFailed.Parsed, options.Scopes, c.cloud, tenantID); ok { + if loginErr, ok := newActionableAuthError(authFailed.Parsed, options.Scopes, c.cloud, tenantID, authFailed); ok { log.Println(authFailed.httpErrorDetails()) if options.Claims != "" { diff --git a/cli/azd/pkg/auth/claims_test.go b/cli/azd/pkg/auth/claims_test.go index ce737b6a394..c5a02a09b7a 100644 --- a/cli/azd/pkg/auth/claims_test.go +++ b/cli/azd/pkg/auth/claims_test.go @@ -431,13 +431,13 @@ func TestReLoginRequiredError_NonRetriable(t *testing.T) { e.NonRetriable() // marker — should not panic } -func TestNewReLoginRequiredError(t *testing.T) { +func TestNewActionableAuthError_ClaimsScenarios(t *testing.T) { t.Parallel() t.Run("nil_response_returns_false", func(t *testing.T) { t.Parallel() - err, ok := newReLoginRequiredError( - nil, nil, cloud.AzurePublic(), "") + err, ok := newActionableAuthError( + nil, nil, cloud.AzurePublic(), "", nil) assert.Nil(t, err) assert.False(t, ok) }) @@ -448,8 +448,8 @@ func TestNewReLoginRequiredError(t *testing.T) { Error: "server_error", ErrorDescription: "something else", } - err, ok := newReLoginRequiredError( - resp, nil, cloud.AzurePublic(), "") + err, ok := newActionableAuthError( + resp, nil, cloud.AzurePublic(), "", nil) assert.Nil(t, err) assert.False(t, ok) }) @@ -460,11 +460,12 @@ func TestNewReLoginRequiredError(t *testing.T) { Error: "invalid_grant", ErrorDescription: "AADSTS700082: expired", } - err, ok := newReLoginRequiredError( + err, ok := newActionableAuthError( resp, []string{"https://management.azure.com//.default"}, cloud.AzurePublic(), "", + nil, ) assert.True(t, ok) require.Error(t, err) @@ -477,11 +478,12 @@ func TestNewReLoginRequiredError(t *testing.T) { Error: "interaction_required", ErrorDescription: "need consent", } - err, ok := newReLoginRequiredError( + err, ok := newActionableAuthError( resp, []string{"https://management.azure.com//.default"}, cloud.AzurePublic(), "", + nil, ) assert.True(t, ok) require.Error(t, err) @@ -493,7 +495,7 @@ func TestNewReLoginRequiredError(t *testing.T) { Error: "invalid_grant", ErrorDescription: "expired", } - err, ok := newReLoginRequiredError( + err, ok := newActionableAuthError( resp, []string{ "https://management.azure.com//.default", @@ -501,6 +503,7 @@ func TestNewReLoginRequiredError(t *testing.T) { }, cloud.AzurePublic(), "", + nil, ) assert.True(t, ok) require.Error(t, err) @@ -514,8 +517,8 @@ func TestNewReLoginRequiredError(t *testing.T) { ErrorDescription: "AADSTS70043: expired", ErrorCodes: []int{70043}, } - err, ok := newReLoginRequiredError( - resp, nil, cloud.AzurePublic(), "") + err, ok := newActionableAuthError( + resp, nil, cloud.AzurePublic(), "", nil) assert.True(t, ok) require.Error(t, err) }) @@ -527,12 +530,13 @@ func TestNewReLoginRequiredError(t *testing.T) { ErrorDescription: "AADSTS700082: expired", ErrorCodes: []int{700082}, } - err, ok := newReLoginRequiredError(resp, nil, cloud.AzurePublic(), "") + err, ok := newActionableAuthError(resp, nil, cloud.AzurePublic(), "", nil) assert.True(t, ok) require.Error(t, err) var errWithSuggestion *internal.ErrorWithSuggestion require.True(t, errors.As(err, &errWithSuggestion)) + assert.Equal(t, "Login expired.", errWithSuggestion.Message) assert.Contains(t, errWithSuggestion.Suggestion, "login expired") }) @@ -543,10 +547,17 @@ func TestNewReLoginRequiredError(t *testing.T) { ErrorDescription: "conditional access", ErrorCodes: []int{50005}, } - err, ok := newReLoginRequiredError( - resp, nil, cloud.AzurePublic(), "") + err, ok := newActionableAuthError( + resp, nil, cloud.AzurePublic(), "", nil) assert.True(t, ok) require.Error(t, err) + + errWithSuggestion, ok := errors.AsType[*internal.ErrorWithSuggestion](err) + require.True(t, ok) + assert.Equal(t, "Reauthentication required.", errWithSuggestion.Message) + require.Len(t, errWithSuggestion.Links, 1) + assert.Equal(t, "https://aka.ms/azd/troubleshoot/conditional-access-policy", errWithSuggestion.Links[0].URL) + assert.Equal(t, "Conditional Access policy troubleshooting", errWithSuggestion.Links[0].Title) }) t.Run("tenant_id_included_in_suggestion", func(t *testing.T) { @@ -557,8 +568,8 @@ func TestNewReLoginRequiredError(t *testing.T) { ErrorCodes: []int{70043}, } tenantID := "72f988bf-86f1-41af-91ab-2d7cd011db47" - err, ok := newReLoginRequiredError( - resp, nil, cloud.AzurePublic(), tenantID) + err, ok := newActionableAuthError( + resp, nil, cloud.AzurePublic(), tenantID, nil) assert.True(t, ok) require.Error(t, err) diff --git a/cli/azd/pkg/auth/errors.go b/cli/azd/pkg/auth/errors.go index 8655982b1ec..e923160d3fd 100644 --- a/cli/azd/pkg/auth/errors.go +++ b/cli/azd/pkg/auth/errors.go @@ -12,10 +12,12 @@ import ( "log" "net/http" "slices" + "strings" 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" + "github.com/azure/azure-dev/cli/azd/pkg/errorhandler" ) // ErrNoCurrentUser indicates that the current user is not logged in. @@ -34,21 +36,33 @@ type ReLoginRequiredError struct { errText string - helpLink string + helpLink *errorhandler.ErrorLink } -// newReLoginRequiredError returns an error if the response indicates that the user needs to reauthenticate. -// If it is not a reauthentication error, it returns false. -func newReLoginRequiredError( +// newActionableAuthError inspects an AAD error response and, if it matches a known +// pattern that azd can surface with actionable guidance, returns a wrapped error and true. +// +// For Conditional Access token protection (AADSTS530084), the wrapper preserves the original +// *AuthFailedError as the inner error (via failedErr) so the AAD error semantics are not +// shadowed by an azd-specific type. Callers that need to distinguish behavior should match on +// the AAD error code from *AuthFailedError.Parsed. +// +// If the response does not match a known pattern, it returns (nil, false). +func newActionableAuthError( response *AadErrorResponse, scopes []string, cloud *cloud.Cloud, tenantID string, + failedErr *AuthFailedError, ) (error, bool) { if response == nil { return nil, false } + if err, ok := newTokenProtectionBlockedSuggestion(response, scopes, failedErr); ok { + return err, true + } + //nolint:lll // https://learn.microsoft.com/azure/active-directory/develop/reference-aadsts-error-codes#handling-error-codes-in-your-application switch response.Error { @@ -59,18 +73,91 @@ func newReLoginRequiredError( // Note: Do not prefix with "Suggestion:" here — the UX renderer // (ErrorWithSuggestion.ToString) already adds that prefix when displaying. suggestion := fmt.Sprintf("%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{ + suggestionErr := &internal.ErrorWithSuggestion{ Err: &err, + Message: scenarioMessage(err.scenario), Suggestion: suggestion, - }, true + } + if err.helpLink != nil { + suggestionErr.Links = []errorhandler.ErrorLink{*err.helpLink} + } + return suggestionErr, true } return nil, false } +const ( + conditionalAccessDocsLink = "https://aka.ms/TBCADocs" + // #nosec G101 -- documentation URL, not a credential. + tokenProtectionFAQLink = "https://aka.ms/TokenProtectionFAQ#troubleshooting" +) + +// newTokenProtectionBlockedSuggestion returns an *internal.ErrorWithSuggestion that wraps +// the original *AuthFailedError when the AAD response indicates AADSTS530084 (Conditional +// Access token protection blocked the request). Until #7704 is addressed, re-running +// `azd auth login` will not help, so the suggestion directs the user to their IT admin. +// +// The inner Err is the original *AuthFailedError so the AAD error code/description remain +// the source of truth for downstream classification (telemetry, gRPC ErrorInfo, etc.). +func newTokenProtectionBlockedSuggestion( + response *AadErrorResponse, + scopes []string, + failedErr *AuthFailedError, +) (error, bool) { + if response == nil { + return nil, false + } + + if !slices.Contains(response.ErrorCodes, 530084) { + return nil, false + } + + message := "A Conditional Access token protection policy blocked this token request." + if usesGraphScope(scopes) { + message = "A Conditional Access token protection policy blocked this Microsoft Graph token request." + } + + var inner error + if failedErr == nil { + // Defensive: callers should always pass the originating *AuthFailedError, but if not + // available, surface the AAD description so the wrapper still carries detail. + inner = errors.New(response.ErrorDescription) + } else { + inner = failedErr + } + + return &internal.ErrorWithSuggestion{ + Err: inner, + Message: message, + Suggestion: "Contact your IT administrator or request a policy exception.", + Links: []errorhandler.ErrorLink{ + { + URL: conditionalAccessDocsLink, + Title: "Conditional Access token protection guidance", + }, + { + URL: tokenProtectionFAQLink, + Title: "Token protection FAQ", + }, + }, + }, true +} + +func usesGraphScope(scopes []string) bool { + return slices.ContainsFunc(scopes, func(scope string) bool { + return strings.HasPrefix(scope, "https://graph.microsoft.com/") + }) +} + +func scenarioMessage(scenario string) string { + if scenario == "" { + return "" + } + + return strings.ToUpper(scenario[:1]) + scenario[1:] + "." +} + func (e *ReLoginRequiredError) init( response *AadErrorResponse, scopes []string, @@ -103,7 +190,10 @@ func (e *ReLoginRequiredError) init( // 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" + e.helpLink = &errorhandler.ErrorLink{ + URL: "https://aka.ms/azd/troubleshoot/conditional-access-policy", + Title: "Conditional Access policy troubleshooting", + } } } @@ -187,7 +277,13 @@ func (e *AuthFailedError) Unwrap() error { func (e *AuthFailedError) Error() string { if e.RawResp == nil { // non-http error, simply append inner error - return fmt.Sprintf("%s: %s", authFailedPrefix, e.innerErr.Error()) + if e.innerErr != nil { + return fmt.Sprintf("%s: %s", authFailedPrefix, e.innerErr.Error()) + } + if e.Parsed != nil && e.Parsed.ErrorDescription != "" { + return fmt.Sprintf("%s: %s", authFailedPrefix, e.Parsed.ErrorDescription) + } + return authFailedPrefix } if e.Parsed == nil { // unable to parse, provide HTTP error details diff --git a/cli/azd/pkg/auth/errors_test.go b/cli/azd/pkg/auth/errors_test.go index 38beea34ef7..2cb4bd225ee 100644 --- a/cli/azd/pkg/auth/errors_test.go +++ b/cli/azd/pkg/auth/errors_test.go @@ -13,6 +13,7 @@ import ( "testing" 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" "github.com/stretchr/testify/require" ) @@ -86,7 +87,7 @@ func TestAuthFailedError(t *testing.T) { } } -func TestReLoginRequired(t *testing.T) { +func TestNewActionableAuthError_RecognizesLoginRequiredErrors(t *testing.T) { tests := []struct { name string resp *AadErrorResponse @@ -98,13 +99,13 @@ func TestReLoginRequired(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, got := newReLoginRequiredError(tt.resp, LoginScopes(cloud.AzurePublic()), cloud.AzurePublic(), "") + _, got := newActionableAuthError(tt.resp, LoginScopes(cloud.AzurePublic()), cloud.AzurePublic(), "", nil) require.Equal(t, tt.want, got) }) } } -func TestReLoginRequiredError(t *testing.T) { +func TestNewActionableAuthError_PreservesUnderlyingErrorText(t *testing.T) { tests := []struct { name string resp *AadErrorResponse @@ -129,9 +130,160 @@ func TestReLoginRequiredError(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err, _ := newReLoginRequiredError(tt.resp, LoginScopes(cloud.AzurePublic()), cloud.AzurePublic(), "") + err, _ := newActionableAuthError(tt.resp, LoginScopes(cloud.AzurePublic()), cloud.AzurePublic(), "", nil) got := err.Error() require.Equal(t, tt.want, got) }) } } + +func TestTokenProtectionBlockedSuggestion(t *testing.T) { + graphScopes := []string{"https://graph.microsoft.com/.default"} + armScopes := LoginScopes(cloud.AzurePublic()) + + tests := []struct { + name string + resp *AadErrorResponse + scopes []string + want bool + wantMessage string + }{ + { + name: "nil_response", + resp: nil, + scopes: armScopes, + want: false, + }, + { + name: "no_token_protection_code", + resp: &AadErrorResponse{Error: "invalid_grant", ErrorCodes: []int{70043}}, + scopes: armScopes, + want: false, + }, + { + name: "token_protection_arm_scope", + resp: &AadErrorResponse{ + Error: "invalid_grant", + ErrorDescription: "AADSTS530084: blocked by token protection", + ErrorCodes: []int{530084}, + }, + scopes: armScopes, + want: true, + wantMessage: "A Conditional Access token protection policy blocked this token request.", + }, + { + name: "token_protection_graph_scope", + resp: &AadErrorResponse{ + Error: "invalid_grant", + ErrorDescription: "AADSTS530084: blocked by token protection", + ErrorCodes: []int{530084}, + }, + scopes: graphScopes, + want: true, + wantMessage: "A Conditional Access token protection policy blocked this Microsoft Graph token request.", + }, + { + name: "token_protection_with_other_codes", + resp: &AadErrorResponse{ + Error: "invalid_grant", + ErrorDescription: "AADSTS530084: blocked by token protection", + ErrorCodes: []int{70043, 530084}, + }, + scopes: armScopes, + want: true, + wantMessage: "A Conditional Access token protection policy blocked this token request.", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var failed *AuthFailedError + if tt.resp != nil { + failed = &AuthFailedError{Parsed: tt.resp, innerErr: errors.New(tt.resp.ErrorDescription)} + } + err, ok := newTokenProtectionBlockedSuggestion(tt.resp, tt.scopes, failed) + require.Equal(t, tt.want, ok) + if !tt.want { + require.Nil(t, err) + return + } + + require.NotNil(t, err) + + // Wrapped as ErrorWithSuggestion with the expected scope-aware message and links. + ews, ok := errors.AsType[*internal.ErrorWithSuggestion](err) + require.True(t, ok, "expected error to be *internal.ErrorWithSuggestion") + require.Equal(t, tt.wantMessage, ews.Message) + require.Equal(t, "Contact your IT administrator or request a policy exception.", ews.Suggestion) + require.Len(t, ews.Links, 2) + require.Equal(t, conditionalAccessDocsLink, ews.Links[0].URL) + require.NotEmpty(t, ews.Links[0].Title) + require.Equal(t, tokenProtectionFAQLink, ews.Links[1].URL) + require.NotEmpty(t, ews.Links[1].Title) + + // Inner error is preserved as the originating *AuthFailedError so AAD semantics + // (error code, description) flow through unmodified. + inner, ok := errors.AsType[*AuthFailedError](err) + require.True(t, ok, "expected inner error to be *AuthFailedError") + require.NotNil(t, inner.Parsed) + require.Contains(t, inner.Parsed.ErrorCodes, 530084) + }) + } +} + +func TestNewActionableAuthError_TokenProtectionTakesPrecedence(t *testing.T) { + // AADSTS530084 paired with invalid_grant should produce a token-protection suggestion + // (not a ReLoginRequiredError), because reauthenticating won't unblock the user. + resp := &AadErrorResponse{ + Error: "invalid_grant", + ErrorDescription: "AADSTS530084: blocked by token protection", + ErrorCodes: []int{530084}, + } + failed := &AuthFailedError{Parsed: resp, innerErr: errors.New(resp.ErrorDescription)} + + err, ok := newActionableAuthError(resp, LoginScopes(cloud.AzurePublic()), cloud.AzurePublic(), "", failed) + require.True(t, ok) + require.NotNil(t, err) + + _, isReLogin := errors.AsType[*ReLoginRequiredError](err) + require.False(t, isReLogin, "should not be classified as ReLoginRequiredError") + + // The wrapper preserves the originating *AuthFailedError (and its AAD code) as the inner err. + inner, ok := errors.AsType[*AuthFailedError](err) + require.True(t, ok, "should preserve inner *AuthFailedError") + require.Contains(t, inner.Parsed.ErrorCodes, 530084) +} + +func TestNewTokenProtectionBlockedSuggestion_NilAuthFailedErrorFallsBackToDescription(t *testing.T) { + resp := &AadErrorResponse{ + Error: "invalid_grant", + ErrorDescription: "AADSTS530084: blocked by token protection", + ErrorCodes: []int{530084}, + } + + err, ok := newTokenProtectionBlockedSuggestion(resp, LoginScopes(cloud.AzurePublic()), nil) + require.True(t, ok) + + ews, ok := errors.AsType[*internal.ErrorWithSuggestion](err) + require.True(t, ok) + require.Equal(t, resp.ErrorDescription, ews.Unwrap().Error()) +} + +func TestUsesGraphScope(t *testing.T) { + tests := []struct { + name string + scopes []string + want bool + }{ + {"empty", nil, false}, + {"arm_only", []string{"https://management.azure.com/.default"}, false}, + {"graph_default", []string{"https://graph.microsoft.com/.default"}, true}, + {"graph_specific_scope", []string{"https://graph.microsoft.com/User.Read"}, true}, + {"mixed", []string{"https://management.azure.com/.default", "https://graph.microsoft.com/.default"}, true}, + {"graph_substring_not_prefix", []string{"https://example.com/https://graph.microsoft.com/.default"}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, usesGraphScope(tt.scopes)) + }) + } +} diff --git a/cli/azd/pkg/azdext/auth_error_reasons.go b/cli/azd/pkg/azdext/auth_error_reasons.go new file mode 100644 index 00000000000..8cf4f4aca49 --- /dev/null +++ b/cli/azd/pkg/azdext/auth_error_reasons.go @@ -0,0 +1,38 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package azdext + +import ( + "google.golang.org/genproto/googleapis/rpc/errdetails" + "google.golang.org/grpc/status" +) + +// Auth error metadata constants used in gRPC ErrorInfo for auth-related host APIs. +const ( + AuthErrorDomain = "azd.auth" +) + +// Auth error reason codes used in gRPC ErrorInfo.Reason. +// +// For AAD-originated failures, the Reason is the originating Entra error code formatted as +// "AADSTS" (e.g. "AADSTS530084") so extensions can match on the AAD code directly without +// azd having to define a synthetic taxonomy. The constants below cover azd-local conditions +// that have no corresponding Entra code. +const ( + AuthErrorReasonNotLoggedIn = "AUTH_NOT_LOGGED_IN" + AuthErrorReasonLoginRequired = "AUTH_LOGIN_REQUIRED" +) + +// AuthErrorReason extracts the ErrorInfo.Reason from a gRPC status when the domain +// matches [AuthErrorDomain]. +func AuthErrorReason(st *status.Status) string { + for _, detail := range st.Details() { + info, ok := detail.(*errdetails.ErrorInfo) + if ok && info.Domain == AuthErrorDomain { + return info.Reason + } + } + + return "" +} diff --git a/cli/azd/pkg/azdext/extension_error.go b/cli/azd/pkg/azdext/extension_error.go index 3c560414947..0b80f882e90 100644 --- a/cli/azd/pkg/azdext/extension_error.go +++ b/cli/azd/pkg/azdext/extension_error.go @@ -56,7 +56,7 @@ func (e *ServiceError) Error() string { // The function applies detection in priority order: // 1. [ServiceError] / [LocalError] — already structured by extension code (highest specificity) // 2. [azcore.ResponseError] — Azure SDK HTTP errors -// 3. gRPC Unauthenticated — auto-classified as auth category (safety net) +// 3. gRPC Unauthenticated — auto-classified as auth category, preserving auth subtypes from ErrorInfo when present // 4. Fallback — unclassified error with original message // // The counterpart [UnwrapError] is called from the azd host to deserialize @@ -121,7 +121,8 @@ func WrapError(err error) *ExtensionError { // Detect gRPC Unauthenticated errors as an auth safety net. // If the extension didn't already classify the error, this ensures auth failures - // from azd host calls are reported with the correct category in telemetry. + // from azd host calls are reported with the correct category in telemetry, + // and preserves specific auth subtypes when the host attached auth ErrorInfo details. // Use errors.AsType to detect gRPC status errors even when wrapped by fmt.Errorf. if grpcErr, ok := errors.AsType[interface { error @@ -132,7 +133,7 @@ func WrapError(err error) *ExtensionError { extErr.Message = st.Message() extErr.Source = &ExtensionError_LocalError{ LocalError: &LocalErrorDetail{ - Code: "auth_failed", + Code: authLocalErrorCode(st), Category: string(LocalErrorCategoryAuth), }, } @@ -143,6 +144,20 @@ func WrapError(err error) *ExtensionError { return extErr } +func authLocalErrorCode(st *status.Status) string { + switch AuthErrorReason(st) { + case AuthErrorReasonNotLoggedIn: + return "not_logged_in" + case AuthErrorReasonLoginRequired: + return "login_required" + } + + // All other reasons (including AAD-originated codes like "AADSTS530084") collapse to a + // generic "auth_failed" label. Extensions that need per-AAD-code granularity can read the + // raw reason directly from the gRPC ErrorInfo via AuthErrorReason. + return "auth_failed" +} + // UnwrapError converts an ExtensionError proto back to a typed Go error. // It is called from the azd host (via [ExtensionService.ReportError] handler // and envelope GetError methods) to deserialize errors received from extensions diff --git a/cli/azd/pkg/azdext/extension_error_test.go b/cli/azd/pkg/azdext/extension_error_test.go index 01cc5747bbb..81cf078fc74 100644 --- a/cli/azd/pkg/azdext/extension_error_test.go +++ b/cli/azd/pkg/azdext/extension_error_test.go @@ -11,6 +11,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "google.golang.org/genproto/googleapis/rpc/errdetails" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) @@ -119,34 +120,87 @@ func TestExtensionError_RoundTrip(t *testing.T) { }, }, { - name: "GrpcUnauthenticatedError", - inputErr: status.Error(codes.Unauthenticated, "not logged in, run `azd auth login` to login"), + name: "GrpcUnauthenticatedError", + inputErr: mustAuthStatusError( + codes.Unauthenticated, + AuthErrorReasonNotLoggedIn, + "not logged in, run `azd auth login` to login", + ), verify: func(t *testing.T, protoErr *ExtensionError, goErr error) { assert.Equal(t, ErrorOrigin_ERROR_ORIGIN_LOCAL, protoErr.GetOrigin()) assert.Contains(t, protoErr.GetMessage(), "not logged in") localDetail := protoErr.GetLocalError() require.NotNil(t, localDetail) + assert.Equal(t, "not_logged_in", localDetail.GetCode()) + assert.Equal(t, "auth", localDetail.GetCategory()) + + var localErr *LocalError + require.ErrorAs(t, goErr, &localErr) + assert.Equal(t, LocalErrorCategoryAuth, localErr.Category) + assert.Equal(t, "not_logged_in", localErr.Code) + }, + }, + { + name: "WrappedGrpcUnauthenticatedError", + inputErr: fmt.Errorf( + "failed to prompt: %w", + mustAuthStatusError( + codes.Unauthenticated, + "AADSTS530084", + "AADSTS530084: blocked by token protection", + ), + ), + verify: func(t *testing.T, protoErr *ExtensionError, goErr error) { + assert.Equal(t, ErrorOrigin_ERROR_ORIGIN_LOCAL, protoErr.GetOrigin()) + assert.Equal(t, "AADSTS530084: blocked by token protection", protoErr.GetMessage()) + + localDetail := protoErr.GetLocalError() + require.NotNil(t, localDetail) + // AAD-originated reasons collapse to the generic "auth_failed" code; the raw + // "AADSTS530084" reason remains available to extensions via the gRPC ErrorInfo. assert.Equal(t, "auth_failed", localDetail.GetCode()) assert.Equal(t, "auth", localDetail.GetCategory()) + }, + }, + { + name: "GrpcUnauthenticatedLoginRequiredError", + inputErr: mustAuthStatusError( + codes.Unauthenticated, + AuthErrorReasonLoginRequired, + "AADSTS70043: token expired\nlogin expired, run `azd auth login`", + ), + verify: func(t *testing.T, protoErr *ExtensionError, goErr error) { + assert.Equal(t, ErrorOrigin_ERROR_ORIGIN_LOCAL, protoErr.GetOrigin()) + assert.Contains(t, protoErr.GetMessage(), "login expired") + + localDetail := protoErr.GetLocalError() + require.NotNil(t, localDetail) + assert.Equal(t, "login_required", localDetail.GetCode()) + assert.Equal(t, "auth", localDetail.GetCategory()) var localErr *LocalError require.ErrorAs(t, goErr, &localErr) assert.Equal(t, LocalErrorCategoryAuth, localErr.Category) - assert.Equal(t, "auth_failed", localErr.Code) + assert.Equal(t, "login_required", localErr.Code) }, }, { - name: "WrappedGrpcUnauthenticatedError", - inputErr: fmt.Errorf("failed to prompt: %w", status.Error(codes.Unauthenticated, "login expired")), + name: "GrpcUnauthenticatedWithoutAuthDetailsFallsBackToAuthFailed", + inputErr: status.Error(codes.Unauthenticated, "generic auth problem"), verify: func(t *testing.T, protoErr *ExtensionError, goErr error) { assert.Equal(t, ErrorOrigin_ERROR_ORIGIN_LOCAL, protoErr.GetOrigin()) - assert.Equal(t, "login expired", protoErr.GetMessage()) + assert.Equal(t, "generic auth problem", protoErr.GetMessage()) localDetail := protoErr.GetLocalError() require.NotNil(t, localDetail) assert.Equal(t, "auth_failed", localDetail.GetCode()) assert.Equal(t, "auth", localDetail.GetCategory()) + + var localErr *LocalError + require.ErrorAs(t, goErr, &localErr) + assert.Equal(t, LocalErrorCategoryAuth, localErr.Category) + assert.Equal(t, "auth_failed", localErr.Code) }, }, } @@ -169,3 +223,16 @@ func TestExtensionError_RoundTrip(t *testing.T) { }) } } + +func mustAuthStatusError(code codes.Code, reason, message string) error { + st := status.New(code, message) + withDetails, err := st.WithDetails(&errdetails.ErrorInfo{ + Reason: reason, + Domain: AuthErrorDomain, + }) + if err != nil { + panic(err) + } + + return withDetails.Err() +}