SONAR-17291 better message for user login requirements

diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/UserUpdater.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/UserUpdater.java
index 7df1a44f9b3e21c40ea595c83835b44bc420fa63..594de62f0a279e84d332da9a91c5c135b9e821c5 100644 (file)
--- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/UserUpdater.java
+++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/UserUpdater.java
@@ -27,6 +27,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Objects;
 import java.util.function.Consumer;
+import java.util.regex.Pattern;
 import java.util.stream.Stream;
 import javax.annotation.Nullable;
 import javax.inject.Inject;
@@ -66,6 +67,8 @@ public class UserUpdater {
   private static final String PASSWORD_PARAM = "Password";
   private static final String NAME_PARAM = "Name";
   private static final String EMAIL_PARAM = "Email";
+  private static final Pattern START_WITH_SPECIFIC_AUTHORIZED_CHARACTERS = Pattern.compile("^[\\.\\-_@].*$");
+  private static final Pattern CONTAINS_ONLY_AUTHORIZED_CHARACTERS = Pattern.compile("\\A\\w[\\w\\.\\-_@]+\\z");
 
   public static final int LOGIN_MIN_LENGTH = 2;
   public static final int LOGIN_MAX_LENGTH = 255;
@@ -313,15 +316,18 @@ public class UserUpdater {
 
   private static boolean validateLoginFormat(@Nullable String login, List<String> messages) {
     boolean isValid = checkNotEmptyParam(login, LOGIN_PARAM, messages);
-    if (!isNullOrEmpty(login)) {
+    if (isValid) {
       if (login.length() < LOGIN_MIN_LENGTH) {
         messages.add(format(Validation.IS_TOO_SHORT_MESSAGE, LOGIN_PARAM, LOGIN_MIN_LENGTH));
         return false;
       } else if (login.length() > LOGIN_MAX_LENGTH) {
         messages.add(format(Validation.IS_TOO_LONG_MESSAGE, LOGIN_PARAM, LOGIN_MAX_LENGTH));
         return false;
-      } else if (!login.matches("\\A\\w[\\w\\.\\-_@]+\\z")) {
-        messages.add("Use only letters, numbers, and .-_@ please.");
+      } else if (START_WITH_SPECIFIC_AUTHORIZED_CHARACTERS.matcher(login).matches()){
+        messages.add("Login should not start with .-_@");
+        return false;
+      } else if (!CONTAINS_ONLY_AUTHORIZED_CHARACTERS.matcher(login).matches()) {
+        messages.add("Login should contain only letters, numbers, and .-_@");
         return false;
       }
     }

diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/HttpHeadersAuthenticationTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/HttpHeadersAuthenticationTest.java
index 6a8b42ab979a14064c29d23799a106d717c29a8e..743e98b50993e3428a2504c8891a5a766f946b84 100644 (file)
--- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/HttpHeadersAuthenticationTest.java
+++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/HttpHeadersAuthenticationTest.java
@@ -344,7 +344,7 @@ public class HttpHeadersAuthenticationTest {
     setNotUserInToken();
 
     assertThatThrownBy(() -> underTest.authenticate(createRequest("invalid login", DEFAULT_NAME, DEFAULT_EMAIL, GROUPS), response))
-      .hasMessage("Use only letters, numbers, and .-_@ please.")
+      .hasMessage("Login should contain only letters, numbers, and .-_@")
       .isInstanceOf(AuthenticationException.class)
       .hasFieldOrPropertyWithValue("source", Source.sso());
 

diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterCreateTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterCreateTest.java
index 5d799c5cc0b78bbb542e761af0eaaac499603500..bd9b077a72704cc1d3dd00396610e481a49ac17a 100644 (file)
--- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterCreateTest.java
+++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterCreateTest.java
@@ -87,7 +87,7 @@ public class UserUpdaterCreateTest {
       .setPassword("PASSWORD")
       .setScmAccounts(ImmutableList.of("u1", "u_1", "User 1"))
       .build(), u -> {
-      });
+    });
 
     assertThat(dto.getUuid()).isNotNull();
     assertThat(dto.getLogin()).isEqualTo("user");
@@ -122,7 +122,7 @@ public class UserUpdaterCreateTest {
       .setLogin("us")
       .setName("User")
       .build(), u -> {
-      });
+    });
 
     UserDto dto = dbClient.userDao().selectByLogin(session, "us");
     assertThat(dto.getUuid()).isNotNull();
@@ -140,7 +140,7 @@ public class UserUpdaterCreateTest {
     UserDto user = underTest.createAndCommit(db.getSession(), NewUser.builder()
       .setName("John Doe")
       .build(), u -> {
-      });
+    });
 
     UserDto dto = dbClient.userDao().selectByLogin(session, user.getLogin());
     assertThat(dto.getLogin()).startsWith("john-doe");
@@ -155,7 +155,7 @@ public class UserUpdaterCreateTest {
       .setLogin("")
       .setName("John Doe")
       .build(), u -> {
-      });
+    });
 
     UserDto dto = dbClient.userDao().selectByLogin(session, user.getLogin());
     assertThat(dto.getLogin()).startsWith("john-doe");
@@ -171,7 +171,7 @@ public class UserUpdaterCreateTest {
       .setName("User")
       .setPassword("password")
       .build(), u -> {
-      });
+    });
 
     UserDto dto = dbClient.userDao().selectByLogin(session, "user");
     assertThat(dto.getExternalLogin()).isEqualTo("user");
@@ -188,7 +188,7 @@ public class UserUpdaterCreateTest {
       .setName("User")
       .setExternalIdentity(new ExternalIdentity("github", "github-user", "ABCD"))
       .build(), u -> {
-      });
+    });
 
     UserDto dto = dbClient.userDao().selectByLogin(session, "user");
     assertThat(dto.isLocal()).isFalse();
@@ -208,7 +208,7 @@ public class UserUpdaterCreateTest {
       .setName("User")
       .setExternalIdentity(new ExternalIdentity(SQ_AUTHORITY, "user", "user"))
       .build(), u -> {
-      });
+    });
 
     UserDto dto = dbClient.userDao().selectByLogin(session, "user");
     assertThat(dto.isLocal()).isFalse();
@@ -229,7 +229,7 @@ public class UserUpdaterCreateTest {
       .setPassword("password")
       .setScmAccounts(asList("u1", "", null))
       .build(), u -> {
-      });
+    });
 
     assertThat(dbClient.userDao().selectByLogin(session, "user").getScmAccountsAsList()).containsOnly("u1");
   }
@@ -244,7 +244,7 @@ public class UserUpdaterCreateTest {
       .setPassword("password")
       .setScmAccounts(asList(""))
       .build(), u -> {
-      });
+    });
 
     assertThat(dbClient.userDao().selectByLogin(session, "user").getScmAccounts()).isNull();
   }
@@ -259,7 +259,7 @@ public class UserUpdaterCreateTest {
       .setPassword("password")
       .setScmAccounts(asList("u1", "u1"))
       .build(), u -> {
-      });
+    });
 
     assertThat(dbClient.userDao().selectByLogin(session, "user").getScmAccountsAsList()).containsOnly("u1");
   }
@@ -275,7 +275,7 @@ public class UserUpdaterCreateTest {
       .setEmail("user@mail.com")
       .setPassword("PASSWORD")
       .build(), u -> {
-      }, otherUser);
+    }, otherUser);
 
     assertThat(es.getIds(UserIndexDefinition.TYPE_USER)).containsExactlyInAnyOrder(created.getUuid(), otherUser.getUuid());
   }
@@ -292,7 +292,7 @@ public class UserUpdaterCreateTest {
       });
     })
       .isInstanceOf(BadRequestException.class)
-      .hasMessage("Use only letters, numbers, and .-_@ please.");
+      .hasMessage("Login should contain only letters, numbers, and .-_@");
   }
 
   @Test
@@ -307,7 +307,7 @@ public class UserUpdaterCreateTest {
       });
     })
       .isInstanceOf(BadRequestException.class)
-      .hasMessage("Use only letters, numbers, and .-_@ please.");
+      .hasMessage("Login should contain only letters, numbers, and .-_@");
   }
 
   @Test
@@ -325,6 +325,23 @@ public class UserUpdaterCreateTest {
       .hasMessage("Login is too short (minimum is 2 characters)");
   }
 
+
+  @Test
+  public void fail_to_create_user_login_start_with_underscore() {
+    assertThatThrownBy(() -> {
+      underTest.createAndCommit(db.getSession(), NewUser.builder()
+        .setLogin("_marbalous")
+        .setName("Marius")
+        .setEmail("marius@mail.com")
+        .setPassword("password")
+        .build(), u -> {
+      });
+    })
+      .isInstanceOf(BadRequestException.class)
+      .hasMessage("Login should not start with .-_@");
+  }
+
+
   @Test
   public void fail_to_create_user_with_too_long_login() {
     assertThatThrownBy(() -> {
@@ -394,7 +411,7 @@ public class UserUpdaterCreateTest {
         .setEmail("marius@mail.com")
         .setPassword("")
         .build(), u -> {
-        });
+      });
       fail();
     } catch (BadRequestException e) {
       assertThat(e.errors()).containsExactlyInAnyOrder("Name can't be empty", "Password can't be empty");
@@ -515,7 +532,7 @@ public class UserUpdaterCreateTest {
       .setPassword("password")
       .setScmAccounts(asList("u1", "u_1"))
       .build(), u -> {
-      });
+    });
 
     verify(newUserNotifier).onNewUser(newUserHandler.capture());
     assertThat(newUserHandler.getValue().getLogin()).isEqualTo("user");
@@ -533,7 +550,7 @@ public class UserUpdaterCreateTest {
       .setEmail("user@mail.com")
       .setPassword("password")
       .build(), u -> {
-      });
+    });
 
     Multimap<String, String> groups = dbClient.groupMembershipDao().selectGroupsByLogins(session, singletonList("user"));
     assertThat(groups.get("user")).containsOnly(defaultGroup.getName());

diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterUpdateTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterUpdateTest.java
index 999dccae463be73b02793776afcddc59beb05ada..e779665c0c228a81f06006c6a65bcc89585012ae 100644 (file)
--- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterUpdateTest.java
+++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterUpdateTest.java
@@ -628,7 +628,19 @@ public class UserUpdaterUpdateTest {
 
     assertThatThrownBy(() -> underTest.updateAndCommit(session, user, updateUser, EMPTY_USER_CONSUMER))
       .isInstanceOf(BadRequestException.class)
-      .hasMessage("Use only letters, numbers, and .-_@ please.");
+      .hasMessage("Login should contain only letters, numbers, and .-_@");
+  }
+
+  @Test
+  public void fail_to_update_login_when_login_start_with_unauthorized_characters() {
+    UserDto user = db.users().insertUser();
+    createDefaultGroup();
+
+    UpdateUser updateUser = new UpdateUser().setLogin("_StartWithUnderscore");
+
+    assertThatThrownBy(() -> underTest.updateAndCommit(session, user, updateUser, EMPTY_USER_CONSUMER))
+      .isInstanceOf(BadRequestException.class)
+      .hasMessage("Login should not start with .-_@");
   }
 
   @Test