group: cannot set name of ldap group

src/groups.js

@@ -5,7 +5,9 @@ exports = module.exports = {
     remove,
     get,
     getByName,
-    update,
+
+    setName,
+
     getWithMembers,
     list,
     listWithMembers,
@@ -73,7 +75,7 @@ async function add(group) {
     if (error && error.code === 'ER_DUP_ENTRY') throw new BoxError(BoxError.ALREADY_EXISTS, error);
     if (error) throw error;
 
-    return { id, name };
+    return { id, name, source };
 }
 
 async function remove(id) {
@@ -200,34 +202,22 @@ async function isMember(groupId, userId) {
     return result.length !== 0;
 }
 
-async function update(id, data) {
-    assert.strictEqual(typeof id, 'string');
-    assert(data && typeof data === 'object');
+async function setName(group, name) {
+    assert.strictEqual(typeof group, 'object');
+    assert.strictEqual(typeof name, 'string');
 
-    if ('name' in data) {
-        assert.strictEqual(typeof data.name, 'string');
-        const error = validateGroupname(data.name);
-        if (error) throw error;
-    }
+    if (group.source === 'ldap') throw new BoxError(BoxError.BAD_STATE, 'Cannot set name of external group');
 
-    const args = [];
-    const fields = [];
-    for (const k in data) {
-        if (k === 'name') {
-            assert.strictEqual(typeof data.name, 'string');
-            fields.push(k + ' = ?');
-            args.push(data.name);
-        }
-    }
-    args.push(id);
+    name = name.toLowerCase(); // we store names in lowercase
+    const error = validateGroupname(name);
+    if (error) throw error;
 
-    const [updateError, result] = await safe(database.query('UPDATE userGroups SET ' + fields.join(', ') + ' WHERE id = ?', args));
+    const [updateError, result] = await safe(database.query('UPDATE userGroups SET name = ? WHERE id = ?', [ name, group.id ]));
     if (updateError && updateError.code === 'ER_DUP_ENTRY' && updateError.sqlMessage.indexOf('userGroups_name') !== -1) throw new BoxError(BoxError.ALREADY_EXISTS, 'name already exists');
     if (updateError) throw updateError;
     if (result.affectedRows !== 1) throw new BoxError(BoxError.NOT_FOUND, 'Group not found');
 }
 
-
 async function resetSource() {
     await database.query('UPDATE userGroups SET source = ?', [ '' ]);
 }

src/routes/groups.js

@@ -6,7 +6,7 @@ exports = module.exports = {
     get,
     list,
     add,
-    update,
+    setName,
     remove,
     setMembers
 };
@@ -47,15 +47,12 @@ async function get(req, res, next) {
     next(new HttpSuccess(200, req.resource));
 }
 
-async function update(req, res, next) {
-    assert.strictEqual(typeof req.params.groupId, 'string');
+async function setName(req, res, next) {
+    assert.strictEqual(typeof req.resource, 'object');
     assert.strictEqual(typeof req.body, 'object');
 
-    if (req.resource.source === 'ldap') return next(new HttpError(409, 'external group cannot be updated'));
-
-    if ('name' in req.body && typeof req.body.name !== 'string') return next(new HttpError(400, 'name must be a string'));
-
-    const [error] = await safe(groups.update(req.params.groupId, req.body));
+    if (typeof req.body.name !== 'string') return next(new HttpError(400, 'name must be a string'));
+    const [error] = await safe(groups.setName(req.resource, req.body.name));
     if (error) return next(BoxError.toHttpError(error));
 
     next(new HttpSuccess(200, { }));

src/routes/test/groups-test.js

@@ -150,6 +150,32 @@ describe('Groups API', function () {
         expect(response.body.groups[1].name).to.eql(group1Object.name);
     });
 
+    it('cannot set missing group name', async function () {
+        const response = await superagent.put(`${serverUrl}/api/v1/groups/${group1Object.id}/name`)
+            .query({ access_token: owner.token })
+            .send({ })
+            .ok(() => true);
+
+        expect(response.statusCode).to.equal(400);
+    });
+
+    it('can set invalid group name', async function () {
+        const response = await superagent.put(`${serverUrl}/api/v1/groups/${group1Object.id}/name`)
+            .query({ access_token: owner.token })
+            .send({ name: '!group1-newname'})
+            .ok(() => true);
+
+        expect(response.statusCode).to.equal(400);
+    });
+
+    it('can set group name', async function () {
+        const response = await superagent.put(`${serverUrl}/api/v1/groups/${group1Object.id}/name`)
+            .query({ access_token: owner.token })
+            .send({ name: 'group1-newname'});
+
+        expect(response.statusCode).to.equal(200);
+    });
+
     it('remove user from group', async function () {
         const response = await superagent.put(`${serverUrl}/api/v1/users/${user.id}/groups`)
             .query({ access_token: owner.token })

src/server.js

@@ -204,7 +204,7 @@ async function initializeExpressSync() {
     router.post('/api/v1/groups',                  json, token, authorizeUserManager, routes.groups.add);
     router.get ('/api/v1/groups/:groupId',               token, authorizeUserManager, routes.groups.load, routes.groups.get);
     router.put ('/api/v1/groups/:groupId/members', json, token, authorizeUserManager, routes.groups.load, routes.groups.setMembers);
-    router.post('/api/v1/groups/:groupId',         json, token, authorizeUserManager, routes.groups.load, routes.groups.update);
+    router.put ('/api/v1/groups/:groupId/name',    json, token, authorizeUserManager, routes.groups.load, routes.groups.setName);
     router.del ('/api/v1/groups/:groupId',               token, authorizeUserManager, routes.groups.load, routes.groups.remove);
 
     // User directory

src/test/groups-test.js

@@ -57,7 +57,7 @@ describe('Groups', function () {
             expect(error).to.be(null);
             group0Object = result;
 
-            [error, result] = await safe(groups.add({ name: group1Name }));
+            [error, result] = await safe(groups.add({ name: group1Name, source: 'ldap' }));
             expect(error).to.be(null);
             group1Object = result;
         });
@@ -171,4 +171,36 @@ describe('Groups', function () {
             await groups.remove(group0Object.id);
         });
     });
+
+    describe('update', function () {
+        let groupObject;
+
+        before(async function () {
+            let [error, result] = await safe(groups.add({ name: 'kootam' }));
+            expect(error).to.be(null);
+            groupObject = result;
+        });
+
+        it('cannot set empty group name', async function () {
+            const [error] = await safe(groups.setName(groupObject, ''));
+            expect(error.reason).to.be(BoxError.BAD_FIELD);
+        });
+
+        it('cannot set bad group name', async function () {
+            const [error] = await safe(groups.setName(groupObject, '!kootam'));
+            expect(error.reason).to.be(BoxError.BAD_FIELD);
+        });
+
+        it('can set group name', async function () {
+            await groups.setName(groupObject, 'kootam2');
+            groupObject = await groups.get(groupObject.id);
+            expect(groupObject.name).to.be('kootam2');
+        });
+
+        it('cannot change ldap group name', async function () {
+            const result = await groups.add({ name: 'ldap-kootam', source: 'ldap' });
+            const [error] = await safe(groups.setName(result, 'ldap-kootam2'));
+            expect(error.reason).to.be(BoxError.BAD_STATE);
+        });
+    });
 });