feat(roles): add list support for roles #5754

apoorvdixit88 · 2024-08-30T11:27:23Z

Type of Change

Description

Add support to

List available roles with info for the user
List roles at entity level for the user

Additional Changes

This PR modifies the API contract
This PR modifies the database schema
This PR modifies application configuration/environment variables

Motivation and Context

Closes #5753

How did you test it?

To List roles available roles in hierarchy with info:

curl --location 'http://localhost:8080/user/role/v2/list' \
--header 'Authorization: Bearer JWT'

Response

[
    {
        "role_id": "merchant_operator",
        "role_name": "operator",
        "entity_type": "merchant",
        "groups": [
            "operations_view",
            "operations_manage",
            "connectors_view",
            "workflows_view",
            "analytics_view",
            "users_view",
            "merchant_details_view"
        ],
        "scope": "organization"
    },
    {
        "role_id": "merchant_developer",
        "role_name": "developer",
        "entity_type": "merchant",
        "groups": [
            "operations_view",
            "connectors_view",
            "analytics_view",
            "users_view",
            "merchant_details_view",
            "merchant_details_manage"
        ],
        "scope": "organization"
    },
    {
        "role_id": "merchant_iam_admin",
        "role_name": "iam",
        "entity_type": "merchant",
        "groups": [
            "operations_view",
            "analytics_view",
            "users_view",
            "users_manage",
            "merchant_details_view"
        ],
        "scope": "organization"
    },
    {
        "role_id": "merchant_view_only",
        "role_name": "view_only",
        "entity_type": "merchant",
        "groups": [
            "operations_view",
            "connectors_view",
            "workflows_view",
            "analytics_view",
            "users_view",
            "merchant_details_view"
        ],
        "scope": "organization"
    },
    {
        "role_id": "merchant_customer_support",
        "role_name": "customer_support",
        "entity_type": "merchant",
        "groups": [
            "operations_view",
            "analytics_view",
            "users_view",
            "merchant_details_view"
        ],
        "scope": "organization"
    },
    {
        "role_id": "merchant_admin",
        "role_name": "admin",
        "entity_type": "merchant",
        "groups": [
            "operations_view",
            "operations_manage",
            "connectors_view",
            "connectors_manage",
            "workflows_view",
            "workflows_manage",
            "analytics_view",
            "users_view",
            "users_manage",
            "merchant_details_view",
            "merchant_details_manage"
        ],
        "scope": "organization"
    },
    {
        "role_id": "org_admin",
        "role_name": "organization_admin",
        "entity_type": "organization",
        "groups": [
            "operations_view",
            "operations_manage",
            "connectors_view",
            "connectors_manage",
            "workflows_view",
            "workflows_manage",
            "analytics_view",
            "users_view",
            "users_manage",
            "merchant_details_view",
            "merchant_details_manage",
            "organization_manage"
        ],
        "scope": "organization"
    }
]

To list roles at entity level for invite:

curl --location 'http://localhost:8080/user/role/list/invite?entity_type=merchant' \
--header 'Authorization: Bearer JWT'

Entity type can be organization, merchant or profile
Response:

[
    {
        "role_id": "merchant_customer_support",
        "role_name": "customer_support"
    },
    {
        "role_id": "merchant_admin",
        "role_name": "admin"
    },
    {
        "role_id": "merchant_view_only",
        "role_name": "view_only"
    },
    {
        "role_id": "merchant_developer",
        "role_name": "developer"
    },
    {
        "role_id": "merchant_iam_admin",
        "role_name": "iam"
    },
    {
        "role_id": "merchant_operator",
        "role_name": "operator"
    }
]

To list all roles at entity level for update

curl --location 'http://localhost:8080/user/role/list/update?entity_type=merchant' \
--header 'Authorization: Bearer JWT'

Response:

[
    {
        "role_id": "merchant_customer_support",
        "role_name": "customer_support"
    },
    {
        "role_id": "merchant_admin",
        "role_name": "admin"
    },
    {
        "role_id": "merchant_view_only",
        "role_name": "view_only"
    },
    {
        "role_id": "merchant_developer",
        "role_name": "developer"
    },
    {
        "role_id": "merchant_iam_admin",
        "role_name": "iam"
    },
    {
        "role_id": "merchant_operator",
        "role_name": "operator"
    }
]

Checklist

I formatted the code cargo +nightly fmt --all
I addressed lints thrown by cargo clippy
I reviewed the submitted code
I added unit tests for my changes where possible

semanticdiff-com · 2024-08-30T11:27:26Z

Review changes with SemanticDiff.

Analyzed 13 of 13 files.

Overall, the semantic diff is 5% smaller than the GitHub diff.

	Filename	Status
✔️	crates/router_env/src/logger/types.rs	Analyzed
✔️	crates/router/src/routes/app.rs	26.88% smaller
✔️	crates/router/src/routes/lock_utils.rs	Analyzed
✔️	crates/router/src/routes/user_role.rs	Analyzed
✔️	crates/router/src/db/kafka_store.rs	Analyzed
✔️	crates/router/src/db/role.rs	Analyzed
✔️	crates/router/src/core/user_role.rs	Analyzed
✔️	crates/router/src/core/user_role/role.rs	0.42% smaller
✔️	crates/diesel_models/src/query/role.rs	14.45% smaller
✔️	crates/api_models/src/user.rs	Analyzed
✔️	crates/api_models/src/user_role.rs	Analyzed
✔️	crates/api_models/src/user_role/role.rs	Analyzed
✔️	crates/api_models/src/events/user_role.rs	63.56% smaller

ThisIsMani · 2024-08-31T12:44:51Z

crates/router/src/core/user_role/role.rs

+        let role_info = roles::RoleInfo::from(role);
+        role_api::MinimalRoleInfo {
+            role_id: role_info.get_role_id().to_string(),
+            role_name: role_info.get_role_name().to_string(),
+        }


Add entity type check and is_updatable / is_invitable.

entity type check is already handled in db query, added check for invitable/updatable

ThisIsMani · 2024-08-31T12:46:11Z

crates/router/src/routes/app.rs

+                .service(
+                    web::scope("/list")
+                        .service(web::resource("").route(web::get().to(list_all_roles)))
+                        .service(web::resource("/v2").route(web::get().to(list_roles_with_info)))


Change this to /v2/list if possible.

ThisIsMani · 2024-08-31T15:24:25Z

crates/router/src/core/user_role/role.rs

+    let custom_roles_map = custom_roles.into_iter().filter_map(|role| {
+        let role_info = roles::RoleInfo::from(role);
+
+        if match check_type {
+            role_api::RoleCheckType::Invite => role_info.is_invitable(),
+            role_api::RoleCheckType::Update => role_info.is_updatable(),
+        } {
+            Some(role_api::MinimalRoleInfo {
+                role_id: role_info.get_role_id().to_string(),
+                role_name: role_info.get_role_name().to_string(),
+            })
+        } else {
+            None
+        }
+    });


NIT: Push every thing to a vector once and then do these checks.

racnan · 2024-08-31T17:13:41Z

crates/router/src/routes/user_role.rs

+        &req,
+        (),
+        |state, user_from_token, _, _| role_core::list_roles_with_info(state, user_from_token),
+        &auth::DashboardNoPermissionAuth,


UserRead Permission required here.

Yes that would be more precise, for now let's keep this auth only.

ThisIsMani

Minor concerns.

ThisIsMani · 2024-09-01T18:03:34Z

crates/router/src/routes/user_role.rs

@@ -305,7 +305,7 @@ pub async fn list_updatable_roles_at_entity_level(
                role_api::RoleCheckType::Update,
            )
        },
-        &auth::DashboardNoPermissionAuth,
+        &auth::JWTAuth(Permission::UsersRead),


Should it be UsersWrite?

No, it should be read only, invite function will check for write, list should be read only.

ThisIsMani · 2024-09-01T18:03:37Z

crates/router/src/routes/user_role.rs

@@ -279,7 +279,7 @@ pub async fn list_invitable_roles_at_entity_level(
                role_api::RoleCheckType::Invite,
            )
        },
-        &auth::DashboardNoPermissionAuth,
+        &auth::JWTAuth(Permission::UsersRead),


Should it be UsersWrite?

racnan · 2024-09-02T06:18:50Z

crates/router/src/core/user_role/role.rs

+    let list_role_info_response = role_info_vec
+        .into_iter()
+        .filter_map(|role_info| {
+            if user_role_entity >= role_info.get_entity_type() {


you can use then on bool.

feat(roles): add list support for roles

3aa96fc

apoorvdixit88 added C-feature Category: Feature request or enhancement S-waiting-on-review Status: This PR has been implemented and needs to be reviewed A-users Area: Users labels Aug 30, 2024

apoorvdixit88 requested review from racnan and ThisIsMani August 30, 2024 11:27

apoorvdixit88 self-assigned this Aug 30, 2024

apoorvdixit88 requested review from a team as code owners August 30, 2024 11:27

apoorvdixit88 added 5 commits August 31, 2024 16:28

fix: routes and updatble check

ef078ff

fix: change flow names

2cb62c9

fix: remove merchant id from response

2092850

fix: remove merchant id

0bb8400

fix: move lists users in ent resp

d70af7c

ThisIsMani reviewed Aug 31, 2024

View reviewed changes

fix: resolve suggestions

fb982f8

apoorvdixit88 requested review from jarnura, lsampras and Narayanbhat166 August 31, 2024 15:24

ThisIsMani previously approved these changes Aug 31, 2024

View reviewed changes

lsampras previously approved these changes Aug 31, 2024

View reviewed changes

racnan previously approved these changes Aug 31, 2024

View reviewed changes

Narayanbhat166 previously approved these changes Sep 1, 2024

View reviewed changes

likhinbopanna added this pull request to the merge queue Sep 1, 2024

github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Sep 1, 2024

apoorvdixit88 added 2 commits September 1, 2024 22:23

Merge branch 'main' into new-roles-list

0913d71

fix: resolve conflicts

05e3483

apoorvdixit88 dismissed Narayanbhat166’s stale review via 05e3483 September 1, 2024 17:56

apoorvdixit88 dismissed stale reviews from racnan, lsampras, and ThisIsMani via 05e3483 September 1, 2024 17:56

ThisIsMani approved these changes Sep 1, 2024

View reviewed changes

jarnura approved these changes Sep 2, 2024

View reviewed changes

racnan approved these changes Sep 2, 2024

View reviewed changes

likhinbopanna added this pull request to the merge queue Sep 2, 2024

Merged via the queue into main with commit e4f1fbc Sep 2, 2024
13 of 14 checks passed

likhinbopanna deleted the new-roles-list branch September 2, 2024 07:08

SanchithHegde removed the S-waiting-on-review Status: This PR has been implemented and needs to be reviewed label Sep 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(roles): add list support for roles #5754

feat(roles): add list support for roles #5754

apoorvdixit88 commented Aug 30, 2024 •

edited

Loading

semanticdiff-com bot commented Aug 30, 2024 •

edited

Loading

ThisIsMani Aug 31, 2024

apoorvdixit88 Aug 31, 2024 •

edited

Loading

ThisIsMani Aug 31, 2024

ThisIsMani Aug 31, 2024

racnan Aug 31, 2024

apoorvdixit88 Sep 1, 2024

ThisIsMani left a comment

ThisIsMani Sep 1, 2024

apoorvdixit88 Sep 1, 2024

ThisIsMani Sep 1, 2024

racnan Sep 2, 2024

feat(roles): add list support for roles #5754

feat(roles): add list support for roles #5754

Conversation

apoorvdixit88 commented Aug 30, 2024 • edited Loading

Type of Change

Description

Additional Changes

Motivation and Context

How did you test it?

Checklist

semanticdiff-com bot commented Aug 30, 2024 • edited Loading

Choose a reason for hiding this comment

apoorvdixit88 Aug 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThisIsMani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apoorvdixit88 commented Aug 30, 2024 •

edited

Loading

semanticdiff-com bot commented Aug 30, 2024 •

edited

Loading

apoorvdixit88 Aug 31, 2024 •

edited

Loading