We don't allow leading and/or trailing whitespace on SCM account names.
We also don't allow duplicate SCM accounts.
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
-import { uniq } from 'lodash';
import * as React from 'react';
import { createUser, updateUser } from '../../../api/users';
import { Button, ResetButtonLink, SubmitButton } from '../../../components/controls/buttons';
login: this.state.login,
name: this.state.name,
password: this.state.password,
- scmAccount: uniq(this.state.scmAccounts)
+ scmAccount: this.state.scmAccounts
}).then(() => {
this.props.onUpdateUsers();
this.props.onClose();
email: user!.local ? this.state.email : undefined,
login: this.state.login,
name: user!.local ? this.state.name : undefined,
- scmAccount: uniq(this.state.scmAccounts)
+ scmAccount: this.state.scmAccounts
}).then(() => {
this.props.onUpdateUsers();
this.props.onClose();
login,
name,
password,
- scmAccount: ['gh', 'bitbucket']
+ scmAccount: ['gh', 'gh', 'bitbucket']
});
});
email,
login,
name,
- scmAccount: ['gh', 'bitbucket']
+ scmAccount: ['gh', 'gh', 'bitbucket']
});
});
*/
package org.sonar.server.user.ws;
+import java.util.HashSet;
import java.util.List;
-import java.util.stream.Collectors;
+import java.util.Set;
import javax.annotation.CheckForNull;
import javax.annotation.Nullable;
import org.sonar.api.server.ws.Change;
private static CreateRequest toWsRequest(Request request) {
return CreateRequest.builder()
.setLogin(request.mandatoryParam(PARAM_LOGIN))
+ .setName(request.mandatoryParam(PARAM_NAME))
.setPassword(request.param(PARAM_PASSWORD))
- .setName(request.param(PARAM_NAME))
.setEmail(request.param(PARAM_EMAIL))
.setScmAccounts(parseScmAccounts(request))
.setLocal(request.mandatoryParamAsBoolean(PARAM_LOCAL))
public static List<String> parseScmAccounts(Request request) {
if (request.hasParam(PARAM_SCM_ACCOUNT)) {
- return formatScmAccounts(request.multiParam(PARAM_SCM_ACCOUNT));
+ List<String> scmAccounts = request.multiParam(PARAM_SCM_ACCOUNT);
+ validateScmAccounts(scmAccounts);
+ return scmAccounts;
}
-
return emptyList();
}
- private static List<String> formatScmAccounts(List<String> scmAccounts) {
- return scmAccounts
- .stream()
- .map(String::trim)
- .collect(Collectors.toList());
+ private static void validateScmAccounts(List<String> scmAccounts) {
+ scmAccounts.forEach(CreateAction::validateScmAccountFormat);
+ validateNoDuplicates(scmAccounts);
+ }
+
+ private static void validateScmAccountFormat(String scmAccount) {
+ checkArgument(scmAccount.equals(scmAccount.strip()), "SCM account cannot start or end with whitespace: '%s'", scmAccount);
+ }
+
+ private static void validateNoDuplicates(List<String> scmAccounts) {
+ Set<String> duplicateCheck = new HashSet<>();
+ for (String account : scmAccounts) {
+ checkArgument(duplicateCheck.add(account), "Duplicate SCM account: '%s'", account);
+ }
}
static class CreateRequest {
.setDescription("Update a user.<br/>" +
"Requires Administer System permission")
.setSince("3.7")
- .setChangelog(new Change("5.2", "User's password can only be changed using the 'change_password' action."))
+ .setChangelog(
+ new Change("5.2", "User's password can only be changed using the 'change_password' action."))
.setPost(true)
.setHandler(this)
.setResponseExample(getClass().getResource("update-example.json"));
// exists in index
assertThat(es.client().search(EsClient.prepareSearch(UserIndexDefinition.TYPE_USER)
- .source(new SearchSourceBuilder()
- .query(boolQuery()
- .must(termQuery(FIELD_LOGIN, "john"))
- .must(termQuery(FIELD_NAME, "John"))
- .must(termQuery(FIELD_EMAIL, "john@email.com"))
- .must(termQuery(FIELD_SCM_ACCOUNTS, "jn")))))
+ .source(new SearchSourceBuilder()
+ .query(boolQuery()
+ .must(termQuery(FIELD_LOGIN, "john"))
+ .must(termQuery(FIELD_NAME, "John"))
+ .must(termQuery(FIELD_EMAIL, "john@email.com"))
+ .must(termQuery(FIELD_SCM_ACCOUNTS, "jn")))))
.getHits().getHits()).hasSize(1);
// exists in db
assertThat(response.getUser().getScmAccountsList()).containsOnly("j,n");
}
-
@Test
- public void create_user_ignores_duplicates_containing_whitespace_characters_in_scm_account_values() {
+ public void fail_when_whitespace_characters_in_scm_account_values() {
logInAsSystemAdministrator();
- CreateWsResponse response = call(CreateRequest.builder()
- .setLogin("john")
- .setName("John")
- .setEmail("john@email.com")
- .setScmAccounts(List.of("admin", " admin "))
- .setPassword("1234")
- .build());
+ assertThatThrownBy(() -> {
+ call(CreateRequest.builder()
+ .setLogin("john")
+ .setName("John")
+ .setEmail("john@email.com")
+ .setScmAccounts(List.of("admin", " admin "))
+ .setPassword("1234")
+ .build());
+ })
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("SCM account cannot start or end with whitespace: ' admin '");
+ }
- assertThat(response.getUser().getScmAccountsList()).containsOnly("admin");
+ @Test
+ public void fail_when_duplicates_characters_in_scm_account_values() {
+ logInAsSystemAdministrator();
+
+ assertThatThrownBy(() -> {
+ call(CreateRequest.builder()
+ .setLogin("john")
+ .setName("John")
+ .setEmail("john@email.com")
+ .setScmAccounts(List.of("admin", "admin"))
+ .setPassword("1234")
+ .build());
+ })
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Duplicate SCM account: 'admin'");
}
@Test
public void fail_when_password_is_set_on_none_local_user() {
logInAsSystemAdministrator();
+ TestRequest request =tester.newRequest()
+ .setParam("login","john")
+ .setParam("name","John")
+ .setParam("password", "1234")
+ .setParam("local", "false");
+
assertThatThrownBy(() -> {
- call(CreateRequest.builder()
- .setLogin("john")
- .setName("John")
- .setPassword("1234")
- .setLocal(false)
- .build());
+ request.execute();
})
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Password should only be set on local user");
public void fail_when_email_is_invalid() {
logInAsSystemAdministrator();
+ CreateRequest request = CreateRequest.builder()
+ .setLogin("pipo")
+ .setName("John")
+ .setPassword("1234")
+ .setEmail("invalid-email")
+ .build();
+
assertThatThrownBy(() -> {
- call(CreateRequest.builder()
- .setLogin("pipo")
- .setName("John")
- .setPassword("1234")
- .setEmail("invalid-email")
- .build());
+ call(request);
})
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Email 'invalid-email' is not valid");
ofNullable(createRequest.getEmail()).ifPresent(e2 -> request.setParam("email", e2));
ofNullable(createRequest.getPassword()).ifPresent(e1 -> request.setParam("password", e1));
ofNullable(createRequest.getScmAccounts()).ifPresent(e -> request.setMultiParam("scmAccount", e));
- request.setParam("local", createRequest.isLocal() ? "true" : "false");
+ request.setParam("local", Boolean.toString(createRequest.isLocal()));
return request.executeProtobuf(CreateWsResponse.class);
}
import org.sonar.server.user.UserUpdater;
import org.sonar.server.user.index.UserIndexer;
import org.sonar.server.usergroups.DefaultGroupFinder;
+import org.sonar.server.ws.TestRequest;
import org.sonar.server.ws.WsActionTester;
import static com.google.common.collect.Lists.newArrayList;
}
@Test
- public void update_scm_account_ignores_duplicates() {
+ public void fail_when_duplicates_characters_in_scm_account_values() {
createUser();
- ws.newRequest()
- .setParam("login", "john")
- .setMultiParam("scmAccount", Arrays.asList("jon.snow", "jon.snow", "jon.jon", "jon.snow"))
- .execute();
+ assertThatThrownBy(() -> {
+ ws.newRequest()
+ .setParam("login", "john")
+ .setMultiParam("scmAccount", Arrays.asList("jon.snow", "jon.snow", "jon.jon", "jon.snow"))
+ .execute();
+ }).isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Duplicate SCM account: 'jon.snow'");
+ ;
- UserDto user = dbClient.userDao().selectByLogin(dbSession, "john");
- assertThat(user.getScmAccountsAsList()).containsExactlyInAnyOrder("jon.jon", "jon.snow");
}
@Test
- public void update_scm_account_ignores_duplicates_containing_whitespace_characters_in_scm_account_values() {
+ public void fail_when_whitespace_characters_in_scm_account_values() {
createUser();
- ws.newRequest()
- .setParam("login", "john")
- .setMultiParam("scmAccount", Arrays.asList("jon.snow", "jon.snow", "jon.jon", " jon.snow "))
- .execute();
+ assertThatThrownBy(() -> {
+ ws.newRequest()
+ .setParam("login", "john")
+ .setMultiParam("scmAccount", Arrays.asList("jon.snow", "jon.snow", "jon.jon", " jon.snow "))
+ .execute();
+ }).isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("SCM account cannot start or end with whitespace: ' jon.snow '");
+ ;
- UserDto user = dbClient.userDao().selectByLogin(dbSession, "john");
- assertThat(user.getScmAccountsAsList()).containsExactlyInAnyOrder("jon.jon", "jon.snow");
}
@Test
public void fail_on_disabled_user() {
db.users().insertUser(u -> u.setLogin("john").setActive(false));
+ TestRequest request = ws.newRequest()
+ .setParam("login", "john");
assertThatThrownBy(() -> {
- ws.newRequest()
- .setParam("login", "john")
- .execute();
+ request.execute();
})
.isInstanceOf(NotFoundException.class)
.hasMessage("User 'john' doesn't exist");
public void fail_on_invalid_email() {
createUser();
+ TestRequest request = ws.newRequest()
+ .setParam("login", "john")
+ .setParam("email", "invalid-email");
assertThatThrownBy(() -> {
- ws.newRequest()
- .setParam("login", "john")
- .setParam("email", "invalid-email")
- .execute();
+ request.execute();
})
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Email 'invalid-email' is not valid");