From 337a67a8f6adbce90fcb573427dfb4f67211501c Mon Sep 17 00:00:00 2001 From: Paulo Gustavo Veiga Date: Thu, 6 Sep 2012 23:52:53 -0300 Subject: [PATCH] Fix ReCaptha NPE Improve error handling when permission are removed. --- .../main/javascript/RestPersistenceManager.js | 4 ++-- mindplot/src/main/javascript/widget/IMenu.js | 8 ++++++-- .../AccessDeniedSecurityException.java | 14 +++++++++++-- .../exceptions/ClientException.java | 20 +++++++++++++++++++ .../java/com/wisemapping/model/Mindmap.java | 3 ++- .../ncontroller/UsersController.java | 14 +++++++++---- .../com/wisemapping/rest/AdminController.java | 6 +++--- .../com/wisemapping/rest/BaseController.java | 20 ++++++++++++++----- .../wisemapping/rest/model/RestErrors.java | 10 +++++----- .../com/wisemapping/validator/Messages.java | 1 + .../src/main/resources/messages_en.properties | 4 ++++ .../src/main/resources/messages_es.properties | 4 ++++ 12 files changed, 84 insertions(+), 24 deletions(-) create mode 100644 wise-webapp/src/main/java/com/wisemapping/exceptions/ClientException.java diff --git a/mindplot/src/main/javascript/RestPersistenceManager.js b/mindplot/src/main/javascript/RestPersistenceManager.js index 82e5b9d8..7d9a5da0 100644 --- a/mindplot/src/main/javascript/RestPersistenceManager.js +++ b/mindplot/src/main/javascript/RestPersistenceManager.js @@ -45,7 +45,8 @@ mindplot.RESTPersistenceManager = new Class({ events.onError(); }, onFailure:function (xhr) { - events.onError(); + var responseText = xhr.responseText; + events.onError(JSON.decode(responseText)); }, headers:{"Content-Type":"application/json", "Accept":"application/json"}, emulation:false, @@ -60,7 +61,6 @@ mindplot.RESTPersistenceManager = new Class({ async:false, method:'post', onSuccess:function () { - console.log("Revert success ...."); }, onException:function () { }, diff --git a/mindplot/src/main/javascript/widget/IMenu.js b/mindplot/src/main/javascript/widget/IMenu.js index 0938b876..487092d8 100644 --- a/mindplot/src/main/javascript/widget/IMenu.js +++ b/mindplot/src/main/javascript/widget/IMenu.js @@ -78,10 +78,14 @@ mindplot.widget.IMenu = new Class({ } menu.setRequireChange(false); }, - onError:function () { + onError:function (error) { if (saveHistory) { saveElem.setStyle('cursor', 'pointer'); - $notify($msg('SAVE_COULD_NOT_BE_COMPLETED')); + var msg = error ? error.globalErrors : null; + if (!msg) { + msg = $msg('SAVE_COULD_NOT_BE_COMPLETED'); + } + $notify(msg); } } }); diff --git a/wise-webapp/src/main/java/com/wisemapping/exceptions/AccessDeniedSecurityException.java b/wise-webapp/src/main/java/com/wisemapping/exceptions/AccessDeniedSecurityException.java index 33698c63..b3fbba89 100755 --- a/wise-webapp/src/main/java/com/wisemapping/exceptions/AccessDeniedSecurityException.java +++ b/wise-webapp/src/main/java/com/wisemapping/exceptions/AccessDeniedSecurityException.java @@ -18,11 +18,21 @@ package com.wisemapping.exceptions; +import org.jetbrains.annotations.NotNull; + public class AccessDeniedSecurityException - extends Exception + extends ClientException { - public AccessDeniedSecurityException(String msg) + public static final String MSG_KEY = "ACCESS_HAS_BEEN_REVOKED"; + + public AccessDeniedSecurityException(@NotNull String msg) { super(msg); } + + @NotNull + @Override + protected String getMsgBundleKey() { + return MSG_KEY; + } } diff --git a/wise-webapp/src/main/java/com/wisemapping/exceptions/ClientException.java b/wise-webapp/src/main/java/com/wisemapping/exceptions/ClientException.java new file mode 100644 index 00000000..35a8e239 --- /dev/null +++ b/wise-webapp/src/main/java/com/wisemapping/exceptions/ClientException.java @@ -0,0 +1,20 @@ +package com.wisemapping.exceptions; + +import org.jetbrains.annotations.NotNull; +import org.springframework.context.MessageSource; + +import java.util.Locale; + +abstract public class ClientException extends WiseMappingException { + public ClientException(@NotNull String message) { + super(message); + } + + protected abstract + @NotNull + String getMsgBundleKey(); + + public String getMessage(@NotNull final MessageSource messageSource, final @NotNull Locale locale) { + return messageSource.getMessage(this.getMsgBundleKey(), null, locale); + } +} diff --git a/wise-webapp/src/main/java/com/wisemapping/model/Mindmap.java b/wise-webapp/src/main/java/com/wisemapping/model/Mindmap.java index e2b78ef4..74b387bc 100644 --- a/wise-webapp/src/main/java/com/wisemapping/model/Mindmap.java +++ b/wise-webapp/src/main/java/com/wisemapping/model/Mindmap.java @@ -18,6 +18,7 @@ package com.wisemapping.model; +import com.wisemapping.exceptions.AccessDeniedSecurityException; import com.wisemapping.exceptions.WiseMappingException; import com.wisemapping.util.ZipUtils; import org.apache.commons.lang.StringEscapeUtils; @@ -233,7 +234,7 @@ public class Mindmap { final Collaboration collaboration = this.findCollaboration(collaborator); if (collaboration == null) { - throw new WiseMappingException("User is not collaborator"); + throw new AccessDeniedSecurityException("Collaborator " + collaborator.getEmail() + " could not access " + this.getId()); } return collaboration.getCollaborationProperties(); } diff --git a/wise-webapp/src/main/java/com/wisemapping/ncontroller/UsersController.java b/wise-webapp/src/main/java/com/wisemapping/ncontroller/UsersController.java index 259cd8ca..20f13c38 100644 --- a/wise-webapp/src/main/java/com/wisemapping/ncontroller/UsersController.java +++ b/wise-webapp/src/main/java/com/wisemapping/ncontroller/UsersController.java @@ -137,10 +137,16 @@ public class UsersController { final String challenge = request.getParameter("recaptcha_challenge_field"); final String uresponse = request.getParameter("recaptcha_response_field"); - final String remoteAddr = request.getRemoteAddr(); - final ReCaptchaResponse reCaptchaResponse = captchaService.checkAnswer(remoteAddr, challenge, uresponse); - if (!reCaptchaResponse.isValid()) { - bindingResult.rejectValue("captcha", Messages.CAPTCHA_ERROR); + if (challenge != null && uresponse != null) { + final String remoteAddr = request.getRemoteAddr(); + final ReCaptchaResponse reCaptchaResponse = captchaService.checkAnswer(remoteAddr, challenge, uresponse); + + if (!reCaptchaResponse.isValid()) { + bindingResult.rejectValue("captcha", Messages.CAPTCHA_ERROR); + } + + } else { + bindingResult.rejectValue("captcha", Messages.CAPTCHA_LOADING_ERROR); } } return bindingResult; diff --git a/wise-webapp/src/main/java/com/wisemapping/rest/AdminController.java b/wise-webapp/src/main/java/com/wisemapping/rest/AdminController.java index ef5cb642..73bc1a85 100644 --- a/wise-webapp/src/main/java/com/wisemapping/rest/AdminController.java +++ b/wise-webapp/src/main/java/com/wisemapping/rest/AdminController.java @@ -60,7 +60,7 @@ public class AdminController extends BaseController { @RequestMapping(method = RequestMethod.POST, value = "admin/users", consumes = {"application/xml", "application/json"}, produces = {"application/json", "text/html", "application/xml"}) @ResponseStatus(value = HttpStatus.CREATED) - public void createUser(@RequestBody RestUser user, HttpServletResponse response) throws IOException, WiseMappingException { + public void createUser(@RequestBody RestUser user, HttpServletResponse response) throws WiseMappingException { if (user == null) { throw new IllegalArgumentException("User could not be found"); } @@ -90,7 +90,7 @@ public class AdminController extends BaseController { @RequestMapping(method = RequestMethod.PUT, value = "admin/users/{id}/password", consumes = {"text/plain"}) @ResponseStatus(value = HttpStatus.NO_CONTENT) - public void changePassword(@RequestBody String password, @PathVariable long id) throws IOException, WiseMappingException { + public void changePassword(@RequestBody String password, @PathVariable long id) throws WiseMappingException { if (password == null) { throw new IllegalArgumentException("Password can not be null"); } @@ -105,7 +105,7 @@ public class AdminController extends BaseController { @RequestMapping(method = RequestMethod.DELETE,value = "admin/users/{id}") @ResponseStatus(value = HttpStatus.NO_CONTENT) - public void getUserByEmail(@PathVariable long id) throws IOException, WiseMappingException { + public void getUserByEmail(@PathVariable long id) throws WiseMappingException { final User user = userService.getUserBy(id); if (user == null) { throw new IllegalArgumentException("User '" + id + "' could not be found"); diff --git a/wise-webapp/src/main/java/com/wisemapping/rest/BaseController.java b/wise-webapp/src/main/java/com/wisemapping/rest/BaseController.java index 471d662b..d36b7edd 100644 --- a/wise-webapp/src/main/java/com/wisemapping/rest/BaseController.java +++ b/wise-webapp/src/main/java/com/wisemapping/rest/BaseController.java @@ -19,6 +19,7 @@ package com.wisemapping.rest; import com.wisemapping.exceptions.AccessDeniedSecurityException; +import com.wisemapping.exceptions.ClientException; import com.wisemapping.filter.UserAgent; import com.wisemapping.mail.NotificationService; import com.wisemapping.model.User; @@ -27,6 +28,7 @@ import com.wisemapping.security.Utils; import org.jetbrains.annotations.NotNull; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.context.i18n.LocaleContextHolder; import org.springframework.context.support.ResourceBundleMessageSource; import org.springframework.http.HttpStatus; import org.springframework.web.bind.annotation.ExceptionHandler; @@ -36,6 +38,7 @@ import org.springframework.web.bind.annotation.ResponseStatus; import javax.servlet.ServletContext; import javax.servlet.http.HttpServletRequest; import java.lang.reflect.UndeclaredThrowableException; +import java.util.Locale; public class BaseController { @@ -75,13 +78,20 @@ public class BaseController { @ExceptionHandler(java.lang.reflect.UndeclaredThrowableException.class) @ResponseStatus(HttpStatus.BAD_REQUEST) public RestErrors handleSecurityErrors(@NotNull UndeclaredThrowableException ex) { - return new RestErrors(ex.getMessage()); + final Throwable cause = ex.getCause(); + RestErrors result; + if (cause instanceof ClientException) { + result = handleClientErrors((ClientException) cause); + } else { + result = new RestErrors(ex.getMessage()); + } + return result; } - @ExceptionHandler(com.wisemapping.exceptions.AccessDeniedSecurityException.class) + @ExceptionHandler(ClientException.class) @ResponseStatus(HttpStatus.BAD_REQUEST) - public RestErrors handleSecurityException(@NotNull AccessDeniedSecurityException ex) { - return new RestErrors(ex.getMessage()); + public RestErrors handleClientErrors(@NotNull ClientException ex) { + final Locale locale = LocaleContextHolder.getLocale(); + return new RestErrors(ex.getMessage(messageSource, locale)); } - } diff --git a/wise-webapp/src/main/java/com/wisemapping/rest/model/RestErrors.java b/wise-webapp/src/main/java/com/wisemapping/rest/model/RestErrors.java index 9ee4ccd5..c5e4ad96 100644 --- a/wise-webapp/src/main/java/com/wisemapping/rest/model/RestErrors.java +++ b/wise-webapp/src/main/java/com/wisemapping/rest/model/RestErrors.java @@ -30,7 +30,7 @@ public class RestErrors { private Errors errors; @JsonIgnore - private List globalErrors; + private List gErrors; @JsonIgnore MessageSource messageSource; @@ -43,12 +43,12 @@ public class RestErrors { this.errors = errors; this.messageSource = messageSource; - this.globalErrors = this.processGlobalErrors(errors, messageSource); + this.gErrors = this.processGlobalErrors(errors, messageSource); } public RestErrors(@NotNull String errorMsg) { - globalErrors = new ArrayList(); - globalErrors.add(errorMsg); + gErrors = new ArrayList(); + gErrors.add(errorMsg); } private List processGlobalErrors(@NotNull Errors errors, @NotNull MessageSource messageSource) { @@ -61,7 +61,7 @@ public class RestErrors { } public List getGlobalErrors() { - return globalErrors; + return gErrors; } public void setGlobalErrors(List list) { diff --git a/wise-webapp/src/main/java/com/wisemapping/validator/Messages.java b/wise-webapp/src/main/java/com/wisemapping/validator/Messages.java index 056fbe76..c39f2575 100644 --- a/wise-webapp/src/main/java/com/wisemapping/validator/Messages.java +++ b/wise-webapp/src/main/java/com/wisemapping/validator/Messages.java @@ -26,4 +26,5 @@ public interface Messages { String MAP_TITLE_ALREADY_EXISTS = "MAP_TITLE_ALREADY_EXISTS"; String PASSWORD_MISSMATCH = "PASSWORD_MISSMATCH"; String CAPTCHA_ERROR = "CAPTCHA_ERROR"; + String CAPTCHA_LOADING_ERROR = "CAPTCHA_LOADING_ERROR"; } diff --git a/wise-webapp/src/main/resources/messages_en.properties b/wise-webapp/src/main/resources/messages_en.properties index 70ced1d6..5436e614 100644 --- a/wise-webapp/src/main/resources/messages_en.properties +++ b/wise-webapp/src/main/resources/messages_en.properties @@ -241,6 +241,10 @@ SUPPORT=Support FEEDBACK=Feedback CONTACT_US=Contact Us +#Pending for translation ... +CAPTCHA_LOADING_ERROR=ReCaptcha could not be loaded. You must have access to Google ReCaptcha service. +ACCESS_HAS_BEEN_REVOKED= Upps. your access permissions to this map has been revoked. Contact map owner. + diff --git a/wise-webapp/src/main/resources/messages_es.properties b/wise-webapp/src/main/resources/messages_es.properties index fdb2ae11..1bfcf4ad 100644 --- a/wise-webapp/src/main/resources/messages_es.properties +++ b/wise-webapp/src/main/resources/messages_es.properties @@ -241,3 +241,7 @@ FEEDBACK=Feedback CONTACT_US=Contáctenos +ACCESS_HAS_BEEN_REVOKED=Los permisos de acceso al mapa han sido revocados. No es posible grabar sus cambios. + + +