eclipse-vorto#2208

* modal for technical user creation now also prompts for subject (required, pattern-validated) * DTO, controller and back-end now validate, handle and persist the subject * few changes in adding user to tenant, e.g. button disabled until some username typed (previous implementation was not working and loading animations are still useless pretty much all over the place still now) * added some failsafes when handling search vs actual usernames in form Signed-off-by: Menahem Julien Raccah Lisei <[email protected]>
bosch-io · Jan 9, 2020 · de5d3db · de5d3db
1 parent 1d174ed
commit de5d3db
Show file tree

Hide file tree

Showing 9 changed files with 109 additions and 44 deletions.
diff --git a/...pository-core/src/main/java/org/eclipse/vorto/repository/account/IUserAccountService.java b/...pository-core/src/main/java/org/eclipse/vorto/repository/account/IUserAccountService.java
@@ -17,6 +17,7 @@
 import org.eclipse.vorto.repository.domain.Role;
 import org.eclipse.vorto.repository.domain.Tenant;
 import org.eclipse.vorto.repository.domain.User;
+import org.eclipse.vorto.repository.web.account.dto.TenantTechnicalUserDto;
 import org.springframework.security.core.Authentication;
 
 /**
@@ -59,11 +60,11 @@ public interface IUserAccountService {
    * the given tenant.
    * @param tenantId
    * @param userId
-   * @param authenticationProviderId
+   * @param user
    * @param roles
    * @return always {@literal true} (fails with runtime exception if some operation failed).
    */
-  boolean createTechnicalUserAndAddToTenant(String tenantId, String userId, String authenticationProviderId, Role... roles);
+  boolean createTechnicalUserAndAddToTenant(String tenantId, String userId, TenantTechnicalUserDto user, Role... roles);
 
   /**
    * Returns if the particular user as the role in the Tenant

diff --git a/...re/src/main/java/org/eclipse/vorto/repository/account/impl/DefaultUserAccountService.java b/...re/src/main/java/org/eclipse/vorto/repository/account/impl/DefaultUserAccountService.java
@@ -33,6 +33,7 @@
 import org.eclipse.vorto.repository.tenant.repository.ITenantRepository;
 import org.eclipse.vorto.repository.tenant.repository.ITenantUserRepo;
 import org.eclipse.vorto.repository.utils.PreConditions;
+import org.eclipse.vorto.repository.web.account.dto.TenantTechnicalUserDto;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.context.ApplicationEventPublisher;
@@ -68,6 +69,14 @@ public class DefaultUserAccountService
 
   private ApplicationEventPublisher eventPublisher = null;
 
+  /**
+   * Defines the minimum validation requirement for a subject string. <br/>
+   * Set arbitrarily to 4+ alphanumeric characters for now. <br/>
+   * This is and should be reflected in the front-end validation - see resource
+   * {@literal createTechnicalUser.html}.
+   */
+  public static final String AUTHENTICATION_SUBJECT_VALIDATION_PATTERN = "^[a-zA-Z0-9]{4,}$";
+
   public void setApplicationEventPublisher(ApplicationEventPublisher applicationEventPublisher) {
     this.eventPublisher = applicationEventPublisher;
   }
@@ -135,23 +144,29 @@ private Tenant validateAndReturnTenant(String tenantId, String userId, String au
    * user exists already, instead.
    * @param tenantId
    * @param userId
-   * @param authenticationProviderId
+   * @param user
    * @param roles
    * @return
    */
-  public boolean createTechnicalUserAndAddToTenant(String tenantId, String userId, String authenticationProviderId, Role... roles) {
+  public boolean createTechnicalUserAndAddToTenant(String tenantId, String userId, TenantTechnicalUserDto user, Role... roles) {
+
+    String authenticationProviderId = user.getAuthenticationProviderId();
+    String subject = user.getSubject();
 
     Tenant tenant = validateAndReturnTenant(tenantId, userId, authenticationProviderId, roles);
 
-    // additional validation for authentication provider id
+    // additional validation for authentication provider id and subject
     PreConditions.notNullOrEmpty(authenticationProviderId, "authenticationProviderId");
+    PreConditions.withPattern(AUTHENTICATION_SUBJECT_VALIDATION_PATTERN, subject, "subject");
 
     // this creates and persists the technical user
-    User user = User.create(userId, authenticationProviderId, null, true);
-    userRepository.save(user);
+    User userToCreate = User.create(userId, authenticationProviderId, subject, true);
+    userRepository.save(userToCreate);
+
     // this creates and persists the "tenant user"
-    TenantUser tenantUser = TenantUser.createTenantUser(tenant, user, roles);
-    tenantUserRepo.save(tenantUser);
+    TenantUser tenantUserToCreate = TenantUser.createTenantUser(tenant, userToCreate, roles);
+    tenantUserRepo.save(tenantUserToCreate);
+
     eventPublisher.publishEvent(new AppEvent(this, userId, EventType.USER_ADDED));
     return true;
   }
@@ -165,7 +180,7 @@ public boolean createTechnicalUserAndAddToTenant(String tenantId, String userId,
    * 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...)},
+   * sibling method here {@link DefaultUserAccountService#createTechnicalUserAndAddToTenant(String, String, TenantTechnicalUserDto, 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

diff --git a/...itory/repository-core/src/main/java/org/eclipse/vorto/repository/utils/PreConditions.java b/...itory/repository-core/src/main/java/org/eclipse/vorto/repository/utils/PreConditions.java
@@ -12,10 +12,17 @@
  */
 package org.eclipse.vorto.repository.utils;
 
-import java.util.Collection;
 import com.google.common.base.Strings;
+import java.util.Collection;
 
 public class PreConditions {
+
+  public static void withPattern(String pattern, String item, String name) {
+    notNullOrEmpty(item, name);
+    if (!item.matches(pattern)) {
+      throw new IllegalArgumentException(String.format("%s '%s' does not match pattern: '%s'", name, item, pattern));
+    }
+  }
 
   public static void notNullOrEmpty(String item, String name) {
     if (Strings.nullToEmpty(item).trim().isEmpty()) {

diff --git a/...sitory-core/src/main/java/org/eclipse/vorto/repository/web/account/AccountController.java b/...sitory-core/src/main/java/org/eclipse/vorto/repository/web/account/AccountController.java
@@ -129,6 +129,10 @@ public ResponseEntity<Boolean> createTechnicalUserForTenant(
       return new ResponseEntity<>(HttpStatus.PRECONDITION_FAILED);
     }
 
+    if (Strings.nullToEmpty(user.getSubject()).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);
@@ -147,7 +151,7 @@ public ResponseEntity<Boolean> createTechnicalUserForTenant(
         return new ResponseEntity<>(HttpStatus.BAD_REQUEST);
       }
       return new ResponseEntity<>(
-        accountService.createTechnicalUserAndAddToTenant(tenantId, userId, user.getAuthenticationProviderId(), toRoles(user)),
+        accountService.createTechnicalUserAndAddToTenant(tenantId, userId, user, toRoles(user)),
         HttpStatus.OK
       );
     } catch (IllegalArgumentException e) {

diff --git a/...re/src/main/java/org/eclipse/vorto/repository/web/account/dto/TenantTechnicalUserDto.java b/...re/src/main/java/org/eclipse/vorto/repository/web/account/dto/TenantTechnicalUserDto.java
@@ -26,10 +26,19 @@
  */
 public class TenantTechnicalUserDto extends TenantUserDto {
   private String authenticationProviderId;
+  private String subject;
   public String getAuthenticationProviderId() {
     return authenticationProviderId;
   }
-  public void setAuthenticationProviderId(String value) {
+  public TenantTechnicalUserDto setAuthenticationProviderId(String value) {
     this.authenticationProviderId = value;
+    return this;
+  }
+  public String getSubject() {
+    return subject;
+  }
+  public TenantTechnicalUserDto setSubject(String value) {
+    this.subject = value;
+    return this;
   }
 }
diff --git a/...itory-core/src/test/java/org/eclipse/vorto/repository/account/UserAccountServiceTest.java b/...itory-core/src/test/java/org/eclipse/vorto/repository/account/UserAccountServiceTest.java
@@ -20,6 +20,7 @@
 import org.eclipse.vorto.repository.domain.Role;
 import org.eclipse.vorto.repository.domain.Tenant;
 import org.eclipse.vorto.repository.domain.User;
+import org.eclipse.vorto.repository.web.account.dto.TenantTechnicalUserDto;
 import org.eclipse.vorto.repository.workflow.IWorkflowService;
 import org.eclipse.vorto.repository.workflow.impl.DefaultWorkflowService;
 import org.junit.Before;
@@ -70,7 +71,11 @@ public void testCreateUserAlreadyExists() throws Exception {
    */
   @Test(expected = IllegalArgumentException.class)
   public void testCreateTechnicalUserAndAddToNonExistingNamespace() {
-    accountService.createTechnicalUserAndAddToTenant("doesNotExist", "pepe", "GITHUB", Role.USER);
+    accountService.createTechnicalUserAndAddToTenant(
+        "doesNotExist", "pepe",
+        new TenantTechnicalUserDto().setAuthenticationProviderId("GITHUB").setSubject("subject"),
+        Role.USER
+    );
   }
 
   /**

diff --git a/repository/repository-web/dist/js/controllers/manage/tenantUserManagement.js b/repository/repository-web/dist/js/controllers/manage/tenantUserManagement.js
@@ -113,6 +113,7 @@ repositoryControllers.controller("createOrUpdateUserController",
         $scope.userPartial = "";
         $scope.selectedUser = null;
         $scope.retrievedUsers = [];
+        $scope.technicalUserSubject = null;
 
         $scope.selectUser = function(user) {
             if (user) {
@@ -123,15 +124,15 @@ repositoryControllers.controller("createOrUpdateUserController",
         }
 
         $scope.highlightUser = function(user) {
-            var element = document.getElementById(user.username);
+            let element = document.getElementById(user.username);
             if (element) {
                 element.style.backgroundColor = '#7fc6e7';
                 element.style.color = '#ffffff'
             }
         }
 
         $scope.unhighlightUser = function(user) {
-            var element = document.getElementById(user.username);
+            let element = document.getElementById(user.username);
             if (element) {
                 element.style.backgroundColor = 'initial';
                 element.style.color = 'initial';
@@ -160,16 +161,15 @@ repositoryControllers.controller("createOrUpdateUserController",
                 $scope.retrievedUsers = [];
                 $scope.selectedUser = null;
             }
+            $scope.toggleSubmitButton();
         };
 
-        //$scope.findUsers();
-
         $scope.cancel = function() {
             $uibModalInstance.dismiss("Canceled.");  
         };
 
         $scope.promptCreateNewTechnicalUser = function() {
-            var modalInstance = $uibModal.open({
+            let modalInstance = $uibModal.open({
                 animation: true,
                 templateUrl: "webjars/repository-web/dist/partials/admin/createTechnicalUser.html",
                 size: "md",
@@ -194,24 +194,42 @@ repositoryControllers.controller("createOrUpdateUserController",
             $http.post("./rest/tenants/" + $scope.tenant.tenantId + "/users/" + $scope.user.username, {
                 "username": $scope.user.username,
                 "roles" : $scope.getRoles($scope.user),
-                "authenticationProviderId": $scope.selectedAuthenticationProviderId
+                "authenticationProviderId": $scope.selectedAuthenticationProviderId,
+                "subject": $scope.technicalUserSubject
             })
-            .then(function(result) {
-                $uibModalInstance.close($scope.user);
-            }, function(reason) {
-                $scope.errorMessage = "Creation of technical user " +
-                    $scope.user.username + " in namespace " +
-                    $scope.tenant.defaultNamespace + " failed. ";
-            });
+            .then(
+                function(result) {
+                    $uibModalInstance.close($scope.user);
+                },
+                function(reason) {
+                    $scope.errorMessage = "Creation of technical user " +
+                        $scope.user.username + " in namespace " +
+                        $scope.tenant.defaultNamespace + " failed. ";
+                }
+            );
         };
 
+        $scope.toggleSubmitButton = function() {
+            let button = document.getElementById("submitButton");
+            if (button) {
+                button.disabled = !(
+                    ($scope.user && $scope.user.username)
+                    ||
+                    $scope.userPartial
+                    ||
+                    $scope.selectedUser
+                );
+            }
+            return button && !button.disabled;
+        }
+
         $scope.addOrUpdateUser = function() {
             // adds username to scope user by either using selected user from
             // drop-down if any, or using the string in user's input box
             if ($scope.selectedUser) {
-                $scope.user.username = $scope.selectedUser.username;
+                $scope.user = $scope.selectedUser;
             }
-            else {
+            else if ($scope.userPartial) {
                 $scope.user.username = $scope.userPartial;
             }
             $scope.validate($scope.user, function(result) {

diff --git a/repository/repository-web/dist/partials/admin/createOrUpdateUser.html b/repository/repository-web/dist/partials/admin/createOrUpdateUser.html
@@ -1,9 +1,5 @@
 <form class="form-horizontal" name="tenantForm">
 	<style>
-		.ng-hide:not(.ng-hide-animate) {
-			display:block;
-			visibility: hidden;
-		}
 		.dropDownUL {
 			list-style-type:none;
 			margin:0;
@@ -35,7 +31,7 @@ <h4 class="modal-title">{{ mode }} User <a class="help-icon"
 				<div class="col-sm-10">
 					<input autofocus type="search" class="search-box-query-filter form-control search-input"
 					 id="userId" placeholder="userId" ng-model="userPartial" ng-readonly="user.edit"
-					 ng-model-options="{ debounce: 1000 }" ng-change="findUsers()">
+					 ng-model-options="{ debounce: 200 }" ng-change="findUsers()" ng-focus="toggleSubmitButton()">
 				</div>
 			</div>
 			<div class="row">
@@ -116,7 +112,8 @@ <h4 class="modal-title">{{ mode }} User <a class="help-icon"
 		<div class="form-group">
 			<div class="col-sm-12">
 				<button type="button" class="btn btn-default pull-right" ng-click="cancel()">Cancel</button>
-				<button type="button" class="btn btn-primary pull-right" ng-click="addOrUpdateUser()" ng-disabled="isCurrentlyAddingOrUpdating">
+				<!-- ng-disabled does not seem to work properly or ever refresh -->
+				<button id="submitButton" type="button" class="btn btn-primary pull-right" ng-click="addOrUpdateUser()" ng-disabled="isCurrentlyAddingOrUpdating">
 					<i class="fa fa-refresh fa-spin" ng-show="isCurrentlyAddingOrUpdating"></i>{{ mode }}
 				</button>
 			</div>

diff --git a/repository/repository-web/dist/partials/admin/createTechnicalUser.html b/repository/repository-web/dist/partials/admin/createTechnicalUser.html
@@ -6,32 +6,41 @@ <h4 class="modal-title">Create technical user
        title="Read more about managing Collaborators"><span class="fa fa-question-circle" /></a>
   </h4>
 </div>
-<div class="modal-body">
+<div class="modal-header">
   <div>
-      User <i>{{user.username}}</i> does not exist. <br/><br/>
-      Create a new technical user and add it to the <i>{{tenant.defaultNamespace}}</i> namespace by selecting the
-      desired authentication provider below.
-    <hr/>
+    <h5>User <i>{{user.username}}</i> does not exist. </h5>
+    <br/>
+    Create a new technical user and add it to the <i>{{tenant.defaultNamespace}}</i> namespace by:
+    <br/>
+    <br/>
+    <ul>
+        <li>Choosing a subject</li>
+        <li>Selecting the desired authentication provider</li>
+      </ul>
   </div>
+</div>
 
-
+<div class="modal-body">
   <div class="row" ng-show="errorMessage !== ''">
     <div class="alert alert-danger" role="alert">{{ errorMessage }}</div>
   </div>
 
-  <div class="table">
-    <br/>
+  <div class="form-group">
+    <!-- -->
+    <input ng-pattern="/^[a-zA-Z0-9]{4,}$/" class="form-control" id="subject" type="text" name="subject" ng-model=technicalUserSubject placeholder="Subject" required>
+    <hr/>
     <div ng-repeat="provider in context.providers" class="table-row-cell" >
         <input id={{provider.id}} type="radio" name="authorizationProvider" ng-model=$parent.selectedAuthenticationProviderId class="btn btn-default" value={{provider.id}} />
         <img src="{{provider.logoHref}}" alt="Select {{provider.label}}"/>
         <label  for={{provider.id}}>{{provider.label}}</label>
     </div>
   </div>
 </div>
+
 <div class="modal-footer">
   <div class="form-group">
     <button type="button" class="btn btn-default pull-right" ng-click="cancel()">Cancel</button>
-    <button type="button" class="btn btn-primary pull-right" ng-click="createNewTechnicalUser()" ng-disabled="isCurrentlyAddingOrUpdating || !selectedAuthenticationProviderId">
+    <button type="button" class="btn btn-primary pull-right" ng-click="createNewTechnicalUser()" ng-disabled="isCurrentlyAddingOrUpdating || !selectedAuthenticationProviderId || !technicalUserSubject">
       <i class="fa fa-refresh fa-spin" ng-show="isCurrentlyAddingOrUpdating"></i> Create
     </button>
   </div>