Skip to content

Update RazorPages, MVC, WebApi templates to use Identity.Web #24167

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
Merged
merged 6 commits into from
Jul 25, 2020
Merged

Update RazorPages, MVC, WebApi templates to use Identity.Web #24167

merged 6 commits into from
Jul 25, 2020

Conversation

JunTaoLuo
Copy link
Contributor

Fixes #24024
Fixes #22726

ported from #24065

fyi @jmprieur

@JunTaoLuo JunTaoLuo requested review from dougbu and a team as code owners July 21, 2020 18:33
@JunTaoLuo JunTaoLuo added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jul 21, 2020
Tratcher
Tratcher reviewed Jul 21, 2020
View reviewed changes
Copy link
Member

@Tratcher Tratcher left a comment

Choose a reason for hiding this comment

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

I filed AzureAD/microsoft-identity-web#346 for an AzureAd migration guide, but we'll also need a docs issue for the AspNetCore migration guide that links to theirs.

JunTaoLuo
JunTaoLuo commented Jul 23, 2020
View reviewed changes
...eb.ProjectTemplates/content/RazorPagesWeb-CSharp/Services/MicrosoftGraphServiceExtensions.cs
services.AddTokenAcquisition(true);
services.AddSingleton<GraphServiceClient, GraphServiceClient>(serviceProvider =>
{
var tokenAquisitionService = serviceProvider.GetService<ITokenAcquisition>();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

#24065 (comment) This probably needs to be GetRequiredService if you expect this to not be null.

JunTaoLuo
JunTaoLuo commented Jul 23, 2020
View reviewed changes
...jectTemplates/Web.ProjectTemplates/content/RazorPagesWeb-CSharp/Services/DownstreamWebApi.cs

namespace Company.WebApplication1
{
public interface IDownstreamWebApi
Copy link
Contributor Author

Choose a reason for hiding this comment

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

#24065 (comment) Is there a specific reason why we need an interface here?

JunTaoLuo
JunTaoLuo commented Jul 23, 2020
View reviewed changes
src/ProjectTemplates/Web.ProjectTemplates/content/StarterWeb-CSharp/Views/Home/Index.cshtml Outdated
@@ -6,3 +6,8 @@
<h1 class="display-4">Welcome</h1>
<p>Learn about <a href="https://docs.microsoft.com/aspnet/core">building Web apps with ASP.NET Core</a>.</p>
</div>
@* #if(GenerateApiOrGraph)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI this doesn't work. The generated templates just has this section verbatim.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried fixing this with 3b093a8. It's working as intended but doesn't seem like the right syntax.

@JunTaoLuo
Copy link
Contributor Author

@jmprieur and @jennyf19 were able to verify that {mvc, webapp, webapi} x {IndividualB2C + API, SingleOrg + Api, SingleOrg + Graph} all works so this is ready to be merged.

Tratcher
Tratcher approved these changes Jul 24, 2020
View reviewed changes
...jectTemplates/Web.ProjectTemplates/content/RazorPagesWeb-CSharp/Services/DownstreamWebApi.cs
}
else
{
apiResult = $"Error calling the API '{apiUrl}'";
Copy link
Member

Choose a reason for hiding this comment

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

Later: Add logging

src/ProjectTemplates/Web.ProjectTemplates/content/RazorPagesWeb-CSharp/Startup.cs
Comment on lines +76 to +77
.AddMicrosoftWebAppCallsWebApi(Configuration,
"AzureAd")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.AddMicrosoftWebAppCallsWebApi(Configuration,
"AzureAd")
.AddMicrosoftWebAppCallsWebApi(Configuration, "AzureAd")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll address this in the Blazor templates PR, this appears in several places IIRC.

@JunTaoLuo JunTaoLuo merged commit dc53cd0 into release/5.0-preview8 Jul 25, 2020
@JunTaoLuo JunTaoLuo deleted the johluo/identity-web-templates2 branch July 25, 2020 00:36
This was referenced Jul 27, 2020
Merged
Closed
Closed
Closed
@satishrabari
Copy link

Hi, Would you please help me with the following things?
I am using Microsoft Identity Platform.
I want to redirect to account/logincallback URL after successful login.
Where I put the redirect URL for this?

I want to configure using startup.cs because I want to save user info in DB and add custom claims in the claim principle.
Is it possible?
I don't have a login URL on my web.

following is my Startup.cs
`

        services.AddAuthentication(OpenIdConnectDefaults.AuthenticationScheme)
            .AddMicrosoftIdentityWebApp(Configuration.GetSection("AzureAd"));

        services.AddControllersWithViews(options =>
        {
            var policy = new AuthorizationPolicyBuilder()
                .RequireAuthenticatedUser()
                .Build();
            options.Filters.Add(new AuthorizeFilter(policy));
        });
        services.AddRazorPages()
             .AddMicrosoftIdentityUI();

`

@ghost
Copy link

ghost commented Jul 12, 2021

Hi @satishrabari. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@satishrabari satishrabari mentioned this pull request Jul 12, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tratcher Tratcher Tratcher approved these changes

@blowdart blowdart Awaiting requested review from blowdart

@javiercn javiercn Awaiting requested review from javiercn

@dougbu dougbu Awaiting requested review from dougbu

Assignees
No one assigned
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JunTaoLuo @satishrabari @Tratcher