Skip to content

Commit

Permalink
Role permissions order tool and workflow (opensearch-project#2733)
Browse files Browse the repository at this point in the history
* Check Permissions Order tool and workflow

Adds a NodeJS tool that can inspect yaml role definitions, check if they are in alphabetical order, correct them if required.

Signed-off-by: Peter Nied <[email protected]>

* Apply fixes to roles.yml files

Signed-off-by: Peter Nied <[email protected]>

* Fixing busted test, adding findArrayInJson for response bodies

Signed-off-by: Peter Nied <[email protected]>

---------

Signed-off-by: Peter Nied <[email protected]>
Signed-off-by: Peter Nied <[email protected]>
  • Loading branch information
peternied authored and RyanL1997 committed Jun 29, 2023
1 parent 4bb144f commit 9cd0198
Show file tree
Hide file tree
Showing 16 changed files with 1,959 additions and 1,770 deletions.
23 changes: 23 additions & 0 deletions .github/workflows/code-hygiene.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,26 @@ jobs:
- uses: gradle/gradle-build-action@v2
with:
arguments: spotbugsMain

check-permissions-order:
runs-on: ubuntu-latest
name: Check permissions orders
steps:
- uses: actions/checkout@v2
- run: npm install yaml

- name: Check permissions order
run: |
exclude_pattern="(^|/)roles_invalidxcontent.yml($|/)
(^|/)invalid_config/config.yml($|/)"
# Set pattern to exclude certain files
set -e
exit_code=0
for file in $(find . -name '*.yml' | grep -Ev "$exclude_pattern"); do
if ! node check-permissions-order.js "$file" --slient; then
exit_code=1
echo "Error: $file requires changes. Run the following command to fix:"
echo "node check-permissions-order.js $file --fix"
fi
done
exit $exit_code
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,7 @@ out/
build/
gradle-build/
.gradle/

# nodejs
node_modules/
package-lock.json
86 changes: 86 additions & 0 deletions check-permissions-order.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
* Modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/

const fs = require('fs')
const yaml = require('yaml')

function checkPermissionsOrder(file, fix = false) {
const contents = fs.readFileSync(file, 'utf8')
const doc = yaml.parseDocument(contents, { keepCstNodes: true })
const roles = doc.contents.items
let requiresChanges = false
roles.forEach(role => {
const itemsFromRole = role?.value?.items;

const clusterPermissions = itemsFromRole?.filter(item => item.key && item.key.value === 'cluster_permissions');
requiresChanges |= checkPermissionsOrdering(clusterPermissions);


const indexPermissionsArray = itemsFromRole?.filter(item => item.key && item.key.value === 'index_permissions');
const indexPermissionObj = indexPermissionsArray?.[0]?.value;
const indexPermissionItems = indexPermissionObj?.items[0]?.items;
const allowedIndexActions = indexPermissionItems?.filter(item => item.key && item.key.value === 'allowed_actions');

requiresChanges |= checkPermissionsOrdering(allowedIndexActions);
})

if (fix && requiresChanges) {
const newContents = doc.toString()
fs.writeFileSync(file, newContents, 'utf8')
}

return requiresChanges
}

/*
Checks the permissions ordering
returns false if they are already stored
returns true if the permissions were not sored, note the permissions object are sorted as a side effect of this function
*/
function checkPermissionsOrdering(permissions) {
let requiresChanges = false;
if (!permissions) {
return requiresChanges;
}
permissions.forEach(permission => {
const items = permission.value.items;
const originalItems = JSON.stringify(items);
items.sort();
const sortedItems = JSON.stringify(items);

// If the original items and sorted items are not the same, then changes are required
if (originalItems !== sortedItems) {
requiresChanges = true;
}
});
return requiresChanges;
}

// Example usage
const args = process.argv.slice(2)
if (args.length === 0) {
console.error('Usage: node check-permissions-order.js <file> [--fix] [--silent]')
process.exit(1)
}
const filePath = args[0]
const fix = args.includes('--fix')
const slient = args.includes('--slient')
if (checkPermissionsOrder(filePath, fix)) {
if (fix) {
if (!slient) { console.log(`${filePath} has been updated.`) }
} else {
if (!slient) { console.error(`Error: ${filePath} requires changes.`) }
process.exit(1)
}
} else {
if (!slient) { console.log(`${filePath} is up-to-date.`) }
}
59 changes: 29 additions & 30 deletions config/roles.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,83 +43,83 @@ alerting_ack_alerts:
alerting_full_access:
reserved: true
cluster_permissions:
- 'cluster_monitor'
- 'cluster:admin/opendistro/alerting/*'
- 'cluster:admin/opensearch/alerting/*'
- 'cluster:admin/opensearch/notifications/feature/publish'
- 'cluster_monitor'
index_permissions:
- index_patterns:
- '*'
allowed_actions:
- 'indices_monitor'
- 'indices:admin/aliases/get'
- 'indices:admin/mappings/get'
- 'indices_monitor'

# Allow users to read Anomaly Detection detectors and results
anomaly_read_access:
reserved: true
cluster_permissions:
- 'cluster:admin/opendistro/ad/detector/info'
- 'cluster:admin/opendistro/ad/detector/search'
- 'cluster:admin/opendistro/ad/detector/validate'
- 'cluster:admin/opendistro/ad/detectors/get'
- 'cluster:admin/opendistro/ad/result/search'
- 'cluster:admin/opendistro/ad/tasks/search'
- 'cluster:admin/opendistro/ad/detector/validate'
- 'cluster:admin/opendistro/ad/result/topAnomalies'
- 'cluster:admin/opendistro/ad/tasks/search'

# Allows users to use all Anomaly Detection functionality
anomaly_full_access:
reserved: true
cluster_permissions:
- 'cluster_monitor'
- 'cluster:admin/opendistro/ad/*'
- 'cluster_monitor'
index_permissions:
- index_patterns:
- '*'
allowed_actions:
- 'indices_monitor'
- 'indices:admin/aliases/get'
- 'indices:admin/mappings/get'
- 'indices_monitor'

# Allow users to execute read only k-NN actions
knn_read_access:
reserved: true
cluster_permissions:
- 'cluster:admin/knn_search_model_action'
- 'cluster:admin/knn_get_model_action'
- 'cluster:admin/knn_search_model_action'
- 'cluster:admin/knn_stats_action'

# Allow users to use all k-NN functionality
knn_full_access:
reserved: true
cluster_permissions:
- 'cluster:admin/knn_training_model_action'
- 'cluster:admin/knn_training_job_router_action'
- 'cluster:admin/knn_training_job_route_decision_info_action'
- 'cluster:admin/knn_warmup_action'
- 'cluster:admin/knn_delete_model_action'
- 'cluster:admin/knn_get_model_action'
- 'cluster:admin/knn_remove_model_from_cache_action'
- 'cluster:admin/knn_update_model_graveyard_action'
- 'cluster:admin/knn_search_model_action'
- 'cluster:admin/knn_get_model_action'
- 'cluster:admin/knn_stats_action'
- 'cluster:admin/knn_training_job_route_decision_info_action'
- 'cluster:admin/knn_training_job_router_action'
- 'cluster:admin/knn_training_model_action'
- 'cluster:admin/knn_update_model_graveyard_action'
- 'cluster:admin/knn_warmup_action'

# Allows users to read Notebooks
notebooks_read_access:
reserved: true
cluster_permissions:
- 'cluster:admin/opendistro/notebooks/list'
- 'cluster:admin/opendistro/notebooks/get'
- 'cluster:admin/opendistro/notebooks/list'

# Allows users to all Notebooks functionality
notebooks_full_access:
reserved: true
cluster_permissions:
- 'cluster:admin/opendistro/notebooks/create'
- 'cluster:admin/opendistro/notebooks/update'
- 'cluster:admin/opendistro/notebooks/delete'
- 'cluster:admin/opendistro/notebooks/get'
- 'cluster:admin/opendistro/notebooks/list'
- 'cluster:admin/opendistro/notebooks/update'

# Allows users to read observability objects
observability_read_access:
Expand All @@ -132,9 +132,9 @@ observability_full_access:
reserved: true
cluster_permissions:
- 'cluster:admin/opensearch/observability/create'
- 'cluster:admin/opensearch/observability/update'
- 'cluster:admin/opensearch/observability/delete'
- 'cluster:admin/opensearch/observability/get'
- 'cluster:admin/opensearch/observability/update'

# Allows users to all PPL functionality
ppl_full_access:
Expand All @@ -153,8 +153,8 @@ ppl_full_access:
reports_instances_read_access:
reserved: true
cluster_permissions:
- 'cluster:admin/opendistro/reports/instance/list'
- 'cluster:admin/opendistro/reports/instance/get'
- 'cluster:admin/opendistro/reports/instance/list'
- 'cluster:admin/opendistro/reports/menu/download'

# Allows users to read and download Reports and Report-definitions
Expand All @@ -163,22 +163,22 @@ reports_read_access:
cluster_permissions:
- 'cluster:admin/opendistro/reports/definition/get'
- 'cluster:admin/opendistro/reports/definition/list'
- 'cluster:admin/opendistro/reports/instance/list'
- 'cluster:admin/opendistro/reports/instance/get'
- 'cluster:admin/opendistro/reports/instance/list'
- 'cluster:admin/opendistro/reports/menu/download'

# Allows users to all Reports functionality
reports_full_access:
reserved: true
cluster_permissions:
- 'cluster:admin/opendistro/reports/definition/create'
- 'cluster:admin/opendistro/reports/definition/update'
- 'cluster:admin/opendistro/reports/definition/on_demand'
- 'cluster:admin/opendistro/reports/definition/delete'
- 'cluster:admin/opendistro/reports/definition/get'
- 'cluster:admin/opendistro/reports/definition/list'
- 'cluster:admin/opendistro/reports/instance/list'
- 'cluster:admin/opendistro/reports/definition/on_demand'
- 'cluster:admin/opendistro/reports/definition/update'
- 'cluster:admin/opendistro/reports/instance/get'
- 'cluster:admin/opendistro/reports/instance/list'
- 'cluster:admin/opendistro/reports/menu/download'

# Allows users to use all asynchronous-search functionality
Expand Down Expand Up @@ -234,14 +234,14 @@ cross_cluster_replication_follower_full_access:
- index_patterns:
- '*'
allowed_actions:
- "indices:admin/plugins/replication/index/setup/validate"
- "indices:data/write/plugins/replication/changes"
- "indices:admin/plugins/replication/index/start"
- "indices:admin/plugins/replication/index/pause"
- "indices:admin/plugins/replication/index/resume"
- "indices:admin/plugins/replication/index/setup/validate"
- "indices:admin/plugins/replication/index/start"
- "indices:admin/plugins/replication/index/status_check"
- "indices:admin/plugins/replication/index/stop"
- "indices:admin/plugins/replication/index/update"
- "indices:admin/plugins/replication/index/status_check"
- "indices:data/write/plugins/replication/changes"

# Allows users to use all cross cluster search functionality at remote cluster
cross_cluster_search_remote_full_access:
Expand All @@ -257,7 +257,6 @@ cross_cluster_search_remote_full_access:
ml_read_access:
reserved: true
cluster_permissions:
- 'cluster:admin/opensearch/ml/stats/nodes'
- 'cluster:admin/opensearch/ml/model_groups/search'
- 'cluster:admin/opensearch/ml/models/get'
- 'cluster:admin/opensearch/ml/models/search'
Expand All @@ -268,8 +267,8 @@ ml_read_access:
ml_full_access:
reserved: true
cluster_permissions:
- 'cluster_monitor'
- 'cluster:admin/opensearch/ml/*'
- 'cluster_monitor'
index_permissions:
- index_patterns:
- '*'
Expand All @@ -286,26 +285,26 @@ notifications_full_access:
notifications_read_access:
reserved: true
cluster_permissions:
- 'cluster:admin/opensearch/notifications/channels/get'
- 'cluster:admin/opensearch/notifications/configs/get'
- 'cluster:admin/opensearch/notifications/features'
- 'cluster:admin/opensearch/notifications/channels/get'

# Allows users to use all snapshot management functionality
snapshot_management_full_access:
reserved: true
cluster_permissions:
- 'cluster:admin/opensearch/snapshot_management/*'
- 'cluster:admin/opensearch/notifications/feature/publish'
- 'cluster:admin/opensearch/snapshot_management/*'
- 'cluster:admin/repository/*'
- 'cluster:admin/snapshot/*'

# Allows users to see snapshots, repositories, and snapshot management policies
snapshot_management_read_access:
reserved: true
cluster_permissions:
- 'cluster:admin/opensearch/snapshot_management/policy/explain'
- 'cluster:admin/opensearch/snapshot_management/policy/get'
- 'cluster:admin/opensearch/snapshot_management/policy/search'
- 'cluster:admin/opensearch/snapshot_management/policy/explain'
- 'cluster:admin/repository/get'
- 'cluster:admin/snapshot/get'

Expand Down
20 changes: 10 additions & 10 deletions src/integrationTest/resources/roles.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@ _meta:
config_version: 2
user_admin__all_access:
cluster_permissions:
- "*"
index_permissions:
- index_patterns:
- "*"
allowed_actions:
- "*"
index_permissions:
- index_patterns:
- "*"
allowed_actions:
- "*"
user_limited-user__limited-role:
index_permissions:
- index_patterns:
- "user-${user.name}"
allowed_actions:
- "indices:data/read/search"
- "indices:data/read/get"
- index_patterns:
- "user-${user.name}"
allowed_actions:
- "indices:data/read/get"
- "indices:data/read/search"
Loading

0 comments on commit 9cd0198

Please sign in to comment.