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.

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

(Smaller) PR just for the DownstreamApiService on top of the Graph service #447

Merged
merged 40 commits into from
Aug 13, 2020

Conversation

jmprieur
Copy link
Collaborator

@jmprieur jmprieur commented Aug 13, 2020

Just for #403
I've taken into account the PR review comments of the bigger branch

Proposal for Downstream web API support

  • Downstream API support folder contains options, service, extensions
  • Downstream API options
  • Updated the Blazor sample to use both the Graph service and the Downstream API service (BlazorServer calls graph)

Todo list sample

  • changed the namespace of the model (common to client and service). Ideally we'd put in a shared project. We might do that later.
  • Updated the TodoListService class to leverage the DownstreamWebApi service. The code is way simpler!!!
  • updated the AppSettings.json and the Startup.cs to leverage these services

jmprieur and others added 30 commits August 3, 2020 21:41
Todo:
- see if we could have some commonalities between the Web app and Web API builder (configuration?)
- Enfoce configuration for CallsWebAPI can only be called if configuration for the AddMicrosoftWebApp/Api
- Renaming AddMicrosoftWebApp to AddMicrosoftIdentityPlatformWebApp,
- Renaming AddMicrosoftWebApi to AddMicrosoftIdentityPlatformWebApi,
 - MicrososoftAppCallingWebApiAuthenticationBuilder.AddInXXXTokenCaches etc ... return their parent builder.
… (#424)

* Make GetTokenForAppAsync less confusing and allow to pass tenantId #413

Checked with @hpsin and here is what we agreed to:
- Change the signature of `GetAccessTokenForUserAsync` to take a `string` (instead of a `IEnumerable<string>`) as there is only one possible string for a given resource of App Id URI AppIdUri: "AppIdUri/.default". Check that the resource ends in "./default"
- Add an additional optional parameter `tenant` to support this scenario, and verify that this tenant is not organizations (and of course common and consumers, which don't make sense)

```CSharp
public async Task<string> GetAccessTokenForAppAsync(string scope, string? tenant = null)
```

* Update src/Microsoft.Identity.Web/ITokenAcquisition.cs
Co-authored-by: jennyf19 <jeferrie@microsoft.com>

* Addressing PR feedback:
- Adding an aka.ms link to the error messages (https://aka.ms/ms-id-web/daemon-scenarios)
- using constants for the meta-tenants
- Testing all the meta tenant
Thanks @jennyf19 for this PR feedback

* Addressing @hpsin 's PR feedback.
…ntityWebApp

- Updating the ITokenAcquisition.GetTokenForAppAsync signature to match the class.
@jmprieur jmprieur changed the base branch from jennyf/newApiPlusGraphService to master August 13, 2020 19:05
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:

@jmprieur jmprieur merged commit 3343fb0 into master Aug 13, 2020
@jmprieur jmprieur deleted the jennyf/newApiPlusGraphServicePlusDownstreamApiService branch August 19, 2020 16:29
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.

3 participants