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.

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

blcham · 2024-10-07T23:54:52Z

@palagdan I rebased this PR

blcham · 2024-11-13T17:38:20Z

@palagdan could you describe what the issue is and in which profile (I've lost it somehow :(

…epository

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

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

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 force-pushed the 202-fine-grained branch from ecd0d6d to 4be2f47 Compare September 5, 2024 18:18

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

blcham reviewed Sep 17, 2024

View reviewed changes

palagdan force-pushed the 202-fine-grained branch from 7e5bb61 to a8624e2 Compare September 30, 2024 15:03

palagdan requested a review from blcham September 30, 2024 15:05

palagdan force-pushed the 202-fine-grained branch 2 times, most recently from 6b23dc1 to 5050dec Compare September 30, 2024 15:45

blcham mentioned this pull request Oct 7, 2024

Users should be persisted in separate context kbss-cvut/record-manager#74

Open

blcham force-pushed the 202-fine-grained branch from 0a9b093 to 26f1cdc Compare October 7, 2024 23:54

blcham force-pushed the 202-fine-grained branch from 26f1cdc to 3e4992c Compare October 9, 2024 10:53

blcham force-pushed the 202-fine-grained branch 6 times, most recently from 4351437 to 9b15576 Compare November 13, 2024 16:55

palagdan added 3 commits November 13, 2024 18:56

[#202] Add new types and roles

2d09f2a

[#202] Change implementation based on new roles

bea10f9

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

4384547

…epository

palagdan added 10 commits November 13, 2024 18:56

[#202] Fix tests

eea6819

[#202] Fix convention. Add group input

cd0aa73

[#202] Implement EDIT_USERS role logic

f73b01c

[#202] Implement COMPLETE_RECORDS and REJECT_RECORDS role logic

d9e5999

[#202] Implement role group assignment logic

34304fa

[#202] Implement Reducer for Role Group

b9dba6a

[#202] Mark constants as deprecated

1999a65

[#202] Load RoleGroups only in internal profile

ec541a7

[#202] Refactor SecurityUtils

ec5d8f6

[#202] Change the role logic in User component for the 1 iteration

ff5b232

blcham force-pushed the 202-fine-grained branch from 9b15576 to ff5b232 Compare November 13, 2024 17:56

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

blcham commented Oct 7, 2024

blcham commented Nov 13, 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

blcham commented Oct 7, 2024

blcham commented Nov 13, 2024

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