Skip to content

Commit 0b00925

Browse files
JeffreyCACopilotCopilot
authored
Provide error suggestions for AADSTS530084 and include auth error details when emitted as ErrorWithSuggestion in telemetry (#7797)
* Preserve auth subtypes in gRPC error details * Include auth error details when emitted as `error.suggestion` Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1c486f0 commit 0b00925

14 files changed

Lines changed: 600 additions & 73 deletions

cli/azd/cmd/auth_login.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -314,10 +314,12 @@ func (la *loginAction) Run(ctx context.Context) (*actions.ActionResult, error) {
314314
// We print any non-setup related errors to stderr.
315315
// We always return a zero exit code.
316316
token, err := la.verifyLoggedIn(ctx)
317-
_, loginExpiryError := errors.AsType[*auth.ReLoginRequiredError](err)
317+
// An *internal.ErrorWithSuggestion already carries actionable, user-facing guidance
318+
// surfaced through the login result; avoid double-printing the raw error.
319+
_, hasSuggestion := errors.AsType[*internal.ErrorWithSuggestion](err)
318320
if err != nil &&
319321
!errors.Is(err, auth.ErrNoCurrentUser) &&
320-
!loginExpiryError {
322+
!hasSuggestion {
321323
fmt.Fprintln(la.console.Handles().Stderr, err.Error())
322324
}
323325

cli/azd/cmd/auth_status.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,12 @@ func (a *authStatusAction) Run(ctx context.Context) (*actions.ActionResult, erro
8181

8282
// get user account information
8383
details, err := a.authManager.LogInDetails(ctx)
84-
_, loginExpiryError := errors.AsType[*auth.ReLoginRequiredError](err)
84+
// An *internal.ErrorWithSuggestion already carries actionable, user-facing guidance
85+
// surfaced through the status result; avoid double-printing the raw error.
86+
_, hasSuggestion := errors.AsType[*internal.ErrorWithSuggestion](err)
8587
if err != nil {
8688
if !errors.Is(err, auth.ErrNoCurrentUser) &&
87-
!loginExpiryError {
89+
!hasSuggestion {
8890
// print a useful message for unknown errors
8991
fmt.Fprintln(a.console.Handles().Stderr, err.Error())
9092
}

cli/azd/internal/cmd/errors.go

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,11 @@ func MapError(err error, span tracing.Span) {
5252
errDetails = append(errDetails, fields.ErrCategory.String("auth"))
5353
} else if errWithSuggestion, ok := errors.AsType[*internal.ErrorWithSuggestion](err); ok {
5454
errCode = "error.suggestion"
55-
span.SetAttributes(fields.ErrType.String(classifySuggestionType(errWithSuggestion.Unwrap())))
55+
innerErr := errWithSuggestion.Unwrap()
56+
span.SetAttributes(fields.ErrType.String(classifySuggestionType(innerErr)))
57+
if authFailedErr, ok := errors.AsType[*auth.AuthFailedError](innerErr); ok {
58+
errDetails = append(errDetails, authFailedTelemetryDetails(authFailedErr)...)
59+
}
5660
} else if respErr, ok := errors.AsType[*azcore.ResponseError](err); ok {
5761
serviceName := "other"
5862
statusCode := -1
@@ -173,18 +177,7 @@ func MapError(err error, span tracing.Span) {
173177
errDetails = append(errDetails, fields.ToolName.String(strings.Join(toolCheckErr.ToolNames, ",")))
174178
}
175179
} else if authFailedErr, ok := errors.AsType[*auth.AuthFailedError](err); ok {
176-
errDetails = append(errDetails, fields.ServiceName.String("aad"))
177-
if authFailedErr.Parsed != nil {
178-
codes := make([]string, 0, len(authFailedErr.Parsed.ErrorCodes))
179-
for _, code := range authFailedErr.Parsed.ErrorCodes {
180-
codes = append(codes, fmt.Sprintf("%d", code))
181-
}
182-
serviceErr := strings.Join(codes, ",")
183-
errDetails = append(errDetails,
184-
fields.ServiceStatusCode.String(authFailedErr.Parsed.Error),
185-
fields.ServiceErrorCode.String(serviceErr),
186-
fields.ServiceCorrelationId.String(authFailedErr.Parsed.CorrelationId))
187-
}
180+
errDetails = append(errDetails, authFailedTelemetryDetails(authFailedErr)...)
188181
errCode = "service.aad.failed"
189182
} else if errors.Is(err, auth.ErrNoCurrentUser) {
190183
errCode = "auth.not_logged_in"
@@ -216,6 +209,24 @@ func MapError(err error, span tracing.Span) {
216209
span.SetStatus(codes.Error, errCode)
217210
}
218211

212+
func authFailedTelemetryDetails(authFailedErr *auth.AuthFailedError) []attribute.KeyValue {
213+
errDetails := []attribute.KeyValue{fields.ServiceName.String("aad")}
214+
if authFailedErr == nil || authFailedErr.Parsed == nil {
215+
return errDetails
216+
}
217+
218+
codes := make([]string, 0, len(authFailedErr.Parsed.ErrorCodes))
219+
for _, code := range authFailedErr.Parsed.ErrorCodes {
220+
codes = append(codes, fmt.Sprintf("%d", code))
221+
}
222+
223+
return append(errDetails,
224+
fields.ServiceStatusCode.String(authFailedErr.Parsed.Error),
225+
fields.ServiceErrorCode.String(strings.Join(codes, ",")),
226+
fields.ServiceCorrelationId.String(authFailedErr.Parsed.CorrelationId),
227+
)
228+
}
229+
219230
// classifySentinel checks if the error matches a known sentinel
220231
// and returns the corresponding telemetry code, or "" if no match.
221232
func classifySentinel(err error) string {

cli/azd/internal/cmd/errors_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,28 @@ func Test_MapError(t *testing.T) {
211211
fields.ErrorKey(fields.ErrCategory.Key).String("auth"),
212212
},
213213
},
214+
{
215+
name: "WithTokenProtectionBlockedAuthFailedError",
216+
err: &internal.ErrorWithSuggestion{
217+
Err: &auth.AuthFailedError{
218+
Parsed: &auth.AadErrorResponse{
219+
Error: "invalid_grant",
220+
ErrorCodes: []int{530084},
221+
CorrelationId: "blocked-token-protection",
222+
},
223+
},
224+
},
225+
// AADSTS530084 surfaced via the suggestion wrapper still classifies as the generic
226+
// AAD service failure for telemetry, and should preserve the wrapped AAD details.
227+
wantErrReason: "error.suggestion",
228+
wantErrDetails: []attribute.KeyValue{
229+
fields.ErrType.String("service.aad.failed"),
230+
fields.ErrorKey(fields.ServiceName.Key).String("aad"),
231+
fields.ErrorKey(fields.ServiceErrorCode.Key).String("530084"),
232+
fields.ErrorKey(fields.ServiceStatusCode.Key).String("invalid_grant"),
233+
fields.ErrorKey(fields.ServiceCorrelationId.Key).String("blocked-token-protection"),
234+
},
235+
},
214236
{
215237
name: "WithAzidentityAuthenticationFailedError",
216238
err: &azidentity.AuthenticationFailedError{},

cli/azd/internal/grpcserver/server.go

Lines changed: 60 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/azure/azure-dev/cli/azd/pkg/auth"
1616
"github.com/azure/azure-dev/cli/azd/pkg/azdext"
1717
"github.com/azure/azure-dev/cli/azd/pkg/extensions"
18+
"google.golang.org/genproto/googleapis/rpc/errdetails"
1819
"google.golang.org/grpc"
1920
"google.golang.org/grpc/codes"
2021
"google.golang.org/grpc/metadata"
@@ -272,31 +273,84 @@ func (s *authenticatedStream) Context() context.Context {
272273
// This ensures that helpful suggestions (like "run azd auth login") are preserved
273274
// when errors are transmitted over gRPC, where only the error message string is sent.
274275
//
275-
// Auth-related errors (ReLoginRequiredError, ErrNoCurrentUser) are returned with
276-
// codes.Unauthenticated so that extensions can detect auth failures via gRPC status code.
276+
// Auth-related errors are returned with codes.Unauthenticated and a structured
277+
// ErrorInfo reason so extensions can reliably classify them as auth failures
278+
// while still distinguishing the remediation path.
277279
func wrapErrorWithSuggestion(err error) error {
278280
if err == nil {
279281
return nil
280282
}
281283

282-
_, loginErr := errors.AsType[*auth.ReLoginRequiredError](err)
283-
isAuthErr := errors.Is(err, auth.ErrNoCurrentUser) || loginErr
284+
isAuthErr := isAuthError(err)
284285

285286
if suggestionErr, ok := errors.AsType[*internal.ErrorWithSuggestion](err); ok {
286287
msg := fmt.Sprintf("%s\n%s", err.Error(), suggestionErr.Suggestion)
287288
if isAuthErr {
288-
return status.Error(codes.Unauthenticated, msg)
289+
return grpcAuthStatus(err, msg)
289290
}
290291
return fmt.Errorf("%w\n%s", err, suggestionErr.Suggestion)
291292
}
292293

293294
if isAuthErr {
294-
return status.Error(codes.Unauthenticated, err.Error())
295+
return grpcAuthStatus(err, err.Error())
295296
}
296297

297298
return err
298299
}
299300

301+
// isAuthError reports whether err's chain contains a known auth-failure type that should be
302+
// surfaced over gRPC as codes.Unauthenticated.
303+
func isAuthError(err error) bool {
304+
if errors.Is(err, auth.ErrNoCurrentUser) {
305+
return true
306+
}
307+
if _, ok := errors.AsType[*auth.ReLoginRequiredError](err); ok {
308+
return true
309+
}
310+
if _, ok := errors.AsType[*auth.AuthFailedError](err); ok {
311+
return true
312+
}
313+
return false
314+
}
315+
316+
func grpcAuthStatus(err error, msg string) error {
317+
st := status.New(codes.Unauthenticated, msg)
318+
reason := grpcAuthReason(err)
319+
if reason == "" {
320+
return st.Err()
321+
}
322+
323+
withDetails, detailErr := st.WithDetails(&errdetails.ErrorInfo{
324+
Reason: reason,
325+
Domain: azdext.AuthErrorDomain,
326+
})
327+
if detailErr != nil {
328+
return st.Err()
329+
}
330+
331+
return withDetails.Err()
332+
}
333+
334+
func grpcAuthReason(err error) string {
335+
if errors.Is(err, auth.ErrNoCurrentUser) {
336+
return azdext.AuthErrorReasonNotLoggedIn
337+
}
338+
339+
// Pass through the originating AAD error code (e.g., "AADSTS530084") when available.
340+
// This preserves Entra's own semantics rather than redefining them on azd's side.
341+
if authFailed, ok := errors.AsType[*auth.AuthFailedError](err); ok {
342+
if authFailed.Parsed != nil && len(authFailed.Parsed.ErrorCodes) > 0 {
343+
return fmt.Sprintf("AADSTS%d", authFailed.Parsed.ErrorCodes[0])
344+
}
345+
}
346+
347+
if _, ok := errors.AsType[*auth.ReLoginRequiredError](err); ok {
348+
return azdext.AuthErrorReasonLoginRequired
349+
}
350+
351+
return ""
352+
}
353+
300354
func generateSigningKey() ([]byte, error) {
301355
bytes := make([]byte, 32) // 256-bit HMAC signing key
302356
if _, err := rand.Read(bytes); err != nil {

cli/azd/internal/grpcserver/server_coverage3_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@ import (
1111

1212
"github.com/azure/azure-dev/cli/azd/internal"
1313
"github.com/azure/azure-dev/cli/azd/pkg/auth"
14+
"github.com/azure/azure-dev/cli/azd/pkg/azdext"
1415
"github.com/stretchr/testify/assert"
1516
"github.com/stretchr/testify/require"
17+
"google.golang.org/genproto/googleapis/rpc/errdetails"
1618
"google.golang.org/grpc"
1719
"google.golang.org/grpc/codes"
1820
"google.golang.org/grpc/metadata"
@@ -77,6 +79,33 @@ func TestWrapErrorWithSuggestion_ReLoginRequired(t *testing.T) {
7779
require.Equal(t, codes.Unauthenticated, st.Code())
7880
}
7981

82+
func TestWrapErrorWithSuggestion_TokenProtectionBlocked(t *testing.T) {
83+
t.Parallel()
84+
authFailed := &auth.AuthFailedError{
85+
Parsed: &auth.AadErrorResponse{
86+
Error: "invalid_grant",
87+
ErrorCodes: []int{530084},
88+
},
89+
}
90+
// In production the wrapper is built by newActionableAuthError; mirror that shape here so
91+
// wrapErrorWithSuggestion classifies the wrapped *AuthFailedError as an auth interaction.
92+
err := fmt.Errorf("token protection: %w", &internal.ErrorWithSuggestion{
93+
Err: authFailed,
94+
Suggestion: "Contact your IT administrator or request a policy exception.",
95+
})
96+
97+
wrapped := wrapErrorWithSuggestion(err)
98+
st, ok := status.FromError(wrapped)
99+
require.True(t, ok)
100+
require.Equal(t, codes.Unauthenticated, st.Code())
101+
details := st.Details()
102+
require.Len(t, details, 1)
103+
info, ok := details[0].(*errdetails.ErrorInfo)
104+
require.True(t, ok)
105+
require.Equal(t, azdext.AuthErrorDomain, info.Domain)
106+
require.Equal(t, "AADSTS530084", info.Reason)
107+
}
108+
80109
func TestWrapErrorWithSuggestion_AuthErrorWithSuggestion(t *testing.T) {
81110
t.Parallel()
82111
inner := auth.ErrNoCurrentUser

cli/azd/internal/grpcserver/server_test.go

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/azure/azure-dev/cli/azd/pkg/azdext"
1919
"github.com/azure/azure-dev/cli/azd/pkg/extensions"
2020
"github.com/stretchr/testify/require"
21+
"google.golang.org/genproto/googleapis/rpc/errdetails"
2122
"google.golang.org/grpc/codes"
2223
"google.golang.org/grpc/status"
2324
)
@@ -239,6 +240,7 @@ func Test_wrapErrorWithSuggestion(t *testing.T) {
239240
wantContain string
240241
wantSameInstance bool
241242
wantGrpcCode codes.Code
243+
wantAuthReason string
242244
}{
243245
{
244246
name: "nil error returns nil",
@@ -268,25 +270,43 @@ func Test_wrapErrorWithSuggestion(t *testing.T) {
268270
wantContain: "azd auth login",
269271
},
270272
{
271-
name: "ErrNoCurrentUser returns Unauthenticated",
272-
err: auth.ErrNoCurrentUser,
273-
wantContain: "not logged in",
274-
wantGrpcCode: codes.Unauthenticated,
273+
name: "ErrNoCurrentUser returns Unauthenticated",
274+
err: auth.ErrNoCurrentUser,
275+
wantContain: "not logged in",
276+
wantGrpcCode: codes.Unauthenticated,
277+
wantAuthReason: azdext.AuthErrorReasonNotLoggedIn,
275278
},
276279
{
277-
name: "wrapped ErrNoCurrentUser returns Unauthenticated",
278-
err: fmt.Errorf("failed to list subscriptions: %w", auth.ErrNoCurrentUser),
279-
wantContain: "not logged in",
280-
wantGrpcCode: codes.Unauthenticated,
280+
name: "wrapped ErrNoCurrentUser returns Unauthenticated",
281+
err: fmt.Errorf("failed to list subscriptions: %w", auth.ErrNoCurrentUser),
282+
wantContain: "not logged in",
283+
wantGrpcCode: codes.Unauthenticated,
284+
wantAuthReason: azdext.AuthErrorReasonNotLoggedIn,
281285
},
282286
{
283287
name: "ReLoginRequiredError with suggestion returns Unauthenticated",
284288
err: &internal.ErrorWithSuggestion{
285289
Err: &auth.ReLoginRequiredError{},
286290
Suggestion: "login expired, run `azd auth login` to acquire a new token.",
287291
},
288-
wantContain: "azd auth login",
289-
wantGrpcCode: codes.Unauthenticated,
292+
wantContain: "azd auth login",
293+
wantGrpcCode: codes.Unauthenticated,
294+
wantAuthReason: azdext.AuthErrorReasonLoginRequired,
295+
},
296+
{
297+
name: "TokenProtectionBlockedError with suggestion returns Unauthenticated",
298+
err: &internal.ErrorWithSuggestion{
299+
Err: &auth.AuthFailedError{
300+
Parsed: &auth.AadErrorResponse{
301+
Error: "invalid_grant",
302+
ErrorCodes: []int{530084},
303+
},
304+
},
305+
Suggestion: "Contact your IT administrator or request a policy exception.",
306+
},
307+
wantContain: "policy exception",
308+
wantGrpcCode: codes.Unauthenticated,
309+
wantAuthReason: "AADSTS530084",
290310
},
291311
}
292312

@@ -306,6 +326,14 @@ func Test_wrapErrorWithSuggestion(t *testing.T) {
306326
st, ok := status.FromError(result)
307327
require.True(t, ok, "expected gRPC status error")
308328
require.Equal(t, tt.wantGrpcCode, st.Code())
329+
if tt.wantAuthReason != "" {
330+
details := st.Details()
331+
require.Len(t, details, 1)
332+
info, ok := details[0].(*errdetails.ErrorInfo)
333+
require.True(t, ok, "expected ErrorInfo detail")
334+
require.Equal(t, azdext.AuthErrorDomain, info.Domain)
335+
require.Equal(t, tt.wantAuthReason, info.Reason)
336+
}
309337
}
310338
})
311339
}

cli/azd/pkg/auth/azd_credential.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func (c *azdCredential) GetToken(ctx context.Context, options policy.TokenReques
7575
c.cacheTracer.LogSnapshotOnce(failurePhase)
7676

7777
if authFailed, ok := errors.AsType[*AuthFailedError](err); ok {
78-
if loginErr, ok := newReLoginRequiredError(authFailed.Parsed, options.Scopes, c.cloud, tenantID); ok {
78+
if loginErr, ok := newActionableAuthError(authFailed.Parsed, options.Scopes, c.cloud, tenantID, authFailed); ok {
7979
log.Println(authFailed.httpErrorDetails())
8080

8181
if options.Claims != "" {

0 commit comments

Comments
 (0)