From 1b7d305d06af42f0996ae54fea223ecc5a081b72 Mon Sep 17 00:00:00 2001 From: Michal Konecny Date: Dec 06 2023 15:48:26 +0000 Subject: Add api endpoints for adding/removing user to group This change will add new API endpoints that will allow users to add/remove new members to group. It also adds new ACL group_modify, which adds group add/remove ability to token. Closes #5333 Signed-off-by: Michal Konecny --- diff --git a/pagure/api/__init__.py b/pagure/api/__init__.py index 7d4cb14..14486f0 100644 --- a/pagure/api/__init__.py +++ b/pagure/api/__init__.py @@ -613,7 +613,12 @@ def api(): ] sections.append(build_docs_section("users", users_methods)) - groups_methods = [group.api_groups, group.api_view_group] + groups_methods = [ + group.api_groups, + group.api_view_group, + group.api_group_add_member, + group.api_group_remove_member, + ] sections.append(build_docs_section("groups", groups_methods)) plugins_methods = [ diff --git a/pagure/api/group.py b/pagure/api/group.py index 664e37b..dcb687f 100644 --- a/pagure/api/group.py +++ b/pagure/api/group.py @@ -12,6 +12,7 @@ from __future__ import absolute_import, unicode_literals import flask +from sqlalchemy.exc import SQLAlchemyError import pagure import pagure.exceptions @@ -20,6 +21,7 @@ from pagure.api import ( API, APIERROR, api_login_optional, + api_login_required, api_method, get_page, get_per_page, @@ -289,3 +291,179 @@ def api_view_group(group): jsonout = flask.jsonify(output) jsonout.status_code = 200 return jsonout + + +@API.route("/group//add", methods=["POST"]) +@api_login_required(acls=["group_modify"]) +@api_method +def api_group_add_member(group): + """ + Add member to group + ------------------- + Add new member to group. To be able to add users to group the requester + needs to have permissions to do that. + + :: + + POST /api/0/group//add + + Input + ^^^^^ + + +---------------------+--------+-------------+-----------------------------+ + | Key | Type | Optionality | Description | + +=====================+========+=============+=============================+ + | ``user`` | string | Mandatory | | User to add as member | + | | | | of group | + +---------------------+--------+-------------+-----------------------------+ + + Sample response + ^^^^^^^^^^^^^^^ + + :: + + { + "creator": { + "default_email": "user1@example.com", + "emails": [ + "user1@example.com" + ], + "fullname": "User1", + "name": "user1" + }, + "date_created": "1492011511", + "description": "Some Group", + "display_name": "Some Group", + "group_type": "user", + "members": [ + "user1", + "user2" + ], + "name": "some_group_name" + } + + """ # noqa + + group = pagure.lib.query.search_groups(flask.g.session, group_name=group) + if not group: + raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOGROUP) + + # Validate inputs + form = pagure.forms.AddUserToGroupForm(meta={"csrf": False}) + if not form.validate_on_submit(): + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.EINVALIDREQ, errors=form.errors + ) + else: + # Add user to group + try: + pagure.lib.query.add_user_to_group( + flask.g.session, + username=form.user.data, + group=group, + user=flask.g.fas_user.username, + is_admin=pagure.utils.is_admin(), + ) + flask.g.session.commit() + pagure.lib.git.generate_gitolite_acls( + project=None, group=group.group_name + ) + except (pagure.exceptions.PagureException, SQLAlchemyError) as err: + flask.g.session.rollback() + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.EDBERROR, errors=[str(err)] + ) + + # Return the updated group + output = group.to_json(public=(not pagure.utils.api_authenticated())) + jsonout = flask.jsonify(output) + jsonout.status_code = 200 + return jsonout + + +@API.route("/group//remove", methods=["POST"]) +@api_login_required(acls=["group_modify"]) +@api_method +def api_group_remove_member(group): + """ + Remove member from group + ------------------------ + Remove member from group. To be able to remove users from group the requester + needs to have permissions to do that. + + :: + + POST /api/0/group//remove + + Input + ^^^^^ + + +---------------------+--------+-------------+-----------------------------+ + | Key | Type | Optionality | Description | + +=====================+========+=============+=============================+ + | ``user`` | string | Mandatory | | User to add as member | + | | | | of group | + +---------------------+--------+-------------+-----------------------------+ + + Sample response + ^^^^^^^^^^^^^^^ + + :: + + { + "creator": { + "default_email": "user1@example.com", + "emails": [ + "user1@example.com" + ], + "fullname": "User1", + "name": "user1" + }, + "date_created": "1492011511", + "description": "Some Group", + "display_name": "Some Group", + "group_type": "user", + "members": [ + "user1", + "user2" + ], + "name": "some_group_name" + } + + """ # noqa + + group = pagure.lib.query.search_groups(flask.g.session, group_name=group) + if not group: + raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOGROUP) + + # Validate inputs + form = pagure.forms.AddUserToGroupForm(meta={"csrf": False}) + if not form.validate_on_submit(): + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.EINVALIDREQ, errors=form.errors + ) + else: + # Remove user to group + try: + pagure.lib.query.delete_user_of_group( + flask.g.session, + username=form.user.data, + groupname=group.group_name, + user=flask.g.fas_user.username, + is_admin=pagure.utils.is_admin(), + ) + flask.g.session.commit() + pagure.lib.git.generate_gitolite_acls( + project=None, group=group.group_name + ) + except (pagure.exceptions.PagureException, SQLAlchemyError) as err: + flask.g.session.rollback() + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.EDBERROR, errors=[str(err)] + ) + + # Return the updated group + output = group.to_json(public=(not pagure.utils.api_authenticated())) + jsonout = flask.jsonify(output) + jsonout.status_code = 200 + return jsonout diff --git a/pagure/default_config.py b/pagure/default_config.py index 528aae8..be14972 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -373,6 +373,7 @@ ACLS = { "modify_git_alias": "Modify git aliases (create or delete)", "create_git_alias": "Create git aliases", "delete_git_alias": "Delete git aliases", + "group_modify": "Add/Remove members from group", } # List of ACLs which a regular user is allowed to associate to an API token @@ -389,6 +390,7 @@ CROSS_PROJECT_ACLS = [ "create_project", "fork_project", "modify_project", + "group_modify", "update_watch_status", "pull_request_create", "commit", @@ -404,6 +406,7 @@ ADMIN_API_ACLS = [ "pull_request_comment", "pull_request_merge", "generate_acls_project", + "group_modify", "commit_flag", "create_branch", "tag_project", diff --git a/tests/test_pagure_admin.py b/tests/test_pagure_admin.py index 026ade0..b64ad67 100644 --- a/tests/test_pagure_admin.py +++ b/tests/test_pagure_admin.py @@ -2291,6 +2291,7 @@ class PagureAdminUpdateAclsTests(tests.Modeltests): "delete_git_alias", "fork_project", "generate_acls_project", + "group_modify", "internal_access", "issue_assign", "issue_change_status", @@ -2343,6 +2344,7 @@ class PagureAdminUpdateAclsTests(tests.Modeltests): "dummy_acls", "fork_project", "generate_acls_project", + "group_modify", "internal_access", "issue_assign", "issue_change_status", diff --git a/tests/test_pagure_flask_api_group.py b/tests/test_pagure_flask_api_group.py index 8199add..35178cb 100644 --- a/tests/test_pagure_flask_api_group.py +++ b/tests/test_pagure_flask_api_group.py @@ -15,6 +15,8 @@ import unittest import sys import os import json +from unittest.mock import patch +from sqlalchemy.exc import SQLAlchemyError sys.path.insert( 0, os.path.join(os.path.dirname(os.path.abspath(__file__)), "..") @@ -22,6 +24,7 @@ sys.path.insert( import pagure.api import pagure.lib.query +from pagure.exceptions import PagureException import tests @@ -773,6 +776,388 @@ class PagureFlaskApiGroupTests(tests.SimplePagureTest): # a different order that was the order of requests assert projects == ["test", "test2"] + def test_api_group_add_member_authenticated(self): + """ + Test the api_group_add_member method of the flask api with an + authenticated user. + """ + tests.create_tokens(self.session) + tests.create_tokens_acl(self.session) + + headers = {"Authorization": "token aaabbbcccddd"} + payload = {"user": "foo"} + output = self.app.post( + "/api/0/group/some_group/add", data=payload, headers=headers + ) + self.assertEqual(output.status_code, 200) + exp = { + "display_name": "Some Group", + "full_url": "http://localhost.localdomain/group/some_group", + "description": None, + "creator": { + "fullname": "PY C", + "full_url": "http://localhost.localdomain/user/pingou", + "url_path": "user/pingou", + "default_email": "bar@pingou.com", + "emails": ["bar@pingou.com", "foo@pingou.com"], + "name": "pingou", + }, + "members": ["pingou", "foo"], + "date_created": "1492020239", + "group_type": "user", + "name": "some_group", + } + data = json.loads(output.get_data(as_text=True)) + data["date_created"] = "1492020239" + self.assertDictEqual(data, exp) + + def test_api_group_add_member_unauthenticated(self): + """ + Assert that api_group_add_member method will fail with + unauthenticated user. + """ + headers = {"Authorization": "token aaabbbcccddd"} + payload = {"user": "foo"} + output = self.app.post( + "/api/0/group/some_group/add", data=payload, headers=headers + ) + self.assertEqual(output.status_code, 401) + exp = { + "error": ( + "Invalid or expired token. " + "Please visit " + "http://localhost.localdomain/settings#nav-api-tab " + "to get or renew your API token." + ), + "error_code": "EINVALIDTOK", + "errors": "Invalid token", + } + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, exp) + + def test_api_group_add_member_no_permission(self): + """ + Assert that api_group_add_member method will fail with + user that don't have permissions to add member to group. + """ + # Create tokens for foo user + tests.create_tokens(self.session, user_id=2) + tests.create_tokens_acl(self.session) + headers = {"Authorization": "token aaabbbcccddd"} + payload = {"user": "foo"} + output = self.app.post( + "/api/0/group/some_group/add", data=payload, headers=headers + ) + self.assertEqual(output.status_code, 400) + exp = { + "error": ( + "An error occurred at the database level " + "and prevent the action from reaching completion" + ), + "error_code": "EDBERROR", + "errors": ["You are not allowed to add user to this group"], + } + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, exp) + + def test_api_group_add_member_no_group(self): + """ + Assert that api_group_add_member method will fail when group doesn't + exist. + """ + tests.create_tokens(self.session) + tests.create_tokens_acl(self.session) + + headers = {"Authorization": "token aaabbbcccddd"} + payload = {"user": "foo"} + output = self.app.post( + "/api/0/group/no_group/add", data=payload, headers=headers + ) + self.assertEqual(output.status_code, 404) + exp = {"error": "Group not found", "error_code": "ENOGROUP"} + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, exp) + + def test_api_group_add_member_invalid_request(self): + """ + Assert that api_group_add_member method will fail when request + is invalid. + """ + tests.create_tokens(self.session) + tests.create_tokens_acl(self.session) + + headers = {"Authorization": "token aaabbbcccddd"} + payload = {"dummy": "foo"} + output = self.app.post( + "/api/0/group/some_group/add", data=payload, headers=headers + ) + self.assertEqual(output.status_code, 400) + exp = { + "error": "Invalid or incomplete input submitted", + "error_code": "EINVALIDREQ", + "errors": {"user": ["This field is required."]}, + } + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, exp) + + @patch("pagure.lib.query.add_user_to_group") + def test_api_group_add_member_pagure_error(self, mock_add_user): + """ + Assert that api_group_add_member method will fail when pagure + throws exception. + """ + tests.create_tokens(self.session) + tests.create_tokens_acl(self.session) + mock_add_user.side_effect = PagureException("Error") + + headers = {"Authorization": "token aaabbbcccddd"} + payload = {"user": "foo"} + output = self.app.post( + "/api/0/group/some_group/add", data=payload, headers=headers + ) + self.assertEqual(output.status_code, 400) + exp = { + "error": ( + "An error occurred at the database level " + "and prevent the action from reaching completion" + ), + "error_code": "EDBERROR", + "errors": ["Error"], + } + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, exp) + + @patch("pagure.lib.query.add_user_to_group") + def test_api_group_add_member_sqlalchemy_error(self, mock_add_user): + """ + Assert that api_group_add_member method will fail when SQLAlchemy + throws exception. + """ + tests.create_tokens(self.session) + tests.create_tokens_acl(self.session) + mock_add_user.side_effect = SQLAlchemyError("Error") + + headers = {"Authorization": "token aaabbbcccddd"} + payload = {"user": "foo"} + output = self.app.post( + "/api/0/group/some_group/add", data=payload, headers=headers + ) + self.assertEqual(output.status_code, 400) + exp = { + "error": ( + "An error occurred at the database level " + "and prevent the action from reaching completion" + ), + "error_code": "EDBERROR", + "errors": ["Error"], + } + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, exp) + + def test_api_group_remove_member_authenticated(self): + """ + Test the api_group_remove_member method of the flask api with an + authenticated user. + """ + tests.create_tokens(self.session) + tests.create_tokens_acl(self.session) + + headers = {"Authorization": "token aaabbbcccddd"} + payload = {"user": "foo"} + # Add user first + output = self.app.post( + "/api/0/group/some_group/add", data=payload, headers=headers + ) + exp = { + "display_name": "Some Group", + "full_url": "http://localhost.localdomain/group/some_group", + "description": None, + "creator": { + "fullname": "PY C", + "full_url": "http://localhost.localdomain/user/pingou", + "url_path": "user/pingou", + "default_email": "bar@pingou.com", + "emails": ["bar@pingou.com", "foo@pingou.com"], + "name": "pingou", + }, + "members": ["pingou", "foo"], + "date_created": "1492020239", + "group_type": "user", + "name": "some_group", + } + data = json.loads(output.get_data(as_text=True)) + data["date_created"] = "1492020239" + self.assertDictEqual(data, exp) + + # Then remove it + output = self.app.post( + "/api/0/group/some_group/remove", data=payload, headers=headers + ) + self.assertEqual(output.status_code, 200) + exp = { + "display_name": "Some Group", + "full_url": "http://localhost.localdomain/group/some_group", + "description": None, + "creator": { + "fullname": "PY C", + "full_url": "http://localhost.localdomain/user/pingou", + "url_path": "user/pingou", + "default_email": "bar@pingou.com", + "emails": ["bar@pingou.com", "foo@pingou.com"], + "name": "pingou", + }, + "members": ["pingou"], + "date_created": "1492020239", + "group_type": "user", + "name": "some_group", + } + data = json.loads(output.get_data(as_text=True)) + data["date_created"] = "1492020239" + self.assertDictEqual(data, exp) + + def test_api_group_remove_member_unauthenticated(self): + """ + Assert that api_group_remove_member method will fail with + unauthenticated user. + """ + headers = {"Authorization": "token aaabbbcccddd"} + payload = {"user": "foo"} + output = self.app.post( + "/api/0/group/some_group/remove", data=payload, headers=headers + ) + self.assertEqual(output.status_code, 401) + exp = { + "error": ( + "Invalid or expired token. " + "Please visit " + "http://localhost.localdomain/settings#nav-api-tab " + "to get or renew your API token." + ), + "error_code": "EINVALIDTOK", + "errors": "Invalid token", + } + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, exp) + + def test_api_group_remove_member_no_permission(self): + """ + Assert that api_group_remove_member method will fail with + user that don't have permissions to remove member to group. + """ + # Create tokens for foo user + tests.create_tokens(self.session, user_id=2) + tests.create_tokens_acl(self.session) + headers = {"Authorization": "token aaabbbcccddd"} + payload = {"user": "foo"} + output = self.app.post( + "/api/0/group/some_group/remove", data=payload, headers=headers + ) + self.assertEqual(output.status_code, 400) + exp = { + "error": ( + "An error occurred at the database level " + "and prevent the action from reaching completion" + ), + "error_code": "EDBERROR", + "errors": ["You are not allowed to remove user from this group"], + } + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, exp) + + def test_api_group_remove_member_no_group(self): + """ + Assert that api_group_remove_member method will fail when group doesn't + exist. + """ + tests.create_tokens(self.session) + tests.create_tokens_acl(self.session) + + headers = {"Authorization": "token aaabbbcccddd"} + payload = {"user": "foo"} + output = self.app.post( + "/api/0/group/no_group/remove", data=payload, headers=headers + ) + self.assertEqual(output.status_code, 404) + exp = {"error": "Group not found", "error_code": "ENOGROUP"} + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, exp) + + def test_api_group_remove_member_invalid_request(self): + """ + Assert that api_group_remove_member method will fail when request + is invalid. + """ + tests.create_tokens(self.session) + tests.create_tokens_acl(self.session) + + headers = {"Authorization": "token aaabbbcccddd"} + payload = {"dummy": "foo"} + output = self.app.post( + "/api/0/group/some_group/remove", data=payload, headers=headers + ) + self.assertEqual(output.status_code, 400) + exp = { + "error": "Invalid or incomplete input submitted", + "error_code": "EINVALIDREQ", + "errors": {"user": ["This field is required."]}, + } + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, exp) + + @patch("pagure.lib.query.delete_user_of_group") + def test_api_group_remove_member_pagure_error(self, mock_remove_user): + """ + Assert that api_group_remove_member method will fail when pagure + throws exception. + """ + tests.create_tokens(self.session) + tests.create_tokens_acl(self.session) + mock_remove_user.side_effect = PagureException("Error") + + headers = {"Authorization": "token aaabbbcccddd"} + payload = {"user": "foo"} + output = self.app.post( + "/api/0/group/some_group/remove", data=payload, headers=headers + ) + self.assertEqual(output.status_code, 400) + exp = { + "error": ( + "An error occurred at the database level " + "and prevent the action from reaching completion" + ), + "error_code": "EDBERROR", + "errors": ["Error"], + } + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, exp) + + @patch("pagure.lib.query.delete_user_of_group") + def test_api_group_remove_member_sqlalchemy_error(self, mock_remove_user): + """ + Assert that api_group_remove_member method will fail when SQLAlchemy + throws exception. + """ + tests.create_tokens(self.session) + tests.create_tokens_acl(self.session) + mock_remove_user.side_effect = SQLAlchemyError("Error") + + headers = {"Authorization": "token aaabbbcccddd"} + payload = {"user": "foo"} + output = self.app.post( + "/api/0/group/some_group/remove", data=payload, headers=headers + ) + self.assertEqual(output.status_code, 400) + exp = { + "error": ( + "An error occurred at the database level " + "and prevent the action from reaching completion" + ), + "error_code": "EDBERROR", + "errors": ["Error"], + } + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, exp) + if __name__ == "__main__": unittest.main(verbosity=2) diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index 3401fa0..9c8c7f8 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -5668,6 +5668,7 @@ foo bar "delete_git_alias", "fork_project", "generate_acls_project", + "group_modify", "internal_access", "issue_assign", "issue_change_status",