pet-rescue icon indicating copy to clipboard operation
pet-rescue copied to clipboard

Validate Roles

Open mononoken opened this issue 10 months ago • 3 comments

If you can think of additional validations, please list them. We can release this for work once we have a good consensus on the validations we want.

Description

Roles have minimal validations, and some of them may not even be desirable. This is bad because we are currently vulnerable to developer mistakes or even malicious form manipulation to assign non-existent roles or other undesirable behavior. Authz are currently set to prevent such role problems from being an actual problem in regards to our data, but the roles themselves are vulnerable.

For example, if you try this in bin/rails c --sandbox:

> User.last.add_role(:wizard)
> User.last.roles
[#<Role:0x00000001112558c0
  id: 5,
  name: "staff",
  resource_type: "Organization",
  resource_id: 1,
  created_at: Thu, 28 Mar 2024 04:34:05.742340000 UTC +00:00,
  updated_at: Thu, 28 Mar 2024 04:34:05.742340000 UTC +00:00>,
 #<Role:0x00000001112557f8
  id: 6,
  name: "wizard",
  resource_type: nil,
  resource_id: nil,
  created_at: Thu, 28 Mar 2024 06:48:51.940040000 UTC +00:00,
  updated_at: Thu, 28 Mar 2024 06:48:51.940040000 UTC +00:00>]

We see two problems above. One, wizards don't exist in this app. Two, the role validations currently permit resources to be nil (which means any checks that forget to check for resource scoping would pass).

Note: The acceptance criteria includes some validations that may already exist, but we should confirm that rolify is enforcing them as we expect or else implement ourselves.

Acceptance criteria

Add validations for these to models/role:

  • [x] Duplicate roles with the same name and resource cannot be created (I think this is implemented by rolify's default, but we need to confirm so)
  • [x] Limit roles to the following: [:adopter, :fosterer, :staff, :admin] (inclusion validation will be useful)
  • [ ] Resource (organization) must match User's organization
  • [x] Name cannot be nil (this may be covered by above inclusion validation)
  • [ ] Resource type cannot be nil
  • [ ] Resource cannot be nil

Others?

  • [ ] User cannot have duplicate roles (think rolify also covers this but confirm)

mononoken avatar Mar 28 '24 07:03 mononoken