[PR #36] Better check for cipher access #3905

New Issue

OVERLORD · 2025-10-09T18:32:23+03:00

OVERLORD commented

2025-10-09 18:32:23 +03:00

Original Pull Request: https://github.com/dani-garcia/vaultwarden/pull/36

State: closed
Merged: Yes

This checks out two TODOs:

Extends Cipher::is_write_accessible_to_user with a check for any collection that might give R/W access to cipher.
Implements Cipher::is_accessible_to_user with a proper check for any RO access.

The current solution works quite well, but I don't like the code repetition. Any input would be greatly appreciated how to solve this in some reasonable manner. I was thinking about using into_boxed() at some stage to promote some code reuse, but I just don't know enough about diesel to pull this off successfully.

Other approach would be to split it into multiple sub-checks and aggregate those but that would lead to 5x the amount of queries, which I'd like to avoid.

Any ideas?

**Original Pull Request:** https://github.com/dani-garcia/vaultwarden/pull/36 **State:** closed **Merged:** Yes --- This checks out two TODOs: 1. Extends `Cipher::is_write_accessible_to_user` with a check for any collection that might give R/W access to cipher. 2. Implements `Cipher::is_accessible_to_user` with a proper check for any RO access. The current solution works quite well, but I don't like the code repetition. Any input would be greatly appreciated how to solve this in some reasonable manner. I was thinking about using `into_boxed()` at some stage to promote some code reuse, but I just don't know enough about diesel to pull this off successfully. Other approach would be to split it into multiple sub-checks and aggregate those but that would lead to 5x the amount of queries, which I'd like to avoid. Any ideas?

OVERLORD added the pull-request label 2025-10-09 18:32:23 +03:00

OVERLORD closed this issue

2025-10-09 18:32:24 +03:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/vaultwarden#3905