Skip to content

Account for ASP.NET Core changes around proxy header handling#1525

Merged
TimHess merged 11 commits into
mainfrom
unknown_reverse_proxy
Jun 27, 2025
Merged

Account for ASP.NET Core changes around proxy header handling#1525
TimHess merged 11 commits into
mainfrom
unknown_reverse_proxy

Conversation

@TimHess
Copy link
Copy Markdown
Member

@TimHess TimHess commented Jun 11, 2025 •
edited
Loading

Description

Provides a fix for #1524 that is automatically implemented when using the CloudFoundry configuration provider

Quality checklist

  • Your code complies with our Coding Style.
  • You've updated unit and/or integration tests for your change, where applicable.
  • You've updated documentation for your change, where applicable.
    If your change affects other repositories, such as Documentation, Samples and/or MainSite, add linked PRs here.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.
  • You've added required license files and/or file headers (explaining where the code came from with proper attribution), where code is copied from StackOverflow, a blog, or OSS.

bart-vmware
bart-vmware reviewed Jun 12, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@bart-vmware bart-vmware left a comment

Choose a reason for hiding this comment

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

Thanks for diving into this. I haven't tested the changes, assuming you did.

Comment thread src/Security/src/Authorization.Certificate/CertificateAuthorizationBuilderExtensions.cs Outdated
Comment thread src/Security/src/Authorization.Certificate/CertificateAuthorizationBuilderExtensions.cs Outdated
@bart-vmware bart-vmware added Component/Security Issues related to Steeltoe Security components (not app-sec) ReleaseLine/4.x Identified as a feature/fix for the 4.x release line labels Jun 12, 2025
@bart-vmware bart-vmware added this to the 4.0.0-rc1 milestone Jun 12, 2025
@TimHess
Copy link
Copy Markdown
Member Author

TimHess commented Jun 12, 2025

Thanks for diving into this. I haven't tested the changes, assuming you did.

Yes, but also because SteeltoeOSS/Samples#391 works with the same set of sample apps and I was hitting weird probably-environment issues during my first round of testing, I was already planning to leave this PR draft/open while wrapping up the other effort

@TimHess TimHess force-pushed the unknown_reverse_proxy branch 2 times, most recently from 9f1e858 to 020bf64 Compare June 12, 2025 19:35
@TimHess TimHess mentioned this pull request Jun 13, 2025
Closed
@TimHess TimHess force-pushed the unknown_reverse_proxy branch 2 times, most recently from 80a4e65 to e4ad593 Compare June 13, 2025 20:19
@TimHess TimHess marked this pull request as ready for review June 13, 2025 20:42
@TimHess TimHess requested a review from bart-vmware June 13, 2025 21:15
bart-vmware
bart-vmware reviewed Jun 16, 2025
View reviewed changes
Comment thread src/Common/src/Hosting/ServiceCollectionExtensions.cs Outdated
Comment thread src/Common/src/Hosting/ServiceCollectionExtensions.cs Outdated
Comment thread src/Common/src/Hosting/ServiceCollectionExtensions.cs Outdated
Comment thread src/Common/test/Hosting.Test/ServiceCollectionExtensionsTest.cs Outdated
bart-vmware
bart-vmware reviewed Jun 16, 2025
View reviewed changes
Comment thread src/Common/test/Hosting.Test/ServiceCollectionExtensionsTest.cs Outdated
bart-vmware
bart-vmware reviewed Jun 16, 2025
View reviewed changes
Comment thread src/Common/test/Hosting.Test/ServiceCollectionExtensionsTest.cs Outdated
@TimHess TimHess force-pushed the unknown_reverse_proxy branch from e4ad593 to 9c0da20 Compare June 18, 2025 18:55
@TimHess TimHess changed the title When present, add CF_INSTANCE_INTERNAL_IP to ASP.NET Core Known Proxies Account for ASP.NET Core changes around proxy header handling Jun 18, 2025
@TimHess TimHess self-assigned this Jun 18, 2025
@TimHess TimHess requested a review from bart-vmware June 18, 2025 21:06
bart-vmware
bart-vmware reviewed Jun 19, 2025
View reviewed changes
Comment thread src/Configuration/src/CloudFoundry/ForwardedHeadersSettings.cs Outdated
Comment thread src/Configuration/src/CloudFoundry/ForwardedHeadersSettings.cs Outdated
Comment thread src/Configuration/src/CloudFoundry/ForwardedHeadersSettings.cs Outdated
Comment thread src/Configuration/test/CloudFoundry.Test/CloudFoundryServiceCollectionExtensionsTest.cs Outdated
@TimHess TimHess force-pushed the unknown_reverse_proxy branch from 6060fe5 to 4c090b6 Compare June 24, 2025 21:39
TimHess and others added 6 commits June 24, 2025 16:46
@TimHess
Improve CF reverse proxy support
2114c7d 
- Move reverse proxy configuration to Common
- When present, add networks for CF_INSTANCE IP vars to KnownNetworks
- Parameter-less UseCertificateAuthorization now tries to retrieve ForwardedHeadersOptions from DI container before falling back on creating a new instance
@TimHess
PR feedback
58e48d0 
- Move new ServiceCollectionExtension to Configuration.CloudFoundry
- Use IConfigureOptions<ForwardedHeadersOptions>
- add missing using on ServiceProviders, true in BuildServiceProvider
- more consistent usage of EnvironmentVariableScope
- remove CF_INSTANCE var parsing option
@TimHess
evaluate KnownNetworks before TrustAll, more tests
a211bd4
@TimHess
suppress S4792
c2da963
@TimHess @bart-vmware
Update src/Configuration/src/CloudFoundry/ForwardedHeadersSettings.cs
0357f4b 
Co-authored-by: Bart Koelman <104792814+bart-vmware@users.noreply.github.com>
@TimHess
scrap custom code in favor of just setting FORWARDEDHEADERS_ENABLED
4ab97af
@TimHess TimHess force-pushed the unknown_reverse_proxy branch from 4c090b6 to 4ab97af Compare June 24, 2025 21:46
@TimHess TimHess requested a review from bart-vmware June 24, 2025 21:46
bart-vmware
bart-vmware reviewed Jun 25, 2025
View reviewed changes
Comment thread src/Configuration/test/CloudFoundry.Test/CloudfoundryConfigurationProviderTest.cs Outdated
Comment thread src/Security/src/Authorization.Certificate/CertificateApplicationBuilderExtensions.cs Outdated
Comment thread src/Security/src/Authorization.Certificate/CertificateApplicationBuilderExtensions.cs
Comment thread src/Configuration/src/CloudFoundry/CloudFoundryConfigurationProvider.cs
Comment thread src/Configuration/src/CloudFoundry/ForwardedHeadersSettings.cs Outdated
Comment thread src/Configuration/test/CloudFoundry.Test/CloudfoundryConfigurationProviderTest.cs Outdated
Comment thread src/Configuration/test/CloudFoundry.Test/CloudfoundryConfigurationProviderTest.cs Outdated
Comment thread src/Configuration/test/CloudFoundry.Test/CloudfoundryConfigurationProviderTest.cs Outdated
Comment thread src/Configuration/test/CloudFoundry.Test/CloudfoundryConfigurationProviderTest.cs Outdated
Comment thread src/Configuration/test/CloudFoundry.Test/CloudfoundryConfigurationProviderTest.cs Outdated
bart-vmware
bart-vmware reviewed Jun 26, 2025
View reviewed changes
Comment thread src/Configuration/test/CloudFoundry.Test/CloudfoundryConfigurationProviderTest.cs Outdated
Comment thread src/Configuration/src/CloudFoundry/CloudFoundryConfigurationProvider.cs
Comment thread src/Configuration/test/CloudFoundry.Test/CloudfoundryConfigurationProviderTest.cs Outdated
Comment thread src/Configuration/test/CloudFoundry.Test/CloudfoundryConfigurationProviderTest.cs Outdated
Comment thread src/Configuration/test/CloudFoundry.Test/CloudfoundryConfigurationProviderTest.cs Outdated
Comment thread src/Configuration/test/CloudFoundry.Test/CloudfoundryConfigurationProviderTest.cs Outdated
bart-vmware
bart-vmware reviewed Jun 26, 2025
View reviewed changes
Comment thread src/Configuration/test/CloudFoundry.Test/CloudfoundryConfigurationProviderTest.cs Outdated
bart-vmware
bart-vmware reviewed Jun 26, 2025
View reviewed changes
Comment thread src/Configuration/test/CloudFoundry.Test/CloudfoundryConfigurationProviderTest.cs Outdated
TimHess and others added 2 commits June 26, 2025 07:54
@TimHess @bart-vmware
Apply suggestions from code review
818f410 
Co-authored-by: Bart Koelman <104792814+bart-vmware@users.noreply.github.com>
@TimHess
HttpContext variable names
cc6b824
@TimHess TimHess requested a review from bart-vmware June 26, 2025 13:56
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jun 26, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@TimHess TimHess merged commit 4e31f14 into main Jun 27, 2025
23 checks passed
@TimHess TimHess deleted the unknown_reverse_proxy branch June 27, 2025 13:08
@TimHess TimHess mentioned this pull request Jul 1, 2025
Merged
@TimHess TimHess linked an issue Jul 3, 2025 that may be closed by this pull request
Change to .NET Runtime breaks reverse proxy configuration for auth libraries #1524
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bart-vmware bart-vmware bart-vmware approved these changes

Assignees

@TimHess TimHess

Labels

Component/Security Issues related to Steeltoe Security components (not app-sec) ReleaseLine/4.x Identified as a feature/fix for the 4.x release line

Projects

None yet

Milestone

4.0.0-rc1

Development

Successfully merging this pull request may close these issues.

Change to .NET Runtime breaks reverse proxy configuration for auth libraries

2 participants

@TimHess @bart-vmware