Modelling a simple User/Group system

code review
1

As a little exercise for myself, I wanted to model a User-Group system in Rust, similar to other systems that manage permissions (e.g. LDAP). The requirements I set for myself are:

  • Two types of entities: Users and Groups
  • Users can be added to Groups. Groups can also be added to other groups.
  • I want to be able to see all members of a group, as well as all groups the user is a member of. This action is explicitly non-recursive (no need to traverse nested groups - I only need to see direct group membership)
  • A recursive action to traverse groups (e.g. to check permissions) is planned for later
  • When a user is deleted, they should be removed from all groups they are a member of

After creating the User and Group structs, I started off with a Directory struct that stores the mappings. The mappings store the respective IDs of their members. I also decided that it would be nice to have an API that can deal with both users and groups, e.g. a add_member() that can deal with both users and groups. To do so, I used a trait on the IDs of User and Group.

However, I ran into problems as the code doesn't compile anymore. And this is where I come to you to get a bit of advice on how to proceed. I have the following questions:

  • Is the current model I use any good or should I switch to something else? Especially using this "mapping" structure in Directory and my attempt with the Member trait
  • Does it make sense to offer this "unified" API or does it introduce should I offer a add_user()/add_group() function instead
  • Should I check for circular group members on insertion, leave it to the "downstream" functions (like the upcoming permission function) to handle circular members or can I get away with ignoring that problem altogether?

Here is my current code. Note that because I just wanted to focus on modelling, there is no validation or error handling yet.

use std::collections::{HashMap, HashSet};

// u64 for simplicity, would change into UUID or similar later
type UserId = u64;
type GroupId = u64;

trait Member {}

struct User {
    id: UserId,
    name: String,
}

struct Group {
    id: GroupId,
    name: String,
}

struct Directory {
    users: HashSet<User>,
    groups: HashSet<Group>,
    members_in_group: HashMap<GroupId, HashSet<Box<dyn Member>>>,
    groups_of_user: HashMap<UserId, HashSet<GroupId>>,
}

impl Member for UserId {}
impl Member for GroupId {}

impl User {
    /// Returns all groups the user is in.
    fn groups<'a>(&self, dir: &'a Directory) -> &'a HashSet<GroupId> {
        &dir.groups_of_user[&self.id]
    }
}

impl Group {
    /// Returns all users and groups in this group.
    fn members<'a>(&self, dir: &'a Directory) -> &'a HashSet<Box<dyn Member>> {
        &dir.members_in_group[&self.id]
    }

    /// Add a new user or group to the group.
    fn add_member(&self, dir: &mut Directory, member: &dyn Member) {
        // TODO: Add cycle checking before adding new groups

        dir.members_in_group
            .entry(self.id)
            .and_modify(|members| *members.insert(member));
        dir.groups_of_user
            .entry(member)
            .and_modify(|groups| groups.insert(self.id));
    }


    fn remove_member(&self, dir: &mut Directory, member: &dyn Member) {
        dir.members_in_group
            .entry(self.id)
            .and_modify(|members| members.remove(member));
        dir.groups_of_user
            .entry(member)
            .and_modify(|groups| groups.remove(&self.id));
    }
}

impl Directory {
    fn get_user(&self, user_id: UserId) -> &User {
        self.users.iter().find(|user| user.id == user_id).unwrap()
    }

    fn get_group(&self, group_id: GroupId) -> &Group {
        self.groups.iter().find(|group| group.id == group_id).unwrap()
    }
}
2

The problem here is that UserId and GroupId are not different types. They are both aliases (additional names) for the type u64:

type UserId = u64;
type GroupId = u64;

This means that UserId and GroupId are the same type. Then, your two impl Member conflict because they are implementations of the same trait for the same type. Also, you don't get any type checking that you haven’t confused the two IDs. There are several possible solutions here, but there are some important design question to answer first, about your data and not about your code:

  1. Is it allowed for a user and a group to have the same ID number, or should this be prohibited?
  2. Is this Directory authoritative about what users and groups exist, or does it have to accept user IDs and group IDs that might or might not accurately reflect the source of truth?

If a user and a group cannot have the same ID number, and this Directory is not authoritative, then every ID is probably best considered a “user or group ID”, and this means you should not attempt to distinguish whether a group member is a user or a group by different types. This will remove your your need for a trait.

// these are all the same type, and so these aliases are documentation only
type UserId = u64;
type GroupId = u64;
type UserOrGroupId = u64;

struct Directory {
    users: HashSet<User>,
    groups: HashSet<Group>,
    members_in_group: HashMap<GroupId, HashSet<UserOrGroupId>>,
    groups_of_user: HashMap<UserId, HashSet<GroupId>>,
}

If a user and a group can have the same ID number, then you should use actual distinct types ("newtypes") for user IDs and group IDs. And, while this is not strictly related, your Member trait should be an enum instead:

#[derive(Clone, Copy, Eq, Hash, PartialEq)]
struct UserId(u64);
#[derive(Clone, Copy, Eq, Hash, PartialEq)]
struct GroupId(u64);

#[derive(Clone, Copy, Eq, Hash, PartialEq)]
enum Member {
    User(UserId),
    Group(GroupId),
}

struct Directory {
    users: HashSet<User>,
    groups: HashSet<Group>,
    members_in_group: HashMap<GroupId, HashSet<Member>>,
    groups_of_user: HashMap<UserId, HashSet<GroupId>>,
}

In general, you should usually not use a trait object (dyn) for simple data when an enum will do. Trait objects are best used when you have an extensible category of things, like “UI widget” or “device driver”.