From 9e0d84c639beb1048d38c03b1b3969db63fba9ff Mon Sep 17 00:00:00 2001
From: Sonia Zorba <sonia.zorba@inaf.it>
Date: Thu, 14 May 2020 15:16:35 +0200
Subject: [PATCH] #3 Improved error messages
---
gms-ui/src/App.vue | 4 +-
gms-ui/src/api/server/index.js | 12 +--
gms/pom.xml | 2 +-
.../it/inaf/ia2/gms/authn/SecurityConfig.java | 12 ++-
.../gms/controller/HomePageController.java | 3 +-
.../gms/controller/KeepAliveController.java | 7 +-
.../gms/exception/BadRequestException.java | 2 +-
.../ia2/gms/exception/ErrorController.java | 92 +++++++++++++++++++
.../inaf/ia2/gms/exception/GmsException.java | 8 ++
.../ia2/gms/exception/NotFoundException.java | 2 +-
.../gms/exception/UnauthorizedException.java | 2 +-
.../inaf/ia2/gms/persistence/LoggingDAO.java | 4 +-
gms/src/main/resources/application.properties | 6 +-
gms/src/main/resources/public/error/404.html | 14 +++
.../main/resources/public/error/error.html | 16 ++++
15 files changed, 163 insertions(+), 23 deletions(-)
create mode 100644 gms/src/main/java/it/inaf/ia2/gms/exception/ErrorController.java
create mode 100644 gms/src/main/java/it/inaf/ia2/gms/exception/GmsException.java
create mode 100644 gms/src/main/resources/public/error/404.html
create mode 100644 gms/src/main/resources/public/error/error.html
diff --git a/gms-ui/src/App.vue b/gms-ui/src/App.vue
index 152a93a..cd5b2f3 100644
--- a/gms-ui/src/App.vue
+++ b/gms-ui/src/App.vue
@@ -41,8 +41,8 @@ export default {
mounted: function() {
var self = this;
document.addEventListener('apiError', function(event) {
- self.$bvToast.toast(event.message, {
- title: "Error",
+ self.$bvToast.toast(event.message.body, {
+ title: event.message.title,
variant: 'danger',
solid: true
});
diff --git a/gms-ui/src/api/server/index.js b/gms-ui/src/api/server/index.js
index d5c81cc..61abe17 100644
--- a/gms-ui/src/api/server/index.js
+++ b/gms-ui/src/api/server/index.js
@@ -26,15 +26,11 @@ function apiRequest(url, options, showLoading = true) {
}
function dispatchApiErrorEvent(error) {
- let message;
- if (error.message) {
- message = error.message;
- } else {
- message = 'Generic error';
- }
-
let event = new CustomEvent('apiError');
- event.message = message;
+ event.message = {
+ title: error.error || 'Error',
+ body: error.message || 'Unknown error'
+ };
document.dispatchEvent(event);
}
diff --git a/gms/pom.xml b/gms/pom.xml
index fb2694e..cc40470 100644
--- a/gms/pom.xml
+++ b/gms/pom.xml
@@ -5,7 +5,7 @@
<parent>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-parent</artifactId>
- <version>2.1.7.RELEASE</version>
+ <version>2.2.7.RELEASE</version>
<relativePath/> <!-- lookup parent from repository -->
</parent>
<groupId>it.inaf.ia2</groupId>
diff --git a/gms/src/main/java/it/inaf/ia2/gms/authn/SecurityConfig.java b/gms/src/main/java/it/inaf/ia2/gms/authn/SecurityConfig.java
index 63b992e..8d47685 100644
--- a/gms/src/main/java/it/inaf/ia2/gms/authn/SecurityConfig.java
+++ b/gms/src/main/java/it/inaf/ia2/gms/authn/SecurityConfig.java
@@ -18,6 +18,8 @@ import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.builders.WebSecurity;
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
import org.springframework.security.oauth2.provider.token.store.jwk.JwkTokenStore;
+import org.springframework.security.web.authentication.Http403ForbiddenEntryPoint;
+import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
import org.springframework.web.cors.CorsConfiguration;
import org.springframework.web.cors.UrlBasedCorsConfigurationSource;
import org.springframework.web.filter.CorsFilter;
@@ -45,14 +47,20 @@ public class SecurityConfig extends WebSecurityConfigurerAdapter {
@Override
public void configure(HttpSecurity http) throws Exception {
- super.configure(http);
-
// CORS are necessary only for development (API access from npm server)
if (Arrays.asList(env.getActiveProfiles()).contains("dev")) {
http.authorizeRequests()
.antMatchers(HttpMethod.OPTIONS, "/**").permitAll();
}
+ super.configure(http);
+
+ // avoid displaying the annoying BasicAuth browser popup when the
+ // session expires (this should happen mostly during development)
+ // [401 WWW-Authenticate is converted to 403]
+ http.exceptionHandling().defaultAuthenticationEntryPointFor(
+ new Http403ForbiddenEntryPoint(), new AntPathRequestMatcher("/keepAlive"));
+
http.csrf().disable();
}
diff --git a/gms/src/main/java/it/inaf/ia2/gms/controller/HomePageController.java b/gms/src/main/java/it/inaf/ia2/gms/controller/HomePageController.java
index ced3a3d..bb5240f 100644
--- a/gms/src/main/java/it/inaf/ia2/gms/controller/HomePageController.java
+++ b/gms/src/main/java/it/inaf/ia2/gms/controller/HomePageController.java
@@ -34,7 +34,7 @@ public class HomePageController {
private InvitedRegistrationManager invitedRegistrationManager;
@ResponseBody
- @GetMapping(value = "/home", produces = MediaType.APPLICATION_JSON_UTF8_VALUE)
+ @GetMapping(value = "/home", produces = MediaType.APPLICATION_JSON_VALUE)
public ResponseEntity<HomePageResponse> getMainPage(@Valid GroupsRequest request) {
HomePageResponse response = new HomePageResponse();
@@ -56,7 +56,6 @@ public class HomePageController {
if (optReg.isPresent()) {
request.setAttribute("invited-registration", optReg.get());
return "/registration-completed";
- //request.getRequestDispatcher("/registration-completed").forward(request, response);
}
return "index.html";
diff --git a/gms/src/main/java/it/inaf/ia2/gms/controller/KeepAliveController.java b/gms/src/main/java/it/inaf/ia2/gms/controller/KeepAliveController.java
index f543633..c41f012 100644
--- a/gms/src/main/java/it/inaf/ia2/gms/controller/KeepAliveController.java
+++ b/gms/src/main/java/it/inaf/ia2/gms/controller/KeepAliveController.java
@@ -2,9 +2,11 @@ package it.inaf.ia2.gms.controller;
import it.inaf.ia2.gms.authn.SessionData;
import it.inaf.ia2.gms.rap.RapClient;
+import java.util.HashMap;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RestController;
@@ -20,13 +22,14 @@ public class KeepAliveController {
@Autowired
private RapClient rapClient;
- @GetMapping("/keepAlive")
+ @GetMapping(value = "/keepAlive", produces = MediaType.APPLICATION_JSON_VALUE)
public ResponseEntity<?> keepAlive() {
LOG.trace("Keepalive called");
if (sessionData.getExpiresIn() < 60) {
rapClient.refreshToken();
LOG.trace("RAP token refreshed");
}
- return ResponseEntity.noContent().build();
+ // empty JSON object response
+ return ResponseEntity.ok(new HashMap<>());
}
}
diff --git a/gms/src/main/java/it/inaf/ia2/gms/exception/BadRequestException.java b/gms/src/main/java/it/inaf/ia2/gms/exception/BadRequestException.java
index 50f633f..535c5a9 100644
--- a/gms/src/main/java/it/inaf/ia2/gms/exception/BadRequestException.java
+++ b/gms/src/main/java/it/inaf/ia2/gms/exception/BadRequestException.java
@@ -4,7 +4,7 @@ import org.springframework.http.HttpStatus;
import org.springframework.web.bind.annotation.ResponseStatus;
@ResponseStatus(value = HttpStatus.BAD_REQUEST)
-public class BadRequestException extends RuntimeException {
+public class BadRequestException extends GmsException {
public BadRequestException(String message) {
super(message);
diff --git a/gms/src/main/java/it/inaf/ia2/gms/exception/ErrorController.java b/gms/src/main/java/it/inaf/ia2/gms/exception/ErrorController.java
new file mode 100644
index 0000000..7fb46c4
--- /dev/null
+++ b/gms/src/main/java/it/inaf/ia2/gms/exception/ErrorController.java
@@ -0,0 +1,92 @@
+package it.inaf.ia2.gms.exception;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.Map;
+import java.util.Scanner;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.boot.autoconfigure.web.servlet.error.AbstractErrorController;
+import org.springframework.boot.web.servlet.error.ErrorAttributes;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.MediaType;
+import org.springframework.http.ResponseEntity;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RestController;
+
+@RestController
+@RequestMapping("${server.error.path:${error.path:/error}}")
+public class ErrorController extends AbstractErrorController {
+
+ @Value("${support.contact.label}")
+ private String supportContactLabel;
+ @Value("${support.contact.email}")
+ private String supportContactEmail;
+
+ @Autowired
+ public ErrorController(ErrorAttributes errorAttributes) {
+ super(errorAttributes);
+ }
+
+ @RequestMapping(produces = MediaType.TEXT_HTML_VALUE)
+ public void errorHtml(HttpServletRequest request, HttpServletResponse response) throws Exception {
+
+ Map<String, Object> errors = super.getErrorAttributes(request, true);
+
+ HttpStatus status = getStatus(request);
+
+ String responseText;
+ if (status == HttpStatus.NOT_FOUND) {
+ responseText = getFileContent("404.html");
+ } else {
+ responseText = getFileContent("error.html")
+ .replace("#ERROR_TITLE#", (String) errors.get("error"))
+ .replace("#ERROR_MESSAGE#", (String) errors.get("message"))
+ .replace("#ADDITIONAL_MESSAGE#", getAdditionalMessage(status));
+ }
+
+ response.setContentType("text/html;charset=UTF-8");
+ response.getOutputStream().print(responseText);
+ }
+
+ private String getAdditionalMessage(HttpStatus status) {
+ if (status.is5xxServerError()) {
+ // unexpected error -> let users report the issue
+ return "<br/>If you need support please contact"
+ + " <a href=\"mailto:" + supportContactEmail + "\">" + supportContactLabel + "</a>";
+ }
+ return "";
+ }
+
+ @RequestMapping(produces = MediaType.TEXT_PLAIN_VALUE)
+ public void errorText(HttpServletRequest request, HttpServletResponse response) throws Exception {
+ Map<String, Object> errors = super.getErrorAttributes(request, true);
+ response.setContentType("text/plain;charset=UTF-8");
+ response.getOutputStream().print(errors.get("error") + ": " + errors.get("message"));
+ }
+
+ @RequestMapping
+ public ResponseEntity<Map<String, Object>> error(HttpServletRequest request) {
+ HttpStatus status = getStatus(request);
+ if (status == HttpStatus.NO_CONTENT) {
+ return new ResponseEntity<>(status);
+ }
+ Map<String, Object> body = getErrorAttributes(request, false);
+ return new ResponseEntity<>(body, status);
+ }
+
+ private String getFileContent(String templateFileName) throws IOException {
+ try (InputStream in = ErrorController.class.getClassLoader()
+ .getResourceAsStream("public/error/" + templateFileName)) {
+ Scanner s = new Scanner(in).useDelimiter("\\A");
+ return s.hasNext() ? s.next() : "";
+ }
+ }
+
+ @Override
+ public String getErrorPath() {
+ return null;
+ }
+}
diff --git a/gms/src/main/java/it/inaf/ia2/gms/exception/GmsException.java b/gms/src/main/java/it/inaf/ia2/gms/exception/GmsException.java
new file mode 100644
index 0000000..c7a5ce4
--- /dev/null
+++ b/gms/src/main/java/it/inaf/ia2/gms/exception/GmsException.java
@@ -0,0 +1,8 @@
+package it.inaf.ia2.gms.exception;
+
+public abstract class GmsException extends RuntimeException {
+
+ public GmsException(String message) {
+ super(message);
+ }
+}
diff --git a/gms/src/main/java/it/inaf/ia2/gms/exception/NotFoundException.java b/gms/src/main/java/it/inaf/ia2/gms/exception/NotFoundException.java
index f04bc1f..da5acb8 100644
--- a/gms/src/main/java/it/inaf/ia2/gms/exception/NotFoundException.java
+++ b/gms/src/main/java/it/inaf/ia2/gms/exception/NotFoundException.java
@@ -4,7 +4,7 @@ import org.springframework.http.HttpStatus;
import org.springframework.web.bind.annotation.ResponseStatus;
@ResponseStatus(value = HttpStatus.NOT_FOUND)
-public class NotFoundException extends RuntimeException {
+public class NotFoundException extends GmsException {
public NotFoundException(String message) {
super(message);
diff --git a/gms/src/main/java/it/inaf/ia2/gms/exception/UnauthorizedException.java b/gms/src/main/java/it/inaf/ia2/gms/exception/UnauthorizedException.java
index e4f73db..70f4465 100644
--- a/gms/src/main/java/it/inaf/ia2/gms/exception/UnauthorizedException.java
+++ b/gms/src/main/java/it/inaf/ia2/gms/exception/UnauthorizedException.java
@@ -4,7 +4,7 @@ import org.springframework.http.HttpStatus;
import org.springframework.web.bind.annotation.ResponseStatus;
@ResponseStatus(value = HttpStatus.UNAUTHORIZED)
-public class UnauthorizedException extends RuntimeException {
+public class UnauthorizedException extends GmsException {
public UnauthorizedException(String message) {
super(message);
diff --git a/gms/src/main/java/it/inaf/ia2/gms/persistence/LoggingDAO.java b/gms/src/main/java/it/inaf/ia2/gms/persistence/LoggingDAO.java
index b82e83d..b04b810 100644
--- a/gms/src/main/java/it/inaf/ia2/gms/persistence/LoggingDAO.java
+++ b/gms/src/main/java/it/inaf/ia2/gms/persistence/LoggingDAO.java
@@ -90,8 +90,10 @@ public class LoggingDAO {
private String getUser(HttpServletRequest request) {
if (request.getUserPrincipal() != null && request.getUserPrincipal() instanceof RapPrincipal) {
return request.getUserPrincipal().getName();
- } else {
+ } else if (request.getSession(false) != null) {
return sessionData.getUserId();
+ } else {
+ return null;
}
}
}
diff --git a/gms/src/main/resources/application.properties b/gms/src/main/resources/application.properties
index 902956e..19b3dac 100644
--- a/gms/src/main/resources/application.properties
+++ b/gms/src/main/resources/application.properties
@@ -2,6 +2,7 @@ server.port=8082
server.servlet.context-path=/gms
spring.main.allow-bean-definition-overriding=true
+server.error.whitelabel.enabled=false
security.oauth2.client.client-id=gms
security.oauth2.client.client-secret=gms-secret
@@ -16,12 +17,13 @@ logging.level.org.springframework.security=DEBUG
logging.level.org.springframework.jdbc=TRACE
logging.level.org.springframework.web=TRACE
-spring.datasource.url=jdbc:postgresql://localhost:5432/postgres
+spring.datasource.url=jdbc:postgresql://localhost:5433/postgres
spring.datasource.username=gms
spring.datasource.password=gms
rap.ws-url=http://localhost/rap-ia2/ws
-rap.ws.basic-auth=false
+support.contact.label=IA2 team
+support.contact.email=ia2@inaf.it
# For development only:
spring.profiles.active=dev
diff --git a/gms/src/main/resources/public/error/404.html b/gms/src/main/resources/public/error/404.html
new file mode 100644
index 0000000..f9d3cfe
--- /dev/null
+++ b/gms/src/main/resources/public/error/404.html
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <title>Page Not Found</title>
+ <meta charset="UTF-8" />
+ <meta name="viewport" content="width=device-width, initial-scale=1.0" />
+ <link rel="stylesheet" href="https://stackpath.bootstrapcdn.com/bootstrap/4.4.1/css/bootstrap.min.css" integrity="sha384-Vkoo8x4CGsO3+Hhxv8T/Q5PaXtkKtu6ug5TOeNV6gBiFeWPGFN9MuhOf23Q9Ifjh" crossorigin="anonymous" />
+ </head>
+ <body>
+ <div class="container mt-4">
+ <h1 class="mb-3 text-primary">Page Not Found</h1>
+ </div>
+ </body>
+</html>
diff --git a/gms/src/main/resources/public/error/error.html b/gms/src/main/resources/public/error/error.html
new file mode 100644
index 0000000..b0fa515
--- /dev/null
+++ b/gms/src/main/resources/public/error/error.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <title>#ERROR_TITLE#</title>
+ <meta charset="UTF-8" />
+ <meta name="viewport" content="width=device-width, initial-scale=1.0" />
+ <link rel="stylesheet" href="https://stackpath.bootstrapcdn.com/bootstrap/4.4.1/css/bootstrap.min.css" integrity="sha384-Vkoo8x4CGsO3+Hhxv8T/Q5PaXtkKtu6ug5TOeNV6gBiFeWPGFN9MuhOf23Q9Ifjh" crossorigin="anonymous" />
+ </head>
+ <body>
+ <div class="container mt-4">
+ <h1 class="mb-3 text-danger">#ERROR_TITLE#</h1>
+ <p><strong>#ERROR_MESSAGE#</strong></p>
+ <p>#ADDITIONAL_MESSAGE#</p>
+ </div>
+ </body>
+</html>
--
GitLab