Initial security migration #5901
Conversation
Separate dataflow and skipper configs instead of trying to "reuse" common configs. Skipper config did extend classes from comman but then override everything. Remove unused CF configs related to tweaking things for "space developer" as this was never used. Essentially minimal changes to get new security configuration model working. There's still some deprecations left to handle but it's better to get better integration tests before those are handled.
...ig-web/src/main/java/org/springframework/cloud/common/security/OAuthClientConfiguration.java
Outdated
Show resolved
Hide resolved
...ig-web/src/main/java/org/springframework/cloud/common/security/OAuthClientConfiguration.java
Show resolved
Hide resolved
| @Autowired | ||
| protected AuthorizationProperties authorizationProperties; | ||
|
|
||
| @Autowired | ||
| protected OAuth2ResourceServerProperties oAuth2ResourceServerProperties; | ||
|
|
||
| @Autowired(required = false) | ||
| protected OpaqueTokenIntrospector opaqueTokenIntrospector; |
There was a problem hiding this comment.
Does this need to be Autowired since it has an autowired setter at line 208?
There was a problem hiding this comment.
Actually I think I forget to remove this OAuthSecurityConfiguration class as DataflowOAuthSecurityConfiguration used to extend it which is not a case anymore so it became dead code.
...web/src/main/java/org/springframework/cloud/common/security/support/SecurityConfigUtils.java
Outdated
Show resolved
Hide resolved
...-web/src/main/java/org/springframework/cloud/common/security/OAuthSecurityConfiguration.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Thanks @jvalkeal for tackling this and improving it w/ the refactoring. I have a handful of comments, suggestions, and questions. Once these are addressed/answered I will probably do one more pass through - trying to keep it high level on this 1st pass. Nice work!
|
|
||
| @Configuration(proxyBeanMethods = false) | ||
| @AutoConfigureBefore({ | ||
| SecurityAutoConfiguration.class, | ||
| ManagementWebSecurityAutoConfiguration.class, | ||
| OAuth2ClientAutoConfiguration.class, | ||
| OAuth2ResourceServerAutoConfiguration.class}) | ||
| @Import({IgnoreAllSecurityConfiguration.class, OAuthSecurityConfiguration.class}) |
There was a problem hiding this comment.
It appears that CommonSecurityAutoConfiguration no longer has any purpose other than enforcing @AutoConfigureBefore. Previously, it would import OAuthSecurityConfiguration. My concern is that other modules may be expecting to get this OAuth security auto-configured when they declare a dependency on spring-cloud-common-security-config-web. One example is CTR.
There was a problem hiding this comment.
Good catch. Looking comment in IgnoreAllSecurityConfiguration
The org.springframework.cloud.common.security.enabled=true property disables this configuration...
Gives a feeling that something got tangled and "shared" stuff didn't work as planned and yet another "enabled" option was added.
It may be better to remove this auto-config, see what breaks i.e. in ctr and then bring something back in a better form.
There was a problem hiding this comment.
Created #5926 to track this
...-web/src/main/java/org/springframework/cloud/common/security/OAuthSecurityConfiguration.java
Show resolved
Hide resolved
...ig-web/src/main/java/org/springframework/cloud/common/security/OAuthClientConfiguration.java
Show resolved
Hide resolved
.../springframework/cloud/skipper/server/config/security/SkipperOAuthSecurityConfiguration.java
Show resolved
Hide resolved
.../springframework/cloud/skipper/server/config/security/SkipperOAuthSecurityConfiguration.java
Outdated
Show resolved
Hide resolved
.../springframework/cloud/skipper/server/config/security/SkipperOAuthSecurityConfiguration.java
Outdated
Show resolved
Hide resolved
...ava/org/springframework/cloud/dataflow/server/config/DataflowOAuthSecurityConfiguration.java
Show resolved
Hide resolved
...src/main/java/org/springframework/cloud/common/security/CommonSecurityAutoConfiguration.java
Show resolved
Hide resolved
Separate dataflow and skipper configs instead of trying to "reuse" common configs. Skipper config did extend classes from comman but then override everything.
Remove unused CF configs related to tweaking things for "space developer" as this was never used.
Essentially minimal changes to get new security
configuration model working. There's still some
deprecations left to handle but it's better to
get better integration tests before those
are handled.