From 10eebe672981b57181ba34f65aaa1a40e6f47afa Mon Sep 17 00:00:00 2001 From: claravox Date: Wed, 22 Nov 2023 16:27:05 +0100 Subject: [PATCH] YDA-5150: Add ability for priv rodsuser to create datamanager group --- tests/features/api/api_group.feature | 72 ++++++++----- tests/features/ui/ui_group.feature | 32 +++--- tests/step_defs/api/test_api_group.py | 21 +++- tests/step_defs/ui/test_ui_group.py | 24 ++--- uuGroup.r | 2 +- uuGroupPolicyChecks.r | 142 +++++++++++++++++++------- 6 files changed, 199 insertions(+), 94 deletions(-) diff --git a/tests/features/api/api_group.feature b/tests/features/api/api_group.feature index a09eb526c..3de21aa73 100644 --- a/tests/features/api/api_group.feature +++ b/tests/features/api/api_group.feature @@ -61,10 +61,26 @@ Feature: Group API And the group "" is created Examples: - | user | group_name | - | functionaladminpriv | research-api-test-group | - | technicaladmin | datamanager-api-test-group | - | technicaladmin | testGroupie | + | user | group_name | + | functionaladminpriv | research-api-test-group | + | functionaladminpriv | research-api-test1-group | + | technicaladmin | not-a-yoda-group | + + + Scenario Outline: Datamanager group creation + Given user is authenticated + And the group "" does not exist + And the user creates a new datamanager group "" + Then the response status code is "200" + And the group "" is created + # api-test is for creating a datamanager group with functionaladminpriv. + # api-test1 is for making sure that can still create a datamanager + # group with technical admin. + + Examples: + | user | group_name | + | functionaladminpriv | datamanager-api-test | + | technicaladmin | datamanager-api-test1 | Scenario Outline: Group update @@ -75,10 +91,11 @@ Feature: Group API And the update to group "" is persisted Examples: - | user | group_name | - | functionaladminpriv | research-api-test-group | - | technicaladmin | datamanager-api-test-group | - | technicaladmin | testGroupie | + | user | group_name | + | functionaladminpriv | research-api-test-group | + | functionaladminpriv | datamanager-api-test | + | technicaladmin | datamanager-api-test1 | + | technicaladmin | not-a-yoda-group | Scenario Outline: Adding user to group @@ -89,10 +106,11 @@ Feature: Group API And user "sterlingarcher" is now a member of the group "" Examples: - | user | group_name | - | functionaladminpriv | research-api-test-group | - | technicaladmin | datamanager-api-test-group | - | technicaladmin | testGroupie | + | user | group_name | + | functionaladminpriv | research-api-test-group | + | functionaladminpriv | datamanager-api-test | + | technicaladmin | datamanager-api-test1 | + | technicaladmin | not-a-yoda-group | Scenario Outline: Group user update role @@ -103,10 +121,11 @@ Feature: Group API And the role of user "sterlingarcher" in group "" is updated Examples: - | user | group_name | - | functionaladminpriv | research-api-test-group | - | technicaladmin | datamanager-api-test-group | - | technicaladmin | testGroupie | + | user | group_name | + | functionaladminpriv | research-api-test-group | + | functionaladminpriv | datamanager-api-test | + | technicaladmin | datamanager-api-test1 | + | technicaladmin | not-a-yoda-group | Scenario Outline: Remove user from group @@ -117,10 +136,11 @@ Feature: Group API And user "sterlingarcher" is no longer a member of the group "" Examples: - | user | group_name | - | functionaladminpriv | research-api-test-group | - | technicaladmin | datamanager-api-test-group | - | technicaladmin | testGroupie | + | user | group_name | + | functionaladminpriv | research-api-test-group | + | functionaladminpriv | datamanager-api-test | + | technicaladmin | datamanager-api-test1 | + | technicaladmin | not-a-yoda-group | Scenario Outline: Group import CSV @@ -142,8 +162,10 @@ Feature: Group API And the group "" no longer exists Examples: - | user | group_name | - | functionaladminpriv | research-api-test-group | - | technicaladmin | datamanager-api-test-group | - | technicaladmin | testGroupie | - | technicaladmin | research-csvtestgroup | + | user | group_name | + | functionaladminpriv | research-api-test-group | + | functionaladminpriv | datamanager-api-test | + | functionaladminpriv | research-api-test1-group | + | technicaladmin | datamanager-api-test1 | + | technicaladmin | research-csvtestgroup | + | technicaladmin | not-a-yoda-group | diff --git a/tests/features/ui/ui_group.feature b/tests/features/ui/ui_group.feature index 9222b94c6..0350f0092 100644 --- a/tests/features/ui/ui_group.feature +++ b/tests/features/ui/ui_group.feature @@ -94,7 +94,7 @@ Feature: Group UI Scenario Outline: Group research create with default schema id - Given user functionaladminpriv is logged in + Given user is logged in And module "group_manager" is shown When user opens add group dialog And groupname is set to @@ -106,8 +106,10 @@ Feature: Group UI And check whether research group properties , , and for user functionaladminpriv Examples: - | category | subcategory | group | expiration_date | - | test-automation | test-automation | ui-test-group | 2030-12-25 | + | user | category | subcategory | group | expiration_date | + | functionaladminpriv | test-automation | test-automation | ui-test-group | 2030-12-25 | + | functionaladminpriv | test-datamanager | test-datamanager | test-datamanager | 2030-12-25 | + | technicaladmin | test-datamanager1 | test-datamanager1 | test-datamanager1 | 2030-12-25 | Scenario Outline: Create new research group starting from same (sub)category of active group at that moment @@ -141,7 +143,7 @@ Feature: Group UI Scenario Outline: Group datamanager create - Given user technicaladmin is logged in + Given user is logged in And module "group_manager" is shown When user opens add group dialog And category is set to @@ -152,8 +154,9 @@ Feature: Group UI And check whether datamanager group properties and are correct Examples: - | category | subcategory | group | - | test-datamanager | test-datamanager | test-datamanager | + | user | category | subcategory | group | + | functionaladminpriv | test-datamanager | test-datamanager | test-datamanager | + | technicaladmin | test-datamanager1 | test-datamanager1 | test-datamanager1 | Scenario Outline: Group remove @@ -164,13 +167,16 @@ Feature: Group UI And user confirms group removal Examples: - | user | category | subcategory | group | - | functionaladminpriv | test-automation | test-automation | research-ui-test-group | - | functionaladminpriv | test-automation | csv-test | research-csv-test-group1 | - | functionaladminpriv | test-automation | csv-test | research-csv-test-group2 | - | functionaladminpriv | test-automation | csv-test | research-csv-test-group3 | - | functionaladminpriv | test-automation | csv-test | research-csv-test-group4 | - | technicaladmin | test-datamanager | test-datamanager | datamanager-test-datamanager | + | user | category | subcategory | group | + | functionaladminpriv | test-automation | test-automation | research-ui-test-group | + | functionaladminpriv | test-automation | csv-test | research-csv-test-group1 | + | functionaladminpriv | test-automation | csv-test | research-csv-test-group2 | + | functionaladminpriv | test-automation | csv-test | research-csv-test-group3 | + | functionaladminpriv | test-automation | csv-test | research-csv-test-group4 | + | functionaladminpriv | test-datamanager | test-datamanager | datamanager-test-datamanager | + | functionaladminpriv | test-datamanager | test-datamanager | research-test-datamanager | + | technicaladmin | test-datamanager1 | test-datamanager1 | datamanager-test-datamanager1 | + | technicaladmin | test-datamanager1 | test-datamanager1 | research-test-datamanager1 | Scenario Outline: Select group in tree view and check group properties are set and active in tree view diff --git a/tests/step_defs/api/test_api_group.py b/tests/step_defs/api/test_api_group.py index e0ae55f09..59693a44f 100644 --- a/tests/step_defs/api/test_api_group.py +++ b/tests/step_defs/api/test_api_group.py @@ -67,12 +67,13 @@ def api_group_does_not_exist(user, group_name): @given(parsers.parse('the user creates a new group "{group_name}"'), target_fixture="api_response") def api_group_create(user, group_name): + cat_and_subcat = "-".join(group_name.split("-")[1:-1]) return api_request( user, "group_create", {"group_name": group_name, - "category": "api-test", - "subcategory": "api-test", + "category": cat_and_subcat, + "subcategory": cat_and_subcat, "schema_id": "default-2", "expiration_date": "", "description": "", @@ -80,6 +81,22 @@ def api_group_create(user, group_name): ) +@given(parsers.parse('the user creates a new datamanager group "{group_name}"'), target_fixture="api_response") +def api_datamanager_group_create(user, group_name): + cat_and_subcat = group_name.split("-", 1)[1] + return api_request( + user, + "group_create", + {"group_name": group_name, + "category": cat_and_subcat, + "subcategory": cat_and_subcat, + "schema_id": "", + "expiration_date": "", + "description": "", + "data_classification": ""} + ) + + @given(parsers.parse('the group "{group_name}" exists')) def given_group_exists(user, group_name): _, body = api_request( diff --git a/tests/step_defs/ui/test_ui_group.py b/tests/step_defs/ui/test_ui_group.py index 93e8a14bb..2d85fa76e 100644 --- a/tests/step_defs/ui/test_ui_group.py +++ b/tests/step_defs/ui/test_ui_group.py @@ -444,39 +444,29 @@ def ui_group_schema_category_is_updated(browser, category): @when(parsers.parse("subcategory is set to {subcategory}")) def ui_group_schema_subcategory_is_set(browser, subcategory): - # Subcategory already exists. browser.find_by_css('#f-group-create-subcategory').find_by_xpath('..').find_by_css('span .select2-selection').click() time.sleep(1) + browser.find_by_css('.select2-search__field').fill(subcategory) + time.sleep(1) options = browser.find_by_css('#select2-f-group-create-subcategory-results') for option in options: - if option.text == subcategory: + if subcategory in option.text: option.click() return True - # Subcategory does not exist. - time.sleep(1) - browser.find_by_css('.select2-search__field').fill(subcategory) - time.sleep(1) - browser.find_by_css('.select2-results__option--highlighted').click() - @when(parsers.parse("subcategory is updated to {subcategory}")) def ui_group_schema_subcategory_is_updated(browser, subcategory): - # Subcategory already exists. browser.find_by_css('#f-group-update-subcategory').find_by_xpath('..').find_by_css('span .select2-selection').click() time.sleep(1) - options = browser.find_by_css('#select2-f-group-create-subcategory-results') + browser.find_by_css('.select2-search__field').fill(subcategory) + time.sleep(1) + options = browser.find_by_css('#select2-f-group-update-subcategory-results') for option in options: - if option.text == subcategory: + if subcategory in option.text: option.click() return True - # Subcategory does not exist. - time.sleep(1) - browser.find_by_css('.select2-search__field').fill(subcategory) - time.sleep(1) - browser.find_by_css('.select2-results__option--highlighted').click() - @when(parsers.parse("schema id is set to {schema_id}")) def ui_group_schema_set_schema_id(browser, schema_id): diff --git a/uuGroup.r b/uuGroup.r index 4c4691890..3b6b46318 100644 --- a/uuGroup.r +++ b/uuGroup.r @@ -770,7 +770,7 @@ uuGroupAdd(*groupName, *category, *subcategory, *schema_id, *expiration_date, *d *kv."category" = *category; *kv."subcategory" = *subcategory; *kv."schema_id" = *schema_id; - *kv."expiration_date" = *expiration_date; + *kv."expiration_date" = *expiration_date; *kv."description" = *description; *kv."data_classification" = *dataClassification; *kv."co_identifier" = *co_identifier; diff --git a/uuGroupPolicyChecks.r b/uuGroupPolicyChecks.r index 90862dc69..7a649644d 100644 --- a/uuGroupPolicyChecks.r +++ b/uuGroupPolicyChecks.r @@ -44,15 +44,16 @@ uuUserNameIsValid(*name) # # NB: Update the category name check below if you change the second half of this pattern. # -# NB: Datamanager is missing in this list. It can only be created by rodsadmin, -# and rodsadmin currently bypasses all checks anyway. -# This check is applicable only to rodsusers with priv-group-add. +# NB: Datamanager is missing in this list. uuGroupNameIsDatamanager is for the specific datamanager case. # # \param[in] name # uuGroupNameIsValid(*name) = *name like regex ``(intake|research|deposit)-([a-z0-9]|[a-z0-9][a-z0-9-]*[a-z0-9])``; +uuGroupNameIsDatamanager(*name) + = *name like regex ``(datamanager)-([a-z0-9]|[a-z0-9][a-z0-9-]*[a-z0-9])``; + # \brief Check if a category name is valid. # # This must be exactly the same as the part of the group name check after the group prefix. @@ -140,6 +141,28 @@ uuGroupExpirationDateIsValid(*expiration_date, *valid) { *valid = bool(*result); } +# \brief Check if actor is a group manager in a group in the category +# +# \param[in] actor +# \param[in] category +# \param[out] isGroupManager +# +uuGroupIsGroupManagerInCategory(*actor, *category, *isGroupManager) { + *isGroupManager = false; + + uuUserGetGroups(*actor, false, *groups); + + foreach (*actorGroup in *groups) { + uuGroupGetCategory(*actorGroup, *agCategory, *agSubcategory); + if (*agCategory == *category) { + uuGroupUserIsManager(*actorGroup, *actor, *isGroupManager); + if (*isGroupManager) { + break; + } + } + } +} + # }}} # \brief Group Policy: Can the user create a new group? @@ -162,8 +185,9 @@ uuGroupPolicyCanGroupAdd(*actor, *groupName, *category, *subcategory, *expiratio *allowed = 0; *reason = ""; - uuGroupUserExists("priv-group-add", *actor, false, *hasPriv); - if (*hasPriv || *actorUserType == "rodsadmin") { + uuGroupUserExists("priv-group-add", *actor, false, *hasPrivGroupAdd); + uuGroupUserExists("priv-category-add", *actor, false, *hasPrivCatAdd); + if (*hasPrivGroupAdd || *actorUserType == "rodsadmin") { if (uuGroupNameIsValid(*groupName)) { uuUserNameIsAvailable(*groupName, *nameAvailable, *existingType); if (*nameAvailable) { @@ -220,8 +244,37 @@ uuGroupPolicyCanGroupAdd(*actor, *groupName, *category, *subcategory, *expiratio } *reason = "The name '*groupName' is already in use by another *existingType."; } + } else if (uuGroupNameIsDatamanager(*groupName)) { + if ((*hasPrivGroupAdd && *hasPrivCatAdd) || *actorUserType == "rodsadmin") { + # Special case for a datamanager group. + uuUserNameIsAvailable(*groupName, *nameAvailable, *existingType); + if (*nameAvailable) { + # The category for which the datamanager group is paired with must already exist + # And actor must be a group manager in at least one group + # in the category for them to be allowed to create a datamanager group + *isManagerInCategory = false; + + uuGroupIsGroupManagerInCategory(*actor, *category, *isManagerInCategory) + if (*isManagerInCategory) { + *allowed = 1 + } + else { + *reason = "You must be a manager of a group in category '*category' to create a datamanager- group" + } + } else { + if (*existingType == "rodsuser") { + *existingType = "user"; + } else if (*existingType == "rodsgroup") { + *existingType = "group"; + } + *reason = "The name '*groupName' is already in use by another *existingType."; + } + + } else { + *reason = "You must have priv-group-add and priv-cat-add to add a datamanger group" + } } else { - *reason = "Group names must start with one of 'intake-' or 'research-' or 'deposit-' and may only contain lowercase letters (a-z) and hyphens (-)."; + *reason = "Group names must start with one of 'intake-', 'research-', 'deposit-', or 'datamanager-' and may only contain lowercase letters (a-z) and hyphens (-)."; } } else { *reason = "You cannot create groups because you are not a member of the priv-group-add group."; @@ -360,12 +413,11 @@ uuGroupPolicyCanGroupRemove(*actor, *groupName, *allowed, *reason) { *reason = ""; uuGroupUserIsManager(*groupName, *actor, *isManager); - if (*isManager || *actorUserType == "rodsadmin") { - # Only a rodsadmin can remove a datamanager-group - # Even datamanager group managers cannot remove their own group. - # v These groups are user-removable v - if (*groupName like regex "(grp|intake|research|deposit|vault)-.*" - || (*groupName like regex "(datamanager)-.*") && *actorUserType == "rodsadmin") { + # Only a rodsadmin or a privileged group manager in + # that category can remove a datamanager-group. + # v These groups are user-removable v + if (*groupName like regex "(grp|intake|research|deposit|vault)-.*" ) { + if (*isManager || *actorUserType == "rodsadmin") { *homeCollection = "/$rodsZoneClient/home/*groupName"; *homeCollectionIsEmpty = true; @@ -377,36 +429,54 @@ uuGroupPolicyCanGroupRemove(*actor, *groupName, *allowed, *reason) { } if (*homeCollectionIsEmpty) { - if (*groupName like regex "(research)-.*") { - # Research groups can only be removed when no vault packages exist - uuChop(*groupName, *prefix, *base, "-", true); - *vaultName = "vault-*base"; - *zoneName = $rodsZoneClient; - *vaultIsEmpty = true; - - # Check whether vault still holds data - msiMakeGenQuery("COLL_NAME", "COLL_NAME like '/*zoneName/home/*vaultName/%'", *genQIn); - msiExecGenQuery(*genQIn, *genQOut); - foreach(*genQOut){ - *vaultIsEmpty = false; - break; - } - if (*vaultIsEmpty) { - *allowed = 1; - } else { - *reason = "There are still datapackages in the vault for group: *groupName. Please remove these first before removing this group."; - } - } else { - *allowed = 1; - } + if (*groupName like regex "(research)-.*") { + # Research groups can only be removed when no vault packages exist + uuChop(*groupName, *prefix, *base, "-", true); + *vaultName = "vault-*base"; + *zoneName = $rodsZoneClient; + *vaultIsEmpty = true; + + # Check whether vault still holds data + msiMakeGenQuery("COLL_NAME", "COLL_NAME like '/*zoneName/home/*vaultName/%'", *genQIn); + msiExecGenQuery(*genQIn, *genQOut); + foreach(*genQOut){ + *vaultIsEmpty = false; + break; + } + if (*vaultIsEmpty) { + *allowed = 1; + } else { + *reason = "There are still datapackages in the vault for group: *groupName. Please remove these first before removing this group."; + } + } else { + *allowed = 1; + } } else { *reason = "The group's directory is not empty. Please remove all of its files and subdirectories before removing this group."; } } else { - *reason = "'*groupName' is not a regular group. You can only remove groups that have one of the following prefixes: grp, intake, research, vault."; + *reason = "You are not a manager of group *groupName."; + } + } else if (*groupName like regex "(datamanager)-.*") { + # Confirm actor is a manager of at least one group in the category + # and has the privileges + uuGroupGetCategory(*groupName, *category, *agSubcategory); + uuGroupIsGroupManagerInCategory(*actor, *category, *isManagerInCategory) + if (*isManagerInCategory) { + uuGroupUserExists("priv-group-add", *actor, false, *hasPrivGroupAdd); + uuGroupUserExists("priv-category-add", *actor, false, *hasPrivCatAdd); + if (*hasPrivGroupAdd && *hasPrivCatAdd) { + *allowed = 1 + } + else { + *reason = "You must have group and category add privileges to delete this datamanager- group"; + } + } + else { + *reason = "You must be a manager of a group in category '*category' to delete this datamanager- group" } } else { - *reason = "You are not a manager of group *groupName."; + *reason = "'*groupName' is not a regular group. You can only remove groups that have one of the following prefixes: grp, intake, research, vault."; } }