From b550d376b0f62bbe914da26500524acf5be22494 Mon Sep 17 00:00:00 2001 From: Vitaly Takmazov Date: Thu, 21 May 2020 12:10:32 +0300 Subject: Show correct signup errors --- src/main/java/com/juick/service/UserService.java | 3 +- .../java/com/juick/service/UserServiceImpl.java | 5 +-- .../com/juick/util/UsernameTakenException.java | 24 +++++++++++++ .../java/com/juick/www/controllers/SignUp.java | 39 ++++++++++++++++------ .../resources/templates/views/signup_result.html | 6 ++++ .../java/com/juick/server/tests/ServerTests.java | 5 ++- 6 files changed, 67 insertions(+), 15 deletions(-) create mode 100644 src/main/java/com/juick/util/UsernameTakenException.java create mode 100644 src/main/resources/templates/views/signup_result.html diff --git a/src/main/java/com/juick/service/UserService.java b/src/main/java/com/juick/service/UserService.java index fbbab0ad..16d76659 100644 --- a/src/main/java/com/juick/service/UserService.java +++ b/src/main/java/com/juick/service/UserService.java @@ -20,6 +20,7 @@ package com.juick.service; import com.juick.model.Message; import com.juick.model.User; import com.juick.model.AuthResponse; +import com.juick.util.UsernameTakenException; import javax.annotation.Nonnull; import java.util.Collection; @@ -39,7 +40,7 @@ public interface UserService { String getSignUpHashByTelegramID(Long telegramId, String username); - Optional createUser(String username, String password); + Optional createUser(String username, String password) throws UsernameTakenException; Optional getUserByUID(int uid); diff --git a/src/main/java/com/juick/service/UserServiceImpl.java b/src/main/java/com/juick/service/UserServiceImpl.java index 84ff1ff5..cf1c838d 100644 --- a/src/main/java/com/juick/service/UserServiceImpl.java +++ b/src/main/java/com/juick/service/UserServiceImpl.java @@ -21,6 +21,7 @@ import com.juick.model.Message; import com.juick.model.User; import com.juick.model.AnonymousUser; import com.juick.model.AuthResponse; +import com.juick.util.UsernameTakenException; import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.StringUtils; @@ -108,7 +109,7 @@ public class UserServiceImpl extends BaseJdbcService implements UserService { @Transactional @Override - public Optional createUser(final String username, final String password) { + public Optional createUser(final String username, final String password) throws UsernameTakenException { KeyHolder holder = new GeneratedKeyHolder(); try { getJdbcTemplate().update( @@ -122,7 +123,7 @@ public class UserServiceImpl extends BaseJdbcService implements UserService { }, holder); } catch (DuplicateKeyException e) { - return Optional.empty(); + throw new UsernameTakenException(); } int uid = holder.getKeys().size() > 1 ? (int)holder.getKeys().get("id") : holder.getKey().intValue(); diff --git a/src/main/java/com/juick/util/UsernameTakenException.java b/src/main/java/com/juick/util/UsernameTakenException.java new file mode 100644 index 00000000..ee787e99 --- /dev/null +++ b/src/main/java/com/juick/util/UsernameTakenException.java @@ -0,0 +1,24 @@ +/* + * Copyright (C) 2008-2020, Juick + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package com.juick.util; + +public class UsernameTakenException extends RuntimeException { + public UsernameTakenException() { + super("Username taken", null, false, false); + } +} diff --git a/src/main/java/com/juick/www/controllers/SignUp.java b/src/main/java/com/juick/www/controllers/SignUp.java index 3b052e18..9516ddf7 100644 --- a/src/main/java/com/juick/www/controllers/SignUp.java +++ b/src/main/java/com/juick/www/controllers/SignUp.java @@ -19,6 +19,7 @@ package com.juick.www.controllers; import com.juick.model.User; import com.juick.util.HttpBadRequestException; import com.juick.util.HttpForbiddenException; +import com.juick.util.UsernameTakenException; import com.juick.www.WebApp; import com.juick.service.CrosspostService; import com.juick.service.EmailService; @@ -103,14 +104,18 @@ public class SignUp { User current; if (hash.length() > 36 || !type.matches("^[a-zA-Z0-9\\-]+$") || !hash.matches("^[a-zA-Z0-9\\-]+$")) { - throw new HttpBadRequestException(); + modelMap.addAttribute("result", "Invalid request"); + modelMap.addAttribute("visitor", visitor); + return "views/signup_result"; } if (action.charAt(0) == 'l') { if (visitor.isAnonymous()) { if (username.length() > 32) { - throw new HttpBadRequestException(); + modelMap.addAttribute("result", "Invalid request"); + modelMap.addAttribute("visitor", visitor); + return "views/signup_result"; } current = userService.checkPassword(username, password).orElseThrow(HttpForbiddenException::new); } else { @@ -118,7 +123,9 @@ public class SignUp { } if (current.getUid() <= 0) { - throw new HttpForbiddenException(); + modelMap.addAttribute("result", "Invalid request"); + modelMap.addAttribute("visitor", visitor); + return "views/signup_result"; } if (!(type.charAt(0) == 'f' && crosspostService.setFacebookUser(hash, current.getUid())) @@ -132,20 +139,29 @@ public class SignUp { emailService.deleteAuthCode(hash); } else { if (type.equals("xmpp")) { - modelMap.addAttribute("visitor", visitor); modelMap.addAttribute("result", "XMPP support is disabled for new users"); - return "views/settings_result"; + } else { + modelMap.addAttribute("result", "Invalid request"); } - throw new HttpBadRequestException(); + modelMap.addAttribute("visitor", visitor); + return "views/signup_result"; } } } else { // Create new account if (username.length() < 2 || username.length() > 16 || !username.matches("^[a-zA-Z0-9\\-]+$") || password.length() < 6 || password.length() > 32) { - throw new HttpBadRequestException(); + modelMap.addAttribute("visitor", visitor); + modelMap.addAttribute("result", "Bad username or password"); + return "views/signup_result"; } - current = userService.createUser(username, password).orElseThrow(HttpBadRequestException::new); + try { + current = userService.createUser(username, password).orElseThrow(HttpBadRequestException::new); + } catch(UsernameTakenException e) { + modelMap.addAttribute("visitor", visitor); + modelMap.addAttribute("result", e.getMessage()); + return "views/signup_result"; + } if (!(type.charAt(0) == 'f' && crosspostService.setFacebookUser(hash, current.getUid())) && !(type.charAt(0) == 'v' && crosspostService.setVKUser(hash, current.getUid())) @@ -156,11 +172,12 @@ public class SignUp { emailService.deleteAuthCode(hash); } else { if (type.equals("xmpp")) { - modelMap.addAttribute("visitor", visitor); modelMap.addAttribute("result", "XMPP support is disabled for new users"); - return "views/settings_result"; + } else { + modelMap.addAttribute("result", "Invalid request"); } - throw new HttpBadRequestException(); + modelMap.addAttribute("visitor", visitor); + return "views/signup_result"; } } } diff --git a/src/main/resources/templates/views/signup_result.html b/src/main/resources/templates/views/signup_result.html new file mode 100644 index 00000000..b204e1b8 --- /dev/null +++ b/src/main/resources/templates/views/signup_result.html @@ -0,0 +1,6 @@ +{% extends "layouts/default" %} +{% block content %} +
+

{{ result | raw }}

+
+{% endblock %} \ No newline at end of file diff --git a/src/test/java/com/juick/server/tests/ServerTests.java b/src/test/java/com/juick/server/tests/ServerTests.java index 51e17ecc..aad85884 100644 --- a/src/test/java/com/juick/server/tests/ServerTests.java +++ b/src/test/java/com/juick/server/tests/ServerTests.java @@ -381,7 +381,10 @@ public class ServerTests { int mid2 = messagesService.createMessage(user.getUid(), "yo2", null, tagList); Message msg2 = messagesService.getMessage(mid2).get(); assertEquals(1, msg2.getTags().size()); - assertEquals("we already have ugnich", Optional.empty(), userService.createUser("ugnich", "x")); + Exception exc = assertThrows(UsernameTakenException.class, () -> { + userService.createUser("ugnich", "x"); + }); + assertEquals("Username taken", exc.getMessage()); User hugnich = userService.createUser("hugnich", "x").orElseThrow(IllegalStateException::new); int rid = messagesService.createReply(msg2.getMid(), 0, hugnich, "bla-bla", null); assertEquals(1, rid); -- cgit v1.2.3