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

[WAM Broker] Check if scopes are passed when broker is invoked #3664

Merged
merged 5 commits into from
Sep 13, 2022
Merged

[WAM Broker] Check if scopes are passed when broker is invoked #3664

merged 5 commits into from
Sep 13, 2022

Conversation

gladjohn
Copy link
Contributor

@gladjohn gladjohn commented Sep 9, 2022

Fixes #3654

Changes proposed in this request

  • checks if user passes scopes in the request. MSAL Runtime needs scopes to be passed. Else users will hit this issue

GlobalExceptionHandler: Status: ApiContractViolation Context: (pii) Tag: 0x2039c1cb

  • few dev app fixes to test this
  • typos
  • more logging

Testing
Dev Apps

Performance impact
none

Documentation

  • All relevant documentation is updated.

@gladjohn gladjohn changed the title Check if scopes are passed when broker is invoked [WAM Broker] Check if scopes are passed when broker is invoked Sep 9, 2022
pmaytak
pmaytak reviewed Sep 10, 2022
View reviewed changes
Copy link
Contributor

@pmaytak pmaytak left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


src/client/Microsoft.Identity.Client.Broker/WamAdapters.cs Outdated Show resolved Hide resolved
src/client/Microsoft.Identity.Client.Broker/WamAdapters.cs Outdated Show resolved Hide resolved
src/client/Microsoft.Identity.Client.Broker/WamAdapters.cs Outdated Show resolved Hide resolved
src/client/Microsoft.Identity.Client.Broker/WamAdapters.cs Show resolved Hide resolved
trwalke
trwalke approved these changes Sep 13, 2022
View reviewed changes
Copy link
Member

@trwalke trwalke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gladjohn gladjohn merged commit 41831c0 into main Sep 13, 2022
@gladjohn gladjohn deleted the gladjohn/runtime_scopes branch September 13, 2022 17:40
SameerK-MSFT
SameerK-MSFT approved these changes Sep 13, 2022
View reviewed changes
Copy link
Contributor

@SameerK-MSFT SameerK-MSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make the method private.

src/client/Microsoft.Identity.Client.Broker/WamAdapters.cs
/// <param name="authenticationRequestParameters"></param>
/// <param name="logger"></param>
/// <exception cref="MsalClientException"></exception>
public static void ValidateAuthParams(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is used in only one place in the same class. Please make it private. (or write unit test :))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bgavrilMS bgavrilMS bgavrilMS left review comments

@trwalke trwalke trwalke approved these changes

@pmaytak pmaytak pmaytak approved these changes

@SameerK-MSFT SameerK-MSFT SameerK-MSFT approved these changes

@neha-bhargava neha-bhargava Awaiting requested review from neha-bhargava

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

Successfully merging this pull request may close these issues.

[Bug] Crashing Microsoft.Identity.Client.Broker - WithBrokerPreview()
6 participants
@gladjohn @bgavrilMS @trwalke @pmaytak @SameerK-MSFT @GladwinJohnson