[PR #5320] [MERGED] rename membership and adopt newtype pattern #3599

New issue

Closed

opened 2026-03-03 10:04:54 +03:00 by kerem · 0 comments

kerem commented

2026-03-03 10:04:54 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/dani-garcia/vaultwarden/pull/5320
Author: @stefan0xC
Created: 12/22/2024
Status: ✅ Merged
Merged: 1/9/2025
Merged by: @dani-garcia

Base: main ← Head: rename-membership

📝 Commits (5)

c1af0cb rename membership
655ac03 use newtype pattern
d0fb15e implement custom derive macro IdFromParam
0fcb4d5 add UuidFromParam macro for UUIDs
c44dfa3 add macros to Docker build

📊 Changes

51 files changed (+2733 additions, -2047 deletions)

View changed files

📝 .dockerignore (+1 -0)
📝 Cargo.lock (+43 -0)
📝 Cargo.toml (+7 -0)
📝 docker/Dockerfile.alpine (+1 -0)
📝 docker/Dockerfile.debian (+1 -0)
📝 docker/Dockerfile.j2 (+1 -0)
➕ macros/Cargo.toml (+13 -0)
➕ macros/src/lib.rs (+58 -0)
📝 src/api/admin.rs (+52 -53)
📝 src/api/core/accounts.rs (+83 -74)
📝 src/api/core/ciphers.rs (+262 -229)
📝 src/api/core/emergency_access.rs (+43 -38)
📝 src/api/core/events.rs (+45 -40)
📝 src/api/core/folders.rs (+22 -16)
📝 src/api/core/organizations.rs (+698 -646)
📝 src/api/core/public.rs (+29 -35)
📝 src/api/core/sends.rs (+44 -38)
📝 src/api/core/two_factor/authenticator.rs (+9 -9)
📝 src/api/core/two_factor/duo.rs (+3 -3)
📝 src/api/core/two_factor/duo_oidc.rs (+3 -3)

...and 31 more files

📄 Description

I've been working on improving Vaultwarden's legibility by renaming the UsersOrganizations to simply Membership and making it clearer when something is referring to the User and when to a Membership relation.

After doing that I was motivated to try my hands on using the newtype pattern so that the Rust type system can be used to check on compile time if we always call the right uuid because there are a lot of Strings that can be used interchangeably otherwise.

For example I noticed that we are passing the wrong id here: github.com/dani-garcia/vaultwarden@ed4ad67e73/src/api/core/organizations.rs (L910)

Because in contrast to CollectionUser the GroupUser::new actually expects a different id: github.com/dani-garcia/vaultwarden@ed4ad67e73/src/db/models/group.rs (L122) This was probably overlooked because it's not a major issue when a user is not correctly invited to a group and it only shows up as a small warning in the logs:

[2024-12-22 21:23:47.022][request][INFO] POST /api/organizations/152bd996-19f9-40ee-b991-e52333a7e719/users/invite
[2024-12-22 21:23:47.031][vaultwarden::db::models::group][WARN] User could not be found!
[2024-12-22 21:23:48.082][response][INFO] (send_invite) POST /api/organizations/<org_id>/users/invite => 200 OK

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/dani-garcia/vaultwarden/pull/5320 **Author:** [@stefan0xC](https://github.com/stefan0xC) **Created:** 12/22/2024 **Status:** ✅ Merged **Merged:** 1/9/2025 **Merged by:** [@dani-garcia](https://github.com/dani-garcia) **Base:** `main` ← **Head:** `rename-membership` --- ### 📝 Commits (5) - [`c1af0cb`](https://github.com/dani-garcia/vaultwarden/commit/c1af0cbbb414f1e806088d40d4d86d12683b224a) rename membership - [`655ac03`](https://github.com/dani-garcia/vaultwarden/commit/655ac0367abfa57b7dc1dcdfb271b75ca7da6877) use newtype pattern - [`d0fb15e`](https://github.com/dani-garcia/vaultwarden/commit/d0fb15e97a1801bd5fa0b090a40b8d3f9f3803f6) implement custom derive macro IdFromParam - [`0fcb4d5`](https://github.com/dani-garcia/vaultwarden/commit/0fcb4d5170d2265f4b271397082414e076f2ad37) add UuidFromParam macro for UUIDs - [`c44dfa3`](https://github.com/dani-garcia/vaultwarden/commit/c44dfa32eeba3572cfd0b9ff6beee01c0159614e) add macros to Docker build ### 📊 Changes **51 files changed** (+2733 additions, -2047 deletions) <details> <summary>View changed files</summary> 📝 `.dockerignore` (+1 -0) 📝 `Cargo.lock` (+43 -0) 📝 `Cargo.toml` (+7 -0) 📝 `docker/Dockerfile.alpine` (+1 -0) 📝 `docker/Dockerfile.debian` (+1 -0) 📝 `docker/Dockerfile.j2` (+1 -0) ➕ `macros/Cargo.toml` (+13 -0) ➕ `macros/src/lib.rs` (+58 -0) 📝 `src/api/admin.rs` (+52 -53) 📝 `src/api/core/accounts.rs` (+83 -74) 📝 `src/api/core/ciphers.rs` (+262 -229) 📝 `src/api/core/emergency_access.rs` (+43 -38) 📝 `src/api/core/events.rs` (+45 -40) 📝 `src/api/core/folders.rs` (+22 -16) 📝 `src/api/core/organizations.rs` (+698 -646) 📝 `src/api/core/public.rs` (+29 -35) 📝 `src/api/core/sends.rs` (+44 -38) 📝 `src/api/core/two_factor/authenticator.rs` (+9 -9) 📝 `src/api/core/two_factor/duo.rs` (+3 -3) 📝 `src/api/core/two_factor/duo_oidc.rs` (+3 -3) _...and 31 more files_ </details> ### 📄 Description I've been working on improving Vaultwarden's legibility by renaming the `UsersOrganizations` to simply `Membership` and making it clearer when something is referring to the `User` and when to a `Membership` relation. After doing that I was motivated to try my hands on using the [newtype pattern](https://doc.rust-lang.org/book/ch19-04-advanced-types.html#using-the-newtype-pattern-for-type-safety-and-abstraction) so that the Rust type system can be used to check on compile time if we always call the right `uuid` because there are a lot of Strings that can be used interchangeably otherwise. For example I noticed that we are passing the wrong id here: https://github.com/dani-garcia/vaultwarden/blob/ed4ad67e732c213beaec78970cdb68e48bee3dc1/src/api/core/organizations.rs#L910 Because in contrast to `CollectionUser` the `GroupUser::new` actually expects a different id: https://github.com/dani-garcia/vaultwarden/blob/ed4ad67e732c213beaec78970cdb68e48bee3dc1/src/db/models/group.rs#L122 This was probably overlooked because it's not a major issue when a user is not correctly invited to a group and it only shows up as a small warning in the logs: ``` [2024-12-22 21:23:47.022][request][INFO] POST /api/organizations/152bd996-19f9-40ee-b991-e52333a7e719/users/invite [2024-12-22 21:23:47.031][vaultwarden::db::models::group][WARN] User could not be found! [2024-12-22 21:23:48.082][response][INFO] (send_invite) POST /api/organizations/<org_id>/users/invite => 200 OK ``` --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>