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

New Issue

OVERLORD · 2025-10-09T18:08:17+03:00

OVERLORD commented

2025-10-09 18:08:17 +03:00

📋 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: ed4ad67e73/src/api/core/organizations.rs (L910)

Because in contrast to CollectionUser the GroupUser::new actually expects a different id: 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>

OVERLORD added the pull-request label 2025-10-09 18:08:17 +03:00

OVERLORD closed this issue

2025-10-09 18:08:19 +03:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/vaultwarden#2614