Steeltoe Security Updates #1311

TimHess · 2024-06-03T20:44:14Z

Description

Split security packages up to align with Microsoft dependencies.
Add support for identity service bindings in CloudFoundry.ServiceBindings
Use new service bindings to directly configure the Microsoft security providers and reduce Steeltoe Security footprint
Drop support for using OAuth directly (for now... this can come back if it is still needed)
Drop Connectors.CloudFoundry

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 · 2024-06-11T12:59:23Z

Adding a reminder to consider adding custom host builders, based on the conversation at #1306 (comment).

- split auth types to separate packages to align with the Microsoft libraries they configure - delete Connectors.Abstractions and Connectors.CloudFoundry

bart-vmware

I've gone over roughly 1/3 of the changes. Great to see IServiceInfo and friends are finally gone!

shared-package.props

src/Common/src/Common.Security/CertificateConfigurationExtensions.cs

src/Security/src/Authorization.Certificate/ApplicationInstanceCertificate.cs

src/Security/src/Authorization.Certificate/CertificateApplicationBuilderExtensions.cs

Co-authored-by: Bart Koelman <[email protected]>

remove special handling for local uaa remove shared named http client code (Microsoft code can handle backchannel creation) remove globalusings

src/Security/src/Authentication.OpenIdConnect/TokenKeyResolver.cs

src/Security/src/Authentication.Shared/TokenKeyResolver.cs

src/Common/src/Common.Security/LocalCertificateWriter.cs

...Authentication.OpenIdConnect.Test/Steeltoe.Security.Authentication.OpenIdConnect.Test.csproj

shared-package.props

src/Security/src/Authentication.JwtBearer/PublicAPI.Shipped.txt

src/Security/test/Authentication.OpenIdConnect.Test/TokenKeyResolverTest.cs

src/Security/src/Authentication.OpenIdConnect/TokenKeyResolver.cs

src/Security/src/Authorization.Certificate/ApplicationInstanceCertificate.cs

src/Security/test/Authentication.JwtBearer.Test/TokenKeyResolverTest.cs

src/Security/test/Authorization.Certificate.Test/CertificateServiceCollectionExtensionsTest.cs

src/Security/test/Authorization.Certificate.Test/ClientCertificatesFixture.cs

src/Security/src/Authorization.Certificate/CertificateAuthorizationDefaults.cs

src/Common/test/Common.Certificate.Test/Steeltoe.Common.Certificate.Test.csproj

src/Bootstrap/src/AutoConfiguration/SteeltoeAssemblyNames.cs

Co-authored-by: Bart Koelman <[email protected]>

pluralize Common.Certificate enforce S3900 in code but not tests (remove unused classes)

bart-vmware

I like how the new extension methods are more focused now, thanks!

src/Security/src/Authentication.JwtBearer/Steeltoe.Security.Authentication.JwtBearer.csproj

...urity/src/Authentication.OpenIdConnect/Steeltoe.Security.Authentication.OpenIdConnect.csproj

src/Common/test/Common.Certificates.Test/Steeltoe.Common.Certificates.Test.csproj

src/Security/src/Authorization.Certificate/CertificateAuthorizationHandler.cs

src/Common/src/Common.Certificates/CertificateConfigurationExtensions.cs

src/Common/src/Common.Certificates/LocalCertificateWriter.cs

src/Common/src/Common.Net/WindowsNetworkFileShare.cs

src/Common/src/Common/Reflection/ReflectionHelpers.cs

src/Security/test/Authorization.Certificate.Test/CertificateAuthorizationTest.cs

Co-authored-by: Bart Koelman <[email protected]>

flags for tag/label processing, null checks, wording changes, test improvements

src/Common/src/Common.Net/WindowsNetworkFileShare.cs

src/Common/src/Common/Reflection/ReflectionHelpers.cs

src/Common/src/Common.Certificates/LocalCertificateWriter.cs

src/Security/test/Authentication.JwtBearer.Test/TokenKeyResolverTest.cs

src/Common/src/Common.Certificates/CertificateServiceCollectionExtensions.cs

src/Security/src/Authorization.Certificate/ApplicationInstanceCertificate.cs

src/Security/src/Authorization.Certificate/CertificateAuthorizationBuilderExtensions.cs

src/Bootstrap/src/AutoConfiguration/BootstrapScanner.cs

src/Security/src/Authorization.Certificate/CertificateAuthorizationPolicies.cs

Co-authored-by: Bart Koelman <[email protected]>

test for non-null feature but null session with trace observer, add a constance for AppInstanceIdentity

TimHess · 2024-06-17T20:26:29Z

Adding a reminder to consider adding custom host builders, based on the conversation at #1306 (comment).

I am not sure that it makes sense to do any host builder extensions here. Since we're working with authentication and authorization, middleware is involved, and we don't have a way to deal with middleware ordering.

I think the original context of this follow-up was along the lines of combining ConfigurationBuilder and ServiceCollection extensions, but that might only make sense for Steeltoe to use internally. The only ConfigurationBuilder extension we're exposing now is specific to the AppInstanceIdentity, and that requires setup of HttpClient (now possibly on user-named instances only) and/or Authorization.

src/Common/test/Common.TestResources/CancellationTokenSourceExtensions.cs

src/Common/test/Common.TestResources/IgnoreLineEndingsComparer.cs

src/Security/test/Authentication.JwtBearer.Test/JwtBearerAuthenticationBuilderExtensionsTest.cs

src/Security/test/Authentication.JwtBearer.Test/TokenKeyResolverTest.cs

src/Bootstrap/test/AutoConfiguration.Test/Steeltoe.Bootstrap.AutoConfiguration.Test.csproj

src/Security/test/Authentication.OpenIdConnect.Test/TokenKeyResolverTest.cs

src/Bootstrap/src/AutoConfiguration/BootstrapScanner.cs

src/Security/src/Authorization.Certificate/CertificateAuthorizationPolicies.cs

src/Security/src/Authorization.Certificate/CertificateAuthorizationBuilderExtensions.cs

src/Security/src/Authorization.Certificate/CertificateHttpClientBuilderExtensions.cs

Co-authored-by: Bart Koelman <[email protected]>

bart-vmware

LGTM.
Thanks, this PR is really a huge improvement!

TimHess added Type/enhancement New feature or request 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 3, 2024

TimHess added this to the 4.0.0-m1 milestone Jun 3, 2024

TimHess self-assigned this Jun 3, 2024

TimHess force-pushed the sso branch 2 times, most recently from 9bbeed0 to 2e3a2ff Compare June 7, 2024 21:28

TimHess mentioned this pull request Jun 10, 2024

Use named options for client certificates #1306

Merged

5 tasks

Base automatically changed from named_certificates to main June 11, 2024 17:36

depend more heavily on Microsoft libraries for auth

6e36ed1

- split auth types to separate packages to align with the Microsoft libraries they configure - delete Connectors.Abstractions and Connectors.CloudFoundry

TimHess force-pushed the sso branch from 2e3a2ff to 6e36ed1 Compare June 11, 2024 19:27

TimHess added 3 commits June 11, 2024 16:47

cleanup, more validation and tests

9b6c3b7

adjust tests to call Steeltoe configure methods before AddAuth

faab613

skip .NET 6 for app security component build

363aabd

TimHess force-pushed the sso branch from 4b92fb6 to 363aabd Compare June 11, 2024 22:53

TimHess added 2 commits June 11, 2024 18:00

use default HeaderConverter for X-Client-Cert processing

9a4b4f9

formatting

681cbcb

TimHess marked this pull request as ready for review June 11, 2024 23:24

bart-vmware requested changes Jun 12, 2024

View reviewed changes

TimHess and others added 3 commits June 12, 2024 08:24

Apply suggestions from code review

2154543

Co-authored-by: Bart Koelman <[email protected]>

Rename Steeltoe.Common.Security to Steeltoe.Common.Certificate

de03386

PR feedback, more simplification

9427171

remove special handling for local uaa remove shared named http client code (Microsoft code can handle backchannel creation) remove globalusings

TimHess force-pushed the sso branch from 44e3cb9 to 9427171 Compare June 12, 2024 21:31

bart-vmware reviewed Jun 13, 2024

View reviewed changes

TimHess and others added 2 commits June 13, 2024 08:27

Apply suggestions from code review

573d476

Co-authored-by: Bart Koelman <[email protected]>

PR feedback

3a81fa9

pluralize Common.Certificate enforce S3900 in code but not tests (remove unused classes)

bart-vmware reviewed Jun 14, 2024

View reviewed changes

Apply suggestions from code review

cad92ae

Co-authored-by: Bart Koelman <[email protected]>

TimHess added 4 commits June 14, 2024 06:41

small fixes

89b9184

reduce sso product name specificity

efd3d3b

certificate subject parsing method signature change

60e4ae5

PR suggestions, move OIDC and JWT extensions to AuthenticationBuilder

dc67946

flags for tag/label processing, null checks, wording changes, test improvements

TimHess requested a review from bart-vmware June 14, 2024 21:38

bart-vmware reviewed Jun 17, 2024

View reviewed changes

TimHess and others added 4 commits June 17, 2024 08:40

Apply suggestions from code review

23f72b7

Co-authored-by: Bart Koelman <[email protected]>

build fixes, revise s3900 again, disposable httpclient

862510f

drop certificates from autoconfiguration,

0231cdc

test for non-null feature but null session with trace observer, add a constance for AppInstanceIdentity

certificateName explicitly nullable, fix Security.slnf

5b0db2c

TimHess requested a review from bart-vmware June 17, 2024 20:26

TimHess added 3 commits June 17, 2024 15:35

rename test class

50454ee

revert more s3900

94ab5bf

cleanup

5cae4a5

bart-vmware reviewed Jun 18, 2024

View reviewed changes

TimHess and others added 5 commits June 18, 2024 08:26

Apply suggestions from code review

97e57f0

Co-authored-by: Bart Koelman <[email protected]>

pr feedback

34f04c6

style

208b1e3

update method description, don't run cert auth tests in parallel

5c8605e

add missing word

cd01711

bart-vmware approved these changes Jun 18, 2024

View reviewed changes

TimHess merged commit 20e0338 into main Jun 18, 2024
17 checks passed

TimHess deleted the sso branch June 18, 2024 16:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Steeltoe Security Updates #1311

Steeltoe Security Updates #1311

TimHess commented Jun 3, 2024 •

edited

Loading

bart-vmware commented Jun 11, 2024

bart-vmware left a comment

bart-vmware left a comment

TimHess commented Jun 17, 2024

bart-vmware left a comment

Steeltoe Security Updates #1311

Steeltoe Security Updates #1311

Conversation

TimHess commented Jun 3, 2024 • edited Loading

Description

Quality checklist

bart-vmware commented Jun 11, 2024

bart-vmware left a comment

Choose a reason for hiding this comment

bart-vmware left a comment

Choose a reason for hiding this comment

TimHess commented Jun 17, 2024

bart-vmware left a comment

Choose a reason for hiding this comment

TimHess commented Jun 3, 2024 •

edited

Loading