fine grained access #208

palagdan · 2024-09-04T14:21:33Z

Resolves partially #202
Resolves partially #158

palagdan · 2024-09-04T14:23:46Z

In the internal authorization, an admin user can assign roles using a multi-select input and can also remove roles as needed. These roles are then saved to the repository.
I can also add a group field, so admin can choose the specific group and roles will be added automatically.

…epository

blcham

see my comments + tests are failing

blcham · 2024-09-05T09:32:51Z

src/components/user/User.jsx

@@ -4,15 +4,16 @@ import withI18n from "../../i18n/withI18n";
 import { injectIntl } from "react-intl";
 import HorizontalInput from "../HorizontalInput";
 import UserValidator from "../../validation/UserValidator";
-import { ACTION_STATUS, ROLE } from "../../constants/DefaultConstants";
+import { ACTION_STATUS, ROLE, ROLE_TYPE } from "../../constants/DefaultConstants";


what is role_type?

it is a mapper between roles and types

blcham · 2024-09-05T09:36:32Z

src/constants/DefaultConstants.js

 };
+
+export const TYPE_ROLE = {


Do we really need to duplicate strings from ROLE?

can we use convention instead? see below

There may be an issue because some roles, such as admin and doctor, do not have the 'rm' prefix, making it difficult to differentiate them. Also there is a problem if we want to display roles in readable form such as "Publish Records" etc.

blcham · 2024-09-05T10:22:02Z

src/constants/Vocabulary.js

+export const DELETE_ALL_RECORDS_TYPE = "http://onto.fel.cvut.cz/ontologies/record-manager/delete-all-records";
+export const EDIT_ALL_RECORDS_TYPE = "http://onto.fel.cvut.cz/ontologies/record-manager/edit-all-records";
+export const VIEW_ALL_RECORDS_TYPE = "http://onto.fel.cvut.cz/ontologies/record-manager/view-all-records";
+export const DELETE_ORGANIZATION_RECORDS_TYPE =


I suggest to infer local name from url by convention:

rm_ <--- 'http://onto.fel.cvut.cz/ontologies/record-manager/'

view_all_records <--- view-all-records

convention is that _ is used instead -

I initially chose the (-) as the divider because I noticed in mode.ttl that other resources and properties use it, such as:

### http://onto.fel.cvut.cz/ontologies/record-manager/has-last-editor rm:has-last-editor rdf:type owl:ObjectProperty ; rdfs:subPropertyOf rm:relates-to . ### http://onto.fel.cvut.cz/ontologies/record-manager/has-member rm:has-member rdf:type owl:ObjectProperty ; rdfs:subPropertyOf rm:relates-to ; owl:inverseOf rm:is-member-of . ### http://onto.fel.cvut.cz/ontologies/record-manager/has-owner rm:has-owner rdf:type owl:ObjectProperty ; rdfs:subPropertyOf rm:relates-to .

However, I will change it to _.

blcham · 2024-09-05T10:37:14Z

src/constants/Vocabulary.js

@@ -8,3 +8,17 @@ export const RDFS_COMMENT = "http://www.w3.org/2000/01/rdf-schema#comment";
 export const ADMIN_TYPE = "http://onto.fel.cvut.cz/ontologies/record-manager/administrator";
 export const DOCTOR_TYPE = "http://onto.fel.cvut.cz/ontologies/record-manager/doctor";
 export const IMPERSONATOR_TYPE = "http://onto.fel.cvut.cz/ontologies/record-manager/impersonator";
+export const COMPLETE_RECORDS_TYPE = "http://onto.fel.cvut.cz/ontologies/record-manager/record-complete";


inconsistent with reject_records ---> complete-records

palagdan · 2024-09-05T18:19:43Z

@blcham
Now it looks like this:

The admin can choose the roles for users if he have role EDIT_USER. The group input currently does not function because we need to discuss how it should be implemented. It might be better to disable the Roles input and assign roles based on the chosen group.

The next point for discussion is how Keycloak will retrieve information about roles. Currently, Keycloak only gets information about roles for the current user, which makes it impossible to display accurate information about other users. I think it would be beneficial to configure the Keycloak plugin to also save this information.

blcham · 2024-09-05T21:34:08Z

we do not want to assign roles, but role groups !! --- as we discussed we want to keep label "role" in RM-UI but assign there ONE role group ... but as i said if it is not changeable or even not visible there i fo not care that much -- it is not that important, we have keycloak for that.

palagdan · 2024-09-06T08:52:23Z

@blcham
I understand that we prefer not to assign roles manually but instead through group roles. However, I made manual role assignment possible solely for testing the UI with specific roles, such as the "edit user," "reject," and "complete" buttons, etc. I have developed a solution using internal authorization where users will only need to select a group role. Based on the selected group role, specific roles will automatically be assigned to the user, as you mentioned in a previous review.

In Keycloak authorization, I recommend not displaying the group role, as doing so would require storing it in a triple (via the plugin change).

So we will have these group roles if undestand correctly:

[OPERATOR_ADMIN]

publishing records
rejecting records
complete records
delete/edit/view organization records
edit users
codelist import

[OPERATOR_USER]

complete records

[SUPPLIER_ADMIN]

rejecting records
complete records
delete/edit/view organization records
delete/edit/view all records
edit users
codelist import

[SUPPIER]

complete records

[NON-ANONYMOUS QUESTIONARE]

complete records

palagdan · 2024-09-06T14:53:31Z

There is now a "Role Group" input that automatically assigns a roles to the user based on a group. I've left disabled roles input because there’s currently no other way to see which roles are assigned to the user, similar to how it's displayed in Keycloak. I can remove this if it’s not necessary for internal authorization.

In Keycloak, there is no "Role Group" or "Roles" option because, as mentioned before, this information should be stored in triples to show valid information. As you said, it’s not necessary for keycloak profile, as all the relevant information is already in Keycloak. Additionally, I’m not sure if we want to retrieve the role group from the token, as you previously mentioned that all the logic is based on roles, and the groups are only for demonstration purposes.

blcham · 2024-09-17T13:55:26Z

src/constants/DefaultConstants.js

@@ -81,8 +82,45 @@ export const ACTION_STATUS = {

 export const ROLE = {
  ADMIN: "Admin",
-  DOCTOR: "Regular User",
+  DOCTOR: "Doctor",


add comment that this is deprecated and will be replaced

palagdan added 2 commits September 3, 2024 20:56

[#202] Add new types and roles

9ff161e

[#202] Change implementation based on new roles

eca01e5

palagdan requested a review from blcham September 4, 2024 14:21

palagdan removed the request for review from blcham September 4, 2024 14:34

[#202] Implement roles in internal authorization and save it to the r…

c94b109

…epository

palagdan force-pushed the 202-fine-grained branch from 7b2e15d to c94b109 Compare September 5, 2024 08:20

blcham requested changes Sep 5, 2024

View reviewed changes

palagdan added 2 commits September 5, 2024 17:53

[#202] Fix tests

4ca36b8

[#202] Fix convention. Add group input

5aa223d

palagdan force-pushed the 202-fine-grained branch from ecd0d6d to 4be2f47 Compare September 5, 2024 18:18

[#202] Implement EDIT_USERS role logic

62d2b09

palagdan force-pushed the 202-fine-grained branch from 4be2f47 to 62d2b09 Compare September 5, 2024 18:26

[#202] Implement COMPLETE_RECORDS and REJECT_RECORDS role logic

19998a7

[#202] Implement role group assignment logic

7e5bb61

blcham reviewed Sep 17, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fine grained access #208

fine grained access #208

palagdan commented Sep 4, 2024

palagdan commented Sep 4, 2024 •

edited

Loading

blcham left a comment

blcham Sep 5, 2024

palagdan Sep 5, 2024

blcham Sep 5, 2024

blcham Sep 5, 2024

palagdan Sep 5, 2024 •

edited

Loading

blcham Sep 5, 2024

palagdan Sep 5, 2024

palagdan Sep 5, 2024

blcham Sep 5, 2024

palagdan Sep 5, 2024

palagdan commented Sep 5, 2024

blcham commented Sep 5, 2024 •

edited

Loading

palagdan commented Sep 6, 2024 •

edited

Loading

palagdan commented Sep 6, 2024 •

edited

Loading

blcham Sep 17, 2024

fine grained access #208

Are you sure you want to change the base?

fine grained access #208

Conversation

palagdan commented Sep 4, 2024

palagdan commented Sep 4, 2024 • edited Loading

blcham 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

palagdan Sep 5, 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

Choose a reason for hiding this comment

palagdan commented Sep 5, 2024

blcham commented Sep 5, 2024 • edited Loading

palagdan commented Sep 6, 2024 • edited Loading

palagdan commented Sep 6, 2024 • edited Loading

Choose a reason for hiding this comment

palagdan commented Sep 4, 2024 •

edited

Loading

palagdan Sep 5, 2024 •

edited

Loading

blcham commented Sep 5, 2024 •

edited

Loading

palagdan commented Sep 6, 2024 •

edited

Loading

palagdan commented Sep 6, 2024 •

edited

Loading