You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@archiva.apache.org by ma...@apache.org on 2020/11/29 12:28:32 UTC

[archiva-redback-core] branch master updated: Improving Role service V2

This is an automated email from the ASF dual-hosted git repository.

martin_s pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/archiva-redback-core.git


The following commit(s) were added to refs/heads/master by this push:
     new 43752fa  Improving Role service V2
43752fa is described below

commit 43752fa7a38dfd25192dfd719e99c1e61257cb85
Author: Martin Stockhammer <ma...@apache.org>
AuthorDate: Sun Nov 29 13:28:25 2020 +0100

    Improving Role service V2
---
 .../archiva/redback/rest/api/MessageKeys.java      |   5 +-
 .../redback/rest/api/model/v2/BaseRoleInfo.java    |  30 ++-
 .../redback/rest/api/model/v2/BaseUserInfo.java    |  30 +++
 .../archiva/redback/rest/api/model/v2/Role.java    |  27 ++-
 .../redback/rest/api/model/v2/RoleInfo.java        |  16 --
 .../redback/rest/api/services/v2/RoleService.java  |  13 +-
 .../interceptors/JacksonJsonConfigurator.java      |   1 +
 .../rest/services/v2/DefaultRoleService.java       |  67 ++++--
 .../rest/services/v2/NativeRoleServiceTest.java    | 229 +++++++++++++++++++++
 .../archiva/redback/role/DefaultRoleManager.java   |  16 +-
 .../redback/role/PermanentRoleDeletionInvalid.java |  35 ++++
 .../template/DefaultRoleTemplateProcessor.java     |   3 +-
 12 files changed, 408 insertions(+), 64 deletions(-)

diff --git a/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/MessageKeys.java b/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/MessageKeys.java
index 51f762d..506ab8f 100644
--- a/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/MessageKeys.java
+++ b/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/MessageKeys.java
@@ -27,7 +27,8 @@ public interface MessageKeys
     String ERR_USERMANAGER_FAIL = "rb.usermanager_error";
     String ERR_ROLEMANAGER_FAIL = "rb.rolemanager_error";
     String ERR_RBACMANAGER_FAIL = "rb.rbacmanager_error";
-    String ERR_KEYMANAGER_FAIL = "reback:keymanager_error";
+    String ERR_KEYMANAGER_FAIL = "rb.keymanager_error";
+    String ERR_EMPTY_DATA = "rb.empty_data_received";
     String ERR_INVALID_POST_DATA = "rb.invalid_post_data";
     String ERR_USER_EXISTS = "rb.user.exists";
     String ERR_USER_ID_EMPTY = "rb.user.id.empty";
@@ -52,6 +53,8 @@ public interface MessageKeys
     String ERR_ROLE_EXISTS = "rb.role.exists";
     // A template instance exists. With arguments templateId, resource
     String ERR_ROLE_INSTANCE_EXISTS = "rb.role.instance.exists";
+    String ERR_ROLE_ID_INVALID = "rb.role.invalid_id";
+    String ERR_ROLE_DELETION_WITH_PERMANENT_FLAG = "rb.role.deletion_with_permanent_flag";
 
     String ERR_AUTH_BAD_CODE = "rb.auth.bad_authorization_code";
     String ERR_AUTH_INVALID_CREDENTIALS = "rb.auth.invalid_credentials";
diff --git a/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/model/v2/BaseRoleInfo.java b/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/model/v2/BaseRoleInfo.java
index 126a5e0..6ba30e3 100644
--- a/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/model/v2/BaseRoleInfo.java
+++ b/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/model/v2/BaseRoleInfo.java
@@ -191,17 +191,6 @@ public class BaseRoleInfo implements Serializable
     }
 
 
-    @Override
-    public boolean equals( Object o )
-    {
-        if ( this == o ) return true;
-        if ( o == null || getClass( ) != o.getClass( ) ) return false;
-
-        BaseRoleInfo that = (BaseRoleInfo) o;
-
-        return id.equals( that.id );
-    }
-
     @Schema( description = "If true, the role is assignable to users or roles. Otherwise, it can be used only as parent role.")
     public boolean isAssignable()
     {
@@ -259,4 +248,23 @@ public class BaseRoleInfo implements Serializable
     public boolean isNotChild() {
         return !isChild;
     }
+
+    @Override
+    public boolean equals( Object o )
+    {
+        if ( this == o ) return true;
+        if ( o == null || getClass( ) != o.getClass( ) ) return false;
+
+        BaseRoleInfo that = (BaseRoleInfo) o;
+
+        if ( permanent != that.permanent ) return false;
+        if ( isTemplateInstance != that.isTemplateInstance ) return false;
+        if ( assignable != that.assignable ) return false;
+        if ( !id.equals( that.id ) ) return false;
+        if ( !name.equals( that.name ) ) return false;
+        if ( description != null ? !description.equals( that.description ) : that.description != null ) return false;
+        if ( modelId != null ? !modelId.equals( that.modelId ) : that.modelId != null ) return false;
+        if ( resource != null ? !resource.equals( that.resource ) : that.resource != null ) return false;
+        return applicationId != null ? applicationId.equals( that.applicationId ) : that.applicationId == null;
+    }
 }
diff --git a/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/model/v2/BaseUserInfo.java b/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/model/v2/BaseUserInfo.java
index 7903a1c..8e19f01 100644
--- a/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/model/v2/BaseUserInfo.java
+++ b/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/model/v2/BaseUserInfo.java
@@ -64,4 +64,34 @@ public class BaseUserInfo implements Serializable
     {
         this.id = id;
     }
+
+    @Override
+    public String toString( )
+    {
+        final StringBuilder sb = new StringBuilder( "BaseUserInfo{" );
+        sb.append( "userId='" ).append( userId ).append( '\'' );
+        sb.append( ", id='" ).append( id ).append( '\'' );
+        sb.append( '}' );
+        return sb.toString( );
+    }
+
+    @Override
+    public boolean equals( Object o )
+    {
+        if ( this == o ) return true;
+        if ( o == null || getClass( ) != o.getClass( ) ) return false;
+
+        BaseUserInfo that = (BaseUserInfo) o;
+
+        if ( !userId.equals( that.userId ) ) return false;
+        return id.equals( that.id );
+    }
+
+    @Override
+    public int hashCode( )
+    {
+        int result = userId.hashCode( );
+        result = 31 * result + id.hashCode( );
+        return result;
+    }
 }
diff --git a/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/model/v2/Role.java b/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/model/v2/Role.java
index f5909a9..c222352 100644
--- a/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/model/v2/Role.java
+++ b/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/model/v2/Role.java
@@ -35,7 +35,9 @@ public class Role implements Serializable
     protected String name;
     protected String id;
     protected String description;
-    protected boolean permanent = false;
+    protected Boolean permanent;
+    protected Boolean assignable;
+
     /**
      * The ids of all the assigned users.
      */
@@ -64,16 +66,18 @@ public class Role implements Serializable
     }
 
     @Schema( description = "True, if this role cannot be deleted.")
-    public boolean isPermanent()
+    public Boolean isPermanent()
     {
         return permanent;
     }
 
-    public void setPermanent( boolean permanent )
+    @Schema( description = "True, if this role can be assigned" )
+    public Boolean isAssignable( )
     {
-        this.permanent = permanent;
+        return assignable;
     }
 
+
     @Schema(description = "The identifier of this role")
     public String getId( )
     {
@@ -101,4 +105,19 @@ public class Role implements Serializable
     }
 
 
+    public Boolean getPermanent( )
+    {
+        return permanent;
+    }
+
+    public void setPermanent( Boolean permanent )
+    {
+        this.permanent = permanent;
+    }
+
+
+    public void setAssignable( Boolean assignable )
+    {
+        this.assignable = assignable;
+    }
 }
diff --git a/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/model/v2/RoleInfo.java b/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/model/v2/RoleInfo.java
index aa77836..01a2698 100644
--- a/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/model/v2/RoleInfo.java
+++ b/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/model/v2/RoleInfo.java
@@ -160,23 +160,7 @@ public class RoleInfo extends BaseRoleInfo
         return sb.toString( );
     }
 
-    @Override
-    public boolean equals( Object o )
-    {
-        if ( this == o )
-        {
-            return true;
-        }
-        if ( o == null || getClass() != o.getClass() )
-        {
-            return false;
-        }
-
-        RoleInfo role = (RoleInfo) o;
 
-        return Objects.equals( getName( ), role.getName( ) );
-
-    }
 
 
 }
diff --git a/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/services/v2/RoleService.java b/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/services/v2/RoleService.java
index de25961..818e650 100644
--- a/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/services/v2/RoleService.java
+++ b/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/services/v2/RoleService.java
@@ -28,18 +28,12 @@ import io.swagger.v3.oas.annotations.security.SecurityRequirement;
 import io.swagger.v3.oas.annotations.tags.Tag;
 import org.apache.archiva.redback.authorization.RedbackAuthorization;
 import org.apache.archiva.redback.integration.security.role.RedbackRoleConstants;
-import org.apache.archiva.redback.rest.api.model.ActionStatus;
-import org.apache.archiva.redback.rest.api.model.Application;
-import org.apache.archiva.redback.rest.api.model.ApplicationRoles;
 import org.apache.archiva.redback.rest.api.model.RedbackRestError;
-import org.apache.archiva.redback.rest.api.model.Role;
-import org.apache.archiva.redback.rest.api.model.User;
-import org.apache.archiva.redback.rest.api.model.VerificationStatus;
 import org.apache.archiva.redback.rest.api.model.v2.PagedResult;
+import org.apache.archiva.redback.rest.api.model.v2.Role;
 import org.apache.archiva.redback.rest.api.model.v2.RoleInfo;
 import org.apache.archiva.redback.rest.api.services.RedbackServiceException;
 
-import javax.ws.rs.Consumes;
 import javax.ws.rs.DELETE;
 import javax.ws.rs.DefaultValue;
 import javax.ws.rs.GET;
@@ -51,7 +45,6 @@ import javax.ws.rs.Path;
 import javax.ws.rs.PathParam;
 import javax.ws.rs.Produces;
 import javax.ws.rs.QueryParam;
-import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 import java.util.List;
 
@@ -386,11 +379,13 @@ public interface RoleService
             ),
             @ApiResponse( responseCode = "404", description = "Role does not exist",
                 content = @Content(mediaType = APPLICATION_JSON, schema = @Schema(implementation = RedbackRestError.class )) ),
+            @ApiResponse( responseCode = "422", description = "Role id does not match the id of the request path",
+                content = @Content(mediaType = APPLICATION_JSON, schema = @Schema(implementation = RedbackRestError.class )) ),
             @ApiResponse( responseCode = "403", description = "The authenticated user has not the permission for role assignment.",
                 content = @Content(mediaType = APPLICATION_JSON, schema = @Schema(implementation = RedbackRestError.class )) )
         }
     )
-    RoleInfo updateRole( @QueryParam("roleId") String roleId, Role role )
+    RoleInfo updateRole( @PathParam("roleId") String roleId, Role role )
     throws RedbackServiceException;
 
 
diff --git a/redback-integrations/redback-rest/redback-rest-services/src/main/java/org/apache/archiva/redback/rest/services/interceptors/JacksonJsonConfigurator.java b/redback-integrations/redback-rest/redback-rest-services/src/main/java/org/apache/archiva/redback/rest/services/interceptors/JacksonJsonConfigurator.java
index 6429e89..93e2cf8 100644
--- a/redback-integrations/redback-rest/redback-rest-services/src/main/java/org/apache/archiva/redback/rest/services/interceptors/JacksonJsonConfigurator.java
+++ b/redback-integrations/redback-rest/redback-rest-services/src/main/java/org/apache/archiva/redback/rest/services/interceptors/JacksonJsonConfigurator.java
@@ -62,6 +62,7 @@ public class JacksonJsonConfigurator
         objectMapper.setDateFormat( new SimpleDateFormat( "yyyy-MM-dd'T'HH:mm:ss.SSSZ" ) );
 
         objectMapperV2.disable( DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES );
+        objectMapperV2.disable( DeserializationFeature.FAIL_ON_MISSING_CREATOR_PROPERTIES );
         objectMapperV2.enable( DeserializationFeature.READ_UNKNOWN_ENUM_VALUES_AS_NULL );
         objectMapperV2.enable( DeserializationFeature.USE_LONG_FOR_INTS );
         objectMapperV2.setAnnotationIntrospector( new JaxbAnnotationIntrospector( objectMapper.getTypeFactory( ) ) );
diff --git a/redback-integrations/redback-rest/redback-rest-services/src/main/java/org/apache/archiva/redback/rest/services/v2/DefaultRoleService.java b/redback-integrations/redback-rest/redback-rest-services/src/main/java/org/apache/archiva/redback/rest/services/v2/DefaultRoleService.java
index a1fae18..a95b499 100644
--- a/redback-integrations/redback-rest/redback-rest-services/src/main/java/org/apache/archiva/redback/rest/services/v2/DefaultRoleService.java
+++ b/redback-integrations/redback-rest/redback-rest-services/src/main/java/org/apache/archiva/redback/rest/services/v2/DefaultRoleService.java
@@ -18,30 +18,22 @@ package org.apache.archiva.redback.rest.services.v2;
  * under the License.
  */
 
-import org.apache.archiva.redback.integration.security.role.RedbackRoleConstants;
-import org.apache.archiva.redback.integration.util.RoleSorter;
-import org.apache.archiva.redback.rbac.Permission;
 import org.apache.archiva.redback.rbac.RBACManager;
 import org.apache.archiva.redback.rbac.RbacManagerException;
 import org.apache.archiva.redback.rbac.RbacObjectNotFoundException;
-import org.apache.archiva.redback.rbac.Resource;
 import org.apache.archiva.redback.rest.api.MessageKeys;
 import org.apache.archiva.redback.rest.api.model.ErrorMessage;
-import org.apache.archiva.redback.rest.api.model.Role;
-import org.apache.archiva.redback.rest.api.model.RoleTemplate;
 import org.apache.archiva.redback.rest.api.model.v2.PagedResult;
+import org.apache.archiva.redback.rest.api.model.v2.Role;
 import org.apache.archiva.redback.rest.api.model.v2.RoleInfo;
 import org.apache.archiva.redback.rest.api.services.RedbackServiceException;
 import org.apache.archiva.redback.rest.api.services.v2.RoleService;
-import org.apache.archiva.redback.rest.services.RedbackAuthenticationThreadLocal;
-import org.apache.archiva.redback.rest.services.RedbackRequestInformation;
+import org.apache.archiva.redback.role.PermanentRoleDeletionInvalid;
 import org.apache.archiva.redback.role.RoleExistsException;
 import org.apache.archiva.redback.role.RoleManager;
 import org.apache.archiva.redback.role.RoleManagerException;
 import org.apache.archiva.redback.role.RoleNotFoundException;
-import org.apache.archiva.redback.role.model.ModelTemplate;
 import org.apache.archiva.redback.role.util.RoleModelUtils;
-import org.apache.archiva.redback.users.User;
 import org.apache.archiva.redback.users.UserManager;
 import org.apache.archiva.redback.users.UserManagerException;
 import org.apache.archiva.redback.users.UserNotFoundException;
@@ -57,16 +49,11 @@ import javax.servlet.http.HttpServletResponse;
 import javax.ws.rs.core.Context;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriInfo;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
-import java.util.Set;
 import java.util.function.BiPredicate;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
@@ -311,6 +298,9 @@ public class DefaultRoleService extends BaseRedbackService
             roleManager.removeTemplatedRole( templateId, resource );
             return Response.ok( ).build( );
         }
+        catch ( PermanentRoleDeletionInvalid e ) {
+            throw new RedbackServiceException( ErrorMessage.of( MessageKeys.ERR_ROLE_DELETION_WITH_PERMANENT_FLAG, RoleModelUtils.getRoleId( templateId, resource ) ), 400 );
+        }
         catch ( RoleNotFoundException e ) {
             throw new RedbackServiceException( ErrorMessage.of( MessageKeys.ERR_ROLE_INSTANCE_NOT_FOUND, templateId, resource ), 404 );
         }
@@ -433,7 +423,52 @@ public class DefaultRoleService extends BaseRedbackService
     @Override
     public RoleInfo updateRole( String roleId, Role role ) throws RedbackServiceException
     {
-        return null;
+        try
+        {
+            if (role==null) {
+                throw new RedbackServiceException( ErrorMessage.of( MessageKeys.ERR_EMPTY_DATA ), 400 );
+            }
+            if ( !StringUtils.equals( roleId, role.getId( ) ) )
+            {
+                throw new RedbackServiceException( ErrorMessage.of( MessageKeys.ERR_ROLE_ID_INVALID ), 422 );
+            }
+            org.apache.archiva.redback.rbac.Role rbacRole = rbacManager.getRoleById( roleId );
+            if (StringUtils.isNotEmpty( role.getName()) && !StringUtils.equals(rbacRole.getName(), role.getName()) ) {
+                rbacRole.setName( role.getName( ) );
+            }
+            if (StringUtils.isNotEmpty( role.getDescription()) && !StringUtils.equals(rbacRole.getDescription(), role.getDescription()) ) {
+                rbacRole.setDescription( role.getDescription( ) );
+            }
+            if (role.isPermanent()!=null && rbacRole.isPermanent()!=role.isPermanent().booleanValue()) {
+                rbacRole.setPermanent( role.isPermanent( ) );
+            }
+            if (role.isAssignable()!=null && rbacRole.isAssignable()!=role.isAssignable().booleanValue()) {
+                rbacRole.setAssignable( role.isAssignable( ) );
+            }
+            if (role.getAssignedUsers()!=null && role.getAssignedUsers().size()>0) {
+                role.getAssignedUsers().stream().forEach( user ->
+                {
+                    try
+                    {
+                        roleManager.assignRole( role.getId( ), user.getUserId( ) );
+                    }
+                    catch ( RoleManagerException e )
+                    {
+                        // silently ignore
+                    }
+                }
+                );
+            }
+            org.apache.archiva.redback.rbac.Role updatedRole = rbacManager.saveRole( rbacRole );
+            return getRoleInfo( updatedRole );
+        }
+        catch (RbacObjectNotFoundException e) {
+            throw new RedbackServiceException( ErrorMessage.of( MessageKeys.ERR_ROLE_NOT_FOUND, roleId ), 404 );
+        }
+        catch ( RbacManagerException e )
+        {
+            throw new RedbackServiceException( ErrorMessage.of( MessageKeys.ERR_RBACMANAGER_FAIL, e.getMessage() ));
+        }
     }
 
 
diff --git a/redback-integrations/redback-rest/redback-rest-services/src/test/java/org/apache/archiva/redback/rest/services/v2/NativeRoleServiceTest.java b/redback-integrations/redback-rest/redback-rest-services/src/test/java/org/apache/archiva/redback/rest/services/v2/NativeRoleServiceTest.java
index b62b4fa..8ded8e8 100644
--- a/redback-integrations/redback-rest/redback-rest-services/src/test/java/org/apache/archiva/redback/rest/services/v2/NativeRoleServiceTest.java
+++ b/redback-integrations/redback-rest/redback-rest-services/src/test/java/org/apache/archiva/redback/rest/services/v2/NativeRoleServiceTest.java
@@ -163,6 +163,7 @@ public class NativeRoleServiceTest extends AbstractNativeRestServices
 
     }
 
+
     @Test
     void checkTemplatedRole( )
     {
@@ -721,4 +722,232 @@ public class NativeRoleServiceTest extends AbstractNativeRestServices
         }
     }
 
+    @Test
+    void updateRole( )
+    {
+        String token = getAdminToken( );
+        try
+        {
+            Response response = given( ).spec( getRequestSpec( token ) ).contentType( JSON )
+                .when( )
+                .put( "template/archiva-repository-manager/repository13" )
+                .then( ).statusCode( 201 ).extract( ).response( );
+            assertNotNull( response );
+            RoleInfo roleInfo = response.getBody( ).jsonPath( ).getObject( "", RoleInfo.class );
+            Map<String, Object> jsonAsMap = new HashMap<>( );
+            jsonAsMap.put( "id", roleInfo.getId( ) );
+            jsonAsMap.put( "name", roleInfo.getName( ) );
+            jsonAsMap.put( "description", "This description was updated." );
+            response = given( ).spec( getRequestSpec( token ) ).contentType( JSON )
+                .when( )
+                .body( jsonAsMap )
+                .patch( roleInfo.getId( ) )
+                .then( ).statusCode( 200 ).extract( ).response( );
+            assertNotNull( response );
+            RoleInfo updatedRole = response.getBody( ).jsonPath( ).getObject( "", RoleInfo.class );
+            assertEquals( roleInfo.getId( ), updatedRole.getId( ) );
+            assertEquals( roleInfo.getName( ), updatedRole.getName( ) );
+            assertEquals( "This description was updated.", updatedRole.getDescription( ) );
+            assertEquals( true, updatedRole.isAssignable( ) );
+            assertEquals( false, updatedRole.isPermanent( ) );
+            assertArrayEquals( roleInfo.getAssignedUsers( ).toArray( ), updatedRole.getAssignedUsers( ).toArray( ) );
+        }
+        finally
+        {
+            given( ).spec( getRequestSpec( token ) ).contentType( JSON )
+                .when( )
+                .delete( "template/archiva-repository-manager/repository13" )
+                .then( ).statusCode( 200 );
+            given( ).spec( getRequestSpec( token ) ).contentType( JSON )
+                .when( )
+                .delete( "template/archiva-repository-observer/repository13" )
+                .then( ).statusCode( 200 );
+        }
+    }
+
+    @Test
+    void updateRoleWithAssignedUsers( )
+    {
+        String token = getAdminToken( );
+        Map<String, Object> jsonAsMap = new HashMap<>( );
+        jsonAsMap.put( "user_id", "aragorn" );
+        jsonAsMap.put( "email", "aragorn@lordoftherings.org" );
+        jsonAsMap.put( "full_name", "Aragorn King of Gondor " );
+        jsonAsMap.put( "password", "pAssw0rD" );
+        String id = "";
+
+        try
+        {
+            given( ).spec( getRequestSpec( token, getUserServicePath( ) ) ).contentType( JSON )
+                .body( jsonAsMap )
+                .when( )
+                .post( )
+                .then( ).statusCode( 201 );
+
+            Response response = given( ).spec( getRequestSpec( token ) ).contentType( JSON )
+                .when( )
+                .put( "template/archiva-repository-manager/repository14" )
+                .then( ).statusCode( 201 ).extract( ).response( );
+            assertNotNull( response );
+            RoleInfo roleInfo = response.getBody( ).jsonPath( ).getObject( "", RoleInfo.class );
+            id = roleInfo.getId( );
+            jsonAsMap = new HashMap<>( );
+            jsonAsMap.put( "id", roleInfo.getId( ) );
+            jsonAsMap.put( "name", roleInfo.getName( ) );
+            jsonAsMap.put( "description", "New description" );
+            jsonAsMap.put( "assignable", "false" );
+            jsonAsMap.put( "permanent", "true" );
+
+            HashMap<Object, Object> aragornMap = new HashMap<>( );
+            aragornMap.put( "id", "jpa:aragorn" );
+            aragornMap.put( "user_id", "aragorn" );
+            jsonAsMap.put( "assigned_users", Arrays.asList( aragornMap ) );
+            response = given( ).spec( getRequestSpec( token ) ).contentType( JSON )
+                .when( )
+                .body( jsonAsMap )
+                .patch( roleInfo.getId( ) )
+                .then( ).statusCode( 200 ).extract( ).response( );
+            assertNotNull( response );
+            RoleInfo updatedRole = response.getBody( ).jsonPath( ).getObject( "", RoleInfo.class );
+            assertEquals( roleInfo.getId( ), updatedRole.getId( ) );
+            assertEquals( roleInfo.getName( ), updatedRole.getName( ) );
+            assertEquals( "New description", updatedRole.getDescription( ) );
+            assertEquals( false, updatedRole.isAssignable( ) );
+            assertEquals( true, updatedRole.isPermanent( ) );
+            assertEquals( 2, updatedRole.getAssignedUsers( ).size() );
+            assertTrue( updatedRole.getAssignedUsers( ).stream( ).filter( user -> "aragorn".equals( user.getUserId( ) ) ).findAny().isPresent() );
+        }
+        finally
+        {
+            // Switching back permanent flag
+            jsonAsMap = new HashMap<>( );
+            jsonAsMap.put( "id", id );
+            jsonAsMap.put( "permanent", "false" );
+            given( ).spec( getRequestSpec( token ) ).contentType( JSON )
+                .when( )
+                .body( jsonAsMap )
+                .patch( id )
+                .then( ).statusCode( 200 ).extract( ).response( );
+
+            given( ).spec( getRequestSpec( token, getUserServicePath( ) ) ).contentType( JSON )
+                .when( )
+                .delete( "aragorn" ).then().statusCode( 200 );
+
+            given( ).spec( getRequestSpec( token ) ).contentType( JSON )
+                .when( )
+                .delete( "template/archiva-repository-manager/repository14" )
+                .then( ).statusCode( 200 );
+            given( ).spec( getRequestSpec( token ) ).contentType( JSON )
+                .when( )
+                .delete( "template/archiva-repository-observer/repository14" )
+                .then( ).statusCode( 200 );
+        }
+    }
+
+
+    @Test
+    void updateRoleWithBadId( )
+    {
+        String token = getAdminToken( );
+        try
+        {
+            Response response = given( ).spec( getRequestSpec( token ) ).contentType( JSON )
+                .when( )
+                .put( "template/archiva-repository-manager/repository15" )
+                .then( ).statusCode( 201 ).extract( ).response( );
+            assertNotNull( response );
+            RoleInfo roleInfo = response.getBody( ).jsonPath( ).getObject( "", RoleInfo.class );
+            Map<String, Object> jsonAsMap = new HashMap<>( );
+            jsonAsMap.put( "id", "abcdefg" );
+            jsonAsMap.put( "name", roleInfo.getName( ) );
+            jsonAsMap.put( "description", "This description was updated." );
+            given( ).spec( getRequestSpec( token ) ).contentType( JSON )
+                .when( )
+                .body( jsonAsMap )
+                .patch( roleInfo.getId( ) )
+                .then( ).statusCode( 422 );
+        }
+        finally
+        {
+            given( ).spec( getRequestSpec( token ) ).contentType( JSON )
+                .when( )
+                .delete( "template/archiva-repository-manager/repository15" )
+                .then( ).statusCode( 200 );
+            given( ).spec( getRequestSpec( token ) ).contentType( JSON )
+                .when( )
+                .delete( "template/archiva-repository-observer/repository15" )
+                .then( ).statusCode( 200 );
+        }
+    }
+
+    @Test
+    void deleteTemplatedRolePermanentThrowsError( )
+    {
+        String token = getAdminToken( );
+        String id = "";
+        try
+        {
+            Response response = given( ).spec( getRequestSpec( token ) ).contentType( JSON )
+                .when( )
+                .put( "template/archiva-repository-manager/repository16" )
+                .then( ).statusCode( 201 ).extract( ).response( );
+            assertNotNull( response );
+            RoleInfo roleInfo = response.getBody( ).jsonPath( ).getObject( "", RoleInfo.class );
+            id = roleInfo.getId( );
+            Map<String, Object> jsonAsMap = new HashMap<>( );
+            jsonAsMap.put( "id", roleInfo.getId( ) );
+            jsonAsMap.put( "name", roleInfo.getName( ) );
+            jsonAsMap.put( "description", "This description was updated." );
+            jsonAsMap.put( "permanent", "true" );
+            response = given( ).spec( getRequestSpec( token ) ).contentType( JSON )
+                .when( )
+                .body( jsonAsMap )
+                .patch( roleInfo.getId( ) )
+                .then( ).statusCode( 200 ).extract( ).response( );
+            assertNotNull( response );
+
+            given( ).spec( getRequestSpec( token ) ).contentType( JSON )
+                .when( )
+                .delete( "template/archiva-repository-manager/repository16" )
+                .then( ).statusCode( 400 );
+
+        }
+        finally
+        {
+            Map<String, Object> jsonAsMap = new HashMap<>( );
+            jsonAsMap.put( "id", id );
+            jsonAsMap.put( "permanent", "false" );
+            given( ).spec( getRequestSpec( token ) ).contentType( JSON )
+                .when( )
+                .body( jsonAsMap )
+                .patch( id )
+                .then( ).statusCode( 200 ).extract( ).response( );
+            given( ).spec( getRequestSpec( token ) ).contentType( JSON )
+                .when( )
+                .delete( "template/archiva-repository-manager/repository16" )
+                .then( ).statusCode( 200 );
+            given( ).spec( getRequestSpec( token ) ).contentType( JSON )
+                .when( )
+                .delete( "template/archiva-repository-observer/repository16" )
+                .then( ).statusCode( 200 );
+        }
+
+
+    }
+
+    @Test
+    void updateRoleNotExist( )
+    {
+        String token = getAdminToken( );
+            Map<String, Object> jsonAsMap = new HashMap<>( );
+            jsonAsMap.put( "id", "abcdefg" );
+            jsonAsMap.put( "name", "abcdefg" );
+            jsonAsMap.put( "description", "This description was updated." );
+            given( ).spec( getRequestSpec( token ) ).contentType( JSON )
+                .when( )
+                .body( jsonAsMap )
+                .patch( "abcdefg" )
+                .then( ).statusCode( 404 );
+    }
+
 }
diff --git a/redback-rbac/redback-rbac-role-manager/src/main/java/org/apache/archiva/redback/role/DefaultRoleManager.java b/redback-rbac/redback-rbac-role-manager/src/main/java/org/apache/archiva/redback/role/DefaultRoleManager.java
index 5969633..c06ac63 100644
--- a/redback-rbac/redback-rbac-role-manager/src/main/java/org/apache/archiva/redback/role/DefaultRoleManager.java
+++ b/redback-rbac/redback-rbac-role-manager/src/main/java/org/apache/archiva/redback/role/DefaultRoleManager.java
@@ -269,13 +269,17 @@ public class DefaultRoleManager
     public void assignRole( String roleId, String principal )
         throws RoleManagerException
     {
-        ModelRole modelRole = RoleModelUtils.getModelRole( blessedModel, roleId );
-
-        if ( modelRole == null )
+        try
+        {
+            rbacManager.getRoleById( roleId );
+        }
+        catch ( RbacObjectNotFoundException e ) {
+            throw new RoleNotFoundException( e.getMessage(), e );
+        }
+        catch ( RbacManagerException e )
         {
-            throw new RoleNotFoundException( "Unable to assign role: " + roleId + " does not exist." );
+            throw new RoleManagerException( e.getMessage( ), e );
         }
-
         try
         {
             UserAssignment userAssignment;
@@ -289,7 +293,7 @@ public class DefaultRoleManager
                 userAssignment = rbacManager.createUserAssignment( principal );
             }
 
-            userAssignment.addRoleId( modelRole.getId() );
+            userAssignment.addRoleId( roleId );
             rbacManager.saveUserAssignment( userAssignment );
         }
         catch ( RbacManagerException e )
diff --git a/redback-rbac/redback-rbac-role-manager/src/main/java/org/apache/archiva/redback/role/PermanentRoleDeletionInvalid.java b/redback-rbac/redback-rbac-role-manager/src/main/java/org/apache/archiva/redback/role/PermanentRoleDeletionInvalid.java
new file mode 100644
index 0000000..cd8968d
--- /dev/null
+++ b/redback-rbac/redback-rbac-role-manager/src/main/java/org/apache/archiva/redback/role/PermanentRoleDeletionInvalid.java
@@ -0,0 +1,35 @@
+package org.apache.archiva.redback.role;/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Thrown, if the deletion of a permanent role was tried.
+ *
+ * @author Martin Stockhammer <ma...@apache.org>
+ */
+public class PermanentRoleDeletionInvalid extends RoleManagerException
+{
+    public PermanentRoleDeletionInvalid( String string )
+    {
+        super( string );
+    }
+
+    public PermanentRoleDeletionInvalid( String string, Throwable throwable )
+    {
+        super( string, throwable );
+    }
+}
diff --git a/redback-rbac/redback-rbac-role-manager/src/main/java/org/apache/archiva/redback/role/template/DefaultRoleTemplateProcessor.java b/redback-rbac/redback-rbac-role-manager/src/main/java/org/apache/archiva/redback/role/template/DefaultRoleTemplateProcessor.java
index dd801d1..e7cee60 100644
--- a/redback-rbac/redback-rbac-role-manager/src/main/java/org/apache/archiva/redback/role/template/DefaultRoleTemplateProcessor.java
+++ b/redback-rbac/redback-rbac-role-manager/src/main/java/org/apache/archiva/redback/role/template/DefaultRoleTemplateProcessor.java
@@ -25,6 +25,7 @@ import org.apache.archiva.redback.rbac.RbacManagerException;
 import org.apache.archiva.redback.rbac.Resource;
 import org.apache.archiva.redback.rbac.Role;
 import org.apache.archiva.redback.rbac.RBACManager;
+import org.apache.archiva.redback.role.PermanentRoleDeletionInvalid;
 import org.apache.archiva.redback.role.RoleExistsException;
 import org.apache.archiva.redback.role.RoleManagerException;
 import org.apache.archiva.redback.role.RoleNotFoundException;
@@ -145,7 +146,7 @@ public class DefaultRoleTemplateProcessor
             }
             else
             {
-                throw new RoleManagerException( "Unable to remove role, it is flagged permanent" );
+                throw new PermanentRoleDeletionInvalid( "Unable to remove role, it is flagged permanent: "+roleId );
             }
         }
         catch ( RbacManagerException e )