search

SAP / cloud-security-services-integration-library

Integration libraries and samples for authenticating users and clients bound to XSUAA authentication and authorization service or Identity authentication service.
Apache License 2.0
151 stars 135 forks source link

Improve the custom SecurityContext Strategy registration #1526

Closed beckermarc closed 6 months ago

beckermarc commented 6 months ago

The library now registers a security context strategy by default. In CAP Java we ran into situations where this registration happened too late. The effect of this was, that SecurityConfigs used an obsolete SecurityContextStrategy (which stored by Spring Security as a member variable in the SecurityConfig) and the library set a new strategy in the static holder class. As the SecurityConfigs stored the authentication objects in the obsolete strategy, CAP wasn't able anymore to get the current authentication from the static holder class.

This PR tries to avoid this situation, by ensuring that the auto-configuration has a high precedence and is run early in the application initialization. Additional this PR avoids reflection usage to set the strategy, as reflection is not really required.

beckermarc commented 6 months ago

@liga-oz Please have a look at this PR, thanks! It is currently breaking some scenarios of CAP Java when using XSUAA 3.4.2 with the now by default enabled security context strategy.

beckermarc commented 6 months ago

Do not merge yet, still found some scenarios where this way of initialization creates problems.

beckermarc commented 6 months ago

I found an approach that now works in all scenarios and makes sure the strategy is registered early enough. It is a little ugly, as it integrates with the web servlet initialization, which is slightly unrelated (although not totally).

Alternatively I also made another proposal to set the strategy based on an environment post processor. However this is slightly "incompatible" as it changes the way in which the configuration can be disabled (only properties, no auto config exclusion anymore): https://github.com/beckermarc/cloud-security-services-integration-library/pull/1

@liga-oz Let me know which approach you are preferring or if you see any problems with any of these suggestions. We can also have a sync, if you would like to discuss further details.

beckermarc commented 5 months ago

I actually noticed that this approach apparently seems to fail in MockMvc scenarios and runs too late there. This means that in those cases the registration happens too late again, breaking CAPs security configuration. Unfortunately I didn't seem to that that scenario thoroughly.

I reopened my alternative approach, that really makes sure to run this as early as possible, with the above mentioned limitations: No auto config exclusion any more, but only disabling through properties possible.

https://github.com/SAP/cloud-security-services-integration-library/pull/1536

@liga-oz Can you take another look at this please?