Skip to content

Commit

Permalink
Feature/2088 namespace admin is able to create a technical user for a…
Browse files Browse the repository at this point in the history
… namespace (#2192)

* #2088:

* end-to-end workflow for technical user creation when adding a non-existing user to a namespace:
   1. Admin edits collaborators in a given namespace
   2. Admin selects "add user"
   3. Given username does not exist
   4. A new modal opens, prompting admin to create a new technical user with a given authorization provider ID among supported
   5. Once created, modals close and the new technical user is created

* no tests present yet

Signed-off-by: Menahem Julien Raccah Lisei <[email protected]>

* #2088:

Small UI/UX improvements:

* nicer design /spacing for the new "create technical user form"
* add user autofocuses on the user name input box

Signed-off-by: Menahem Julien Raccah Lisei <[email protected]>

* #2088:

Large UI/UX improvements:

* User search when adding to namespace, with a drop down activating once user input field text length is >= 4 characters
* Search retrieves users whose name contains search characters, shows and adds to drop-down
* If no option selected or none available, username interpreted as is (can still be added as non-existing / technical user then)

Signed-off-by: Menahem Julien Raccah Lisei <[email protected]>

* #2088:

* Made user search case-insensitive (not well optimized, sadly)
* Added a few tests for search
* Removed some extraneous import in TenantTechnicalUserDto

Signed-off-by: Menahem Julien Raccah Lisei <[email protected]>

* #2088:

* Cleaned up some code
* Added some javadoc where missing
* Added a couple of account service tests (note that user and tenant repositories are mocked by the parent class, so the only applicable tests are the ones that will fail due to validation issues)

Signed-off-by: Menahem Julien Raccah Lisei <[email protected]>

* #2088:

* Minimal javadoc on new tests

Signed-off-by: Menahem Julien Raccah Lisei <[email protected]>

* #2088:

* Improved combobox
* Cleaned up some garbage

Signed-off-by: Menahem Julien Raccah Lisei <[email protected]>
  • Loading branch information
Menahem Julien Raccah Lisei authored and aedelmann committed Jan 9, 2020
1 parent 73aaa1a commit 1d174ed
Show file tree
Hide file tree
Showing 11 changed files with 894 additions and 447 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright (c) 2018 Contributors to the Eclipse Foundation
* Copyright (c) 2018, 2019 Contributors to the Eclipse Foundation
*
* See the NOTICE file(s) distributed with this work for additional
* information regarding copyright ownership.
Expand Down Expand Up @@ -29,7 +29,14 @@ public interface IUserAccountService {
* @return users who are system administrators
*/
Collection<User> getSystemAdministrators();


/**
* Finds a list of users matching the given partial username.
* @param partial
* @return
*/
Collection<User> findUsers(String partial);

/**
*
* @param tenantId the tenant from which to remove the user
Expand All @@ -46,12 +53,23 @@ public interface IUserAccountService {
* @return status if the user was added successfully
*/
boolean addUserToTenant(String tenantId, String userId, Role ... roles);


/**
* Creates a technical user with the given id, authentication provider and roles, then adds it to
* the given tenant.
* @param tenantId
* @param userId
* @param authenticationProviderId
* @param roles
* @return always {@literal true} (fails with runtime exception if some operation failed).
*/
boolean createTechnicalUserAndAddToTenant(String tenantId, String userId, String authenticationProviderId, Role... roles);

/**
* Returns if the particular user as the role in the Tenant
*
* @param tenantId the tenant to check
* @param userId the user id
* @param authentication the authentication
* @param role the role to check (e.g ROLE_TENANT_ADMIN, ROLE_USER,..)
* @return
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright (c) 2018 Contributors to the Eclipse Foundation
* Copyright (c) 2018, 2019 Contributors to the Eclipse Foundation
*
* See the NOTICE file(s) distributed with this work for additional
* information regarding copyright ownership.
Expand Down Expand Up @@ -51,15 +51,18 @@ public class DefaultUserAccountService
@Value("${server.admin:#{null}}")
private String[] admins;

@SuppressWarnings("SpringJavaInjectionPointsAutowiringInspection")
@Autowired
private IUserRepository userRepository;

@Autowired
private INotificationService notificationService;

@SuppressWarnings("SpringJavaInjectionPointsAutowiringInspection")
@Autowired
private ITenantRepository tenantRepo;

@SuppressWarnings("SpringJavaInjectionPointsAutowiringInspection")
@Autowired
private ITenantUserRepo tenantUserRepo;

Expand Down Expand Up @@ -87,14 +90,92 @@ public boolean removeUserFromTenant(String tenantId, String userId) {
return true;
}

public boolean addUserToTenant(String tenantId, String userId, Role... roles) {
/**
* This method does to extraneous things - ugly but convenient to centralize some validation logic:
* <ul>
* <li>
* Validates all "id" string parameters and fails if any invalid ({@literal null} or empty).
* </li>
* <li>
* Validates that the given roles are not empty.
* </li>
* <li>
* Attempts to find an existing tenant (namespace) with the given id, and fails if not found.
* </li>
* <li>
* If found, returns the tenant (namespace).
* </li>
* </ul>
* With the current usage, it's probably not worth re-writing / separating scopes.
*
* @param tenantId
* @param userId
* @param authenticationProviderId
* @param roles
* @return
*/
private Tenant validateAndReturnTenant(String tenantId, String userId, String authenticationProviderId, Role... roles) {
PreConditions.notNullOrEmpty(tenantId, "tenantId");
PreConditions.notNullOrEmpty(userId, "userId");
PreConditions.notNullOrEmpty(roles, "roles should not be empty");

if (authenticationProviderId != null) {
if (authenticationProviderId.trim().isEmpty()) {
throw new IllegalArgumentException("Given authentication provider cannot be empty.");
}
}
Tenant tenant = tenantRepo.findByTenantId(tenantId);
PreConditions.notNull(tenant, "Tenant with given tenantId does not exist");
return tenant;
}

PreConditions.notNull(tenant, "Tenant with given tenantId doesnt exists");
/**
* This is invoked through a {@code POST} request in the {@link org.eclipse.vorto.repository.web.account.AccountController},
* when an administrator wants to add a new technical user to a given namespace. <br/>
* @see DefaultUserAccountService#addUserToTenant(String, String, Role...) for situations where the
* user exists already, instead.
* @param tenantId
* @param userId
* @param authenticationProviderId
* @param roles
* @return
*/
public boolean createTechnicalUserAndAddToTenant(String tenantId, String userId, String authenticationProviderId, Role... roles) {

Tenant tenant = validateAndReturnTenant(tenantId, userId, authenticationProviderId, roles);

// additional validation for authentication provider id
PreConditions.notNullOrEmpty(authenticationProviderId, "authenticationProviderId");

// this creates and persists the technical user
User user = User.create(userId, authenticationProviderId, null, true);
userRepository.save(user);
// this creates and persists the "tenant user"
TenantUser tenantUser = TenantUser.createTenantUser(tenant, user, roles);
tenantUserRepo.save(tenantUser);
eventPublisher.publishEvent(new AppEvent(this, userId, EventType.USER_ADDED));
return true;
}

/**
* As the name implies, adds a given user to the given tenant/namespace, with the given roles.<br/>
* As the name does <b>not</b> imply, the given user represented by the {@code userId} parameter
* must exist.<br/>
* For the purpose of ensuring the user exists, the front-end will first invoke a user search. <br/>
* If the user is found, then this method gets eventually invoked after the {@code PUT} method is
* called in the {@link org.eclipse.vorto.repository.web.account.AccountController}.<br/>
* If the user search returns {@literal 404}, then the {@code POST} method is invoked instead in
* the {@link org.eclipse.vorto.repository.web.account.AccountController}, which may end up invoking
* sibling method here {@link DefaultUserAccountService#createTechnicalUserAndAddToTenant(String, String, String, Role...)},
* upon administrative user confirmation that they want to create a technical user instead.
* @param tenantId the tenant to add this user to
* @param userId the user id
* @param roles the roles to be given to the user
* @return
*/
public boolean addUserToTenant(String tenantId, String userId, Role... roles) {

// cannot validate authentication provider within context
Tenant tenant = validateAndReturnTenant(tenantId, userId, null, roles);

Optional<TenantUser> maybeUser = tenant.getUser(userId);
if (maybeUser.isPresent()) {
Expand Down Expand Up @@ -261,6 +342,11 @@ public User getUser(String username) {
return this.userRepository.findByUsername(username);
}

@Override
public Collection<User> findUsers(String partial) {
return this.userRepository.findUserByPartial(partial);
}

@Override
public void saveUser(User user) {
this.userRepository.save(user);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,15 @@ public interface IUserRepository extends CrudRepository<User, Long> {
* @return
*/
User findByUsername(String username);


/**
* Finds a list of users matching the given partial username.
* @param partial
* @return
*/
@Query("SELECT u from User u WHERE LOWER(u.username) LIKE %?1%")
Collection<User> findUserByPartial(String partial);

@Query("SELECT u from User u, TenantUser tu, UserRole r " +
"WHERE u.id = tu.user.id AND " +
"tu.id = r.user.id AND " +
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright (c) 2018 Contributors to the Eclipse Foundation
* Copyright (c) 2018, 2019 Contributors to the Eclipse Foundation
*
* See the NOTICE file(s) distributed with this work for additional
* information regarding copyright ownership.
Expand All @@ -12,6 +12,8 @@
*/
package org.eclipse.vorto.repository.web.account;

import com.google.common.base.Strings;
import io.swagger.annotations.ApiParam;
import java.security.Principal;
import java.util.Arrays;
import java.util.Collection;
Expand All @@ -34,6 +36,7 @@
import org.eclipse.vorto.repository.tenant.ITenantService;
import org.eclipse.vorto.repository.upgrade.IUpgradeService;
import org.eclipse.vorto.repository.web.ControllerUtils;
import org.eclipse.vorto.repository.web.account.dto.TenantTechnicalUserDto;
import org.eclipse.vorto.repository.web.account.dto.TenantUserDto;
import org.eclipse.vorto.repository.web.account.dto.UserDto;
import org.slf4j.Logger;
Expand All @@ -52,8 +55,6 @@
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod;
import org.springframework.web.bind.annotation.RestController;
import com.google.common.base.Strings;
import io.swagger.annotations.ApiParam;

@RestController
public class AccountController {
Expand Down Expand Up @@ -108,6 +109,55 @@ public ResponseEntity<Boolean> addOrUpdateUsersForTenant(
}
}

@RequestMapping(method = RequestMethod.POST, value = "/rest/tenants/{tenantId}/users/{userId}")
@PreAuthorize("hasRole('ROLE_SYS_ADMIN') or hasPermission(#tenantId, 'org.eclipse.vorto.repository.domain.Tenant', 'ROLE_TENANT_ADMIN')")
public ResponseEntity<Boolean> createTechnicalUserForTenant(
@ApiParam(value = "tenantId", required = true) @PathVariable String tenantId,
@ApiParam(value = "userId", required = true) @PathVariable String userId,
@RequestBody @ApiParam(value = "The user to be added to the tenant",
required = true) final TenantTechnicalUserDto user) {

if (Strings.nullToEmpty(userId).trim().isEmpty()) {
return new ResponseEntity<>(HttpStatus.PRECONDITION_FAILED);
}

if (Strings.nullToEmpty(tenantId).trim().isEmpty()) {
return new ResponseEntity<>(HttpStatus.PRECONDITION_FAILED);
}

if (Strings.nullToEmpty(user.getAuthenticationProviderId()).trim().isEmpty()) {
return new ResponseEntity<>(HttpStatus.PRECONDITION_FAILED);
}

if (user.getRoles().stream()
.anyMatch(role -> role.equals(UserRole.ROLE_SYS_ADMIN))) {
return new ResponseEntity<>(HttpStatus.PRECONDITION_FAILED);
}

try {
LOGGER.info(
String.format(
"Creating technical user [%s] and adding to tenant [%s] with role(s) [%s]",
userId,
tenantId,
user.getRoles()
)
);
if (userId.equals(SecurityContextHolder.getContext().getAuthentication().getName())) {
return new ResponseEntity<>(HttpStatus.BAD_REQUEST);
}
return new ResponseEntity<>(
accountService.createTechnicalUserAndAddToTenant(tenantId, userId, user.getAuthenticationProviderId(), toRoles(user)),
HttpStatus.OK
);
} catch (IllegalArgumentException e) {
return new ResponseEntity<>(HttpStatus.PRECONDITION_FAILED);
} catch (Exception e) {
LOGGER.error("error at addOrUpdateUsersForTenant()", e);
return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR);
}
}

private Role[] toRoles(TenantUserDto user) {
Set<Role> roles = user.getRoles().stream()
.filter(role -> !(role.equals(UserRole.ROLE_SYS_ADMIN)))
Expand Down Expand Up @@ -236,7 +286,22 @@ public ResponseEntity<UserDto> getUser(
User user = accountService.getUser(ControllerUtils.sanitize(username));
if (user != null) {
return new ResponseEntity<>(UserDto.fromUser(user), HttpStatus.OK);
} else {
}
else {
return new ResponseEntity<>(HttpStatus.NOT_FOUND);
}
}

@RequestMapping(method = RequestMethod.GET, value = "/rest/accounts/search/{partial:.+}")
@PreAuthorize("isAuthenticated()")
public ResponseEntity<Collection<UserDto>> findUsers(
@ApiParam(value = "Username", required = true) @PathVariable String partial) {

Collection<User> users = accountService.findUsers(ControllerUtils.sanitize(partial.toLowerCase()));
if (users != null) {
return new ResponseEntity<>(users.stream().map(UserDto::fromUser).collect(Collectors.toList()), HttpStatus.OK);
}
else {
return new ResponseEntity<>(HttpStatus.NOT_FOUND);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/**
* Copyright (c) 2019 Contributors to the Eclipse Foundation
*
* See the NOTICE file(s) distributed with this work for additional
* information regarding copyright ownership.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License 2.0 which is available at
* https://www.eclipse.org/legal/epl-2.0
*
* SPDX-License-Identifier: EPL-2.0
*/
package org.eclipse.vorto.repository.web.account.dto;

/**
* Represents a payload for requests to perform both:
* <ul>
* <li>
* Create a new technical user with given name, roles and authentication provider ID
* </li>
* <li>
* Add to the given namespace
* </li>
* </ul>
* @author mena-bosch
*/
public class TenantTechnicalUserDto extends TenantUserDto {
private String authenticationProviderId;
public String getAuthenticationProviderId() {
return authenticationProviderId;
}
public void setAuthenticationProviderId(String value) {
this.authenticationProviderId = value;
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright (c) 2018 Contributors to the Eclipse Foundation
* Copyright (c) 2018, 2019 Contributors to the Eclipse Foundation
*
* See the NOTICE file(s) distributed with this work for additional
* information regarding copyright ownership.
Expand All @@ -14,6 +14,7 @@

import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.when;

import org.eclipse.vorto.repository.AbstractIntegrationTest;
import org.eclipse.vorto.repository.core.IUserContext;
import org.eclipse.vorto.repository.domain.Role;
Expand Down Expand Up @@ -64,6 +65,24 @@ public void testCreateUserAlreadyExists() throws Exception {
accountService.create(user.getUsername(), "GITHUB", null);
}

/**
* Tests that creating a technical user and adding to a non-existing namespace fails.
*/
@Test(expected = IllegalArgumentException.class)
public void testCreateTechnicalUserAndAddToNonExistingNamespace() {
accountService.createTechnicalUserAndAddToTenant("doesNotExist", "pepe", "GITHUB", Role.USER);
}

/**
* Tests that creating a technical user with no authentication provider fails.
*/
@Test(expected = IllegalArgumentException.class)
public void testCreateTechnicalUserWithoutAuthenticationProvider() {
// this implicitly creates the "playground" namespace
User user = setupUserWithRoles("alex");
accountService.createTechnicalUserAndAddToTenant("playground", "pepe", null, Role.USER);
}

private User setupUserWithRoles(String username) {
return User.create(username, "GITHUB", null, new Tenant("playground"), Role.SYS_ADMIN, Role.MODEL_CREATOR);
}
Expand Down
Loading

0 comments on commit 1d174ed

Please sign in to comment.