Op-21881:CVE-fixes with java-17 and spring-boot upgrade-v4.0 #466
Conversation
… along with CVE fixes-v4.0
… along with CVE fixes-v4.0
gate-core/src/main/groovy/com/netflix/spinnaker/gate/security/anonymous/AnonymousConfig.groovy
Outdated
Show resolved
Hide resolved
gate-core/src/main/groovy/com/netflix/spinnaker/gate/security/anonymous/AnonymousConfig.groovy
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
remove commented out code
also we have to fix the bean creation here properly.
There was a problem hiding this comment.
remove commented out code
also, we have to fix the bean creation here properly.
| @Bean | ||
| public AuthenticationManager authenticationManager( | ||
| AuthenticationConfiguration authConfig) throws Exception { | ||
| return authConfig.getAuthenticationManager(); |
There was a problem hiding this comment.
do we have getAuthenticationManager defined in AuthConfig? but why do we need to have the bean declaration then?
There was a problem hiding this comment.
No, it is not defined in AuthConfig
There was a problem hiding this comment.
than why it used here authConfig.getAuthenticationManager();
There was a problem hiding this comment.
will remove it
gate-ldap/src/main/groovy/com/netflix/spinnaker/gate/security/ldap/LdapSsoConfig.groovy
Show resolved
Hide resolved
| import org.springframework.security.oauth2.provider.authentication.OAuth2AuthenticationDetails; | ||
| import org.springframework.security.web.authentication.preauth.PreAuthenticatedAuthenticationToken; | ||
|
|
||
| public class BearerTokenExtractor { |
There was a problem hiding this comment.
Attach a java doc explaining where this is needed
| SamlAuthTokenUpdateFilter authTokenUpdateFilter = new SamlAuthTokenUpdateFilter() | ||
| http.addFilterAfter(authTokenUpdateFilter, | ||
| BasicAuthenticationFilter.class) | ||
| // saml.init(http) |
gate-saml/src/main/groovy/com/netflix/spinnaker/gate/security/saml/SamlSsoConfig.groovy
Show resolved
Hide resolved
|
|
||
| @Slf4j | ||
| @ControllerAdvice(basePackageClasses = [OpsmxSaporPolicyController.class, OpsmxAutopilotController.class, | ||
| OpsmxAuditClientServiceController.class, OpsmxDashboardController.class, OpsmxPlatformController.class, |
There was a problem hiding this comment.
why OpsmxDashboardController has been removed?
There was a problem hiding this comment.
will add back
There was a problem hiding this comment.
@emanipravallika overall good work, highly appreciated.
Did you test swagger after this upgrade?
Did you test saml config? what was the outcome?
What other auth configs you tested other than ldap?
There are a lot of places where cleanup is needed. I have pointed most of them but you too review them and remove.
| @@ -94,60 +96,54 @@ public void authorizeUserForApplicationId( | |||
|
|
|||
| log.debug("authorizing the endpoint : {}", endpointUrl); | |||
|
|
|||
| switch (method) { | |||
There was a problem hiding this comment.
why the switch has been replaced by if-else?
There was a problem hiding this comment.
if switch case is used - encountered this error{Incompatible types. Found: 'org.springframework.http.HttpMethod', required: 'char, byte, short, int, Character, Byte, Short, Integer, String, or an enum'}, SO, I replaced with if-else
| @@ -182,64 +178,59 @@ public void authorizeUserForServiceId(String username, String endpointUrl, Strin | |||
|
|
|||
| log.info("authorizing the endpoint for service Id : {}", endpointUrl); | |||
|
|
|||
| switch (method) { | |||
There was a problem hiding this comment.
if switch case is used - encountered this error{Incompatible types. Found: 'org.springframework.http.HttpMethod', required: 'char, byte, short, int, Character, Byte, Short, Integer, String, or an enum'}, SO, I replaced with if-else
| @@ -13,6 +13,8 @@ | |||
| * See the License for the specific language governing permissions and | |||
| * limitations under the License. | |||
| */ | |||
| //file:noinspection GroovyAssignabilityCheck | |||
| //file:noinspection GroovyAccessibility | |||
| org.gradle.parallel=true | ||
| spinnakerGradleVersion=8.25.0 | ||
| targetJava11=true | ||
| spinnakerGradleVersion=1-0-SNAPSHOT |
There was a problem hiding this comment.
from where did we pick these versions?
There was a problem hiding this comment.
From spinnaker gate-oes
gradle.properties
Outdated
| spinnakerGradleVersion=8.25.0 | ||
| targetJava11=true | ||
| spinnakerGradleVersion=1-0-SNAPSHOT | ||
| #targetJava11=true |
| @@ -21,3 +21,4 @@ targetJava11=true | |||
| # | |||
| #fiatComposite=true | |||
| #korkComposite=true | |||
| org.gradle.jvmargs=-Xmx6g -Xms6g | |||
There was a problem hiding this comment.
why are we passing JVM args in props? do these shouldn't be part of the run command? how are we sure about the values like 6g?
(https://devopsmx.atlassian.net/browse/OP-21881)