Skip to content

Comments

Implements GetAuthorizationHeaderAsync to support 1P specific scenarios#2896

Merged
sruke merged 4 commits intorel/v2from
sruthi/HeaderProviderEx
Jun 24, 2024
Merged

Implements GetAuthorizationHeaderAsync to support 1P specific scenarios#2896
sruke merged 4 commits intorel/v2from
sruthi/HeaderProviderEx

Conversation

@sruke
Copy link
Contributor

@sruke sruke commented Jun 14, 2024

What?

  • Implements IAuthorizationHeaderProvider.GetAuthorizationHeaderAsync in DefaultAuthorizationHeaderProvider
  • Updates DownstreamApi, MicrosoftGraph, GraphServiceClient and BaseAuthorizationHeaderProvider to use the new higher level API that can create a header for on-behalf-of an app or user instead of separate APIs (GetAuthorizationHeaderForAppAsync and GetAuthorizationHeaderForUserAsync). There is no change to the existing DevEx.
  • Adds ConfigureAwait which was previously missing for some async calls

Why?
This is being done to accommodate 1P specific protocols.

Copy link
Collaborator

@jmprieur jmprieur left a comment

Choose a reason for hiding this comment

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

This looks good.
did you run the test apps, in particular the Web App Calls Graph (there are links on the index page)

@sruke
Copy link
Contributor Author

sruke commented Jun 21, 2024

This looks good. did you run the test apps, in particular the Web App Calls Graph (there are links on the index page)

Yes, all the ID.Web tests are passing including WebAppCallsApiCallsGraph test.

@sruke sruke self-assigned this Jun 21, 2024
@sruke sruke marked this pull request as ready for review June 21, 2024 19:32
@sruke sruke changed the title Implements IAuthorizationHeaderProviderExtension Implements GetAuthorizationHeaderAsync to support 1P specific scenarios Jun 21, 2024
Copy link
Collaborator

@jennyf19 jennyf19 left a comment

Choose a reason for hiding this comment

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

:shipit:
we want this in master as well, correct?

Copy link
Collaborator

@jmprieur jmprieur left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks @sruke

@sruke
Copy link
Contributor Author

sruke commented Jun 24, 2024

:shipit: we want this in master as well, correct?

Yes. Once this PR is merged, I'll cherry pick this change into master.

@sruke sruke merged commit d242d0a into rel/v2 Jun 24, 2024
@sruke sruke deleted the sruthi/HeaderProviderEx branch June 24, 2024 17:38
sruke added a commit that referenced this pull request Jun 24, 2024
…os (#2896)

* Implements IAuthorizationHeaderProviderExtension

* Use DownstreamApiOptions.RequestAppToken to check if header is for app or user

* Updates Abstractions version

* Updates CAE tests

---------

Co-authored-by: Sruthi Keerthi Rangavajhula (from Dev Box) <[email protected]>
@sruke sruke mentioned this pull request Jun 24, 2024
sruke added a commit that referenced this pull request Jun 25, 2024
* Implements GetAuthorizationHeaderAsync to support 1P specific scenarios (#2896)

* Implements IAuthorizationHeaderProviderExtension

* Use DownstreamApiOptions.RequestAppToken to check if header is for app or user

* Updates Abstractions version

* Updates CAE tests

---------

Co-authored-by: Sruthi Keerthi Rangavajhula (from Dev Box) <[email protected]>

* Add missing using

* Update test

---------

Co-authored-by: Sruthi Keerthi Rangavajhula (from Dev Box) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Implement GetAuthorizationHeaderAsync in ID.Web and use it for DowmstreamApi, GraphServiceClient,

3 participants