Fail fast on azd ai agent init when not logged in#7614
Fail fast on azd ai agent init when not logged in#7614therealjohn wants to merge 1 commit intoAzure:mainfrom
Conversation
…in init command. Fixes Azure#7547
therealjohn
requested review from
JeffreyCA,
hemarina,
jongio,
rajeshkamal5050,
tg-msft,
trangevi,
trrwilson,
vhvb1989,
wbreza and
weikanglim
as code owners
There was a problem hiding this comment.
Pull request overview
Adds an early authentication probe to azd ai agent init so the command fails immediately when the user isn’t logged in, avoiding partially-initialized project state (Fixes #7547).
Changes:
- Introduces
ensureLoggedIn()which callsAccount.ListSubscriptionsand converts gRPCUnauthenticatedinto a structured auth error with a login suggestion. - Invokes
ensureLoggedIn()near the start ofinit’sRunE, before prompts and file-modifying work.
| // only gRPC Unauthenticated errors are treated as failures. Other errors (e.g. | ||
| // network issues) are ignored so they don't block init for unrelated reasons. | ||
| func ensureLoggedIn(ctx context.Context, azdClient *azdext.AzdClient) error { | ||
| _, err := azdClient.Account().ListSubscriptions(ctx, &azdext.ListSubscriptionsRequest{}) | ||
| if err == nil { | ||
| return nil | ||
| } | ||
|
|
||
| if st, ok := status.FromError(err); ok && st.Code() == codes.Unauthenticated { | ||
| return exterrors.Auth( | ||
| exterrors.CodeNotLoggedIn, | ||
| "not logged in", | ||
| "run `azd auth login` to authenticate before running init", | ||
| ) | ||
| } | ||
|
|
There was a problem hiding this comment.
ensureLoggedIn currently returns nil for all non-Unauthenticated errors, which also swallows cancellation/deadline errors (e.g., Ctrl+C or context timeout). That can cause init to continue after the user has cancelled. Consider explicitly propagating context cancellations (errors.Is(err, context.Canceled) or gRPC codes.Canceled/DeadlineExceeded) instead of returning nil.
| // only gRPC Unauthenticated errors are treated as failures. Other errors (e.g. | |
| // network issues) are ignored so they don't block init for unrelated reasons. | |
| func ensureLoggedIn(ctx context.Context, azdClient *azdext.AzdClient) error { | |
| _, err := azdClient.Account().ListSubscriptions(ctx, &azdext.ListSubscriptionsRequest{}) | |
| if err == nil { | |
| return nil | |
| } | |
| if st, ok := status.FromError(err); ok && st.Code() == codes.Unauthenticated { | |
| return exterrors.Auth( | |
| exterrors.CodeNotLoggedIn, | |
| "not logged in", | |
| "run `azd auth login` to authenticate before running init", | |
| ) | |
| } | |
| // gRPC Unauthenticated errors are treated as login failures, cancellation and | |
| // deadline errors are propagated, and other errors (e.g. network issues) are | |
| // ignored so they don't block init for unrelated reasons. | |
| func ensureLoggedIn(ctx context.Context, azdClient *azdext.AzdClient) error { | |
| _, err := azdClient.Account().ListSubscriptions(ctx, &azdext.ListSubscriptionsRequest{}) | |
| if err == nil { | |
| return nil | |
| } | |
| if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { | |
| return err | |
| } | |
| if st, ok := status.FromError(err); ok { | |
| if st.Code() == codes.Canceled || st.Code() == codes.DeadlineExceeded { | |
| return err | |
| } | |
| if st.Code() == codes.Unauthenticated { | |
| return exterrors.Auth( | |
| exterrors.CodeNotLoggedIn, | |
| "not logged in", | |
| "run `azd auth login` to authenticate before running init", | |
| ) | |
| } | |
| } |
| if err := ensureLoggedIn(ctx, azdClient); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
The new fail-fast auth behavior via ensureLoggedIn is important user-facing flow control, but there’s no unit test asserting it. Consider adding a test that starts a minimal in-process gRPC server with AccountService.ListSubscriptions returning codes.Unauthenticated and verifies init errors out before doing any work; also cover the non-Unauthenticated path (should not block).
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Clean fix for #7547. The early auth check prevents the user from going through manifest detection, scaffolding, and environment creation only to fail later at subscription selection. The function mirrors the checkAiModelServiceAvailable pattern right above it - consistent and easy to follow. One suggestion below.
Worth considering as a follow-up: azd core's AccountService doesn't have a dedicated CheckAuth or IsAuthenticated RPC. ListSubscriptions works as a probe here but it's indirect - it may enumerate all subscriptions just to verify auth. A lightweight auth-check method in core would give extensions a cleaner, cheaper way to gate on authentication, and could handle error classification (not_logged_in vs login_expired) server-side where the info is available.
| if st, ok := status.FromError(err); ok && st.Code() == codes.Unauthenticated { | ||
| return exterrors.Auth( | ||
| exterrors.CodeNotLoggedIn, | ||
| "not logged in", | ||
| "run `azd auth login` to authenticate before running init", | ||
| ) | ||
| } |
There was a problem hiding this comment.
The hardcoded "not logged in" message discards whatever the gRPC server actually reported. The exterrors package already has authFromGrpcMessage (unexported) that classifies "not logged in" vs "token expired" vs generic auth failure based on the server's message.
At minimum, forwarding st.Message() preserves useful diagnostic info:
| if st, ok := status.FromError(err); ok && st.Code() == codes.Unauthenticated { | |
| return exterrors.Auth( | |
| exterrors.CodeNotLoggedIn, | |
| "not logged in", | |
| "run `azd auth login` to authenticate before running init", | |
| ) | |
| } | |
| if st, ok := status.FromError(err); ok && st.Code() == codes.Unauthenticated { | |
| msg := st.Message() | |
| if msg == "" { | |
| msg = "not logged in" | |
| } | |
| return exterrors.Auth( | |
| exterrors.CodeNotLoggedIn, | |
| msg, | |
| "run `azd auth login` to authenticate before running init", | |
| ) | |
| } |
Longer term, exporting authFromGrpcMessage (or wrapping it) would let callers here get proper code classification too.
| if err := ensureLoggedIn(ctx, azdClient); err != nil { | ||
| return err | ||
| } | ||
|
|
There was a problem hiding this comment.
It might be better to arrange this after WaitForDebugger so debugging still works when logged out
| // only gRPC Unauthenticated errors are treated as failures. Other errors (e.g. | ||
| // network issues) are ignored so they don't block init for unrelated reasons. | ||
| func ensureLoggedIn(ctx context.Context, azdClient *azdext.AzdClient) error { | ||
| _, err := azdClient.Account().ListSubscriptions(ctx, &azdext.ListSubscriptionsRequest{}) |
There was a problem hiding this comment.
this function ensureLogged in should not depend on trying to run one API that requires logged in.
If there is not a unique API to check for auth, consider using the workflow api like in
You you can call azd auth status here
Problem: Running azd ai agent init without being authenticated lets the user proceed through manifest detection, project scaffolding (azd init -t), and environment creation before finally failing at subscription selection with "not logged in". This leaves the directory in a partially-initialized state.
Fix: Add an ensureLoggedIn check early in the init command's RunE, before any interactive prompts or file-modifying operations. It calls ListSubscriptions as a lightweight auth probe — if the user isn't authenticated, they see the error immediately with a suggestion to run azd auth login.
Fixes #7547