From d668817288efdf972006c2d492e7b0a2629fd687 Mon Sep 17 00:00:00 2001 From: Matthew Evans Date: Tue, 11 Mar 2025 12:58:40 +0000 Subject: [PATCH 01/18] Move `/search-users` to users blueprint --- pydatalab/src/pydatalab/routes/v0_1/items.py | 41 ------------------ pydatalab/src/pydatalab/routes/v0_1/users.py | 45 +++++++++++++++++++- 2 files changed, 44 insertions(+), 42 deletions(-) diff --git a/pydatalab/src/pydatalab/routes/v0_1/items.py b/pydatalab/src/pydatalab/routes/v0_1/items.py index f9d1c1d23..c3b22c301 100644 --- a/pydatalab/src/pydatalab/routes/v0_1/items.py +++ b/pydatalab/src/pydatalab/routes/v0_1/items.py @@ -14,7 +14,6 @@ from pydatalab.logger import LOGGER from pydatalab.models import ITEM_MODELS from pydatalab.models.items import Item -from pydatalab.models.people import Person from pydatalab.models.relationships import RelationshipType from pydatalab.models.utils import generate_unique_refcode from pydatalab.mongo import ITEMS_FTS_FIELDS, flask_mongo @@ -1106,43 +1105,3 @@ def save_item(): ) return jsonify(status="success", last_modified=updated_data["last_modified"]), 200 - - -@ITEMS.route("/search-users/", methods=["GET"]) -def search_users(): - """Perform free text search on users and return the top results. - GET parameters: - query: String with the search terms. - nresults: Maximum number of (default 100) - - Returns: - response list of dictionaries containing the matching items in order of - descending match score. - """ - - query = request.args.get("query", type=str) - nresults = request.args.get("nresults", default=100, type=int) - types = request.args.get("types", default=None) - - match_obj = {"$text": {"$search": query}} - if types is not None: - match_obj["type"] = {"$in": types} - - cursor = flask_mongo.db.users.aggregate( - [ - {"$match": match_obj}, - {"$sort": {"score": {"$meta": "textScore"}}}, - {"$limit": nresults}, - { - "$project": { - "_id": 1, - "identities": 1, - "display_name": 1, - "contact_email": 1, - } - }, - ] - ) - return jsonify( - {"status": "success", "users": list(json.loads(Person(**d).json()) for d in cursor)} - ), 200 diff --git a/pydatalab/src/pydatalab/routes/v0_1/users.py b/pydatalab/src/pydatalab/routes/v0_1/users.py index 4d7e82dc8..3ba83308b 100644 --- a/pydatalab/src/pydatalab/routes/v0_1/users.py +++ b/pydatalab/src/pydatalab/routes/v0_1/users.py @@ -1,9 +1,11 @@ +import json + from bson import ObjectId from flask import Blueprint, jsonify, request from flask_login import current_user from pydatalab.config import CONFIG -from pydatalab.models.people import DisplayName, EmailStr +from pydatalab.models.people import DisplayName, EmailStr, Person from pydatalab.mongo import flask_mongo from pydatalab.permissions import active_users_or_get_only @@ -77,3 +79,44 @@ def save_user(user_id): ) return (jsonify({"status": "success"}), 200) + + +@active_users_or_get_only +@USERS.route("/search-users/", methods=["GET"]) +def search_users(): + """Perform free text search on users and return the top results. + GET parameters: + query: String with the search terms. + nresults: Maximum number of (default 100) + + Returns: + response list of dictionaries containing the matching items in order of + descending match score. + """ + + query = request.args.get("query", type=str) + nresults = request.args.get("nresults", default=100, type=int) + types = request.args.get("types", default=None) + + match_obj = {"$text": {"$search": query}} + if types is not None: + match_obj["type"] = {"$in": types} + + cursor = flask_mongo.db.users.aggregate( + [ + {"$match": match_obj}, + {"$sort": {"score": {"$meta": "textScore"}}}, + {"$limit": nresults}, + { + "$project": { + "_id": 1, + "identities": 1, + "display_name": 1, + "contact_email": 1, + } + }, + ] + ) + return jsonify( + {"status": "success", "users": list(json.loads(Person(**d).json()) for d in cursor)} + ), 200 From 2578bb95c3b8e6c14cf8a2a60e21877d3b74e93a Mon Sep 17 00:00:00 2001 From: Matthew Evans Date: Tue, 11 Mar 2025 13:03:53 +0000 Subject: [PATCH 02/18] Add endpoints for group management --- pydatalab/schemas/cell.json | 66 ++++++++++++++ pydatalab/schemas/equipment.json | 66 ++++++++++++++ pydatalab/schemas/sample.json | 66 ++++++++++++++ pydatalab/schemas/startingmaterial.json | 66 ++++++++++++++ pydatalab/src/pydatalab/login.py | 25 +++++- pydatalab/src/pydatalab/models/people.py | 29 +++++- pydatalab/src/pydatalab/mongo.py | 31 +++++++ .../src/pydatalab/routes/v0_1/__init__.py | 3 +- pydatalab/src/pydatalab/routes/v0_1/users.py | 90 ++++++++++++++++++- 9 files changed, 435 insertions(+), 7 deletions(-) diff --git a/pydatalab/schemas/cell.json b/pydatalab/schemas/cell.json index a94011179..0c43c6c3b 100644 --- a/pydatalab/schemas/cell.json +++ b/pydatalab/schemas/cell.json @@ -275,6 +275,65 @@ "name" ] }, + "Group": { + "title": "Group", + "description": "A model that describes a group of users, for the sake\nof applying group permissions.\n\nEach `Person` model can point to a given group.", + "type": "object", + "properties": { + "type": { + "title": "Type", + "default": "groups", + "const": "groups", + "type": "string" + }, + "immutable_id": { + "title": "Immutable ID", + "format": "uuid", + "type": "string" + }, + "last_modified": { + "title": "Last Modified", + "type": "string", + "format": "date-time" + }, + "relationships": { + "title": "Relationships", + "type": "array", + "items": { + "$ref": "#/definitions/TypedRelationship" + } + }, + "group_id": { + "title": "Group Id", + "minLength": 1, + "maxLength": 40, + "pattern": "^(?:[a-zA-Z0-9]+|[a-zA-Z0-9][a-zA-Z0-9._-]+[a-zA-Z0-9])$", + "type": "string" + }, + "display_name": { + "title": "Display Name", + "minLength": 1, + "maxLength": 150, + "type": "string" + }, + "description": { + "title": "Description", + "type": "string" + }, + "group_admins": { + "title": "Group Admins", + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "group_id", + "display_name", + "group_admins" + ] + }, "AccountStatus": { "title": "AccountStatus", "description": "A string enum representing the account status.", @@ -331,6 +390,13 @@ "type": "string", "format": "email" }, + "groups": { + "title": "Groups", + "type": "array", + "items": { + "$ref": "#/definitions/Group" + } + }, "managers": { "title": "Managers", "type": "array", diff --git a/pydatalab/schemas/equipment.json b/pydatalab/schemas/equipment.json index 9d7da376a..d27eb6f73 100644 --- a/pydatalab/schemas/equipment.json +++ b/pydatalab/schemas/equipment.json @@ -239,6 +239,65 @@ "name" ] }, + "Group": { + "title": "Group", + "description": "A model that describes a group of users, for the sake\nof applying group permissions.\n\nEach `Person` model can point to a given group.", + "type": "object", + "properties": { + "type": { + "title": "Type", + "default": "groups", + "const": "groups", + "type": "string" + }, + "immutable_id": { + "title": "Immutable ID", + "format": "uuid", + "type": "string" + }, + "last_modified": { + "title": "Last Modified", + "type": "string", + "format": "date-time" + }, + "relationships": { + "title": "Relationships", + "type": "array", + "items": { + "$ref": "#/definitions/TypedRelationship" + } + }, + "group_id": { + "title": "Group Id", + "minLength": 1, + "maxLength": 40, + "pattern": "^(?:[a-zA-Z0-9]+|[a-zA-Z0-9][a-zA-Z0-9._-]+[a-zA-Z0-9])$", + "type": "string" + }, + "display_name": { + "title": "Display Name", + "minLength": 1, + "maxLength": 150, + "type": "string" + }, + "description": { + "title": "Description", + "type": "string" + }, + "group_admins": { + "title": "Group Admins", + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "group_id", + "display_name", + "group_admins" + ] + }, "AccountStatus": { "title": "AccountStatus", "description": "A string enum representing the account status.", @@ -295,6 +354,13 @@ "type": "string", "format": "email" }, + "groups": { + "title": "Groups", + "type": "array", + "items": { + "$ref": "#/definitions/Group" + } + }, "managers": { "title": "Managers", "type": "array", diff --git a/pydatalab/schemas/sample.json b/pydatalab/schemas/sample.json index 602a990a9..42ba3af3d 100644 --- a/pydatalab/schemas/sample.json +++ b/pydatalab/schemas/sample.json @@ -328,6 +328,65 @@ "name" ] }, + "Group": { + "title": "Group", + "description": "A model that describes a group of users, for the sake\nof applying group permissions.\n\nEach `Person` model can point to a given group.", + "type": "object", + "properties": { + "type": { + "title": "Type", + "default": "groups", + "const": "groups", + "type": "string" + }, + "immutable_id": { + "title": "Immutable ID", + "format": "uuid", + "type": "string" + }, + "last_modified": { + "title": "Last Modified", + "type": "string", + "format": "date-time" + }, + "relationships": { + "title": "Relationships", + "type": "array", + "items": { + "$ref": "#/definitions/TypedRelationship" + } + }, + "group_id": { + "title": "Group Id", + "minLength": 1, + "maxLength": 40, + "pattern": "^(?:[a-zA-Z0-9]+|[a-zA-Z0-9][a-zA-Z0-9._-]+[a-zA-Z0-9])$", + "type": "string" + }, + "display_name": { + "title": "Display Name", + "minLength": 1, + "maxLength": 150, + "type": "string" + }, + "description": { + "title": "Description", + "type": "string" + }, + "group_admins": { + "title": "Group Admins", + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "group_id", + "display_name", + "group_admins" + ] + }, "AccountStatus": { "title": "AccountStatus", "description": "A string enum representing the account status.", @@ -384,6 +443,13 @@ "type": "string", "format": "email" }, + "groups": { + "title": "Groups", + "type": "array", + "items": { + "$ref": "#/definitions/Group" + } + }, "managers": { "title": "Managers", "type": "array", diff --git a/pydatalab/schemas/startingmaterial.json b/pydatalab/schemas/startingmaterial.json index 525d81b7f..11bf9d28c 100644 --- a/pydatalab/schemas/startingmaterial.json +++ b/pydatalab/schemas/startingmaterial.json @@ -381,6 +381,65 @@ "name" ] }, + "Group": { + "title": "Group", + "description": "A model that describes a group of users, for the sake\nof applying group permissions.\n\nEach `Person` model can point to a given group.", + "type": "object", + "properties": { + "type": { + "title": "Type", + "default": "groups", + "const": "groups", + "type": "string" + }, + "immutable_id": { + "title": "Immutable ID", + "format": "uuid", + "type": "string" + }, + "last_modified": { + "title": "Last Modified", + "type": "string", + "format": "date-time" + }, + "relationships": { + "title": "Relationships", + "type": "array", + "items": { + "$ref": "#/definitions/TypedRelationship" + } + }, + "group_id": { + "title": "Group Id", + "minLength": 1, + "maxLength": 40, + "pattern": "^(?:[a-zA-Z0-9]+|[a-zA-Z0-9][a-zA-Z0-9._-]+[a-zA-Z0-9])$", + "type": "string" + }, + "display_name": { + "title": "Display Name", + "minLength": 1, + "maxLength": 150, + "type": "string" + }, + "description": { + "title": "Description", + "type": "string" + }, + "group_admins": { + "title": "Group Admins", + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "group_id", + "display_name", + "group_admins" + ] + }, "AccountStatus": { "title": "AccountStatus", "description": "A string enum representing the account status.", @@ -437,6 +496,13 @@ "type": "string", "format": "email" }, + "groups": { + "title": "Groups", + "type": "array", + "items": { + "$ref": "#/definitions/Group" + } + }, "managers": { "title": "Managers", "type": "array", diff --git a/pydatalab/src/pydatalab/login.py b/pydatalab/src/pydatalab/login.py index 833550f67..ad11aca3f 100644 --- a/pydatalab/src/pydatalab/login.py +++ b/pydatalab/src/pydatalab/login.py @@ -9,7 +9,7 @@ from flask_login import LoginManager, UserMixin from pydatalab.models import Person -from pydatalab.models.people import AccountStatus, Identity, IdentityType +from pydatalab.models.people import AccountStatus, Group, Identity, IdentityType from pydatalab.models.utils import UserRole from pydatalab.mongo import flask_mongo @@ -72,6 +72,11 @@ def identity_types(self) -> list[IdentityType]: """Returns a list of the identity types associated with the user.""" return [_.identity_type for _ in self.person.identities] + @property + def groups(self) -> list[Group]: + """Returns the list of groups that the user is a member of.""" + return self.person.groups + def refresh(self) -> None: """Reconstruct the user object from their database entry, to be used when, e.g., a new identity has been associated with them. @@ -87,7 +92,25 @@ def get_by_id_cached(user_id): return get_by_id(user_id) +<<<<<<< HEAD def get_by_id(user_id: str) -> LoginUser | None: +======= +def groups_lookup() -> dict: + return { + "from": "groups", + "let": {"group_ids": "$group_ids"}, + "pipeline": [ + {"$match": {"$expr": {"$in": ["$_id", {"$ifNull": ["$$group_ids", []]}]}}}, + {"$addFields": {"__order": {"$indexOfArray": ["$$group_ids", "$_id"]}}}, + {"$sort": {"__order": 1}}, + {"$project": {"_id": 1, "display_name": 1}}, + ], + "as": "groups", + } + + +def get_by_id(user_id: str) -> Optional[LoginUser]: +>>>>>>> 82a1eb81 (Add endpoints for group management) """Lookup the user database ID and create a new `LoginUser` with the relevant metadata. diff --git a/pydatalab/src/pydatalab/models/people.py b/pydatalab/src/pydatalab/models/people.py index 096250cce..57d7f6d18 100644 --- a/pydatalab/src/pydatalab/models/people.py +++ b/pydatalab/src/pydatalab/models/people.py @@ -6,7 +6,7 @@ from pydantic import EmailStr as PydanticEmailStr from pydatalab.models.entries import Entry -from pydatalab.models.utils import PyObjectId +from pydatalab.models.utils import HumanReadableIdentifier, PyObjectId class IdentityType(str, Enum): @@ -97,6 +97,30 @@ class AccountStatus(str, Enum): DEACTIVATED = "deactivated" +class Group(Entry): + """A model that describes a group of users, for the sake + of applying group permissions. + + Each `Person` model can point to a given group. + + """ + + type: str = Field("groups", const=True) + """The entry type as a string.""" + + group_id: HumanReadableIdentifier + """A short, locally-unique ID for the group.""" + + display_name: DisplayName + """The chosen display name for the group""" + + description: Optional[str] + """A description of the group""" + + group_admins: List[PyObjectId] + """A list of user IDs that can manage this group.""" + + class Person(Entry): """A model that describes an individual and their digital identities.""" @@ -115,6 +139,9 @@ class Person(Entry): managers: list[PyObjectId] | None """A list of user IDs that can manage this person's items.""" + groups: list[Group] | None + """A list of groups that this person belongs to.""" + account_status: AccountStatus = Field(AccountStatus.UNVERIFIED) """The status of the user's account.""" diff --git a/pydatalab/src/pydatalab/mongo.py b/pydatalab/src/pydatalab/mongo.py index cd4fba24a..170505692 100644 --- a/pydatalab/src/pydatalab/mongo.py +++ b/pydatalab/src/pydatalab/mongo.py @@ -208,4 +208,35 @@ def create_user_fts(): db.users.drop_index(user_fts_name) ret += create_user_fts() + group_fts_fields = {"display_name", "description"} + group_fts_name = "group full-text search" + group_index_name = "unique group identifiers" + + def create_group_index(group_index_name): + return db.groups.create_index( + "group_id", + unique=True, + name=group_index_name, + background=background, + ) + + try: + ret += create_group_index(group_index_name) + except pymongo.errors.OperationFailure: + db.users.drop_index(group_index_name) + ret += create_group_index(group_index_name) + + def create_group_fts(): + return db.groups.create_index( + [(k, pymongo.TEXT) for k in group_fts_fields], + name=group_fts_name, + background=background, + ) + + try: + ret += create_group_fts() + except pymongo.errors.OperationFailure: + db.users.drop_index(group_fts_name) + ret += create_group_fts() + return ret diff --git a/pydatalab/src/pydatalab/routes/v0_1/__init__.py b/pydatalab/src/pydatalab/routes/v0_1/__init__.py index 512a40163..a1577118b 100644 --- a/pydatalab/src/pydatalab/routes/v0_1/__init__.py +++ b/pydatalab/src/pydatalab/routes/v0_1/__init__.py @@ -11,13 +11,14 @@ from .info import INFO from .items import ITEMS from .remotes import REMOTES -from .users import USERS +from .users import GROUPS, USERS BLUEPRINTS: tuple[Blueprint, ...] = ( AUTH, COLLECTIONS, REMOTES, USERS, + GROUPS, ADMIN, ITEMS, BLOCKS, diff --git a/pydatalab/src/pydatalab/routes/v0_1/users.py b/pydatalab/src/pydatalab/routes/v0_1/users.py index 3ba83308b..ac845a46d 100644 --- a/pydatalab/src/pydatalab/routes/v0_1/users.py +++ b/pydatalab/src/pydatalab/routes/v0_1/users.py @@ -1,15 +1,98 @@ import json +import pymongo.errors from bson import ObjectId from flask import Blueprint, jsonify, request from flask_login import current_user from pydatalab.config import CONFIG -from pydatalab.models.people import DisplayName, EmailStr, Person -from pydatalab.mongo import flask_mongo -from pydatalab.permissions import active_users_or_get_only +from pydatalab.models.people import DisplayName, EmailStr, Group, Person +from pydatalab.mongo import _get_active_mongo_client, flask_mongo +from pydatalab.permissions import active_users_or_get_only, admin_only USERS = Blueprint("users", __name__) +GROUPS = Blueprint("groups", __name__) + + +@USERS.before_request +@active_users_or_get_only +def _(): ... + + +@GROUPS.before_request +@admin_only +def _(): ... + + +@GROUPS.route("/groups", methods=["PUT"]) +def create_group(): + request_json = request.get_json() + + group_json = { + "group_id": request_json.get("group_id"), + "display_name": request_json.get("display_name"), + "description": request_json.get("description"), + "group_admins": request_json.get("group_admins"), + } + try: + group = Group(**group_json) + except Exception as e: + return jsonify({"status": "error", "message": f"Invalid group data: {str(e)}"}), 400 + + try: + group_immutable_id = flask_mongo.db.groups.insert_one(group.dict()).inserted_id + except pymongo.errors.DuplicateKeyError: + return jsonify( + {"status": "error", "message": f"Group ID {group.group_id} already exists."} + ), 400 + + if group_immutable_id: + return jsonify({"status": "success", "group_immutable_id": str(group_immutable_id)}), 200 + + return jsonify({"status": "error", "message": "Unable to create group."}), 400 + + +@GROUPS.route("/groups", methods=["DELETE"]) +def delete_group(): + request_json = request.get_json() + + group_id = request_json.get("immutable_id") + if group_id is not None: + result = flask_mongo.db.groups.delete_one({"_id": ObjectId(group_id)}) + + if result.deleted_count == 1: + return jsonify({"status": "success"}), 200 + + return jsonify({"status": "error", "message": "Unable to delete group."}), 400 + + +@GROUPS.route("/groups/", methods=["PATCH"]) +def add_user_to_group(group_immutable_id): + request_json = request.get_json() + + user_id = request_json.get("user_id") + + if not user_id: + return jsonify({"status": "error", "message": "No user ID provided."}), 400 + + client = _get_active_mongo_client() + with client.start_session(causal_consistency=True) as session: + group_exists = flask_mongo.db.groups.find_one( + {"_id": ObjectId(group_immutable_id)}, session=session + ) + if not group_exists: + return jsonify({"status": "error", "message": "Group does not exist."}), 400 + + update_user = flask_mongo.db.users.update_one( + {"_id": ObjectId(user_id)}, + {"$addToSet": {"groups": group_immutable_id}}, + session=session, + ) + + if not update_user.modified_count == 1: + return jsonify({"status": "error", "message": "Unable to add user to group."}), 400 + + return jsonify({"status": "error", "message": "Unable to add user to group."}), 400 @USERS.before_request @@ -81,7 +164,6 @@ def save_user(user_id): return (jsonify({"status": "success"}), 200) -@active_users_or_get_only @USERS.route("/search-users/", methods=["GET"]) def search_users(): """Perform free text search on users and return the top results. From 323785679b09f50522566689c177ea04d2dceba2 Mon Sep 17 00:00:00 2001 From: Matthew Evans Date: Tue, 11 Mar 2025 13:39:03 +0000 Subject: [PATCH 03/18] Add tests for group creation --- pydatalab/src/pydatalab/models/people.py | 2 +- pydatalab/tests/server/test_users.py | 58 +++++++++++++++++++++++- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/pydatalab/src/pydatalab/models/people.py b/pydatalab/src/pydatalab/models/people.py index 57d7f6d18..7736b82aa 100644 --- a/pydatalab/src/pydatalab/models/people.py +++ b/pydatalab/src/pydatalab/models/people.py @@ -139,7 +139,7 @@ class Person(Entry): managers: list[PyObjectId] | None """A list of user IDs that can manage this person's items.""" - groups: list[Group] | None + groups: list[Group] = Field(default_factory=list) """A list of groups that this person belongs to.""" account_status: AccountStatus = Field(AccountStatus.UNVERIFIED) diff --git a/pydatalab/tests/server/test_users.py b/pydatalab/tests/server/test_users.py index 64eab34e8..04950fa67 100644 --- a/pydatalab/tests/server/test_users.py +++ b/pydatalab/tests/server/test_users.py @@ -13,6 +13,7 @@ def test_get_current_user(client): assert (resp_json := resp.json) assert resp_json["immutable_id"] == 24 * "1" assert resp_json["role"] == "user" + assert resp_json["groups"] == [] def test_get_current_user_admin(admin_client): @@ -41,7 +42,7 @@ def test_role_update_by_user(client, real_mongo_client, user_id): assert user["role"] == "manager" -def test_user_update(client, real_mongo_client, user_id, admin_user_id): +def test_user_update(client, unauthenticated_client, real_mongo_client, user_id, admin_user_id): endpoint = f"/users/{str(user_id)}" # Test display name update user_request = {"display_name": "Test Person II"} @@ -105,6 +106,16 @@ def test_user_update(client, real_mongo_client, user_id, admin_user_id): user = real_mongo_client.get_database().users.find_one({"_id": admin_user_id}) assert user["display_name"] == "Test Admin" + # Test that differing user auth can/cannot search for users + endpoint = "/search-users/" + resp = client.get(endpoint + "?query='Test Person'") + assert resp.status_code == 200 + assert len(resp.json["users"]) == 4 + + # Test that differing user auth can/cannot search for users + resp = unauthenticated_client.get(endpoint + "?query='Test Person'") + assert resp.status_code == 401 + def test_user_update_admin(admin_client, real_mongo_client, user_id): endpoint = f"/users/{str(user_id)}" @@ -114,3 +125,48 @@ def test_user_update_admin(admin_client, real_mongo_client, user_id): assert resp.status_code == 200 user = real_mongo_client.get_database().users.find_one({"_id": user_id}) assert user["display_name"] == "Test Person" + + +def test_create_group(admin_client, client, unauthenticated_client, real_mongo_client): + from bson import ObjectId + + good_group = { + "display_name": "My New Group", + "group_id": "my-new-group", + "description": "A group for testing", + "group_admins": [], + } + + # Group ID cannot be None + bad_group = good_group.copy() + bad_group["group_id"] = None + resp = admin_client.put("/groups", json=bad_group) + assert resp.status_code == 400 + + # Successfully create group + resp = admin_client.put("/groups", json=good_group) + assert resp.status_code == 200 + group_immutable_id = ObjectId(resp.json["group_immutable_id"]) + assert real_mongo_client.get_database().groups.find_one({"_id": group_immutable_id}) + + # Group ID must be unique + resp = admin_client.put("/groups", json=good_group) + assert resp.status_code == 400 + + # Request must come from admin + # Make ID unique so that this would otherwise pass + good_group["group_id"] = "my-new-group-2" + resp = unauthenticated_client.put("/groups", json=good_group) + assert resp.status_code == 401 + assert ( + real_mongo_client.get_database().groups.find_one({"group_id": good_group["group_id"]}) + is None + ) + + # Request must come from admin + resp = client.put("/groups", json=good_group) + assert resp.status_code == 403 + assert ( + real_mongo_client.get_database().groups.find_one({"group_id": good_group["group_id"]}) + is None + ) From 0c4d0f9bc5ea49eda38645ed19f9cd24fef97059 Mon Sep 17 00:00:00 2001 From: Matthew Evans Date: Tue, 11 Mar 2025 13:56:45 +0000 Subject: [PATCH 04/18] Add admin endpoints for group listing and tweak user listing --- pydatalab/src/pydatalab/models/people.py | 6 ++++- pydatalab/src/pydatalab/routes/v0_1/admin.py | 26 +++++++++++++++++--- pydatalab/tests/server/test_users.py | 14 +++++++++++ 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/pydatalab/src/pydatalab/models/people.py b/pydatalab/src/pydatalab/models/people.py index 7736b82aa..c6f19bbc0 100644 --- a/pydatalab/src/pydatalab/models/people.py +++ b/pydatalab/src/pydatalab/models/people.py @@ -6,7 +6,7 @@ from pydantic import EmailStr as PydanticEmailStr from pydatalab.models.entries import Entry -from pydatalab.models.utils import HumanReadableIdentifier, PyObjectId +from pydatalab.models.utils import HumanReadableIdentifier, PyObjectId, UserRole class IdentityType(str, Enum): @@ -195,3 +195,7 @@ def new_user_from_identity( contact_email=contact_email, account_status=account_status, ) + + +class User(Person): + role: UserRole diff --git a/pydatalab/src/pydatalab/routes/v0_1/admin.py b/pydatalab/src/pydatalab/routes/v0_1/admin.py index c95334073..0a2fbd729 100644 --- a/pydatalab/src/pydatalab/routes/v0_1/admin.py +++ b/pydatalab/src/pydatalab/routes/v0_1/admin.py @@ -3,8 +3,9 @@ from flask_login import current_user from pydatalab.config import CONFIG +from pydatalab.models.people import Group, User from pydatalab.mongo import flask_mongo -from pydatalab.permissions import admin_only, get_default_permissions +from pydatalab.permissions import admin_only ADMIN = Blueprint("admins", __name__) @@ -18,7 +19,6 @@ def _(): ... def get_users(): users = flask_mongo.db.users.aggregate( [ - {"$match": get_default_permissions(user_only=True)}, { "$lookup": { "from": "roles", @@ -27,6 +27,19 @@ def get_users(): "as": "role", } }, + { + "$lookup": { + "from": "groups", + "let": {"group_ids": "$group_ids"}, + "pipeline": [ + {"$match": {"$expr": {"$in": ["$_id", {"$ifNull": ["$$group_ids", []]}]}}}, + {"$addFields": {"__order": {"$indexOfArray": ["$$group_ids", "$_id"]}}}, + {"$sort": {"__order": 1}}, + {"$project": {"_id": 1, "display_name": 1}}, + ], + "as": "groups", + }, + }, { "$addFields": { "role": { @@ -41,7 +54,7 @@ def get_users(): ] ) - return jsonify({"status": "success", "data": list(users)}) + return jsonify({"status": "success", "data": list(User(**u).json() for u in users)}) @ADMIN.route("/roles/", methods=["PATCH"]) @@ -93,3 +106,10 @@ def save_role(user_id): ) return (jsonify({"status": "success"}), 200) + + +@ADMIN.route("/groups", methods=["GET"]) +def get_groups(): + return jsonify( + {"status": "success", "data": [Group(**d).json() for d in flask_mongo.db.groups.find()]} + ), 200 diff --git a/pydatalab/tests/server/test_users.py b/pydatalab/tests/server/test_users.py index 04950fa67..3c1799b87 100644 --- a/pydatalab/tests/server/test_users.py +++ b/pydatalab/tests/server/test_users.py @@ -42,6 +42,20 @@ def test_role_update_by_user(client, real_mongo_client, user_id): assert user["role"] == "manager" +def test_list_users(admin_client, client): + resp = admin_client.get("/users") + assert resp.status_code == 200 + resp = client.get("/users") + assert resp.status_code == 403 + + +def test_list_groups(admin_client, client): + resp = admin_client.get("/groups") + assert resp.status_code == 200 + resp = client.get("/groups") + assert resp.status_code == 403 + + def test_user_update(client, unauthenticated_client, real_mongo_client, user_id, admin_user_id): endpoint = f"/users/{str(user_id)}" # Test display name update From be62ebe8bb334eed1fdd344b0e7c615327776e0a Mon Sep 17 00:00:00 2001 From: Matthew Evans Date: Tue, 11 Mar 2025 21:19:00 +0000 Subject: [PATCH 05/18] Restructure group routes and add test for search-groups --- pydatalab/schemas/cell.json | 3 +- pydatalab/schemas/equipment.json | 3 +- pydatalab/schemas/sample.json | 3 +- pydatalab/schemas/startingmaterial.json | 3 +- pydatalab/src/pydatalab/models/people.py | 2 +- .../src/pydatalab/routes/v0_1/__init__.py | 3 +- pydatalab/src/pydatalab/routes/v0_1/admin.py | 84 ++++++++++++++++++- pydatalab/src/pydatalab/routes/v0_1/groups.py | 46 ++++++++++ pydatalab/src/pydatalab/routes/v0_1/users.py | 11 +-- pydatalab/tests/server/test_users.py | 5 ++ 10 files changed, 141 insertions(+), 22 deletions(-) create mode 100644 pydatalab/src/pydatalab/routes/v0_1/groups.py diff --git a/pydatalab/schemas/cell.json b/pydatalab/schemas/cell.json index 0c43c6c3b..6f247657c 100644 --- a/pydatalab/schemas/cell.json +++ b/pydatalab/schemas/cell.json @@ -330,8 +330,7 @@ }, "required": [ "group_id", - "display_name", - "group_admins" + "display_name" ] }, "AccountStatus": { diff --git a/pydatalab/schemas/equipment.json b/pydatalab/schemas/equipment.json index d27eb6f73..1ba99e442 100644 --- a/pydatalab/schemas/equipment.json +++ b/pydatalab/schemas/equipment.json @@ -294,8 +294,7 @@ }, "required": [ "group_id", - "display_name", - "group_admins" + "display_name" ] }, "AccountStatus": { diff --git a/pydatalab/schemas/sample.json b/pydatalab/schemas/sample.json index 42ba3af3d..10312e833 100644 --- a/pydatalab/schemas/sample.json +++ b/pydatalab/schemas/sample.json @@ -383,8 +383,7 @@ }, "required": [ "group_id", - "display_name", - "group_admins" + "display_name" ] }, "AccountStatus": { diff --git a/pydatalab/schemas/startingmaterial.json b/pydatalab/schemas/startingmaterial.json index 11bf9d28c..2f0045723 100644 --- a/pydatalab/schemas/startingmaterial.json +++ b/pydatalab/schemas/startingmaterial.json @@ -436,8 +436,7 @@ }, "required": [ "group_id", - "display_name", - "group_admins" + "display_name" ] }, "AccountStatus": { diff --git a/pydatalab/src/pydatalab/models/people.py b/pydatalab/src/pydatalab/models/people.py index c6f19bbc0..5e11b30e4 100644 --- a/pydatalab/src/pydatalab/models/people.py +++ b/pydatalab/src/pydatalab/models/people.py @@ -117,7 +117,7 @@ class Group(Entry): description: Optional[str] """A description of the group""" - group_admins: List[PyObjectId] + group_admins: Optional[List[PyObjectId]] """A list of user IDs that can manage this group.""" diff --git a/pydatalab/src/pydatalab/routes/v0_1/__init__.py b/pydatalab/src/pydatalab/routes/v0_1/__init__.py index a1577118b..687f9030f 100644 --- a/pydatalab/src/pydatalab/routes/v0_1/__init__.py +++ b/pydatalab/src/pydatalab/routes/v0_1/__init__.py @@ -7,11 +7,12 @@ from .collections import COLLECTIONS from .files import FILES from .graphs import GRAPHS +from .groups import GROUPS from .healthcheck import HEALTHCHECK from .info import INFO from .items import ITEMS from .remotes import REMOTES -from .users import GROUPS, USERS +from .users import USERS BLUEPRINTS: tuple[Blueprint, ...] = ( AUTH, diff --git a/pydatalab/src/pydatalab/routes/v0_1/admin.py b/pydatalab/src/pydatalab/routes/v0_1/admin.py index 0a2fbd729..167e91bb1 100644 --- a/pydatalab/src/pydatalab/routes/v0_1/admin.py +++ b/pydatalab/src/pydatalab/routes/v0_1/admin.py @@ -1,10 +1,13 @@ +import json + +import pymongo.errors from bson import ObjectId from flask import Blueprint, jsonify, request from flask_login import current_user from pydatalab.config import CONFIG from pydatalab.models.people import Group, User -from pydatalab.mongo import flask_mongo +from pydatalab.mongo import _get_active_mongo_client, flask_mongo from pydatalab.permissions import admin_only ADMIN = Blueprint("admins", __name__) @@ -54,7 +57,7 @@ def get_users(): ] ) - return jsonify({"status": "success", "data": list(User(**u).json() for u in users)}) + return jsonify({"status": "success", "data": list(json.loads(User(**u).json()) for u in users)}) @ADMIN.route("/roles/", methods=["PATCH"]) @@ -111,5 +114,80 @@ def save_role(user_id): @ADMIN.route("/groups", methods=["GET"]) def get_groups(): return jsonify( - {"status": "success", "data": [Group(**d).json() for d in flask_mongo.db.groups.find()]} + { + "status": "success", + "data": [json.loads(Group(**d).json()) for d in flask_mongo.db.groups.find()], + } ), 200 + + +@ADMIN.route("/groups", methods=["PUT"]) +@admin_only +def create_group(): + request_json = request.get_json() + + group_json = { + "group_id": request_json.get("group_id"), + "display_name": request_json.get("display_name"), + "description": request_json.get("description"), + "group_admins": request_json.get("group_admins"), + } + try: + group = Group(**group_json) + except Exception as e: + return jsonify({"status": "error", "message": f"Invalid group data: {str(e)}"}), 400 + + try: + group_immutable_id = flask_mongo.db.groups.insert_one(group.dict()).inserted_id + except pymongo.errors.DuplicateKeyError: + return jsonify( + {"status": "error", "message": f"Group ID {group.group_id} already exists."} + ), 400 + + if group_immutable_id: + return jsonify({"status": "success", "group_immutable_id": str(group_immutable_id)}), 200 + + return jsonify({"status": "error", "message": "Unable to create group."}), 400 + + +@ADMIN.route("/groups", methods=["DELETE"]) +def delete_group(): + request_json = request.get_json() + + group_id = request_json.get("immutable_id") + if group_id is not None: + result = flask_mongo.db.groups.delete_one({"_id": ObjectId(group_id)}) + + if result.deleted_count == 1: + return jsonify({"status": "success"}), 200 + + return jsonify({"status": "error", "message": "Unable to delete group."}), 400 + + +@ADMIN.route("/groups/", methods=["PATCH"]) +def add_user_to_group(group_immutable_id): + request_json = request.get_json() + + user_id = request_json.get("user_id") + + if not user_id: + return jsonify({"status": "error", "message": "No user ID provided."}), 400 + + client = _get_active_mongo_client() + with client.start_session(causal_consistency=True) as session: + group_exists = flask_mongo.db.groups.find_one( + {"_id": ObjectId(group_immutable_id)}, session=session + ) + if not group_exists: + return jsonify({"status": "error", "message": "Group does not exist."}), 400 + + update_user = flask_mongo.db.users.update_one( + {"_id": ObjectId(user_id)}, + {"$addToSet": {"groups": group_immutable_id}}, + session=session, + ) + + if not update_user.modified_count == 1: + return jsonify({"status": "error", "message": "Unable to add user to group."}), 400 + + return jsonify({"status": "error", "message": "Unable to add user to group."}), 400 diff --git a/pydatalab/src/pydatalab/routes/v0_1/groups.py b/pydatalab/src/pydatalab/routes/v0_1/groups.py new file mode 100644 index 000000000..8b3aa1376 --- /dev/null +++ b/pydatalab/src/pydatalab/routes/v0_1/groups.py @@ -0,0 +1,46 @@ +import json + +from flask import Blueprint, jsonify, request + +from pydatalab.models.people import Group +from pydatalab.mongo import flask_mongo +from pydatalab.permissions import active_users_or_get_only + +GROUPS = Blueprint("groups", __name__) + + +@GROUPS.route("/search/groups", methods=["GET"]) +@active_users_or_get_only +def search_groups(): + """Perform free text search on groups and return the top results. + GET parameters: + query: String with the search terms. + nresults: Maximum number of results (default 100) + + Returns: + response list of dictionaries containing the matching groups in order of + descending match score. + """ + + query = request.args.get("query", type=str) + nresults = request.args.get("nresults", default=100, type=int) + match_obj = {"$text": {"$search": query}} + + cursor = flask_mongo.db.groups.aggregate( + [ + {"$match": match_obj}, + {"$sort": {"score": {"$meta": "textScore"}}}, + {"$limit": nresults}, + { + "$project": { + "_id": 1, + "display_name": 1, + "description": 1, + "group_id": 1, + } + }, + ] + ) + return jsonify( + {"status": "success", "data": list(json.loads(Group(**d).json()) for d in cursor)} + ), 200 diff --git a/pydatalab/src/pydatalab/routes/v0_1/users.py b/pydatalab/src/pydatalab/routes/v0_1/users.py index ac845a46d..e73dc6278 100644 --- a/pydatalab/src/pydatalab/routes/v0_1/users.py +++ b/pydatalab/src/pydatalab/routes/v0_1/users.py @@ -1,17 +1,15 @@ import json -import pymongo.errors from bson import ObjectId from flask import Blueprint, jsonify, request from flask_login import current_user from pydatalab.config import CONFIG -from pydatalab.models.people import DisplayName, EmailStr, Group, Person -from pydatalab.mongo import _get_active_mongo_client, flask_mongo +from pydatalab.models.people import DisplayName, EmailStr, Person +from pydatalab.mongo import flask_mongo from pydatalab.permissions import active_users_or_get_only, admin_only USERS = Blueprint("users", __name__) -GROUPS = Blueprint("groups", __name__) @USERS.before_request @@ -95,11 +93,6 @@ def add_user_to_group(group_immutable_id): return jsonify({"status": "error", "message": "Unable to add user to group."}), 400 -@USERS.before_request -@active_users_or_get_only -def _(): ... - - @USERS.route("/users/", methods=["PATCH"]) def save_user(user_id): request_json = request.get_json() diff --git a/pydatalab/tests/server/test_users.py b/pydatalab/tests/server/test_users.py index 3c1799b87..0704078ed 100644 --- a/pydatalab/tests/server/test_users.py +++ b/pydatalab/tests/server/test_users.py @@ -184,3 +184,8 @@ def test_create_group(admin_client, client, unauthenticated_client, real_mongo_c real_mongo_client.get_database().groups.find_one({"group_id": good_group["group_id"]}) is None ) + + # Check a user can search groups + resp = client.get("/search/groups?query=New") + assert resp.status_code == 200 + assert len(resp.json["data"]) == 1 From 367b138f8c01ec416ca609df5875fa0eb0f31e8e Mon Sep 17 00:00:00 2001 From: Matthew Evans Date: Tue, 11 Mar 2025 21:30:37 +0000 Subject: [PATCH 06/18] Add group display to admin panel --- webapp/src/components/AdminDisplay.vue | 4 +- webapp/src/components/GroupTable.vue | 67 ++++++++++++++++++++++++++ webapp/src/components/UserTable.vue | 2 +- webapp/src/server_fetch_utils.js | 13 +++++ webapp/src/views/Admin.vue | 2 +- 5 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 webapp/src/components/GroupTable.vue diff --git a/webapp/src/components/AdminDisplay.vue b/webapp/src/components/AdminDisplay.vue index c9905f2c6..eb79901dd 100644 --- a/webapp/src/components/AdminDisplay.vue +++ b/webapp/src/components/AdminDisplay.vue @@ -1,15 +1,17 @@ + + diff --git a/webapp/src/components/UserTable.vue b/webapp/src/components/UserTable.vue index 4e4610a2c..6f4355671 100644 --- a/webapp/src/components/UserTable.vue +++ b/webapp/src/components/UserTable.vue @@ -9,7 +9,7 @@ - + {{ user.display_name }} diff --git a/webapp/src/server_fetch_utils.js b/webapp/src/server_fetch_utils.js index 6695eb396..1091a84ac 100644 --- a/webapp/src/server_fetch_utils.js +++ b/webapp/src/server_fetch_utils.js @@ -270,6 +270,19 @@ export function getUsersList() { }); } +export function getGroupsList() { + return fetch_get(`${API_URL}/groups`) + .then(function (response_json) { + const groups = response_json.data; + return groups; + }) + .catch((error) => { + console.error("Error when fetching groups list"); + console.error(error); + throw error; + }); +} + export function getStartingMaterialList() { return fetch_get(`${API_URL}/starting-materials/`) .then(function (response_json) { diff --git a/webapp/src/views/Admin.vue b/webapp/src/views/Admin.vue index 1625e44cf..b34c91f90 100644 --- a/webapp/src/views/Admin.vue +++ b/webapp/src/views/Admin.vue @@ -23,7 +23,7 @@ export default { }, data() { return { - items: ["Users"], + items: ["Users", "Groups"], selectedItem: "Users", user: null, }; From 37b15a058decdb6f345541ed95314784929d12b1 Mon Sep 17 00:00:00 2001 From: Matthew Evans Date: Thu, 13 Mar 2025 18:52:55 +0000 Subject: [PATCH 07/18] Fix user table issue caused by API change --- webapp/src/components/UserTable.vue | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/webapp/src/components/UserTable.vue b/webapp/src/components/UserTable.vue index 6f4355671..fd499a849 100644 --- a/webapp/src/components/UserTable.vue +++ b/webapp/src/components/UserTable.vue @@ -33,7 +33,7 @@