[Fix] Wrong permissions on edit CDB #2665

gabiwassan · 2020-11-27T20:22:36Z

Hi team,

This PR resolves:

Wrong permissions on edit CDB List with Cluster and Manager context configuration server.
Added action to get context server and status.
Context server is dynamically added to the CDBList edit button, add new, and delete.
Note: Policy cluster:status is necessary to get configuration context manager or cluster

Issue #2657

…star or manager).

…ux and used to set dynamic actions.

… manager)

public/redux/reducers/appStateReducers.js

public/controllers/management/components/management/ruleset/ruleset-table.js

…o all components with clusterStatus prop.

…s.ts logic for posible combinations with & in resources.

public/controllers/management/components/management/ruleset/actions-buttons.js

public/controllers/management/components/management/ruleset/ruleset-table.js

public/controllers/management/components/management/ruleset/utils/columns.js

…it-cdb' into fix/2657-wrong-permissions-on-edit-cdb

public/react-services/wz-user-permissions.ts

jsanchez91

I have conducted tests with these users and roles using X-Pack Security:

elastic: administrator
test_user: readonly, cluster_readonly

This is the result I have obtained in both tests

gabiwassan · 2020-12-04T12:24:47Z

I have conducted tests with these users and roles using X-Pack Security:

elastic: administrator

test_user: readonly, cluster_readonly

This is the result I have obtained in both tests

Thanks, I'm checking it right now.

…factor, prettier and lint.

frankeros · 2020-12-10T13:06:47Z

Hi @gabiwassan

Testing this branch I found this bug when you set these policies to the role

You're able to see all files but trying to access one of them throw this error

The same error occurs

With these policies

when you try to add new lists

It requires a specific path (I'm not sure if it is a bug)

Desvelao · 2020-12-10T13:07:14Z

public/controllers/management/components/management/ruleset/actions-buttons.js

+          },
+          {
+            action: `${((this.props || {}).clusterStatus || {}).contextConfigServer}:read`,
+            resource: `file:path:/etc/${this.props.msg}`,


Should it be the section variable instead this.props.msg? I checked the code and didn't found where it is coming from.

Desvelao · 2020-12-10T13:07:42Z

public/controllers/management/components/management/ruleset/actions-buttons.js

+          },
+          {
+            action: `${((this.props || {}).clusterStatus || {}).contextConfigServer}:read_file`,
+            resource: `file:path:/etc/${this.props.msg}`,


Should it be the section variable instead this.props.msg? I checked the code and didn't found where it is coming from.

Desvelao · 2020-12-10T13:22:26Z

I tested the PR and found the next problem:

Logged with deafult admin user in Open Distro, I foudn this problem in the Management/Rules section:

Desvelao · 2020-12-10T15:21:14Z

I don't think we need check the permissions here, allowing edit/remove items of the list.

I think only the Save button is enough to do the validation to upload the file.

I found a problem using the default admin user that has the default administrator role. The Save button is disabled because the required resource is not included in the action that has this role.

Maybe modify the resource to node:id:* (Although the file should really have to be uploaded to the master node of the cluster, i mean the resource should be node:id:<master_node_id> instead node:id:* to be more exactly)

frankeros · 2020-12-11T15:08:11Z

Testing the new commits I found these two bugs

When the user has permissions to a specific file and permissions to upload_file

and try to save a new list file gets this error

but the new item is created anyway
When the user (any with access to this action) tries to export a list detail, gets this error

code: 3034
message: "3034 - An error occurred fetching data from the Wazuh API : Extra query parameter(s) path not in spec"
statusCode: 500

…ue. This resolve error when we create new list and dont have permissions to view all lists.

frankeros

great job @gabiwassan!! 👏 👏

gabiwassan · 2020-12-15T13:23:52Z

Testing the new commits I found these two bugs

When the user has permissions to a specific file and permissions to upload_file

and try to save a new list file gets this error

but the new item is created anyway

When the user (any with access to this action) tries to export a list detail, gets this error
code: 3034
message: "3034 - An error occurred fetching data from the Wazuh API : Extra query parameter(s) path not in spec"
statusCode: 500

The following issue was created #2692 for error 500

Desvelao

LGTM

gabiwassan added 4 commits November 27, 2020 16:56

Added new action and reducer to get status and context of server (clu…

3c825c5

…star or manager).

Added new method isClusterOrManager to get cluster status, set on red…

23b6f1e

…ux and used to set dynamic actions.

Fixed and replaced all permissions by contextConfigServer (cluster or…

f2188c9

… manager)

Rollback, wrong change.

acbb51c

gabiwassan requested review from jsanchez91, frankeros and Desvelao November 27, 2020 20:22

frankeros suggested changes Nov 27, 2020

View reviewed changes

public/redux/reducers/appStateReducers.js Show resolved Hide resolved

gabiwassan added 2 commits November 27, 2020 18:25

Request changes of validation null properties.

f1420b8

Added initial state of clusterStatus

4ed8732

gabiwassan changed the title ~~[Fix] Wrong permissions on edit CDB #2657~~ [Fix] Wrong permissions on edit CDB Nov 27, 2020

gabiwassan requested a review from frankeros November 27, 2020 21:33

Desvelao reviewed Nov 30, 2020

View reviewed changes

public/controllers/management/components/management/ruleset/ruleset-table.js Outdated Show resolved Hide resolved

Moved method isClusterOrManager to management-main.js to distribute t…

f689408

…o all components with clusterStatus prop.

gabiwassan requested a review from Desvelao November 30, 2020 17:17

gabiwassan and others added 3 commits December 1, 2020 18:32

Fixed permissions to edit and export to csv.

9cd4472

Fixed permissions for manager or cluster. Added on wz-user-permission…

5715e9c

…s.ts logic for posible combinations with & in resources.

Merge branch '4.0-7.9' into fix/2657-wrong-permissions-on-edit-cdb

00eeacf

frankeros suggested changes Dec 3, 2020

View reviewed changes

gabiwassan added 2 commits December 3, 2020 16:33

Changed to const permissions declaration.

20ce83a

Merge remote-tracking branch 'origin/fix/2657-wrong-permissions-on-ed…

80aa117

…it-cdb' into fix/2657-wrong-permissions-on-edit-cdb

frankeros suggested changes Dec 3, 2020

View reviewed changes

public/react-services/wz-user-permissions.ts Outdated Show resolved Hide resolved

jsanchez91 suggested changes Dec 4, 2020

View reviewed changes

gabiwassan added 3 commits December 4, 2020 15:55

Fixed user-permissions to support cominations of resources with &. Re…

b81b69d

…factor, prettier and lint.

Refactor Withe and Black mode. (juts simplify code)

129753e

Fixed typo error

3474127

gabiwassan requested review from jsanchez91 and frankeros December 9, 2020 12:07

Desvelao reviewed Dec 10, 2020

View reviewed changes

Fixed errors detected on PR.

191d3f6

gabiwassan requested a review from Desvelao December 10, 2020 21:02

gabiwassan added 5 commits December 11, 2020 16:18

Removed unnecessary request, to update list content, we have this val…

e6f95a6

…ue. This resolve error when we create new list and dont have permissions to view all lists.

First commit scaffolding test wz-users-permissions

a611ff6

Fixed mock of userPermissions and requiredPermissions

c793aaf

Implemented a basic suite of test for wz-user-permissions.ts

754c5b5

Removed import.

6448c21

frankeros approved these changes Dec 15, 2020

View reviewed changes

Added header Wazuh Copyright.

e8dd674

Desvelao approved these changes Dec 17, 2020

View reviewed changes

Desvelao removed the request for review from jsanchez91 December 18, 2020 10:32

Added CHANGELOG.md

0c6be16

gabiwassan merged commit 4df6d4a into 4.0-7.9 Dec 18, 2020

gabiwassan deleted the fix/2657-wrong-permissions-on-edit-cdb branch December 18, 2020 11:42

frankeros mentioned this pull request Dec 29, 2020

Merge last changes into cloud/4.0-7.9 #2715

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fix] Wrong permissions on edit CDB #2665

[Fix] Wrong permissions on edit CDB #2665

gabiwassan commented Nov 27, 2020 •

edited by Desvelao

Loading

jsanchez91 left a comment

gabiwassan commented Dec 4, 2020

frankeros commented Dec 10, 2020 •

edited

Loading

Desvelao Dec 10, 2020 •

edited

Loading

Desvelao Dec 10, 2020 •

edited

Loading

Desvelao commented Dec 10, 2020 •

edited

Loading

Desvelao commented Dec 10, 2020

frankeros commented Dec 11, 2020

frankeros left a comment

gabiwassan commented Dec 15, 2020

Desvelao left a comment

[Fix] Wrong permissions on edit CDB #2665

[Fix] Wrong permissions on edit CDB #2665

Conversation

gabiwassan commented Nov 27, 2020 • edited by Desvelao Loading

jsanchez91 left a comment

Choose a reason for hiding this comment

gabiwassan commented Dec 4, 2020

frankeros commented Dec 10, 2020 • edited Loading

Testing this branch I found this bug when you set these policies to the role

With these policies

Desvelao Dec 10, 2020 • edited Loading

Choose a reason for hiding this comment

Desvelao Dec 10, 2020 • edited Loading

Choose a reason for hiding this comment

Desvelao commented Dec 10, 2020 • edited Loading

Desvelao commented Dec 10, 2020

frankeros commented Dec 11, 2020

frankeros left a comment

Choose a reason for hiding this comment

gabiwassan commented Dec 15, 2020

Desvelao left a comment

Choose a reason for hiding this comment

gabiwassan commented Nov 27, 2020 •

edited by Desvelao

Loading

frankeros commented Dec 10, 2020 •

edited

Loading

Desvelao Dec 10, 2020 •

edited

Loading

Desvelao Dec 10, 2020 •

edited

Loading

Desvelao commented Dec 10, 2020 •

edited

Loading