Skip to content
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.

Sign up for GitHub

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

Jump to bottom

auth: Fix getScopes always injecting the management scope, even if a scope for a different resource is specified #1594

Merged
merged 2 commits into from
Sep 21, 2023
Merged

auth: Fix getScopes always injecting the management scope, even if a scope for a different resource is specified #1594

merged 2 commits into from
Sep 21, 2023

Conversation

MRayermannMSFT
Copy link
Member

@MRayermannMSFT MRayermannMSFT commented Sep 21, 2023 •
edited
Loading

When trying to get a credential with scope http://storage.azure.com, the management scope was being injected. This caused vscode.authentication.getSession to return an undefined session.

@MRayermannMSFT MRayermannMSFT requested a review from a team as a code owner September 21, 2023 17:49
@alexweininger alexweininger changed the title Fix getScopes always injecting the management scope, even if a scope for a different resource is specified auth: Fix getScopes always injecting the management scope, even if a scope for a different resource is specified Sep 21, 2023
@MRayermannMSFT
Copy link
Member Author

MRayermannMSFT commented Sep 21, 2023 •
edited
Loading

Note for future people looking back at this PR. I just noticed my dumb code new Set<string>([...arrScopes]);. I intentionally choose not to fix to avoid a re-review. :)

@MRayermannMSFT MRayermannMSFT merged commit 7dbd629 into main Sep 21, 2023
4 checks passed
@MRayermannMSFT MRayermannMSFT deleted the dev/marayerm/fix-get-scopes branch September 21, 2023 18:03
@alexweininger
Copy link
Member

Note for future people looking back at this PR. I just noticed my dumb code new Set<string>([...arrScopes]);. I intentionally choose not to fix to avoid a re-review. :)

As the approver I take responsibility #1595

@alexweininger alexweininger mentioned this pull request Oct 19, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexweininger alexweininger alexweininger approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MRayermannMSFT @alexweininger