Skip to content

Commit aa10d51

Browse files
dbyron-sfdbyron-sf
committed
test(web): verify some error handling behavior of AuthConfig
1 parent e1be4c3 commit aa10d51

File tree

4 files changed

+197
-2
lines changed

4 files changed

+197
-2
lines changed

‎gate-basic/gate-basic.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
dependencies {
22
implementation project(":gate-core")
3+
implementation "io.spinnaker.kork:kork-annotations"
34
implementation "io.spinnaker.kork:kork-security"
45
implementation "org.springframework.session:spring-session-core"
56
}

‎gate-basic/src/main/java/com/netflix/spinnaker/gate/security/basic/BasicAuthConfig.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import com.netflix.spinnaker.gate.config.AuthConfig;
2020
import com.netflix.spinnaker.gate.security.SpinnakerAuthConfig;
21+
import com.netflix.spinnaker.kork.annotations.VisibleForTesting;
2122
import org.springframework.beans.factory.annotation.Autowired;
2223
import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression;
2324
import org.springframework.boot.autoconfigure.security.SecurityProperties;
@@ -35,11 +36,11 @@
3536
@EnableWebSecurity
3637
public class BasicAuthConfig extends WebSecurityConfigurerAdapter {
3738

38-
private final AuthConfig authConfig;
39+
@VisibleForTesting protected final AuthConfig authConfig;
3940

4041
private final BasicAuthProvider authProvider;
4142

42-
private final DefaultCookieSerializer defaultCookieSerializer;
43+
@VisibleForTesting protected final DefaultCookieSerializer defaultCookieSerializer;
4344

4445
@Autowired
4546
public BasicAuthConfig(

‎gate-core/src/main/java/com/netflix/spinnaker/gate/config/AuthConfig.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,16 @@ public void configure(HttpSecurity http) throws Exception {
7676
.authorizeRequests(
7777
registry -> {
7878
registry
79+
// https://github.com/spring-projects/spring-security/issues/11055#issuecomment-1098061598 suggests
80+
//
81+
// filterSecurityInterceptorOncePerRequest(false)
82+
//
83+
// until spring boot 3.0. Since
84+
//
85+
// .antMatchers("/error").permitAll()
86+
//
87+
// permits unauthorized access to /error, filterSecurityInterceptorOncePerRequest
88+
// isn't relevant.
7989
.antMatchers("/error")
8090
.permitAll()
8191
.antMatchers("/favicon.ico")

‎gate-web/src/test/java/com/netflix/spinnaker/gate/AuthConfigTest.java

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
/*
2+
* Copyright 2024 Salesforce, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.netflix.spinnaker.gate;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
20+
import com.fasterxml.jackson.databind.ObjectMapper;
21+
import com.netflix.spinnaker.gate.config.AuthConfig;
22+
import com.netflix.spinnaker.gate.health.DownstreamServicesHealthIndicator;
23+
import com.netflix.spinnaker.gate.security.basic.BasicAuthConfig;
24+
import com.netflix.spinnaker.gate.services.ApplicationService;
25+
import com.netflix.spinnaker.gate.services.DefaultProviderLookupService;
26+
import java.io.IOException;
27+
import java.util.Map;
28+
import javax.servlet.http.HttpServletResponse;
29+
import org.junit.jupiter.api.BeforeEach;
30+
import org.junit.jupiter.api.Test;
31+
import org.junit.jupiter.api.TestInfo;
32+
import org.springframework.beans.factory.annotation.Autowired;
33+
import org.springframework.boot.autoconfigure.security.SecurityProperties;
34+
import org.springframework.boot.test.context.SpringBootTest;
35+
import org.springframework.boot.test.mock.mockito.MockBean;
36+
import org.springframework.boot.test.web.client.TestRestTemplate;
37+
import org.springframework.context.annotation.Bean;
38+
import org.springframework.context.annotation.Configuration;
39+
import org.springframework.context.annotation.Primary;
40+
import org.springframework.core.ParameterizedTypeReference;
41+
import org.springframework.http.HttpMethod;
42+
import org.springframework.http.HttpStatus;
43+
import org.springframework.http.ResponseEntity;
44+
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
45+
import org.springframework.session.web.http.DefaultCookieSerializer;
46+
import org.springframework.test.context.TestPropertySource;
47+
import org.springframework.web.bind.annotation.GetMapping;
48+
import org.springframework.web.bind.annotation.RestController;
49+
50+
/** AuthConfig is in gate-core, but is about matching http requests, so use gate-web to test it. */
51+
@SpringBootTest(
52+
classes = {Main.class, AuthConfigTest.TestConfiguration.class},
53+
webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
54+
@TestPropertySource(
55+
properties = {
56+
"spring.config.location=classpath:gate-test.yml",
57+
"spring.security.user.name=testuser",
58+
"spring.security.user.password=testpassword",
59+
"security.basicform.enabled=true"
60+
})
61+
class AuthConfigTest {
62+
63+
private static final String TEST_USER = "testuser";
64+
65+
private static final String TEST_PASSWORD = "testpassword";
66+
67+
private static final ParameterizedTypeReference<Map<String, String>> mapType =
68+
new ParameterizedTypeReference<>() {};
69+
70+
@Autowired TestRestTemplate restTemplate;
71+
72+
@Autowired ObjectMapper objectMapper;
73+
74+
/** To prevent periodic calls to service's /health endpoints */
75+
@MockBean DownstreamServicesHealthIndicator downstreamServicesHealthIndicator;
76+
77+
/** to prevent period application loading */
78+
@MockBean ApplicationService applicationService;
79+
80+
/** To prevent attempts to load accounts */
81+
@MockBean DefaultProviderLookupService defaultProviderLookupService;
82+
83+
@BeforeEach
84+
void init(TestInfo testInfo) {
85+
System.out.println("--------------- Test " + testInfo.getDisplayName());
86+
}
87+
88+
@Test
89+
void forwardNoCredsRequiresAuth() {
90+
final ResponseEntity<Map<String, String>> response =
91+
restTemplate.exchange("/forward", HttpMethod.GET, null, mapType);
92+
93+
// Without .antMatchers("/error").permitAll() in AuthConfig, we'd expect to
94+
// get an empty error response since the request is unauthorized.
95+
// https://github.com/spring-projects/spring-boot/issues/26356 has details.
96+
97+
// Leave this test here in case someone gets the urge to restrict access to /error.
98+
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
99+
assertThat(response.getBody()).isNotNull();
100+
assertThat(response.getBody().get("timestamp")).isNotNull();
101+
assertThat(response.getBody().get("status"))
102+
.isEqualTo(String.valueOf(HttpStatus.UNAUTHORIZED.value()));
103+
assertThat(response.getBody().get("error"))
104+
.isEqualTo(HttpStatus.UNAUTHORIZED.getReasonPhrase());
105+
assertThat(response.getBody().get("message"))
106+
.isEqualTo(HttpStatus.UNAUTHORIZED.getReasonPhrase());
107+
}
108+
109+
@Test
110+
void forwardWrongCredsRequiresAuth() {
111+
final ResponseEntity<Map<String, String>> response =
112+
restTemplate
113+
.withBasicAuth(TEST_USER, "wrong" + TEST_PASSWORD)
114+
.exchange("/forward", HttpMethod.GET, null, mapType);
115+
116+
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
117+
assertThat(response.getBody()).isNotNull();
118+
assertThat(response.getBody().get("timestamp")).isNotNull();
119+
assertThat(response.getBody().get("status"))
120+
.isEqualTo(String.valueOf(HttpStatus.UNAUTHORIZED.value()));
121+
assertThat(response.getBody().get("error"))
122+
.isEqualTo(HttpStatus.UNAUTHORIZED.getReasonPhrase());
123+
assertThat(response.getBody().get("message"))
124+
.isEqualTo(HttpStatus.UNAUTHORIZED.getReasonPhrase());
125+
}
126+
127+
@Test
128+
void forwardWithCorrectCreds() {
129+
final ResponseEntity<Object> response =
130+
restTemplate
131+
.withBasicAuth(TEST_USER, TEST_PASSWORD)
132+
.exchange("/forward", HttpMethod.GET, null, Object.class);
133+
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.FOUND);
134+
assertThat(response.getHeaders().getLocation().getPath()).isEqualTo("/hello");
135+
assertThat(response.getBody()).isNull();
136+
}
137+
138+
static class TestAuthConfig extends BasicAuthConfig {
139+
public TestAuthConfig(
140+
AuthConfig authConfig,
141+
SecurityProperties securityProperties,
142+
DefaultCookieSerializer defaultCookieSerializer) {
143+
super(authConfig, securityProperties, defaultCookieSerializer);
144+
}
145+
146+
@Override
147+
protected void configure(HttpSecurity http) throws Exception {
148+
// This is the same as BasicAuthConfig except for
149+
//
150+
// authenticationEntryPoint(new LoginUrlAuthenticationEntryPoint("/login"));
151+
//
152+
// Leaving that out makes it easier to test some behavior of AuthConfig.
153+
defaultCookieSerializer.setSameSite(null);
154+
http.formLogin().and().httpBasic();
155+
authConfig.configure(http);
156+
}
157+
}
158+
159+
@Configuration
160+
static class TestConfiguration {
161+
@RestController
162+
public static class TestController {
163+
@GetMapping("/forward")
164+
public void forward(HttpServletResponse response) throws IOException {
165+
response.sendRedirect("/hello");
166+
}
167+
168+
@GetMapping("/hello")
169+
public String hello() {
170+
return "hello";
171+
}
172+
}
173+
174+
@Bean
175+
@Primary
176+
BasicAuthConfig basicAuthConfig(
177+
AuthConfig autoConfig,
178+
SecurityProperties securityProperties,
179+
DefaultCookieSerializer defaultCookieSerializer) {
180+
return new TestAuthConfig(autoConfig, securityProperties, defaultCookieSerializer);
181+
}
182+
}
183+
}

0 commit comments

Comments
 (0)