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

Update templates to incorporate net7 changes #42802

Merged
merged 5 commits into from
Jul 26, 2022

Conversation

captainsafia
Copy link
Member

@captainsafia captainsafia commented Jul 19, 2022

Description

  • Removed UseAuthentication and UseAuthorization middleware registration from templates
  • Update Swashbuckle.AspNetCore version to 6.4.0
  • Set accelerator key for "Do not use top-level statement option" in Visual Studio

Closes #41893 and #42719

Customer Impact

Over the past few previews, we've been iterating on a feature to automatically register authentication and authorization middlewares when a user has configured authentication schemes or authorization policies. This PR removes the manual registration of these middlewares from the templates and contributes to our overall goal of reducing the amount of boilerplate that users have to write in order to configure authentication.

This PR also updates the Swashbuckle.AspNetCore dependency to incorporate code changes that light up the new WithOpenApi APIs shipping in preview5.

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

Low risk because:

  • Change is limited to ASP.NET templates with authentication configured.
  • Changes require minimal reactions in identity scaffolding.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jul 19, 2022
@captainsafia
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@captainsafia captainsafia changed the title Leverage auto-registration of AuthNZ middlewares Update templates to incorporate net7 changes Jul 20, 2022
@captainsafia
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@captainsafia
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@captainsafia captainsafia requested a review from a team July 20, 2022 22:51
@captainsafia captainsafia marked this pull request as ready for review July 20, 2022 22:51
@captainsafia captainsafia requested review from a team, dougbu, wtgodbe and Pilchie as code owners July 20, 2022 22:51
@dougbu
Copy link
Member

dougbu commented Jul 20, 2022

@captainsafia aren't these changes incomplete w/o removing some of the parameters from template.json files❔ We shouldn't leave dangling bits behind.

@DamianEdwards
Copy link
Member

@dougbu this is simply removing lines that are now not required as the framework effectively calls them on your behalf in the cases that the template would have added them before, i.e. the AuthN/AuthZ middleware getting added automatically now if AuthN/AuthZ services are added.

@dougbu
Copy link
Member

dougbu commented Jul 20, 2022

Got it @DamianEdwards, thanks. I'd resolve my comment if I could 😀

@captainsafia captainsafia added the Servicing-consider Shiproom approval is required for the issue label Jul 21, 2022
@ghost
Copy link

ghost commented Jul 21, 2022

Hi @captainsafia. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

@rbhanda rbhanda added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Jul 21, 2022
@rbhanda rbhanda added this to the 7.0-rc1 milestone Jul 21, 2022
@captainsafia
Copy link
Member Author

cc: @vlada-shubina @phenning for review on the changes related to accelerator keys for the "UseProgramMain" template option.

Copy link
Member

@brunolins16 brunolins16 left a comment

Choose a reason for hiding this comment

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

The template (cs files) changes look good for me; however, I don't know enough to review if the json changes are correct or not :)

@captainsafia captainsafia requested a review from phenning July 21, 2022 21:23
…bAssembly-CSharp/.template.config/localize/templatestrings.en.json
Copy link
Member

@vlada-shubina vlada-shubina left a comment

Choose a reason for hiding this comment

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

Change related to accelerator key looks good to me

@captainsafia
Copy link
Member Author

@phenning Do you mind giving this a green light if you're fine with the changes made?

@captainsafia captainsafia enabled auto-merge (squash) July 26, 2022 15:51
@vijayrkn
Copy link

@deepchoudhery @zahalzel - Tagging so that you can make the related changes to the ASP.NET Core Scaffolding & MS Identity scaffolding templates.

@ghost
Copy link

ghost commented Jul 28, 2022

Hi @vijayrkn. 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.

@deepchoudhery
Copy link
Member

@vijayrkn cool already been on it.

@ghost
Copy link

ghost commented Jul 28, 2022

Hi @deepchoudhery. 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update project templates to leverage new authentication APIs
9 participants