Fix device (#365)

* device in group request added.

* Test now check only admins can see users.

* Fixed api-doc

* fix apidoc again

* Fixed after code review

Co-authored-by: Clare McLennan <>

api/V1/Device.ts

@@ -23,6 +23,9 @@ import responseUtil from "./responseUtil";
 import { body, param, query } from "express-validator/check";
 import Sequelize from "sequelize";
 import { Application } from "express";
+import { AccessLevel } from "../../models/GroupUsers";
+import { AuthorizationError } from "../customErrors";
+import DeviceUsers from "../../models/DeviceUsers";
 
 const Op = Sequelize.Op;
 
@@ -47,7 +50,7 @@ export default function (app: Application, baseUrl: string) {
     apiUrl,
     [
       middleware.getGroupByName(body),
-      middleware.checkNewName("devicename"),
+      middleware.isValidName(body, "devicename"),
       middleware.checkNewPassword("password")
     ],
     middleware.requestWrapper(async (request, response) => {
@@ -123,6 +126,73 @@ export default function (app: Application, baseUrl: string) {
     })
   );
 
+/**
+ * @api {get} /api/v1/devices/:deviceName/in-group/:groupIdOrName Get a single device
+ * @apiName GetDeviceInGroup
+ * @apiGroup Device
+ * @apiParam {string} deviceName Name of the device
+ * @apiParam {stringOrInt} groupIdOrName Identifier of group device belongs to
+ *
+ * @apiDescription Returns the device if the user can access it either through
+ * through both group membership and direct assignment.
+ *
+ * @apiUse V1UserAuthorizationHeader
+ *
+ * @apiUse V1ResponseSuccess
+ * @apiSuccess {JSON} device Object with `deviceName` (string), `id` (int), and device users (if authorized) '
+ * 
+ * @apiUse V1ResponseError
+ */
+    app.get(
+    `${apiUrl}/:deviceName/in-group/:groupIdOrName`,
+    [
+      auth.authenticateUser,
+      middleware.getGroupByNameOrIdDynamic(param, "groupIdOrName"),
+      middleware.isValidName(param, "deviceName"),
+    ],
+    middleware.requestWrapper(async (request, response) => {
+      const device = await models.Device.findOne({
+        where: { devicename: request.params.deviceName, GroupId: request.body.group.id },
+        include: [
+          {
+            model: models.User,
+            attributes: ["id", "username"]
+          }
+        ]
+      });
+
+      let deviceReturn : any  = {};
+      if (device ) {
+        const accessLevel = await device.getAccessLevel(request.user);
+        if (accessLevel < AccessLevel.Read) {
+          throw new AuthorizationError(
+            "User is not authorized to access device"
+          );        
+        }
+
+        deviceReturn = {
+          id: device.id,
+          deviceName: device.devicename,
+          groupName: request.body.group.groupname,
+          userIsAdmin: (accessLevel == AccessLevel.Admin)
+        };
+        if (accessLevel == AccessLevel.Admin) {
+          deviceReturn.users = device.Users.map((user) => {
+            return  {
+              userName: user.username, 
+              admin: user.DeviceUsers.admin, 
+              id: user.DeviceUsers.UserId}
+          })
+        }
+      }
+      return responseUtil.send(response, {
+        statusCode: 200,
+        device: deviceReturn,
+        messages: ["Request succesful"]
+      });
+    })
+  );
+
   /**
    * @api {get} /api/v1/devices/users Get all users who can access a device.
    * @apiName GetDeviceUsers
@@ -142,10 +212,12 @@ export default function (app: Application, baseUrl: string) {
    */
   app.get(
     `${apiUrl}/users`,
-    [auth.authenticateUser, middleware.getDeviceById(query)],
+    [auth.authenticateUser, 
+    middleware.getDeviceById(query),
+    auth.userCanAccessDevices],
     middleware.requestWrapper(async (request, response) => {
       let users = await request.body.device.users(request.user);
-
+      
       users = users.map((u) => {
         u = u.get({ plain: true });
 
@@ -244,7 +316,7 @@ export default function (app: Application, baseUrl: string) {
     middleware.requestWrapper(async function (request, response) {
       const removed = await models.Device.removeUserFromDevice(
         request.user,
-        request.body.device,
+        request.body.device,  
         request.body.user
       );
 
@@ -282,7 +354,7 @@ export default function (app: Application, baseUrl: string) {
     [
       auth.authenticateDevice,
       middleware.getGroupByName(body, "newGroup"),
-      middleware.checkNewName("newName"),
+      middleware.isValidName(body, "newName"),
       middleware.checkNewPassword("newPassword")
     ],
     middleware.requestWrapper(async function (request, response) {
@@ -327,7 +399,6 @@ export default function (app: Application, baseUrl: string) {
     [
       middleware.parseJSON("devices", query).optional(),
       middleware.parseArray("groups", query).optional(),
-
       query("operator").isIn(["or", "and", "OR", "AND"]).optional(),
       auth.authenticateAccess(["user"], { devices: "r" })
     ],

api/V1/Group.ts

@@ -23,6 +23,7 @@ import responseUtil from "./responseUtil";
 import { body, param, query } from "express-validator/check";
 import { Application } from "express";
 import { Validator } from "jsonschema";
+
 const JsonSchema = new Validator();
 
 const validateStationsJson = (val, { req }) => {
@@ -72,7 +73,7 @@ export default function (app: Application, baseUrl: string) {
     apiUrl,
     [
       auth.authenticateUser,
-      middleware.checkNewName("groupname").custom((value) => {
+      middleware.isValidName(body, "groupname").custom((value) => {
         return models.Group.freeGroupname(value);
       })
     ],

api/V1/User.ts

@@ -50,7 +50,7 @@ export default function (app: Application, baseUrl: string) {
   app.post(
     apiUrl,
     [
-      middleware.checkNewName("username").custom((value) => {
+      middleware.isValidName(body, "username").custom((value) => {
         return models.User.freeUsername(value);
       }),
       body("email")
@@ -99,7 +99,7 @@ export default function (app: Application, baseUrl: string) {
     [
       auth.authenticateUser,
       middleware
-        .checkNewName("username")
+        .isValidName(body, "username")
         .custom((value) => {
           return models.User.freeUsername(value);
         })

api/middleware.ts

@@ -204,10 +204,10 @@ function getRecordingById(checkFunc: ValidationChainBuilder): ValidationChain {
   return getModelById(models.Recording, "id", checkFunc);
 }
 
-const checkNewName = function (field: string): ValidationChain {
-  return body(field, "Invalid " + field)
+const isValidName = function (checkFunc: ValidationChainBuilder, field: string): ValidationChain {
+  return checkFunc(field, `${field} must only contain letters, numbers, dash, underscore and space.  It must contain at least one letter`)
     .isLength({ min: 3 })
-    .matches(/(?=.*[A-Za-z])^[a-zA-Z0-9]+([_ -]?[a-zA-Z0-9])*$/);
+    .matches(/(?=.*[A-Za-z])^[a-zA-Z0-9]+([_ \-a-zA-Z0-9])*$/);
 };
 
 const checkNewPassword = function (field: string): ValidationChain {
@@ -338,7 +338,7 @@ export default {
   getDetailSnapshotById,
   getFileById,
   getRecordingById,
-  checkNewName,
+  isValidName,
   checkNewPassword,
   parseJSON,
   parseArray,

models/Device.ts

@@ -24,17 +24,14 @@ import { GroupUsersStatic } from "./GroupUsers";
 import { DeviceUsersStatic } from "./DeviceUsers";
 import { ScheduleId } from "./Schedule";
 import { Event } from "./Event";
+import { AccessLevel } from "./GroupUsers"
 
 const Op = Sequelize.Op;
 export type DeviceId = number;
-type UserDevicePermissions = {
-  canListUsers: boolean;
-  canAddUsers: boolean;
-  canRemoveUsers: boolean;
-};
+
 export interface Device extends Sequelize.Model, ModelCommon<Device> {
   id: DeviceId;
-  userPermissions: (user: User) => UserDevicePermissions;
+  getAccessLevel: (user: User) => AccessLevel;
   addUser: (userId: UserId, options: any) => any;
   devicename: string;
   groupname: string;
@@ -75,7 +72,6 @@ export interface DeviceStatic extends ModelStaticCommon<Device> {
     includeData?: any
   ) => Promise<{ rows: Device[]; count: number }>;
   freeDevicename: (name: string, id: number) => Promise<boolean>;
-  newUserPermissions: (enabled: boolean) => UserDevicePermissions;
   getFromId: (id: DeviceId) => Promise<Device>;
   findDevice: (
     deviceID?: DeviceId,
@@ -186,7 +182,7 @@ export default function (
     if (device == null || userToAdd == null) {
       return false;
     }
-    if (!(await device.userPermissions(authUser)).canAddUsers) {
+    if ((await device.getAccessLevel(authUser)) != AccessLevel.Admin) {
       throw new AuthorizationError(
         "User is not a group, device, or global admin so cannot add users to this device"
       );
@@ -221,7 +217,7 @@ export default function (
     if (device == null || userToRemove == null) {
       return false;
     }
-    if (!(await device.userPermissions(authUser)).canRemoveUsers) {
+    if ((await device.getAccessLevel(authUser)) != AccessLevel.Admin) {
       throw new AuthorizationError(
         "User is not a group, device, or global admin so cannot remove users from this device"
       );
@@ -291,14 +287,6 @@ export default function (
     );
   };
 
-  Device.newUserPermissions = function (enabled) {
-    return {
-      canListUsers: enabled,
-      canAddUsers: enabled,
-      canRemoveUsers: enabled
-    };
-  };
-
   Device.freeDevicename = async function (devicename, groupId) {
     const device = await this.findOne({
       where: { devicename: devicename, GroupId: groupId }
@@ -561,20 +549,20 @@ order by hour;
   // INSTANCE METHODS
   //------------------
 
-  Device.prototype.userPermissions = async function (user) {
+  Device.prototype.getAccessLevel = async function (user) {
     if (user.hasGlobalWrite()) {
-      return Device.newUserPermissions(true);
+      return AccessLevel.Admin;
     }
 
-    const isGroupAdmin = await (models.GroupUsers as GroupUsersStatic).isAdmin(
+    const groupAccessLevel = await (models.GroupUsers as GroupUsersStatic).getAccessLevel(
       this.GroupId,
       user.id
     );
-    const isDeviceAdmin = await (models.DeviceUsers as DeviceUsersStatic).isAdmin(
+    const deviceAccessLevel = await (models.DeviceUsers as DeviceUsersStatic).getAccessLevel(
       this.id,
       user.id
     );
-    return Device.newUserPermissions(isGroupAdmin || isDeviceAdmin);
+    return Math.max(groupAccessLevel, deviceAccessLevel);
   };
 
   Device.prototype.getJwtDataValues = function () {
@@ -604,7 +592,7 @@ order by hour;
     authUser,
     attrs = ["id", "username", "email"]
   ) {
-    if (!(await this.userPermissions(authUser)).canListUsers) {
+    if (!(await this.getAccessLevel(authUser) == AccessLevel.Admin)) {
       return [];
     }
 

models/DeviceUsers.ts

@@ -20,12 +20,14 @@ import Sequelize from "sequelize";
 import { ModelCommon, ModelStaticCommon } from "./index";
 import { UserId } from "./User";
 import { DeviceId } from "./Device";
+import { AccessLevel } from "./GroupUsers"
 
 export interface DeviceUsers
   extends Sequelize.Model,
     ModelCommon<DeviceUsers> {}
 export interface DeviceUsersStatic extends ModelStaticCommon<DeviceUsers> {
   isAdmin: (deviceId: DeviceId, userId: UserId) => Promise<boolean>;
+  getAccessLevel: (deviceId: DeviceId, userId: UserId) => Promise<number>;
 }
 
 export default function (
@@ -53,14 +55,24 @@ export default function (
   DeviceUsers.addAssociations = function () {};
 
   DeviceUsers.isAdmin = async function (deviceId, userId) {
+    return (await this.getAccessLevel(deviceId, userId)) == AccessLevel.Admin;
+  };
+
+    /**
+   * Checks if a user is a admin of a group.
+   */
+  DeviceUsers.getAccessLevel = async function (deviceId, userId) {
     const deviceUser = await this.findOne({
       where: {
         DeviceId: deviceId,
         UserId: userId,
-        admin: true
       }
     });
-    return deviceUser != null;
+
+    if (deviceUser == null) {
+      return AccessLevel.None;
+    }
+    return (deviceUser.admin) ? AccessLevel.Admin : AccessLevel.Read;
   };
 
   return DeviceUsers;

models/GroupUsers.ts

@@ -21,9 +21,16 @@ import { ModelCommon, ModelStaticCommon } from "./index";
 import { GroupId } from "./Group";
 import { UserId } from "./User";
 
+export enum AccessLevel {
+  None = 0,
+  Read = 1,
+  Admin = 2
+}
+
 export interface GroupUsers extends Sequelize.Model, ModelCommon<GroupUsers> {}
 export interface GroupUsersStatic extends ModelStaticCommon<GroupUsers> {
   isAdmin: (groupId: GroupId, userId: UserId) => Promise<boolean>;
+  getAccessLevel: (groupId: GroupId, userId: UserId) => Promise<AccessLevel>;
 }
 
 export default function (sequelize, DataTypes): GroupUsersStatic {
@@ -49,14 +56,24 @@ export default function (sequelize, DataTypes): GroupUsersStatic {
    * Checks if a user is a admin of a group.
    */
   GroupUsers.isAdmin = async function (groupId, userId) {
+    return (await this.getAccessLevel(groupId, userId)) == AccessLevel.Admin;
+  };
+
+  /**
+   * Checks if a user is a admin of a group.
+   */
+  GroupUsers.getAccessLevel = async function (groupId, userId) {
     const groupUsers = await this.findOne({
       where: {
         GroupId: groupId,
         UserId: userId,
-        admin: true
       }
     });
-    return groupUsers != null;
+
+    if (groupUsers == null) {
+      return AccessLevel.None;
+    }
+    return (groupUsers.admin) ? AccessLevel.Admin : AccessLevel.Read;
   };
 
   return GroupUsers;

test-cypress/cypress/commands/api/camera.d.ts

@@ -25,10 +25,13 @@ declare namespace Cypress {
 
     /**
      * use to test when a camera should not be able to be created.
+     * 
+     * Use makeCameraNameTestName = false if you don't want cy_ etc added to the camera name
      */
     apiShouldFailToCreateCamera(
       cameraName: string,
-      group: string
+      group: string,
+      makeCameraNameTestName? : boolean
     ): Chainable<Element>;
   }
 }