Skip to content

Small type-safety risk in the ABAC implementation #1

@mindplay-dk

Description

@mindplay-dk

This might amount to a nit pick, but this example makes me a bit uneasy:

// Can view the `todo` Todo
hasPermission(user, "todos", "view", todo)
// Can view all todos
hasPermission(user, "todos", "view")

It's very easy to accidentally omit the todo argument - the difference between checking if someone can view a specific todo vs checking if they can view all todos is significant enough that this should probably made explicit, e.g.:

// Can view all todos 👎
hasPermission(user, "todos", "view") // should error

// Can view all todos 👍
hasPermission(user, "todos", "view", null) // should be okay

Sidetrack, but to be honest, this is why I personally prefer to make security APIs like these conform to an explicit interfaces with multiple implementations for different roles, instead of attempting to break it down into resources and actions and "framework" it.

That is, explicit methods like can(user).viewTodo(todo), where the language and meaning is explicit - after all, checking if a user can read a specific todo is substantially different from checking if they can read all todos. One is a significantly broader permission than the other. If I had that permission in my security model, it would be can(user).viewAllTodos() - although I'm not sure what the usefulness is, at least with the example you showed in the video, where being able to view all todos sounds more a capability certain roles have, and less like a permission you would actually check from client code?

Anyhow, I digress - show don't tell, right? 🙂

Here's my approach:

// @lib: es2017

type Comment = {
  id: string;
  body: string;
  authorId: string;
  createdAt: Date;
};

type Todo = {
  id: string;
  title: string;
  userId: string;
  completed: boolean;
  invitedUsers: string[];
};

type Role = "admin" | "moderator" | "user";
type User = { blockedBy: string[]; roles: Role[]; id: string };

// Define the permission interface
interface Permissions {
  viewTodo: (todo: Todo) => boolean;
  createTodo: () => boolean;
  updateTodo: (todo: Todo) => boolean;
  deleteTodo: (todo: Todo) => boolean;
  viewComment: (comment: Comment) => boolean;
  createComment: () => boolean;
  updateComment: (comment: Comment) => boolean;
}

// Named type for the factory map
type PermissionsFactoryMap = {
  [key in Role]: (user: User) => Permissions;
};

// Factory map for role-specific permissions
const permissionsFactoryMap: PermissionsFactoryMap = {
  admin: (user: User) => ({
    viewTodo: () => true,
    createTodo: () => true,
    updateTodo: () => true,
    deleteTodo: () => true,
    viewComment: () => true,
    createComment: () => true,
    updateComment: () => true,
  }),
  moderator: (user: User) => ({
    viewTodo: () => true,
    createTodo: () => true,
    updateTodo: () => true,
    deleteTodo: (todo) => todo.completed,
    viewComment: () => true,
    createComment: () => true,
    updateComment: () => true,
  }),
  user: (user: User) => ({
    viewTodo: (todo) => !user.blockedBy.includes(todo.userId),
    createTodo: () => true,
    updateTodo: (todo) => todo.userId === user.id || todo.invitedUsers.includes(user.id),
    deleteTodo: (todo) => (todo.userId === user.id || todo.invitedUsers.includes(user.id)) && todo.completed,
    viewComment: (comment) => !user.blockedBy.includes(comment.authorId),
    createComment: () => true,
    updateComment: (comment) => comment.authorId === user.id,
  }),
};

// Function to create a proxy that checks permissions based on user roles
function can(user: User): Permissions {
  const userRolePermissions = user.roles.map(role => permissionsFactoryMap[role](user));

  return new Proxy({} as Permissions, {
    get(target, action: keyof Permissions) {
      return (data: any) => userRolePermissions.some(permissions => permissions[action](data));
    },
  });
}

// USAGE:
const user: User = { blockedBy: ["2"], id: "1", roles: ["user"] };
const todo: Todo = {
  completed: false,
  id: "3",
  invitedUsers: [],
  title: "Test Todo",
  userId: "1",
};

console.log(can(user).createComment()); // Can create a comment
console.log(can(user).viewTodo(todo)); // Can view the `todo` Todo
console.log(can(user).viewTodo({ ...todo, userId: "2" })); // Cannot view the `todo` Todo
console.log(can(user).deleteTodo(todo)); // Cannot delete the `todo` Todo
console.log(can(user).deleteTodo({ ...todo, completed: true })); // Can delete the `todo` Todo if completed

It's maybe a bit simpler and easier to understand? Avoids all the generic types, coupling data types to actions, string unions and such.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions