Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RBAC to Bitwarden Portal #2853

Merged
merged 18 commits into from
May 4, 2023
Merged

Conversation

trmartin4
Copy link
Member

@trmartin4 trmartin4 commented Apr 14, 2023

Type of change

- [ ] Bug fix
- [X] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

Add role-based access control to the Bitwarden Portal.

Code changes

Preparatory Changes

  • CustomClaimsPrincipalFactory.cs This sets all the standard claims, and additionally sets the user's role claim. The role is retrieved using the AccessControlService.
  • AccessControlService.cs
    • New service added to handle behavior related to access control. The function added here, GetUserRole(), determines the role from the config, and is used to set the claim.
    • A function UserHasPermission(Permission) has been added that serves as the core of the RBAC logic. This function will be called in the Views to determine whether to display certain UI elements, and will be used in the ActionFilterAttribute that will be implemented in the next step.
  • Permissions.cs List of enums that represent all current permissions.
  • RolePermissionMapping.cs Contains a dictionary of Roles:Permissions. This is used to determine if a user has a given permission (their role is used as the key, and if the permission is in the list of values for that key, they have the permission). In the next iteration of the RBAC implementation, this file will be replaced with a db call to retrieve the permissions for a given role.
  • RequirePermissionAttribute.cs Implementation of the Action Filter. It takes a Permission as a parameter and will only successfully resolve if the user has the given permission. It will be used on controller actions to ensure users can only access actions they are authorized to access.
  • UsersController.cs Replaced the ToUser() function with value assignments dependent on the logged in user's permissions. The permission filter attribute was also applied to the Delete action.
  • UserEditModel.cs This function is no longer used. Name, Email, and EmailVerified can no longer be updated, and the other value assignments have been moved to the controller where permission checks can be applied.
  • Users/Edit.cshtml Permission checks have been added to prevent logged in users from seeing or editing information they're not authorized to interact with.
  • _Layout.cshtml
    • Remove the "Users" option in the nav bar if the logged in user does not have permission to view the page the option links to.
    • Add permission check to display "Logs" in the nav bar
    • Add permission check to display "Tools" in the nav bar
    • Add permission check to display "Provider" option in the nav bar
    • Add permission check to display "Organizations" option in the nav bar

Logs

  • LogsController.cs Applied the permission filter attribute at the controller level so it will apply to all actions in the controller

Tools

  • ToolsController.cs Added the permission check action filter attribute to the actions in the controller

Providers

  • ProvidersController.cs Added permission action filter attribute to actions
  • ProviderEditModel.cs Removed the setting of Name and Business Name because these fields can no longer be edited by anyone of any role
  • Admins.cshtml Add permission check to display the button to resend the email invite
  • Edit.cshtml Add permission check to edit the email fields and save the form, also updated the Name and Business Name fields to display as regular text rather than as input fields because these can no longer be edited by any role
  • Index.cshtml Add permission check to display the "Create Provider" button

Organizations:

  • OrganizationsController.cs Applied action filter attribute to appropriate actions, and replaced the model.ToOrganization call with a new function so that permission checks can be applied to the value changes.
  • OrganizationEditModel.cs Removed the ToOrganization function because it was replaced with a function in the controller so permission checks could be applied.
  • Organizations/Edit.cshtml
    • Passes to the partial view that the billing information displayed belongs to an Organization. This information determines what permission will be checked.
    • Added permission checks to the UI elements, and replaced the Name and Business Name input text boxes with plain text as these fields can no longer be edited by any role.
  • _BillingInformation.cshtml Added permission checks to the UI
  • BillingInformationModel.cs This model and accompanying partial view are used on both the User and Organization pages. An Entity property was added to be able to tell if we're in the User or Organization page so the appropriate permissions can be checked.
  • _OrganizationInformation.cshtml Added permission checks to the UI

Before you submit

  • Please check for formatting errors (dotnet format --verify-no-changes) (required)
  • If making database changes - make sure you also update Entity Framework queries and/or migrations
  • Please add unit tests where it makes sense to do so (encouraged but not required)
  • If this change requires a documentation update - notify the documentation team
  • If this change has particular deployment requirements - notify the DevOps team

dgoodman-bw and others added 13 commits February 14, 2023 10:31
* PM-48 - add user's role as a claim and establish access control service

* PM-48 - remove function unrelated to the role claim

* PM-48 - fix whitespace issues

* PM-48 - move registration of CustomClaimsPrincipalFactory, replace role claim type string with constant, streamline code that retrieves the user's role
* PM-48 - add user's role as a claim and establish access control service

* PM-48 - remove function unrelated to the role claim

* PM-48 - fix whitespace issues

* PM-47 - add list of permission enums, role:permissions mapping, and function that determines if the logged in user has the given permission

* PM-47 - remove unneeded service registration, set role to lowercase

* PM-47 - fix code style issues
* PM-54 - add permission gates to User elements

* PM-54 - fix formatting

* PM-54 - remove unused function

* PM-54 - fix variable reference, add permission to billing role

* PM-54 - handle Upgrade Premium button functionality and fix spelling

* PM-54 - change permission name to be more accurate
* PM-50 - add rbac for logs

* PM-50 - remove unnecessary action filter
* PM-52 add rbac for providers

* PM-52 - update redirect action

* PM-52 - add back edit functionality and permission

* PM-52 - reverse changes around removing edit functionality

* PM-52 - moved permission check to variable assignement
@trmartin4 trmartin4 changed the title Feature/rbac bitwarden admin portal Add RBAC to Bitwarden Portal Apr 14, 2023
@trmartin4 trmartin4 force-pushed the feature/rbac-bitwarden-admin-portal branch from 3696a58 to 088089b Compare April 20, 2023 02:27
trmartin4 and others added 3 commits April 20, 2023 15:46
* Updates to add RBAC to changes from reseller.

* Added back checks for delete and initiating a trial.

* Removed extraneous Razor tag.
@bitwarden-devops-bot bitwarden-devops-bot temporarily deployed to QA Cloud May 1, 2023 15:53 Inactive
@bitwarden-devops-bot bitwarden-devops-bot temporarily deployed to QA Cloud May 2, 2023 13:29 Inactive
@trmartin4 trmartin4 requested a review from a team May 4, 2023 18:36
Copy link
Contributor

@JaredSnider-Bitwarden JaredSnider-Bitwarden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quickly re-reviewed. LGTM! Thank you for putting this PR together!

@trmartin4 trmartin4 merged commit 0bd0910 into master May 4, 2023
@trmartin4 trmartin4 deleted the feature/rbac-bitwarden-admin-portal branch May 4, 2023 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants