Skip to content

Commit e5383da

Browse files
fix(auth): re-add proxy-stripped /api prefix to browser-facing OAuth URLs (#1330)
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent b451268 commit e5383da

13 files changed

Lines changed: 285 additions & 53 deletions

server/src/main/java/de/tum/cit/aet/hephaestus/core/auth/AuthProperties.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,16 @@
1515
* integration tests override via {@code @TestPropertySource}.
1616
*
1717
* @param issuer Canonical issuer URI; populates the {@code iss} claim.
18+
* @param apiBasePath Public path prefix the reverse proxy strips before requests reach this app
19+
* (e.g. {@code /api} when Traefik strips {@code /api}); empty when the app is
20+
* served at the origin root (local dev). Prepended when building the absolute,
21+
* browser-reachable OAuth URLs — the authorization-request {@code redirect_uri}
22+
* and the {@code /oauth2/authorization} init redirect — so the IdP and the
23+
* callback land back on the proxied API path, not the SPA. It cannot be inferred
24+
* from the request: prod runs {@code forward-headers-strategy: native} (Tomcat
25+
* {@code RemoteIpValve}, kept for the pre-auth IP rate-limit trust model — see
26+
* {@code ProxyTrustGuard}), which restores forwarded host/proto but NOT
27+
* {@code X-Forwarded-Prefix}. Normalized to leading-slash / no-trailing-slash.
1828
* @param audience Default {@code aud} claim for SPA cookies.
1929
* @param accessTtl Cookie-JWT lifetime.
2030
* @param cookieName Access-token cookie name (the {@code __Host-} prefix is
@@ -69,6 +79,7 @@
6979
@ConfigurationProperties(prefix = "hephaestus.auth")
7080
public record AuthProperties(
7181
@DefaultValue("http://localhost:8080") URI issuer,
82+
@DefaultValue("") String apiBasePath,
7283
@DefaultValue("hephaestus-spa") String audience,
7384
@DefaultValue("15m") Duration accessTtl,
7485
@DefaultValue(DEFAULT_COOKIE_NAME) String cookieName,
@@ -86,6 +97,23 @@ public record AuthProperties(
8697
*/
8798
public AuthProperties {
8899
loginProviders = loginProviders == null ? Map.of() : loginProviders;
100+
apiBasePath = normalizeApiBasePath(apiBasePath);
101+
}
102+
103+
/**
104+
* Coerce {@code apiBasePath} to the leading-slash / no-trailing-slash form the OAuth-URL builders
105+
* concatenate, so {@code api}, {@code /api} and {@code /api/} are equivalent and {@code /} or blank
106+
* mean root — a misconfigured value can't silently produce {@code hostapi/…} or a double slash.
107+
*/
108+
private static String normalizeApiBasePath(String value) {
109+
if (value == null) {
110+
return "";
111+
}
112+
String trimmed = value.trim().replaceAll("/+$", "");
113+
if (trimmed.isEmpty()) {
114+
return "";
115+
}
116+
return trimmed.startsWith("/") ? trimmed : "/" + trimmed;
89117
}
90118

91119
/**

server/src/main/java/de/tum/cit/aet/hephaestus/core/auth/oauth/AuthBeginController.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,21 @@ public class AuthBeginController {
3939
private final de.tum.cit.aet.hephaestus.core.auth.jwt.CookieBearerTokenResolver bearerTokenResolver;
4040
private final org.springframework.security.oauth2.jwt.JwtDecoder jwtDecoder;
4141

42+
/** Proxy-stripped API prefix re-added to the init redirect — see {@code AuthProperties#apiBasePath}. */
43+
private final String apiBasePath;
44+
4245
public AuthBeginController(
4346
ClientRegistrationRepository clientRegistrationRepository,
4447
AuthIntentCookie authIntentCookie,
4548
de.tum.cit.aet.hephaestus.core.auth.jwt.CookieBearerTokenResolver bearerTokenResolver,
46-
de.tum.cit.aet.hephaestus.core.auth.jwt.RevocationAwareJwtDecoder jwtDecoder
49+
de.tum.cit.aet.hephaestus.core.auth.jwt.RevocationAwareJwtDecoder jwtDecoder,
50+
de.tum.cit.aet.hephaestus.core.auth.AuthProperties authProperties
4751
) {
4852
this.clientRegistrationRepository = clientRegistrationRepository;
4953
this.authIntentCookie = authIntentCookie;
5054
this.bearerTokenResolver = bearerTokenResolver;
5155
this.jwtDecoder = jwtDecoder;
56+
this.apiBasePath = authProperties.apiBasePath();
5257
}
5358

5459
@GetMapping("/login")
@@ -89,8 +94,10 @@ public RedirectView begin(
8994
authIntentCookie.write(response, intent);
9095
// 302 to Spring's standard initiation endpoint; the OAuth2AuthorizationRequestRedirectFilter
9196
// takes over from here, building the upstream redirect with state + PKCE (see AuthSecurityConfig).
97+
// apiBasePath re-adds the proxy-stripped prefix so the browser lands on the proxied init endpoint,
98+
// not the SPA. (The /auth/error targets above are SPA routes at the origin root, so they keep none.)
9299
String urlEncodedRegistration = URLEncoder.encode(registrationId, StandardCharsets.UTF_8);
93-
return new RedirectView(OAUTH_INIT_PATH + urlEncodedRegistration, false);
100+
return new RedirectView(apiBasePath + OAUTH_INIT_PATH + urlEncodedRegistration, false);
94101
}
95102

96103
/**

server/src/main/java/de/tum/cit/aet/hephaestus/core/auth/provider/LoginProviderClientRegistrationRepository.java

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,26 @@ public class LoginProviderClientRegistrationRepository
2424
implements ClientRegistrationRepository, Iterable<ClientRegistration>, IdentityProviderCatalog
2525
{
2626

27-
private static final String CALLBACK_TEMPLATE = "{baseUrl}/login/oauth2/code/{registrationId}";
28-
2927
private final LoginProviderRepository loginProviderRepository;
28+
29+
/**
30+
* {@code redirect_uri} template. {@code {baseUrl}} expands per request to the public origin (scheme
31+
* + host, restored by native forward-headers); {@code apiBasePath} re-adds the proxy-stripped prefix
32+
* so the IdP redirects back to the proxied API path, not the SPA — see {@code AuthProperties#apiBasePath}.
33+
*/
34+
private final String callbackTemplate;
35+
3036
private final Cache<String, ClientRegistration> cache = Caffeine.newBuilder()
3137
.expireAfterWrite(Duration.ofSeconds(60))
3238
.maximumSize(256)
3339
.build();
3440

35-
public LoginProviderClientRegistrationRepository(LoginProviderRepository loginProviderRepository) {
41+
public LoginProviderClientRegistrationRepository(
42+
LoginProviderRepository loginProviderRepository,
43+
String apiBasePath
44+
) {
3645
this.loginProviderRepository = loginProviderRepository;
46+
this.callbackTemplate = "{baseUrl}" + apiBasePath + "/login/oauth2/code/{registrationId}";
3747
}
3848

3949
@Override
@@ -79,7 +89,7 @@ private ClientRegistration toRegistration(LoginProvider provider) {
7989
.clientSecret(provider.getClientSecret())
8090
.clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC)
8191
.authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE)
82-
.redirectUri(CALLBACK_TEMPLATE)
92+
.redirectUri(callbackTemplate)
8393
.scope(provider.getScopes().trim().split("\\s+"))
8494
.userNameAttributeName("id")
8595
.clientName(provider.getDisplayName());

server/src/main/java/de/tum/cit/aet/hephaestus/core/auth/provider/LoginProviderConfiguration.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package de.tum.cit.aet.hephaestus.core.auth.provider;
22

3+
import de.tum.cit.aet.hephaestus.core.auth.AuthProperties;
34
import org.springframework.context.annotation.Bean;
45
import org.springframework.context.annotation.Configuration;
56

@@ -14,8 +15,9 @@ public class LoginProviderConfiguration {
1415

1516
@Bean
1617
public LoginProviderClientRegistrationRepository loginProviderClientRegistrationRepository(
17-
LoginProviderRepository loginProviderRepository
18+
LoginProviderRepository loginProviderRepository,
19+
AuthProperties authProperties
1820
) {
19-
return new LoginProviderClientRegistrationRepository(loginProviderRepository);
21+
return new LoginProviderClientRegistrationRepository(loginProviderRepository, authProperties.apiBasePath());
2022
}
2123
}

server/src/main/java/de/tum/cit/aet/hephaestus/core/auth/web/LoginProviderAdminController.java

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,30 @@ public class LoginProviderAdminController {
4343

4444
private final LoginProviderService loginProviderService;
4545

46-
public LoginProviderAdminController(LoginProviderService loginProviderService) {
46+
/** Proxy-stripped API prefix re-added to the displayed callback URL — see {@code AuthProperties#apiBasePath}. */
47+
private final String apiBasePath;
48+
49+
public LoginProviderAdminController(
50+
LoginProviderService loginProviderService,
51+
de.tum.cit.aet.hephaestus.core.auth.AuthProperties authProperties
52+
) {
4753
this.loginProviderService = loginProviderService;
54+
this.apiBasePath = authProperties.apiBasePath();
55+
}
56+
57+
/**
58+
* Public callback base the admin registers on the upstream OAuth app: the request origin (scheme +
59+
* host, restored by native forward-headers) plus the proxy-stripped API prefix. The per-provider
60+
* {@code /login/oauth2/code/{id}} segment is appended in {@link #toView}.
61+
*/
62+
private String callbackBase() {
63+
return ServletUriComponentsBuilder.fromCurrentContextPath().build().toUriString() + apiBasePath;
4864
}
4965

5066
@GetMapping
5167
@Operation(summary = "List login providers", operationId = "adminListLoginProviders")
5268
public ResponseEntity<List<LoginProviderViewDTO>> list() {
53-
String callbackBase = ServletUriComponentsBuilder.fromCurrentContextPath().build().toUriString();
69+
String callbackBase = callbackBase();
5470
return ResponseEntity.ok(
5571
loginProviderService
5672
.listAll()
@@ -75,10 +91,7 @@ public ResponseEntity<LoginProviderViewDTO> create(@Valid @RequestBody CreateLog
7591
body.scopes()
7692
)
7793
);
78-
LoginProviderViewDTO view = toView(
79-
created,
80-
ServletUriComponentsBuilder.fromCurrentContextPath().build().toUriString()
81-
);
94+
LoginProviderViewDTO view = toView(created, callbackBase());
8295
URI location = ServletUriComponentsBuilder.fromCurrentRequest()
8396
.path("/{registrationId}")
8497
.buildAndExpand(created.getRegistrationId())
@@ -103,9 +116,7 @@ public ResponseEntity<LoginProviderViewDTO> update(
103116
body.enabled()
104117
)
105118
);
106-
return ResponseEntity.ok(
107-
toView(updated, ServletUriComponentsBuilder.fromCurrentContextPath().build().toUriString())
108-
);
119+
return ResponseEntity.ok(toView(updated, callbackBase()));
109120
}
110121

111122
@DeleteMapping("/{registrationId}")

server/src/main/resources/application-prod.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,9 @@ hephaestus:
125125
auth:
126126
issuer: ${HEPHAESTUS_AUTH_ISSUER}
127127
state-cookie-key: ${HEPHAESTUS_AUTH_STATE_COOKIE_KEY}
128+
# Traefik strips /api before the request reaches the app (see docker/compose.app.yaml), and
129+
# native forward-headers can't restore the prefix — re-add it to the browser-facing OAuth URLs.
130+
api-base-path: /api
128131

129132
# ═══════════════════════════════════════════════════════════════════════════
130133
# GITLAB INTEGRATION (Production)
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package de.tum.cit.aet.hephaestus.core.auth;
2+
3+
import java.net.URI;
4+
import java.time.Duration;
5+
import java.util.List;
6+
import java.util.Map;
7+
8+
/**
9+
* Shared {@link AuthProperties} builder for unit tests, so the 12-arg record construction lives in one
10+
* place. Values are the production defaults; vary only the component under test.
11+
*/
12+
public final class AuthPropertiesFixture {
13+
14+
private AuthPropertiesFixture() {}
15+
16+
/** Production defaults (no proxy prefix, no seeded providers). */
17+
public static AuthProperties defaults() {
18+
return build("", Map.of());
19+
}
20+
21+
/** Defaults with the given {@code apiBasePath} (the constructor normalizes it). */
22+
public static AuthProperties withApiBasePath(String apiBasePath) {
23+
return build(apiBasePath, Map.of());
24+
}
25+
26+
/** Defaults with the given seeded login providers. */
27+
public static AuthProperties withLoginProviders(Map<String, AuthProperties.LoginProviderSeed> loginProviders) {
28+
return build("", loginProviders);
29+
}
30+
31+
private static AuthProperties build(String apiBasePath, Map<String, AuthProperties.LoginProviderSeed> providers) {
32+
return new AuthProperties(
33+
URI.create("http://localhost:8080"),
34+
apiBasePath,
35+
"hephaestus-spa",
36+
Duration.ofMinutes(15),
37+
AuthProperties.DEFAULT_COOKIE_NAME,
38+
"",
39+
Duration.ofHours(48),
40+
providers,
41+
List.of(),
42+
"",
43+
Duration.ofHours(1),
44+
Duration.ofHours(12)
45+
);
46+
}
47+
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package de.tum.cit.aet.hephaestus.core.auth;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
5+
import de.tum.cit.aet.hephaestus.testconfig.BaseUnitTest;
6+
import org.junit.jupiter.params.ParameterizedTest;
7+
import org.junit.jupiter.params.provider.CsvSource;
8+
9+
/**
10+
* {@code apiBasePath} feeds string-concatenated OAuth URLs ({@code {baseUrl}} + path + callback), so a
11+
* stray missing/duplicated slash silently breaks login. Pin the constructor normalization that makes
12+
* {@code api}, {@code /api} and {@code /api/} equivalent and collapses blank/{@code /} to root.
13+
*/
14+
class AuthPropertiesTest extends BaseUnitTest {
15+
16+
@ParameterizedTest
17+
@CsvSource(
18+
value = {
19+
"/api | /api",
20+
"api | /api",
21+
"/api/ | /api",
22+
"/api/v2/ | /api/v2",
23+
"'' | ''",
24+
"/ | ''",
25+
"' /api/ ' | /api",
26+
},
27+
delimiterString = "|",
28+
emptyValue = ""
29+
)
30+
void apiBasePath_isNormalizedToLeadingSlashNoTrailingSlash(String raw, String expected) {
31+
assertThat(AuthPropertiesFixture.withApiBasePath(raw).apiBasePath()).isEqualTo(expected);
32+
}
33+
}

server/src/test/java/de/tum/cit/aet/hephaestus/core/auth/jwt/CookieBearerTokenResolverTest.java

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,9 @@
33
import static org.assertj.core.api.Assertions.assertThat;
44

55
import de.tum.cit.aet.hephaestus.core.auth.AuthProperties;
6+
import de.tum.cit.aet.hephaestus.core.auth.AuthPropertiesFixture;
67
import de.tum.cit.aet.hephaestus.testconfig.BaseUnitTest;
78
import jakarta.servlet.http.Cookie;
8-
import java.net.URI;
9-
import java.time.Duration;
109
import org.junit.jupiter.api.BeforeEach;
1110
import org.junit.jupiter.api.Test;
1211
import org.springframework.mock.web.MockHttpServletRequest;
@@ -20,28 +19,15 @@
2019
*/
2120
class CookieBearerTokenResolverTest extends BaseUnitTest {
2221

23-
private static final String COOKIE_NAME = "__Host-HEPHAESTUS_AT";
22+
private static final String COOKIE_NAME = AuthProperties.DEFAULT_COOKIE_NAME;
2423
private static final String COOKIE_TOKEN = "cookie-token-trusted";
2524
private static final String HEADER_TOKEN = "attacker-token-hostile";
2625

2726
private CookieBearerTokenResolver resolver;
2827

2928
@BeforeEach
3029
void setUp() {
31-
AuthProperties properties = new AuthProperties(
32-
URI.create("http://localhost:8080"),
33-
"hephaestus-spa",
34-
Duration.ofMinutes(15),
35-
COOKIE_NAME,
36-
"",
37-
Duration.ofHours(48),
38-
java.util.Map.of(),
39-
java.util.List.of(),
40-
"",
41-
Duration.ofHours(1),
42-
Duration.ofHours(12)
43-
);
44-
resolver = new CookieBearerTokenResolver(properties);
30+
resolver = new CookieBearerTokenResolver(AuthPropertiesFixture.defaults());
4531
}
4632

4733
@Test

server/src/test/java/de/tum/cit/aet/hephaestus/core/auth/oauth/AuthBeginControllerLinkTest.java

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import static org.mockito.Mockito.verifyNoInteractions;
77
import static org.mockito.Mockito.when;
88

9+
import de.tum.cit.aet.hephaestus.core.auth.AuthPropertiesFixture;
910
import de.tum.cit.aet.hephaestus.core.auth.jwt.CookieBearerTokenResolver;
1011
import de.tum.cit.aet.hephaestus.core.auth.jwt.RevocationAwareJwtDecoder;
1112
import de.tum.cit.aet.hephaestus.testconfig.BaseUnitTest;
@@ -28,21 +29,32 @@
2829
*/
2930
class AuthBeginControllerLinkTest extends BaseUnitTest {
3031

32+
private ClientRegistrationRepository registrations;
3133
private CookieBearerTokenResolver bearerTokenResolver;
3234
private RevocationAwareJwtDecoder jwtDecoder;
3335
private AuthIntentCookie authIntentCookie;
3436
private AuthBeginController controller;
3537

3638
@BeforeEach
3739
void setUp() {
38-
ClientRegistrationRepository registrations = mock(ClientRegistrationRepository.class);
40+
registrations = mock(ClientRegistrationRepository.class);
3941
when(registrations.findByRegistrationId(any())).thenReturn(githubRegistration());
4042
bearerTokenResolver = mock(CookieBearerTokenResolver.class);
4143
jwtDecoder = mock(RevocationAwareJwtDecoder.class);
4244
byte[] key = new byte[32];
4345
new SecureRandom().nextBytes(key);
4446
authIntentCookie = new AuthIntentCookie(key);
45-
controller = new AuthBeginController(registrations, authIntentCookie, bearerTokenResolver, jwtDecoder);
47+
controller = buildController("");
48+
}
49+
50+
private AuthBeginController buildController(String apiBasePath) {
51+
return new AuthBeginController(
52+
registrations,
53+
authIntentCookie,
54+
bearerTokenResolver,
55+
jwtDecoder,
56+
AuthPropertiesFixture.withApiBasePath(apiBasePath)
57+
);
4658
}
4759

4860
private static ClientRegistration githubRegistration() {
@@ -118,4 +130,18 @@ void loginMode_neverTouchesDecoder() {
118130
assertThat(intent.mode()).isEqualTo(AuthIntentCookie.Intent.Mode.LOGIN);
119131
verifyNoInteractions(jwtDecoder);
120132
}
133+
134+
@Test
135+
void initRedirect_carriesApiBasePath_soItLandsOnTheProxiedEndpointNotTheSpa() {
136+
RedirectView view = buildController("/api").begin(
137+
"github",
138+
"ws",
139+
"/",
140+
"login",
141+
new MockHttpServletRequest(),
142+
new MockHttpServletResponse()
143+
);
144+
145+
assertThat(view.getUrl()).isEqualTo("/api/oauth2/authorization/github");
146+
}
121147
}

0 commit comments

Comments
 (0)