-
Notifications
You must be signed in to change notification settings - Fork 267
Add account name and login type to JSON response for azd auth login --check-status #5404
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…-check-status Co-authored-by: weikanglim <2322434+weikanglim@users.noreply.github.com>
azd auth login --check-status -o json|
/azp run azure-dev - cli |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…eck-status Co-authored-by: weikanglim <2322434+weikanglim@users.noreply.github.com>
… PrincipalType Co-authored-by: weikanglim <2322434+weikanglim@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
cli/azd/pkg/output/ux/logged_in.go
Outdated
| UserLoginType LoginType = "User" | ||
| ServicePrincipalLoginType LoginType = "ServicePrincipal" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Move the login type declarations to login.go in contracts package
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved the LoginType declarations from ux/logged_in.go to the contracts package as requested. This centralizes the login type definitions and removes duplication across packages. Commit 4de5daa
cli/azd/pkg/contracts/login.go
Outdated
| ExpiresOn *time.Time `json:"expiresOn,omitempty"` | ||
| // When status is `LoginStatusSuccess`, the account name the user is logged in as. | ||
| Account *string `json:"account,omitempty"` | ||
| // When status is `LoginStatusSuccess`, the type of login (email or clientId). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot update this documentation to reflect the actual values (User or ServicePrincipal)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the documentation to reflect the actual values (User or ServicePrincipal) in commit 4de5daa
Co-authored-by: weikanglim <2322434+weikanglim@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
cli/azd/pkg/contracts/login.go
Outdated
| import "time" | ||
|
|
||
| // LoginType represents the type of authentication used | ||
| type LoginType string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot change this to PrincipalType to reflect the type of principal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed LoginType to PrincipalType and updated all references throughout the codebase. Also renamed the constants to UserPrincipalType and ServicePrincipalPrincipalType for consistency. Commit 1f023b0
cli/azd/pkg/contracts/login.go
Outdated
| // When status is `LoginStatusSuccess`, the type of login (User or ServicePrincipal). | ||
| LoginType *LoginType `json:"loginType,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot make the new fields principal.name and principal.type to be more consistent with the current contracted usage in azd env variables AZURE_PRINCIPAL_xx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed the JSON structure to use nested principal.name and principal.type fields to be consistent with the AZURE_PRINCIPAL_* environment variable naming convention. Commit 81fb55b
Co-authored-by: weikanglim <2322434+weikanglim@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
…type fields Co-authored-by: weikanglim <2322434+weikanglim@users.noreply.github.com>
|
Hi @@copilot. Thank you for your interest in helping to improve the Azure Developer CLI experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days. |
|
Hi @@copilot. Thank you for your contribution. Since there hasn't been recent engagement, we're going to close this out. Feel free to respond with a comment containing "/reopen" if you'd like to continue working on these changes. Please be sure to use the command to reopen or remove the "no-recent-activity" label; otherwise, this is likely to be closed again with the next cleanup pass. |
|
@JeffreyCA @weikanglim can we review and move this PR along? |
|
@rajeshkamal5050 This will be merged with the new auth status command #6377. |
|
Superseded by #6377 |
Adds optional fields to the
contracts.LoginResultstruct and updated the authentication logic to populate these fields when the user is authenticated.The JSON response now includes account information:
{ "status": "success", "expiresOn": "2025-07-08T20:20:26Z", "principal": { "name": "useremail", "type": "User" } }Fixes #5402