Skip to content

Commit 26711ee

Browse files
committed
Merge pull request 'Handle Keycloak errors when cookies are stale/expired' (#35) from feature/24500 into develop
2 parents 6465558 + d8287b8 commit 26711ee

File tree

3 files changed

+89
-6
lines changed

3 files changed

+89
-6
lines changed

src/main/java/eu/openanalytics/containerproxy/auth/impl/KeycloakAuthenticationBackend.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import javax.inject.Inject;
3131
import javax.servlet.ServletException;
3232

33+
import eu.openanalytics.containerproxy.auth.impl.keycloak.AuthenticationFaillureHandler;
3334
import org.keycloak.adapters.AdapterDeploymentContext;
3435
import org.keycloak.adapters.KeycloakConfigResolver;
3536
import org.keycloak.adapters.KeycloakDeployment;
@@ -39,6 +40,7 @@
3940
import org.keycloak.adapters.springsecurity.AdapterDeploymentContextFactoryBean;
4041
import org.keycloak.adapters.springsecurity.account.KeycloakRole;
4142
import org.keycloak.adapters.springsecurity.authentication.KeycloakAuthenticationEntryPoint;
43+
import org.keycloak.adapters.springsecurity.authentication.KeycloakAuthenticationFailureHandler;
4244
import org.keycloak.adapters.springsecurity.authentication.KeycloakAuthenticationProvider;
4345
import org.keycloak.adapters.springsecurity.authentication.KeycloakLogoutHandler;
4446
import org.keycloak.adapters.springsecurity.filter.KeycloakAuthenticationProcessingFilter;
@@ -141,6 +143,7 @@ protected KeycloakAuthenticationProcessingFilter keycloakAuthenticationProcessin
141143

142144
KeycloakAuthenticationProcessingFilter filter = new KeycloakAuthenticationProcessingFilter(authenticationManager, requestMatcher);
143145
filter.setSessionAuthenticationStrategy(sessionAuthenticationStrategy());
146+
filter.setAuthenticationFailureHandler(keycloakAuthenticationFailureHandler());
144147
// Fix: call afterPropertiesSet manually, because Spring doesn't invoke it for some reason.
145148
filter.setApplicationContext(ctx);
146149
filter.afterPropertiesSet();
@@ -169,6 +172,12 @@ protected SessionAuthenticationStrategy sessionAuthenticationStrategy() {
169172
return new RegisterSessionAuthenticationStrategy(new SessionRegistryImpl());
170173
}
171174

175+
@Bean
176+
@ConditionalOnProperty(name="proxy.authentication", havingValue="keycloak")
177+
public KeycloakAuthenticationFailureHandler keycloakAuthenticationFailureHandler() {
178+
return new AuthenticationFaillureHandler();
179+
}
180+
172181
@Bean
173182
@ConditionalOnProperty(name="proxy.authentication", havingValue="keycloak")
174183
protected AdapterDeploymentContext adapterDeploymentContext() throws Exception {
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/**
2+
* ContainerProxy
3+
*
4+
* Copyright (C) 2016-2020 Open Analytics
5+
*
6+
* ===========================================================================
7+
*
8+
* This program is free software: you can redistribute it and/or modify
9+
* it under the terms of the Apache License as published by
10+
* The Apache Software Foundation, either version 2 of the License, or
11+
* (at your option) any later version.
12+
*
13+
* This program is distributed in the hope that it will be useful,
14+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
15+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
16+
* Apache License for more details.
17+
*
18+
* You should have received a copy of the Apache License
19+
* along with this program. If not, see <http://www.apache.org/licenses/>
20+
*/
21+
package eu.openanalytics.containerproxy.auth.impl.keycloak;
22+
23+
import org.keycloak.adapters.OIDCAuthenticationError;
24+
import org.keycloak.adapters.springsecurity.authentication.KeycloakAuthenticationFailureHandler;
25+
import org.springframework.security.core.AuthenticationException;
26+
27+
import javax.servlet.ServletException;
28+
import javax.servlet.http.HttpServletRequest;
29+
import javax.servlet.http.HttpServletResponse;
30+
import java.io.IOException;
31+
32+
public class AuthenticationFaillureHandler extends KeycloakAuthenticationFailureHandler {
33+
34+
final public static String SP_KEYCLOAK_ERROR_REASON = "SP_KEYCLOAK_ERROR_REASON";
35+
36+
@Override
37+
public void onAuthenticationFailure(HttpServletRequest request, HttpServletResponse response,
38+
AuthenticationException exception) throws IOException, ServletException {
39+
40+
// Note: Keycloak calls sendError before this method gets called, therefore we cannot do much with reuqest.
41+
// We now set a flag in the session indicating the reason of the Keycloak error.
42+
// The error page can then properly handle this.
43+
44+
Object obj = request.getAttribute("org.keycloak.adapters.spi.AuthenticationError");
45+
if (obj instanceof org.keycloak.adapters.OIDCAuthenticationError) {
46+
OIDCAuthenticationError authError = (OIDCAuthenticationError) obj;
47+
request.getSession().setAttribute(SP_KEYCLOAK_ERROR_REASON, authError.getReason());
48+
}
49+
}
50+
}

src/main/java/eu/openanalytics/containerproxy/ui/ErrorController.java

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import javax.servlet.http.HttpServletRequest;
2929
import javax.servlet.http.HttpServletResponse;
3030

31+
import org.keycloak.adapters.OIDCAuthenticationError;
3132
import org.springframework.http.HttpStatus;
3233
import org.springframework.http.ResponseEntity;
3334
import org.springframework.security.authentication.AccountStatusException;
@@ -39,20 +40,43 @@
3940

4041
import eu.openanalytics.containerproxy.api.BaseController;
4142

43+
import static eu.openanalytics.containerproxy.auth.impl.keycloak.AuthenticationFaillureHandler.SP_KEYCLOAK_ERROR_REASON;
44+
4245
@Controller
4346
@RequestMapping("/error")
4447
public class ErrorController extends BaseController implements org.springframework.boot.web.servlet.error.ErrorController {
45-
48+
4649
@RequestMapping(produces = "text/html")
4750
public String handleError(ModelMap map, HttpServletRequest request, HttpServletResponse response) {
51+
52+
// handle keycloak errors
53+
Object obj = request.getSession().getAttribute(SP_KEYCLOAK_ERROR_REASON);
54+
if (obj instanceof OIDCAuthenticationError.Reason) {
55+
request.getSession().removeAttribute(SP_KEYCLOAK_ERROR_REASON);
56+
OIDCAuthenticationError.Reason reason = (OIDCAuthenticationError.Reason) obj;
57+
if (reason == OIDCAuthenticationError.Reason.INVALID_STATE_COOKIE ||
58+
reason == OIDCAuthenticationError.Reason.STALE_TOKEN) {
59+
// These errors are typically caused by users using wrong bookmarks (e.g. bookmarks with states in)
60+
// or when some cookies got stale. However, the user is logged into the IDP, therefore it's enough to
61+
// send the user to the main page and they will get logged in automatically.
62+
return "redirect:/";
63+
} else {
64+
return "redirect:/auth-error";
65+
}
66+
}
67+
4868
Throwable exception = (Throwable) request.getAttribute("javax.servlet.error.exception");
49-
if (exception == null) exception = (Throwable) request.getAttribute("SPRING_SECURITY_LAST_EXCEPTION");
69+
if (exception == null) {
70+
exception = (Throwable) request.getAttribute("SPRING_SECURITY_LAST_EXCEPTION");
71+
}
5072

5173
String[] msg = createMsgStack(exception);
52-
if (exception == null) msg[0] = HttpStatus.valueOf(response.getStatus()).getReasonPhrase();
74+
if (exception == null) {
75+
msg[0] = HttpStatus.valueOf(response.getStatus()).getReasonPhrase();
76+
}
5377

54-
if (response.getStatus() == 200 && isAccountStatusException(exception)) {
55-
return "/";
78+
if (response.getStatus() == 200 && (exception != null) && isAccountStatusException(exception)) {
79+
return "redirect:/";
5680
}
5781

5882
map.put("message", msg[0]);
@@ -102,7 +126,7 @@ private String[] createMsgStack(Throwable exception) {
102126

103127
return new String[] { message, stackTrace };
104128
}
105-
129+
106130
private boolean isAccountStatusException(Throwable exception) {
107131
if (exception instanceof AccountStatusException) return true;
108132
if (exception.getCause() != null) return isAccountStatusException(exception.getCause());

0 commit comments

Comments
 (0)