Show correct signup errors

6 files changed, 67 insertions, 15 deletions

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<User> createUser(String username, String password);
+    Optional<User> createUser(String username, String password) throws UsernameTakenException;
 
     Optional<User> 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<User> createUser(final String username, final String password) {
+    public Optional<User> 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 <http://www.gnu.org/licenses/>.
+ */
+
+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 %}
+<article>
+    <p>{{ result | raw }}</p>
+</article>
+{% 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);