From a8eb8bb0edc0b7210d56bb69bb0f079667059e8d Mon Sep 17 00:00:00 2001 From: Thomas Meyer Date: Fri, 19 Jul 2019 22:41:40 +0200 Subject: [PATCH 1/4] Fix error-page for POST,PUT, etc. the view controller error in authz-config.xml forwards to the resp. JSP, but the error page only supports GET method. Install a real error controller that at least logs all error messages and then forwards to the view controller, so not handled exceptions for e.g. POST request can actually be found. --- .../mitre/openid/connect/ErrorController.java | 27 +++++++++++++++++++ .../src/main/webapp/WEB-INF/web.xml | 6 ++--- pom.xml | 6 ++--- 3 files changed, 33 insertions(+), 6 deletions(-) create mode 100644 openid-connect-server-webapp/src/main/java/org/mitre/openid/connect/ErrorController.java diff --git a/openid-connect-server-webapp/src/main/java/org/mitre/openid/connect/ErrorController.java b/openid-connect-server-webapp/src/main/java/org/mitre/openid/connect/ErrorController.java new file mode 100644 index 0000000000..6c8e591376 --- /dev/null +++ b/openid-connect-server-webapp/src/main/java/org/mitre/openid/connect/ErrorController.java @@ -0,0 +1,27 @@ +package org.mitre.openid.connect; + +import javax.servlet.RequestDispatcher; +import javax.servlet.http.HttpServletRequest; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.RequestMapping; + +@Controller +public class ErrorController { + + private static final Logger logger = LoggerFactory.getLogger(ErrorController.class); + + @RequestMapping("/errorController") + public String handle(HttpServletRequest req) { + Throwable errorException = (Throwable) req.getAttribute(RequestDispatcher.ERROR_EXCEPTION); + String message = (String) req.getAttribute(RequestDispatcher.ERROR_MESSAGE); + String requestUri = (String) req.getAttribute(RequestDispatcher.ERROR_REQUEST_URI); + + logger.error("request {} failed with {}", requestUri, message); + logger.error("exception", errorException); + + return "/error"; + } +} diff --git a/openid-connect-server-webapp/src/main/webapp/WEB-INF/web.xml b/openid-connect-server-webapp/src/main/webapp/WEB-INF/web.xml index 618db1df4a..538d3c6639 100644 --- a/openid-connect-server-webapp/src/main/webapp/WEB-INF/web.xml +++ b/openid-connect-server-webapp/src/main/webapp/WEB-INF/web.xml @@ -71,9 +71,9 @@ true - + - /error + /errorController - + diff --git a/pom.xml b/pom.xml index 6824f13603..6763854e53 100644 --- a/pom.xml +++ b/pom.xml @@ -404,8 +404,8 @@ javax.servlet - servlet-api - 2.5 + javax.servlet-api + 3.0.1 provided @@ -644,7 +644,7 @@ javax.servlet - servlet-api + javax.servlet-api javax.servlet.jsp From 6fc1e4cd1d616adf0aa2bfb8c423eb4119ceb798 Mon Sep 17 00:00:00 2001 From: Thomas Meyer Date: Sat, 3 Aug 2019 14:48:27 +0200 Subject: [PATCH 2/4] ErrorController: user /error as path for controller and /errorview for view --- .../main/java/org/mitre/openid/connect/ErrorController.java | 4 ++-- .../src/main/webapp/WEB-INF/authz-config.xml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/openid-connect-server-webapp/src/main/java/org/mitre/openid/connect/ErrorController.java b/openid-connect-server-webapp/src/main/java/org/mitre/openid/connect/ErrorController.java index 6c8e591376..386a04bf51 100644 --- a/openid-connect-server-webapp/src/main/java/org/mitre/openid/connect/ErrorController.java +++ b/openid-connect-server-webapp/src/main/java/org/mitre/openid/connect/ErrorController.java @@ -13,7 +13,7 @@ public class ErrorController { private static final Logger logger = LoggerFactory.getLogger(ErrorController.class); - @RequestMapping("/errorController") + @RequestMapping("/error") public String handle(HttpServletRequest req) { Throwable errorException = (Throwable) req.getAttribute(RequestDispatcher.ERROR_EXCEPTION); String message = (String) req.getAttribute(RequestDispatcher.ERROR_MESSAGE); @@ -22,6 +22,6 @@ public String handle(HttpServletRequest req) { logger.error("request {} failed with {}", requestUri, message); logger.error("exception", errorException); - return "/error"; + return "/error-view"; } } diff --git a/openid-connect-server-webapp/src/main/webapp/WEB-INF/authz-config.xml b/openid-connect-server-webapp/src/main/webapp/WEB-INF/authz-config.xml index 3b7a4faa87..e1bee6e61a 100644 --- a/openid-connect-server-webapp/src/main/webapp/WEB-INF/authz-config.xml +++ b/openid-connect-server-webapp/src/main/webapp/WEB-INF/authz-config.xml @@ -55,6 +55,6 @@ - + \ No newline at end of file From 8f228e4c8f71004f42251e32a3d425a7f0591dc3 Mon Sep 17 00:00:00 2001 From: Thomas Meyer Date: Sat, 3 Aug 2019 14:49:01 +0200 Subject: [PATCH 3/4] ErrorController: Move code from view into controller --- .../mitre/openid/connect/ErrorController.java | 22 ++++++++++++++++ .../src/main/webapp/WEB-INF/views/error.jsp | 26 ++----------------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/openid-connect-server-webapp/src/main/java/org/mitre/openid/connect/ErrorController.java b/openid-connect-server-webapp/src/main/java/org/mitre/openid/connect/ErrorController.java index 386a04bf51..d810655659 100644 --- a/openid-connect-server-webapp/src/main/java/org/mitre/openid/connect/ErrorController.java +++ b/openid-connect-server-webapp/src/main/java/org/mitre/openid/connect/ErrorController.java @@ -5,6 +5,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.http.HttpStatus; +import org.springframework.security.oauth2.common.exceptions.OAuth2Exception; import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.RequestMapping; @@ -22,6 +24,26 @@ public String handle(HttpServletRequest req) { logger.error("request {} failed with {}", requestUri, message); logger.error("exception", errorException); + processError(req); return "/error-view"; } + + private void processError(HttpServletRequest request) { + if (request.getAttribute("error") != null && request.getAttribute("error") instanceof OAuth2Exception) { + request.setAttribute("errorCode", ((OAuth2Exception)request.getAttribute("error")).getOAuth2ErrorCode()); + request.setAttribute("message", ((OAuth2Exception)request.getAttribute("error")).getMessage()); + } else if (request.getAttribute(RequestDispatcher.ERROR_EXCEPTION) != null) { + Throwable t = (Throwable)request.getAttribute(RequestDispatcher.ERROR_EXCEPTION); + request.setAttribute("errorCode", t.getClass().getSimpleName() + " (" + request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE) + ")"); + request.setAttribute("message", t.getMessage()); + } else if (request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE) != null) { + Integer code = (Integer)request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE); + HttpStatus status = HttpStatus.valueOf(code); + request.setAttribute("errorCode", status.toString() + " " + status.getReasonPhrase()); + request.setAttribute("message", request.getAttribute(RequestDispatcher.ERROR_MESSAGE)); + } else { + request.setAttribute("errorCode", "Server error"); + request.setAttribute("message", "See the logs for details"); + } + } } diff --git a/openid-connect-server-webapp/src/main/webapp/WEB-INF/views/error.jsp b/openid-connect-server-webapp/src/main/webapp/WEB-INF/views/error.jsp index 66c5f585ed..86d3c28850 100644 --- a/openid-connect-server-webapp/src/main/webapp/WEB-INF/views/error.jsp +++ b/openid-connect-server-webapp/src/main/webapp/WEB-INF/views/error.jsp @@ -4,27 +4,7 @@ <%@ taglib prefix="o" tagdir="/WEB-INF/tags"%> <%@ taglib prefix="spring" uri="http://www.springframework.org/tags"%> <%@ taglib prefix="security" uri="http://www.springframework.org/security/tags"%> -<%@page import="org.springframework.security.oauth2.common.exceptions.OAuth2Exception"%> -<% -if (request.getAttribute("error") != null && request.getAttribute("error") instanceof OAuth2Exception) { - request.setAttribute("errorCode", ((OAuth2Exception)request.getAttribute("error")).getOAuth2ErrorCode()); - request.setAttribute("message", ((OAuth2Exception)request.getAttribute("error")).getMessage()); -} else if (request.getAttribute("javax.servlet.error.exception") != null) { - Throwable t = (Throwable)request.getAttribute("javax.servlet.error.exception"); - request.setAttribute("errorCode", t.getClass().getSimpleName() + " (" + request.getAttribute("javax.servlet.error.status_code") + ")"); - request.setAttribute("message", t.getMessage()); -} else if (request.getAttribute("javax.servlet.error.status_code") != null) { - Integer code = (Integer)request.getAttribute("javax.servlet.error.status_code"); - HttpStatus status = HttpStatus.valueOf(code); - request.setAttribute("errorCode", status.toString() + " " + status.getReasonPhrase()); - request.setAttribute("message", request.getAttribute("javax.servlet.error.message")); -} else { - request.setAttribute("errorCode", "Server error"); - request.setAttribute("message", "See the logs for details"); -} - -%> @@ -38,11 +18,9 @@ if (request.getAttribute("error") != null && request.getAttribute("error") insta

-

- +

- - + From e2d182440b11dc5d59ee09e703869d51acbd58ff Mon Sep 17 00:00:00 2001 From: Thomas Meyer Date: Sat, 3 Aug 2019 14:50:25 +0200 Subject: [PATCH 4/4] ErrorController: instanceof check includes null check. --- .../src/main/java/org/mitre/openid/connect/ErrorController.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openid-connect-server-webapp/src/main/java/org/mitre/openid/connect/ErrorController.java b/openid-connect-server-webapp/src/main/java/org/mitre/openid/connect/ErrorController.java index d810655659..7a95e65313 100644 --- a/openid-connect-server-webapp/src/main/java/org/mitre/openid/connect/ErrorController.java +++ b/openid-connect-server-webapp/src/main/java/org/mitre/openid/connect/ErrorController.java @@ -29,7 +29,7 @@ public String handle(HttpServletRequest req) { } private void processError(HttpServletRequest request) { - if (request.getAttribute("error") != null && request.getAttribute("error") instanceof OAuth2Exception) { + if (request.getAttribute("error") instanceof OAuth2Exception) { request.setAttribute("errorCode", ((OAuth2Exception)request.getAttribute("error")).getOAuth2ErrorCode()); request.setAttribute("message", ((OAuth2Exception)request.getAttribute("error")).getMessage()); } else if (request.getAttribute(RequestDispatcher.ERROR_EXCEPTION) != null) {