Authorization.re

Authorization.re

type permission =
  | Permission(string, string);

module PermissionComparision = {
  type t = permission;
  let compare = (Permission(name1, _), Permission(name2, _)) => String.compare(name1, name2);
};

module PermissionSet = Set.Make(PermissionComparision);

type role = {
  name: string,
  description: string,
  permissions: PermissionSet.t
};

type organization = {
  name: string,
  organizationId: string
};

let isSameOrgAs = ({organizationId: id1}, {organizationId: id2}) => id1 == id2;

type user = {
  email: string,
  userId: string,
  organizations: list((organization, list(role)))
};

let isSameUserAs = ({userId: id1}, {userId: id2}) => id1 == id2;

type entity =
  | User(user)
  | Organization(organization);

let isSameEntityAs = (entity1, entity2) =>
  switch (entity1, entity2) {
  | (User(user1), User(user2)) => user1 |> isSameUserAs(user2)
  | (Organization(org1), Organization(org2)) => org1 |> isSameOrgAs(org2)
  | _ => false
  };

type accessToken = {
  name: string,
  permissions: PermissionSet.t,
  owner: entity
};

type credential =
  | UserCredential(user)
  | AccessToken(accessToken);

type verdict =
  | Authorized
  | Unauthorized;

let (%>=) = (set1, set2) => PermissionSet.subset(set2, set1);

let (%+) = (set1, set2) => PermissionSet.union(set1, set2);

let agglomeratePermissions = (roles) =>
  roles |> List.fold_left((set, {permissions}: role) => set %+ permissions, PermissionSet.empty);

let toSome = (x) => Some(x);

let tryFindOrganization = ({organizations}, targetOrg) =>
  try (
    organizations
    |> List.find(((organization, _)) => organization |> isSameOrgAs(targetOrg))
    |> toSome
  ) {
  | _ => None
  };

let isUserAuthorized = (user, org, requiredPermissions) =>
  switch (tryFindOrganization(user, org)) {
  | Some((_, roles)) when agglomeratePermissions(roles) %>= requiredPermissions => Authorized
  | Some(_) => Unauthorized
  | None => Unauthorized
  };

let isAuthorized = (entity, requiredPermissions, credential) => {
  let requiredPermissionSet = PermissionSet.of_list(requiredPermissions);
  switch (credential, entity) {
  /* User trying to gain access to org */
  | (UserCredential(user), Organization(org)) => isUserAuthorized(user, org, requiredPermissionSet)
  /* User trying to gain access to own user account */
  | (UserCredential(authUser), User(user)) when authUser |> isSameUserAs(user) => Authorized
  /* User trying to gain access to ANOTHER user account */
  | (UserCredential(_), User(_)) => Unauthorized
  /* Access token trying to gain access to correct entity */
  | (AccessToken({owner, permissions}), entity)
      when entity |> isSameEntityAs(owner) && permissions %>= requiredPermissionSet =>
    Authorized
  /* Access token trying to gain access to a DIFFERENT entity for which it was issued */
  | (AccessToken(_), _) => Unauthorized
  }
};

Hi, here's some feedback, hope it's useful:

L2: what do the string types mean in this type definition? A good way to clarify is to alias the types:

type name = string; type passwd = string;
type permission = Permission(name, passwd);

L4, 9: I think the intent is to have a permission set; in this case it's idiomatic to put related stuff in a module. Also, no need for an intermediate PermissionComparison module:

module Permission = {
  type name = string; type passwd = string;
  type t = Permission(name, passwd);

  module Set = Set.Make({
    type nonrec t = t;
    let compare = ...;
  });
};

L6: note that we usually use compare for value comparison, so changing its semantics may result in unexpected behaviour. Specifically, think about where the comparison function will be used. With the current implementation, you won't be able to add a permission to a set which contains a permission with the same name but different password field.

L17, 22, 24, 30, 32, 36: I recommend bundling types and their associated operations in their own modules, like I show with Permission. Leads to more succinct function names as well, e.g. Entity.isSameAs.

L64: if you're targeting JavaScript, you can use Js.Option.some.

L69: this probably means that user.organizations should be a map data structure with organization as the key type and list(role) as the value type. This lets you get rid of tryFindOrganization entirely and simplify isUserAuthorized to:

let isUserAuthorized = (user, org, requiredPermissions) =>
  switch (OrgMap.find(org, user.organizations)) {
    | roles when agglomeratePermissions(roles) %>= requiredPermissions =>
      Authorized
    | _ | exception Not_found => Unauthorized
  };

General note: as it is right now, this module is exposing all its type definitions, meaning any consuming module can create the Authorized data directly and possibly bypass your authorisation rule. You can hide these variant tags (I suggest all of them since no one should be trying to pattern match on them outside of this module) to try to secure the authorisation.

Thanks for the detailed review @yawaramin

L6: note that we usually use compare for value comparison, so changing its semantics may result in unexpected behaviour. Specifically, think about where the comparison function will be used. With the current implementation, you won't be able to add a permission to a set which contains a permission with the same name but different password field.

The second parameter is actually for a description field which has no logic significance, so I actually think it'd be best to remove that from this module.