role clients fix

portal/rest/groups.py

@@ -6,7 +6,7 @@ from flask_restplus import Namespace, Resource
 
 from portal.rest import rest_helpers
 from portal.rest.schemas import group_import_schema, group_schema, groups_schema, projects_schema, \
-    user_list_update_schema, users_schema
+    client_list_update_schema, users_schema
 from portal.service import auth, general
 from portal.service.general import find_client_owner
 from portal.service.errors import ForbiddenError
@@ -136,7 +136,7 @@ class GroupUsersList(Resource):
         require_client(client=client, course=course, permissions=permissions)
 
         data = rest_helpers.parse_request_data(
-            user_list_update_schema, action='update', resource='group_membership'
+            client_list_update_schema, action='update', resource='group_membership'
         )
 
         group = general.find_group(course, gid)

portal/rest/roles.py

 import logging
+
+from flask import request
 from flask_jwt_extended import jwt_required
 from flask_restplus import Namespace, Resource
 
+from portal.database.models import ClientType
 from portal.rest import rest_helpers
 from portal.rest.schemas import permissions_schema, role_schema, roles_schema, \
-    user_list_update_schema, users_schema
+    client_list_update_schema, users_schema
 from portal.service import auth, general
 from portal.service.general import find_client_owner
 from portal.service.errors import ForbiddenError
 from portal.service.permissions import check_client, require_client
-from portal.service.roles import add_single_user_to_role, create_role, delete_role, list_roles, \
-    remove_single_user_from_role, update_role, update_role_permissions, \
-    update_users_membership_in_role
+from portal.service.roles import add_single_client_to_role, create_role, delete_role, list_roles, \
+    remove_single_client_from_role, update_role, update_role_permissions, \
+    update_clients_membership_in_role
 
 roles_namespace = Namespace('')
 
@@ -134,7 +137,7 @@ class RolePermissions(Resource):
         return permissions_schema.dump(role.permissions), 204
 
 
-@roles_namespace.route('/courses/<string:cid>/roles/<string:rid>/users')
+@roles_namespace.route('/courses/<string:cid>/roles/<string:rid>/clients')
 @roles_namespace.param('cid', 'Course id')
 @roles_namespace.param('rid', 'Role id')
 @roles_namespace.response(404, 'Course not found')
@@ -146,6 +149,7 @@ class RoleUsersList(Resource):
         client = auth.find_client()
         course = general.find_course(cid)
         role = general.find_role(course, rid)
+        type = request.args.get('type')
         # authorization
 
         if not check_client(client=client, course=course, permissions=['view_course_full']) or \
@@ -153,7 +157,7 @@ class RoleUsersList(Resource):
                               permissions=['view_course_limited']) and find_client_owner(client) in role.clients):
             raise ForbiddenError(uid=client.id)
 
-        return users_schema.dump(role.clients)
+        return users_schema.dump([client for client in role.clients if client.type == ClientType[type.upper()]])
 
     @jwt_required
     @roles_namespace.response(204, 'Updates users membership in role')
@@ -165,29 +169,29 @@ class RoleUsersList(Resource):
         require_client(client=client, course=course, permissions=perm)
 
         data = rest_helpers.parse_request_data(
-            user_list_update_schema, action='update', resource='role_membership'
+            client_list_update_schema, action='update', resource='role_membership'
         )
 
         # everything from users_add is added, THEN everything from users_remove
         # is subtracted
         role = general.find_role(course, rid)
-        update_users_membership_in_role(role, data)
+        update_clients_membership_in_role(role, data)
         return '', 204
 
 
 @roles_namespace.route(
-    '/courses/<string:cid>/roles/<string:rid>/users/<string:uid>')
+    '/courses/<string:cid>/roles/<string:rid>/clients/<string:clid>')
 @roles_namespace.param('cid', 'Course id')
 @roles_namespace.param('rid', 'Role id')
-@roles_namespace.param('uid', 'User id')
+@roles_namespace.param('clid', 'Client id')
 @roles_namespace.response(404, 'Course not found')
 @roles_namespace.response(404, 'Role not found')
-@roles_namespace.response(404, 'User not found')
+@roles_namespace.response(404, 'Client not found')
 class RoleUser(Resource):
 
     @jwt_required
-    @roles_namespace.response(204, 'Adds user to role')
-    def put(self, cid: str, rid: str, uid: str):
+    @roles_namespace.response(204, 'Adds client to role')
+    def put(self, cid: str, rid: str, clid: str):
         client = auth.find_client()
         course = general.find_course(cid)
 
@@ -196,13 +200,13 @@ class RoleUser(Resource):
         require_client(client=client, course=course, permissions=perm)
 
         role = general.find_role(course, rid)
-        user = general.find_user(uid)
-        add_single_user_to_role(role, user)
+        client = general.find_client(clid)
+        add_single_client_to_role(role, client)
         return '', 204
 
     @jwt_required
-    @roles_namespace.response(204, 'Removes user from role')
-    def delete(self, cid: str, rid: str, uid: str):
+    @roles_namespace.response(204, 'Removes client from role')
+    def delete(self, cid: str, rid: str, clid: str):
         client = auth.find_client()
         course = general.find_course(cid)
         # authorization
@@ -210,6 +214,6 @@ class RoleUser(Resource):
         require_client(client=client, course=course, permissions=perm)
 
         role = general.find_role(course, rid)
-        user = general.find_user(uid)
-        remove_single_user_from_role(role, user)
+        user = general.find_client(clid)
+        remove_single_client_from_role(role, user)
         return '', 204

portal/rest/schemas.py

@@ -52,6 +52,7 @@ class NestedCollection:
             'Project': ('id', 'codename', 'course.id'),
             'Course': ('id', 'codename'),
             'User': ('id', 'username', 'uco'),
+            'Client': ('id', 'type' ),
             'Component': ('id', 'name', 'type'),
             'Submission': ('id', 'state', 'user.id', 'user.username',
                            'project.id', 'project.codename',
@@ -188,13 +189,17 @@ class ProjectConfigSchema(BaseSchema, Schema):
 class RoleSchema(BaseSchema, NamedSchema, Schema):
     """Role Schema
     """
-    users = NESTED['users']
+    clients = NESTED['clients']
     course = NESTED['course']
     # this could be done prettier (e.g. flattening the structure, showing only
     # set fields)
     permissions = NESTED("RolePermissions")
 
 
+class ClientSchema(BaseSchema, Schema):
+    type = fields.Str()
+
+
 class RolePermissionsSchema(BaseSchema, Schema):
     """Role Permissions Schema
     """
@@ -302,7 +307,7 @@ class CourseImportSchema(Schema):
     config = NESTED('CourseImportConfig', required=True)
 
 
-class UserListUpdateSchema(Schema):
+class ClientListUpdateSchema(Schema):
     """User List Update Schema
     """
     add = fields.List(fields.Str())
@@ -378,7 +383,7 @@ projects_schema = ProjectSchema(many=True, only=(*CODENAME_W_DESC, 'course'))
 config_schema = ProjectConfigSchema()
 config_schema_reduced = ProjectConfigSchema(only=('id', 'project', 'submissions_allowed_from',
                                                   'submissions_allowed_to', 'file_whitelist'))
-user_list_update_schema = UserListUpdateSchema(strict=True)
+client_list_update_schema = ClientListUpdateSchema(strict=True)
 group_import_schema = GroupImportSchema(strict=True)
 worker_schema = WorkerSchema(strict=True)
 workers_schema = WorkerSchema(strict=True, many=True)

portal/service/roles.py

@@ -8,7 +8,7 @@ from typing import Union, List
 from portal.database.models import Course, Role, RolePermissions, User, Client
 from portal.service.errors import ForbiddenError, PortalAPIError
 from portal.service.filters import filter_roles_from_course
-from portal.service.general import delete_entity, get_new_name, update_entity, write_entity
+from portal.service.general import delete_entity, get_new_name, update_entity, write_entity, find_client
 from portal.service.permissions import check_client
 from portal.service.users import find_users
 
@@ -94,64 +94,75 @@ def update_role_permissions(role: Role, permissions: dict) -> RolePermissions:
     return role.permissions
 
 
-def update_users_membership_in_role(role: Role, data: dict) -> Role:
-    """Updates users membership in the role
+def find_clients(ids: List[str]) -> List[Client]:
+    """Finds all clients based on their ids
+
+        Args:
+            ids(List[str]): List of client ids
+
+        Returns(List[Client]): List of clients
+        """
+    return [find_client(i) for i in ids]
+
+
+def update_clients_membership_in_role(role: Role, data: dict) -> Role:
+    """Updates client membership in the role
     Args:
         role(Role): Role instance
         data(dict): Data provided to update role membership
 
     Returns(Role): Updated role
     """
-    users_old = set(role.clients)
-    users_add = data.get('add')
-    users_remove = data.get('remove')
-    users = set()
-    if users_add:
-        to_add = find_users(data['add'])
-        users = users_old.union(to_add)
-    if users_remove:
-        to_remove = find_users(data['remove'])
-        users = users_old.difference(to_remove)
-    role.clients = list(users)
+    clients_old = set(role.clients)
+    clients_add = data.get('add')
+    clients_remove = data.get('remove')
+    clients = set()
+    if clients_add:
+        to_add = find_clients(data['add'])
+        clients = clients_old.union(to_add)
+    if clients_remove:
+        to_remove = find_clients(data['remove'])
+        clients = clients_old.difference(to_remove)
+    role.clients = list(clients)
     write_entity(role)
-    log.info(f"[UPDATE] Users in Role {role.id}: {data}.")
+    log.info(f"[UPDATE] Clients in Role {role.id}: {data}.")
     return role
 
 
-def add_single_user_to_role(role: Role, user: User):
-    """Adds single user to the role
+def add_single_client_to_role(role: Role, client: Client):
+    """Adds single client to the role
     Args:
         role(Role): Role instance
-        user(User): User instance
+        client(Client): Client instance
     Returns:
     """
-    if user not in role.clients:
-        role.clients.append(user)
+    if client not in role.clients:
+        role.clients.append(client)
         write_entity(role)
-        log.info(f"[ADD] User {user.id} to role "
+        log.info(f"[ADD] Client {client.id} to role "
                  f"{role.id} in course {role.course.id}.")
     else:
-        log.info(f"[ADD] User {user.id} is already "
+        log.info(f"[ADD] Client {client.id} is already "
                  f"in role {role.id} in course {role.course.id}: no change.")
 
 
-def remove_single_user_from_role(role: Role, user: User) -> Role:
-    """Removes single user from the role
+def remove_single_client_from_role(role: Role, client: Client) -> Role:
+    """Removes single client from the role
     Args:
         role(Role): Role instance
-        user(User): User instance
+        client(Client): Client instance
     Returns(Role): Updated role
     """
     course = role.course
     try:
-        role.clients.remove(user)
+        role.clients.remove(client)
         write_entity(role)
     except ValueError:
-        raise PortalAPIError(400, message=f"Could not remove user {user.id} "
+        raise PortalAPIError(400, message=f"Could not remove client {client.id} "
                                           f"from role {role.id} in course {course.id}: "
-                                          f"role does not contain user.")
+                                          f"role does not contain client.")
     log.info(
-        f"[REMOVE] User {user.id} from role {role.id} in course {course.id}.")
+        f"[REMOVE] Client {client.id} from role {role.id} in course {course.id}.")
     return role
 
 

tests/rest/test_role.py

@@ -98,7 +98,7 @@ def test_users_list(client):
     cpp = Course.query.filter_by(codename="testcourse1").first()
     r = cpp.roles[0]
     response = utils.make_request(
-        client, f"/courses/{cpp.codename}/roles/{r.id}/users", method='get')
+        client, f"/courses/{cpp.codename}/roles/{r.id}/clients?type=user", method='get')
 
     assert response.status_code == 200
     assert response.mimetype == 'application/json'
@@ -120,7 +120,7 @@ def test_users_update_add(client):
         add=[user.id]
         )
     request_json = json.dumps(request_dict)
-    response = utils.make_request(client, f"/courses/{c.codename}/roles/{r.name}/users",
+    response = utils.make_request(client, f"/courses/{c.codename}/roles/{r.name}/clients",
                                   data=request_json,
                                   headers={"content-type": "application/json"}, method='put')
 
@@ -145,7 +145,7 @@ def test_users_update_add_duplicate(client):
         add=[user.id]
         )
     request_json = json.dumps(request_dict)
-    response = utils.make_request(client, f"/courses/{c.codename}/roles/{r.name}/users",
+    response = utils.make_request(client, f"/courses/{c.codename}/roles/{r.name}/clients",
                                   data=request_json,
                                   headers={"content-type": "application/json"}, method='put')
 
@@ -170,7 +170,7 @@ def test_users_update_remove(client):
         remove=[user.id]
         )
     request_json = json.dumps(request_dict)
-    response = utils.make_request(client, f"/courses/{c.codename}/roles/{r.name}/users",
+    response = utils.make_request(client, f"/courses/{c.codename}/roles/{r.name}/clients",
                                   data=request_json,
                                   headers={"content-type": "application/json"}, method='put')
 
@@ -195,7 +195,7 @@ def test_users_update_remove_user_not_in(client):
         remove=[user.id]
         )
     request_json = json.dumps(request_dict)
-    response = utils.make_request(client, f"/courses/{c.codename}/roles/{r.name}/users",
+    response = utils.make_request(client, f"/courses/{c.codename}/roles/{r.name}/clients",
                                   data=request_json,
                                   headers={"content-type": "application/json"}, method='put')
 
@@ -218,7 +218,7 @@ def test_add_user(client):
     assert r not in u.roles
 
     response = utils.make_request(
-        client, f"/courses/{c.codename}/roles/{r.name}/users/{u.username}", method='put')
+        client, f"/courses/{c.codename}/roles/{r.name}/clients/{u.id}", method='put')
     assert response.status_code == 204
     assert response.mimetype == 'application/json'
 
@@ -237,7 +237,7 @@ def test_add_user_duplicate(client):
     assert u in r.clients
 
     response = utils.make_request(
-        client, f"/courses/{c.codename}/roles/{r.name}/users/{u.username}", method='put')
+        client, f"/courses/{c.codename}/roles/{r.name}/clients/{u.id}", method='put')
     assert response.status_code == 204
     assert response.mimetype == 'application/json'
 
@@ -255,7 +255,7 @@ def test_remove_user(client):
     u = r.clients[0]
 
     response = utils.make_request(
-        client, f"/courses/{c.codename}/roles/{r.name}/users/{u.username}", method='delete')
+        client, f"/courses/{c.codename}/roles/{r.name}/clients/{u.id}", method='delete')
     assert response.status_code == 204
     assert response.mimetype == 'application/json'
 
@@ -274,7 +274,7 @@ def test_remove_user_not_in(client):
     assert r not in u.roles
 
     response = utils.make_request(
-        client, f"/courses/{c.codename}/roles/{r.name}/users/{u.username}", method='delete')
+        client, f"/courses/{c.codename}/roles/{r.name}/clients/{u.id}", method='delete')
     assert response.status_code == 400
 
     r_updated = Role.query.filter(