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 functions-dotnet-dependency-injection.md #108984

Closed
wants to merge 2 commits into from

Conversation

brettsam
Copy link
Contributor

@brettsam brettsam commented May 2, 2023

Adding note about ASP.NET authentication

We've had support issues with this: Azure/azure-functions-host#6805. This is not likely to be something we add support for in-proc as we're moving towards dotnet-isolated in the future. We should start calling this out to help prevent support issues.

Adding note about ASP.NET authentication
@prmerger-automator
Copy link
Contributor

@brettsam : Thanks for your contribution! The author(s) have been notified to review your proposed change.

@brettsam
Copy link
Contributor Author

brettsam commented May 2, 2023

@fabiocav / @mattchenderson -- would like your eyes on this as well

Adding note to troubleshooting topic...
@learn-build-service-prod
Copy link
Contributor

Learn Build status updates of commit 155fdfe:

✅ Validation status: passed

File Status Preview URL Details
articles/azure-functions/functions-dotnet-dependency-injection.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

@learn-build-service-prod
Copy link
Contributor

Learn Build status updates of commit 9fd0aa5:

✅ Validation status: passed

File Status Preview URL Details
articles/azure-functions/functions-dotnet-dependency-injection.md ✅Succeeded
articles/azure-functions/functions-recover-storage-account.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

@Court72
Copy link
Contributor

Court72 commented May 2, 2023

@ggailey777

Can you review the proposed changes?

When the changes are ready for publication, add a #sign-off comment to signal that the PR is ready for the review team to merge.

#label:"aq-pr-triaged"
@MicrosoftDocs/public-repo-pr-review-team

@prmerger-automator prmerger-automator bot added the aq-pr-triaged tracking label for the PR review team label May 2, 2023
Comment on lines +124 to +126
### ASP.NET authentication overrides

Configuring ASP.NET authentication in a Functions startup class can override services that are required for the Azure portal to communicate with the host. This includes, but is not limited to, any calls to `AddAuthentication()`. If the host's authentication services are overridden and the portal cannot communicate with the host, it will consider the app unreachable. This may result in errors such as `No authentication handler is registered for the scheme 'ArmToken'.`.
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be qualified as specifically a concern for .NET (in process only?). as this article is language agnostic.

@@ -71,6 +71,8 @@ A series of registration steps run before and after the runtime processes the st

- *The dependency injection container only holds explicitly registered types*. The only services available as injectable types are what are setup in the `Configure` method. As a result, Functions-specific types like `BindingContext` and `ExecutionContext` aren't available during setup or as injectable types.

- *Configuring ASP.NET authentication is not supported*. The Functions host configures ASP.NET authentication services in order to properly expose APIs to infrastucture services, such as the Azure portal. Additional configuration in a custom Startup class can override this configuration, causing unintended consequences. For example, calling `builder.Services.AddAuthentication()` can break authentication between the portal and the host, leading to messages such as "Azure Functions runtime is unreachable".
Copy link
Contributor

Choose a reason for hiding this comment

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

The consequences here are broader than just portal scenarios, right? Any non-HttpTrigger endpoint would potentially be in scope, including extension endpoints, etc., right? Regardless, we might reframe that more as a specific example.

Suggested change
- *Configuring ASP.NET authentication is not supported*. The Functions host configures ASP.NET authentication services in order to properly expose APIs to infrastucture services, such as the Azure portal. Additional configuration in a custom Startup class can override this configuration, causing unintended consequences. For example, calling `builder.Services.AddAuthentication()` can break authentication between the portal and the host, leading to messages such as "Azure Functions runtime is unreachable".
- *Configuring ASP.NET authentication is not supported*. The Functions host configures ASP.NET authentication services in order to properly expose APIs for core lifecycle operations. Additional configuration in a custom Startup class can override this configuration, causing unintended consequences. For example, calling `builder.Services.AddAuthentication()` can break authentication between the portal and the host, leading to messages such as "Azure Functions runtime is unreachable".

@American-Dipper
Copy link
Contributor

emailed GG

@American-Dipper
Copy link
Contributor

emailed author

@ggailey777
Copy link
Contributor

@mattchenderson can you just grab BrettSam's commit into your fork with the other DI work? Sadly, he doesn't often return to these github issues (and you know where to find him 😉).

@denrea
Copy link
Contributor

denrea commented Nov 10, 2023

sent reminder to ggailey777

@American-Dipper
Copy link
Contributor

@mattchenderson - Can you address this issue that's addressed to you?

Or, @ggailey777 , should this again PR be closed?

Thanks, jeff review team

@Stacyrch140
Copy link
Contributor

@mattchenderson - Can you address #108984 (comment) that's addressed to you?

Alternatively, should this PR be closed, @ggailey777?

@ggailey777
Copy link
Contributor

ggailey777 commented Feb 5, 2024

Thanks @Stacyrch140 for the reminder. I do have the commits in my own private PR now so #please-close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants