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

Issues within Visual Studio when selecting both Azure AD and Docker Linux support #24151

Closed
SeanKilleen opened this issue Jun 5, 2020 · 23 comments
Assignees
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares ✔️ Resolution: Fixed The bug or enhancement requested in this issue has been checked-in!
Milestone

Comments

@SeanKilleen
Copy link
Contributor

I'm not sure if this is the correct location as this issue deals with Visual Studio and this repository looks to be largely geared toward dotnet new. If I'm not in the right spot, feel free to close or forward me elsewhere.


Recently, I created a project within Visual Studio. I selected .NET Core MVC. I enabled the options for Azure AD Auth (single organization) and Docker container support with Linux.

Upon publishing my container to an Azure Web app for Containers, I noticed that the redirect URI was specified as always using http.

I opened an issue here: #22572

It turns out I needed to do some additional work to enable header forwarding in order for Azure AD auth to work within the container.

I'm hoping that we could get this added to the templating engine so that applications would work out of the box.

I'd be happy to send along a PR if someone points me in the right direction.

Thanks for all you do!

@donJoseLuis
Copy link

Hello @SeanKilleen , we are also trying to find the right place. Please share the relevant template name (if you know it) or steps to reproduce. Based on this we can then find the right repo.

@SeanKilleen
Copy link
Contributor Author

SeanKilleen commented Jun 8, 2020

Thanks @donJoseLuis!

My steps to recreate were:

  • Open Visual Studio 2019
  • Select to create a new project
  • Select ASP.NET Core Web Application
  • Select Web Application (Model-View-Controller)
  • Under Authentication, select Change and change to Work or School Accounts. Select Cloud - Single Organization as the option and choose the domain from the drop-down. Click OK
  • Leave the checkbox for Enable Docker Support checked and set to Linux.
  • Create the project.

Let me know if I can provide more detail. I think one template (Docker or AD) needs to be modified to detect the enablement of the other one and should add the additional code to forward the headers.

One consideration: This may need to be an optional thing. We may want to add a method within Startup that accomplishes this, along with a comment and/or docs reference.

This makes me think it's likely the template for "Work or School Accounts" auth that should be updated to see if there's a dockerfile in the solution.

@SeanKilleen
Copy link
Contributor Author

Hi @donJoseLuis -- wanted to follow up to make sure you'd seen my reply and that this is no longer labeled as need-customer-info. Thanks!

@donJoseLuis
Copy link

Greetings @SeanKilleen . Thanks for the ping.
@grinrag + @ladipro , can you please re-assess the issue in this week's triage?

@genalt
Copy link

genalt commented Jun 23, 2020

@mkArtakMSFT looks like this is ASP.NET template issue. Could you, please, point me to the right repo, where is the source of template located?

@genalt
Copy link

genalt commented Jun 23, 2020

@SeanKilleen this repo contains only template engine source code and couple of templates. The ASP.NET templates are located in a different repo. I asked the guy, who can point us to the right place.

@genalt genalt transferred this issue from dotnet/templating Jul 21, 2020
@javiercn
Copy link
Member

@grinrag this is the right repo for the aspnetcore templates.

@javiercn javiercn added the area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer label Jul 21, 2020
@Tratcher
Copy link
Member

What version are you using? This should have been addressed back in 3.0. See https://devblogs.microsoft.com/aspnet/forwarded-headers-middleware-updates-in-net-core-3-0-preview-6/

@bradygaster

@blowdart blowdart added Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. area-servers and removed area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer labels Jul 23, 2020
@ghost
Copy link

ghost commented Jul 27, 2020

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

See our Issue Management Policies for more information.

@SeanKilleen
Copy link
Contributor Author

Sorry, I'd missed this in a bunch of notifications.

@Tratcher I was using Visual studio 2019 with latest updates at the time and .NET Core 3.1 installed. When using the templates, I had to manually add the change for it to work.

Are you saying that you're currently unable to reproduce it with the steps I provided?

@ghost ghost added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. Status: No Recent Activity labels Jul 27, 2020
@BrennanConroy BrennanConroy added this to the Next sprint planning milestone Jul 29, 2020
@ghost
Copy link

ghost commented Jul 29, 2020

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@sebastienros
Copy link
Member

After some investigation, I confirm that the headers forwarder middleware is registered, but it's only enabled if the ASPNET_ForwardedHeaders_Enabled environment variable is equal to true, or if the appsettings.json contains the "ForwardedHeaders_Enabled": true setting, or if the Docker file defined the ENV.

However when running the template in VS neither of these are set from what I experienced.

{
  "AzureAd": {
    "Instance": "https://login.microsoftonline.com/",
    "Domain": "qualified.domain.name",
    "TenantId": "22222222-2222-2222-2222-222222222222",
    "ClientId": "11111111-1111-1111-11111111111111111",
    "CallbackPath": "/signin-oidc"
  },
  "Logging": {
    "LogLevel": {
      "Default": "Information",
      "Microsoft": "Warning",
      "Microsoft.Hosting.Lifetime": "Information"
    }
  },
  "AllowedHosts": "*"
}

image

@bradygaster
Copy link
Member

bradygaster commented Sep 7, 2021

@Tratcher Might be good to put the setting into the template, but set to false - that way it could cue customers to the existence of the setting and potentially ease this process for other folks. false as in "yes the default" but just to give folks a cue that it is there. Optimally, we could create a more nuanced heuristic to use, that could wire things up in the best possible, pit-of-success sort of way based on what the customer has opted in on (identity, linux, etc.)

@Tratcher
Copy link
Member

Tratcher commented Sep 7, 2021

Where? It's set to true in the actual docker file when published. I wouldn't expect it in appsettings.json since that would conflict with the docker file, but maybe launchsettings.json?

@sebastienros
Copy link
Member

It's set to true in the actual docker file when published

Can you point me to the file? I tried with 3.1 templates.

@sebastienros
Copy link
Member

in the actual docker file

How is the file you shared related to the one that is generated with the template? In the templates the based image is the standard aspnet runtime one.

FROM mcr.microsoft.com/dotnet/aspnet:3.1 AS base

@Tratcher
Copy link
Member

Tratcher commented Sep 7, 2021

And that is probably the disconnect here. We fixed this issue primarily for azure app service linux hosting. I don't know if the same fix was applied to our base image, nor how those are maintained.

@sebastienros
Copy link
Member

I see, these images are used when we upload/publish a .NET app as is, not when a Docker option is selected in VS.
So this doesn't apply in this case and confirms the problem. IMO the simplest change would be to add the setting to the docker file template, and maybe a few others that the one you pointed at has. If not just for discoverability like Brady is saying.

@sebastienros
Copy link
Member

@vijayrkn Should I transfer this issue somewhere, or is there a bug in VS I can point to instead?

@vijayrkn
Copy link

vijayrkn commented Oct 7, 2021

There is an internal tracking bug on our side to set this app setting while publishing to App Service Container. This change is already implemented and should be available in the next update of Visual Studio. I think we can close this issue here.

@SeanKilleen
Copy link
Contributor Author

@vijayrkn that's great to hear! I know this was a tricky one.

If possible, I'd love to keep it open until the next release is available so that I can pull it and verify. Any concerns there?

@vijayrkn
Copy link

vijayrkn commented Oct 7, 2021

No concerns with me. We can keep it open until the validation is complete.

@adityamandaleeka adityamandaleeka added ✔️ Resolution: Fixed The bug or enhancement requested in this issue has been checked-in! and removed Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. labels Oct 19, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Dec 29, 2021
@amcasey amcasey added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares and removed area-runtime labels Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares ✔️ Resolution: Fixed The bug or enhancement requested in this issue has been checked-in!
Projects
None yet
Development

No branches or pull requests