Rework roles and permissions

bradsawadye commented 4 months ago

This is a rework of role based access control in the OpenHIM. This extends the basic RBAC we had by adding more permissions. It makes the openhim more flexible and manageable by multiple users in a secure way

Summary by CodeRabbit

New Features
- Introduced role-based permission checks for user actions across multiple APIs to enhance security and access control.
- Added functionality to handle roles for channel access based on permissions.
- Implemented a new utility function for checking user permissions, improving the flexibility of access control.
Refactor
- Centralized authorization logic to use a unified permission check function across various API endpoints, improving maintainability.
- Enhanced transaction management API with refined user authorization checks and a new function to filter sensitive transaction details.
Tests
- Added new test cases for permission checks on tasks, apps, channels, and roles, ensuring protected operations are only accessible to authorized users.
- Updated integration tests to validate role-based access control for mediators and metrics, ensuring correct permission handling.

coderabbitai[bot] commented 4 months ago

Walkthrough

The recent updates focus on enhancing security and access control across various API functions by implementing role-based permission checks. This involves refactoring existing authorization checks to use a centralized permission verification method, utils.checkUserPermission, and updating tests accordingly. Additionally, a new role management schema has been introduced, along with the integration of default role creation into the server initialization process.

Changes

Files	Change Summary
`src/api/apps.js`, `src/api/audits.js`, `src/api/authentication.js`, `src/api/authorisation.js`, `src/api/channels.js`, `src/api/clients.js`, `src/api/contactGroups.js`, `src/api/events.js`, `src/api/keystore.js`, `src/api/logs.js`, `src/api/mediators.js`, `src/api/metadata.js`, `src/api/restart.js`, `src/api/roles.js`, `src/api/tasks.js`, `src/api/transactions.js`	Refactored authorization checks to utilize `utils.checkUserPermission`, enhancing permission handling across various functions.
`src/model/role.js`	Introduced a new Role schema with predefined roles and permissions, and provided a function to create default roles.
`src/server.js`	Renamed `ensureRootUser` to `ensureRootUserAndRoles` to include default role creation.
Test files in `test/integration/...`	Updated to accommodate new role-based permission checks, added new variables for authentication, and expanded test cases to cover new permission handling logic.

Poem

In code we weave a tapestry tight,
With roles and checks, both day and night.
Permissions guard each action clear,
To keep our data safe and near.
With tests and refactors all in place,
Our system now runs a secure race.
🐇💻✨

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)

🪧 Tips

### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit , please review it.` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` - `@coderabbitai help me debug CodeRabbit configuration file.` Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. ### CodeRabbit Commands (Invoked using PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai full review` to do a full review from scratch and review all the files again. - `@coderabbitai summary` to regenerate the summary of the PR. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository. - `@coderabbitai help` to get help. ### Other keywords and placeholders - Add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. - Add `@coderabbitai summary` to generate the high-level summary at a specific location in the PR description. - Add `@coderabbitai` anywhere in the PR title to generate the title automatically. ### CodeRabbit Configuration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information. - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json` ### Documentation and Community - Visit our [Documentation](https://coderabbit.ai/docs) for detailed information on how to use CodeRabbit. - Join our [Discord Community](http://discord.gg/coderabbit) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 92.93598% with 32 lines in your changes missing coverage. Please review.

Project coverage is 87.07%. Comparing base (f451ea2) to head (96467f1). Report is 53 commits behind head on master.

Files with missing lines	Patch %	Lines
src/api/tasks.js	75.00%	19 Missing :warning:
src/api/transactions.js	84.44%	7 Missing :warning:
src/api/channels.js	94.91%	3 Missing :warning:
src/api/clients.js	97.67%	1 Missing :warning:
src/model/role.js	94.11%	1 Missing :warning:
src/server.js	83.33%	1 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #1230 +/- ## ========================================== + Coverage 86.79% 87.07% +0.28% ========================================== Files 90 91 +1 Lines 6042 5988 -54 ========================================== - Hits 5244 5214 -30 + Misses 798 774 -24 ``` | [Flag](https://app.codecov.io/gh/jembi/openhim-core-js/pull/1230/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jembi) | Coverage Δ | | |---|---|---| | [](https://app.codecov.io/gh/jembi/openhim-core-js/pull/1230/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jembi) | `?` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jembi#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

rcrichton commented 1 month ago

@drizzentic @bradsawadye I just thought of some things while reviewing this:

We also need a mechanism to migrate people from the old roles to the new ones. Which means a new upgradeDB function (see https://github.com/jembi/openhim-core-js/blob/master/src/upgradeDB.js). This is maybe a separate task and PR to this one.
We also need to think if there are any places in the UI where no being able to view something requires changes there. The most obvious would be removing links from the header if they don't have access to manager or view those.

jembi / openhim-core-js