mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Act on effective system tag canAssign permission 

Whenever the server returns true for the can-assign Webdav property of
a system tag, it means the current user is allowed to assign,
regardless of the value of user-assignable.

This commit brings the proper logic to the web UI to make it possible
for users to assign when they have the permission.
This commit is contained in:
Vincent Petry 2016-05-11 16:47:33 +02:00
parent 03d32bc39b
commit 88740f035d
No known key found for this signature in database
GPG Key ID: AF8F9EFC56562186
8 changed files with 109 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
apps/dav/lib/SystemTag/SystemTagPlugin.php
View File

@ -53,6 +53,7 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin {
const DISPLAYNAME_PROPERTYNAME = '{http://owncloud.org/ns}display-name'; const DISPLAYNAME_PROPERTYNAME = '{http://owncloud.org/ns}display-name';
const USERVISIBLE_PROPERTYNAME = '{http://owncloud.org/ns}user-visible'; const USERVISIBLE_PROPERTYNAME = '{http://owncloud.org/ns}user-visible';
const USERASSIGNABLE_PROPERTYNAME = '{http://owncloud.org/ns}user-assignable'; const USERASSIGNABLE_PROPERTYNAME = '{http://owncloud.org/ns}user-assignable';
const CANASSIGN_PROPERTYNAME = '{http://owncloud.org/ns}can-assign';
/** /**
* @var \Sabre\DAV\Server $server * @var \Sabre\DAV\Server $server
@ -206,7 +207,7 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin {
PropFind $propFind, PropFind $propFind,
\Sabre\DAV\INode $node \Sabre\DAV\INode $node
) { ) {
if (!($node instanceof SystemTagNode)) { if (!($node instanceof SystemTagNode) && !($node instanceof SystemTagMappingNode)) {
return; return;
} }
@ -223,8 +224,14 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin {
}); });
$propFind->handle(self::USERASSIGNABLE_PROPERTYNAME, function() use ($node) { $propFind->handle(self::USERASSIGNABLE_PROPERTYNAME, function() use ($node) {
// this is the tag's inherent property "is user assignable"
return $node->getSystemTag()->isUserAssignable() ? 'true' : 'false'; return $node->getSystemTag()->isUserAssignable() ? 'true' : 'false';
}); });
$propFind->handle(self::CANASSIGN_PROPERTYNAME, function() use ($node) {
// this is the effective permission for the current user
return $this->tagManager->canUserAssignTag($node->getSystemTag(), $this->userSession->getUser()) ? 'true' : 'false';
});
} }
/** /**

2
apps/systemtags/js/systemtagsinfoview.js
View File

@ -12,7 +12,7 @@
function modelToSelection(model) { function modelToSelection(model) {
var data = model.toJSON(); var data = model.toJSON();
if (!OC.isUserAdmin() && !data.userAssignable) { if (!OC.isUserAdmin() && !data.canAssign) {
data.locked = true; data.locked = true;
} }
return data; return data;

43
apps/systemtags/tests/js/systemtagsinfoviewSpec.js
View File

@ -62,9 +62,9 @@ describe('OCA.SystemTags.SystemTagsInfoView tests', function() {
fetchStub.yieldTo('success', view.selectedTagsCollection); fetchStub.yieldTo('success', view.selectedTagsCollection);
expect(setDataStub.calledOnce).toEqual(true); expect(setDataStub.calledOnce).toEqual(true);
expect(setDataStub.getCall(0).args[0]).toEqual([{ expect(setDataStub.getCall(0).args[0]).toEqual([{
id: '1', name: 'test1', userVisible: true, userAssignable: true id: '1', name: 'test1', userVisible: true, userAssignable: true, canAssign: true
}, { }, {
id: '3', name: 'test3', userVisible: true, userAssignable: true id: '3', name: 'test3', userVisible: true, userAssignable: true, canAssign: true
}]); }]);
expect(view.$el.hasClass('hidden')).toEqual(false); expect(view.$el.hasClass('hidden')).toEqual(false);
@ -79,7 +79,7 @@ describe('OCA.SystemTags.SystemTagsInfoView tests', function() {
view = new OCA.SystemTags.SystemTagsInfoView(); view = new OCA.SystemTags.SystemTagsInfoView();
view.selectedTagsCollection.add([ view.selectedTagsCollection.add([
{id: '1', name: 'test1'}, {id: '1', name: 'test1'},
{id: '3', name: 'test3', userVisible: false, userAssignable: false} {id: '3', name: 'test3', userVisible: false, userAssignable: false, canAssign: false}
]); ]);
var callback = sinon.stub(); var callback = sinon.stub();
@ -87,9 +87,9 @@ describe('OCA.SystemTags.SystemTagsInfoView tests', function() {
expect(callback.calledOnce).toEqual(true); expect(callback.calledOnce).toEqual(true);
expect(callback.getCall(0).args[0]).toEqual([{ expect(callback.getCall(0).args[0]).toEqual([{
id: '1', name: 'test1', userVisible: true, userAssignable: true id: '1', name: 'test1', userVisible: true, userAssignable: true, canAssign: true
}, { }, {
id: '3', name: 'test3', userVisible: false, userAssignable: false id: '3', name: 'test3', userVisible: false, userAssignable: false, canAssign: false
}]); }]);
inputViewSpy.restore(); inputViewSpy.restore();
@ -103,7 +103,7 @@ describe('OCA.SystemTags.SystemTagsInfoView tests', function() {
view = new OCA.SystemTags.SystemTagsInfoView(); view = new OCA.SystemTags.SystemTagsInfoView();
view.selectedTagsCollection.add([ view.selectedTagsCollection.add([
{id: '1', name: 'test1'}, {id: '1', name: 'test1'},
{id: '3', name: 'test3', userAssignable: false} {id: '3', name: 'test3', userAssignable: false, canAssign: false}
]); ]);
var callback = sinon.stub(); var callback = sinon.stub();
@ -111,9 +111,33 @@ describe('OCA.SystemTags.SystemTagsInfoView tests', function() {
expect(callback.calledOnce).toEqual(true); expect(callback.calledOnce).toEqual(true);
expect(callback.getCall(0).args[0]).toEqual([{ expect(callback.getCall(0).args[0]).toEqual([{
id: '1', name: 'test1', userVisible: true, userAssignable: true id: '1', name: 'test1', userVisible: true, userAssignable: true, canAssign: true
}, { }, {
id: '3', name: 'test3', userVisible: true, userAssignable: false, locked: true id: '3', name: 'test3', userVisible: true, userAssignable: false, canAssign: false, locked: true
}]);
inputViewSpy.restore();
});
it('does not set locked flag on non-assignable tags when canAssign overrides it with true', function() {
isAdminStub.returns(false);
var inputViewSpy = sinon.spy(OC.SystemTags, 'SystemTagsInputField');
var element = $('<input type="hidden" val="1,4"/>');
view.remove();
view = new OCA.SystemTags.SystemTagsInfoView();
view.selectedTagsCollection.add([
{id: '1', name: 'test1'},
{id: '4', name: 'test4', userAssignable: false, canAssign: true}
]);
var callback = sinon.stub();
inputViewSpy.getCall(0).args[0].initSelection(element, callback);
expect(callback.calledOnce).toEqual(true);
expect(callback.getCall(0).args[0]).toEqual([{
id: '1', name: 'test1', userVisible: true, userAssignable: true, canAssign: true
}, {
id: '4', name: 'test4', userVisible: true, userAssignable: false, canAssign: true
}]); }]);
inputViewSpy.restore(); inputViewSpy.restore();
@ -152,7 +176,8 @@ describe('OCA.SystemTags.SystemTagsInfoView tests', function() {
id: '2', id: '2',
name: 'test2', name: 'test2',
userVisible: true, userVisible: true,
userAssignable: true userAssignable: true,
canAssign: true
}); });
createStub.restore(); createStub.restore();

10
core/js/systemtags/systemtagmodel.js
View File

@ -23,14 +23,17 @@
defaults: { defaults: {
userVisible: true, userVisible: true,
userAssignable: true userAssignable: true,
canAssign: true
}, },
davProperties: { davProperties: {
'id': '{' + NS_OWNCLOUD + '}id', 'id': '{' + NS_OWNCLOUD + '}id',
'name': '{' + NS_OWNCLOUD + '}display-name', 'name': '{' + NS_OWNCLOUD + '}display-name',
'userVisible': '{' + NS_OWNCLOUD + '}user-visible', 'userVisible': '{' + NS_OWNCLOUD + '}user-visible',
'userAssignable': '{' + NS_OWNCLOUD + '}user-assignable' 'userAssignable': '{' + NS_OWNCLOUD + '}user-assignable',
// read-only, effective permissions computed by the server,
'canAssign': '{' + NS_OWNCLOUD + '}can-assign'
}, },
parse: function(data) { parse: function(data) {
@ -38,7 +41,8 @@
id: data.id, id: data.id,
name: data.name, name: data.name,
userVisible: data.userVisible === true || data.userVisible === 'true', userVisible: data.userVisible === true || data.userVisible === 'true',
userAssignable: data.userAssignable === true || data.userAssignable === 'true' userAssignable: data.userAssignable === true || data.userAssignable === 'true',
canAssign: data.canAssign === true || data.canAssign === 'true'
}; };
} }
}); });

8
core/js/systemtags/systemtagsinputfield.js
View File

@ -206,7 +206,8 @@
tag = this.collection.create({ tag = this.collection.create({
name: e.object.name.trim(), name: e.object.name.trim(),
userVisible: true, userVisible: true,
userAssignable: true userAssignable: true,
canAssign: true
}, { }, {
success: function(model) { success: function(model) {
self._addToSelect2Selection(model.toJSON()); self._addToSelect2Selection(model.toJSON());
@ -263,7 +264,7 @@
var tagModels = collection.filterByName(query.term.trim()); var tagModels = collection.filterByName(query.term.trim());
if (!self._isAdmin) { if (!self._isAdmin) {
tagModels = _.filter(tagModels, function(tagModel) { tagModels = _.filter(tagModels, function(tagModel) {
return tagModel.get('userAssignable'); return tagModel.get('canAssign');
}); });
} }
query.callback({ query.callback({
@ -331,6 +332,7 @@
name: term, name: term,
userAssignable: true, userAssignable: true,
userVisible: true, userVisible: true,
canAssign: true,
isNew: true isNew: true
}; };
} else { } else {
@ -346,7 +348,7 @@
function modelToSelection(model) { function modelToSelection(model) {
var data = model.toJSON(); var data = model.toJSON();
if (!self._isAdmin && !data.userAssignable) { if (!self._isAdmin && !data.canAssign) {
// lock static tags for non-admins // lock static tags for non-admins
data.locked = true; data.locked = true;
} }

71
core/js/tests/specs/systemtags/systemtagsinputfieldSpec.js
View File

@ -68,7 +68,7 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
view.collection.add([ view.collection.add([
new OC.SystemTags.SystemTagModel({id: '1', name: 'abc'}), new OC.SystemTags.SystemTagModel({id: '1', name: 'abc'}),
new OC.SystemTags.SystemTagModel({id: '2', name: 'def'}), new OC.SystemTags.SystemTagModel({id: '2', name: 'def'}),
new OC.SystemTags.SystemTagModel({id: '3', name: 'abd', userAssignable: false}), new OC.SystemTags.SystemTagModel({id: '3', name: 'abd', userAssignable: false, canAssign: false}),
]); ]);
}); });
it('does not create dummy tag when user types non-matching name', function() { it('does not create dummy tag when user types non-matching name', function() {
@ -84,6 +84,7 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
expect(result.isNew).toEqual(true); expect(result.isNew).toEqual(true);
expect(result.userVisible).toEqual(true); expect(result.userVisible).toEqual(true);
expect(result.userAssignable).toEqual(true); expect(result.userAssignable).toEqual(true);
expect(result.canAssign).toEqual(true);
}); });
it('creates dummy tag when user types non-matching name even with prefix of existing tag', function() { it('creates dummy tag when user types non-matching name even with prefix of existing tag', function() {
var opts = select2Stub.getCall(0).args[0]; var opts = select2Stub.getCall(0).args[0];
@ -93,6 +94,7 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
expect(result.isNew).toEqual(true); expect(result.isNew).toEqual(true);
expect(result.userVisible).toEqual(true); expect(result.userVisible).toEqual(true);
expect(result.userAssignable).toEqual(true); expect(result.userAssignable).toEqual(true);
expect(result.canAssign).toEqual(true);
}); });
it('creates the real tag and fires select event after user selects the dummy tag', function() { it('creates the real tag and fires select event after user selects the dummy tag', function() {
var selectHandler = sinon.stub(); var selectHandler = sinon.stub();
@ -110,14 +112,16 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
expect(createStub.getCall(0).args[0]).toEqual({ expect(createStub.getCall(0).args[0]).toEqual({
name: 'newname', name: 'newname',
userVisible: true, userVisible: true,
userAssignable: true userAssignable: true,
canAssign: true
}); });
var newModel = new OC.SystemTags.SystemTagModel({ var newModel = new OC.SystemTags.SystemTagModel({
id: '123', id: '123',
name: 'newname', name: 'newname',
userVisible: true, userVisible: true,
userAssignable: true userAssignable: true,
canAssign: true
}); });
// not called yet // not called yet
@ -186,7 +190,8 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
expect(createStub.getCall(0).args[0]).toEqual({ expect(createStub.getCall(0).args[0]).toEqual({
name: 'newname', name: 'newname',
userVisible: true, userVisible: true,
userAssignable: true userAssignable: true,
canAssign: true
}); });
var newModel = new OC.SystemTags.SystemTagModel({ var newModel = new OC.SystemTags.SystemTagModel({
@ -320,7 +325,8 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
testTags = [ testTags = [
new OC.SystemTags.SystemTagModel({id: '1', name: 'test1'}), new OC.SystemTags.SystemTagModel({id: '1', name: 'test1'}),
new OC.SystemTags.SystemTagModel({id: '2', name: 'test2'}), new OC.SystemTags.SystemTagModel({id: '2', name: 'test2'}),
new OC.SystemTags.SystemTagModel({id: '3', name: 'test3', userAssignable: false}), new OC.SystemTags.SystemTagModel({id: '3', name: 'test3', userAssignable: false, canAssign: false}),
new OC.SystemTags.SystemTagModel({id: '4', name: 'test4', userAssignable: false, canAssign: true})
]; ];
}); });
afterEach(function() { afterEach(function() {
@ -328,7 +334,7 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
}); });
it('grabs values from the full collection', function() { it('grabs values from the full collection', function() {
var $el = view.$el.find('input'); var $el = view.$el.find('input');
$el.val('1,3'); $el.val('1,3,4');
var opts = select2Stub.getCall(0).args[0]; var opts = select2Stub.getCall(0).args[0];
var callback = sinon.stub(); var callback = sinon.stub();
opts.initSelection($el, callback); opts.initSelection($el, callback);
@ -339,11 +345,16 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
expect(callback.calledOnce).toEqual(true); expect(callback.calledOnce).toEqual(true);
var models = callback.getCall(0).args[0]; var models = callback.getCall(0).args[0];
expect(models.length).toEqual(2); expect(models.length).toEqual(3);
expect(models[0].id).toEqual('1'); expect(models[0].id).toEqual('1');
expect(models[0].name).toEqual('test1'); expect(models[0].name).toEqual('test1');
expect(models[0].locked).toBeFalsy();
expect(models[1].id).toEqual('3'); expect(models[1].id).toEqual('3');
expect(models[1].name).toEqual('test3'); expect(models[1].name).toEqual('test3');
expect(models[1].locked).toBeFalsy();
expect(models[2].id).toEqual('4');
expect(models[2].name).toEqual('test4');
expect(models[2].locked).toBeFalsy();
}); });
}); });
describe('autocomplete', function() { describe('autocomplete', function() {
@ -356,7 +367,7 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
view.collection.add([ view.collection.add([
new OC.SystemTags.SystemTagModel({id: '1', name: 'abc'}), new OC.SystemTags.SystemTagModel({id: '1', name: 'abc'}),
new OC.SystemTags.SystemTagModel({id: '2', name: 'def'}), new OC.SystemTags.SystemTagModel({id: '2', name: 'def'}),
new OC.SystemTags.SystemTagModel({id: '3', name: 'abd', userAssignable: false}), new OC.SystemTags.SystemTagModel({id: '3', name: 'abd', userAssignable: false, canAssign: false}),
new OC.SystemTags.SystemTagModel({id: '4', name: 'Deg'}), new OC.SystemTags.SystemTagModel({id: '4', name: 'Deg'}),
]); ]);
}); });
@ -379,13 +390,15 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
id: '1', id: '1',
name: 'abc', name: 'abc',
userVisible: true, userVisible: true,
userAssignable: true userAssignable: true,
canAssign: true
}, },
{ {
id: '3', id: '3',
name: 'abd', name: 'abd',
userVisible: true, userVisible: true,
userAssignable: false userAssignable: false,
canAssign: false
} }
]); ]);
}); });
@ -405,13 +418,15 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
id: '2', id: '2',
name: 'def', name: 'def',
userVisible: true, userVisible: true,
userAssignable: true userAssignable: true,
canAssign: true
}, },
{ {
id: '4', id: '4',
name: 'Deg', name: 'Deg',
userVisible: true, userVisible: true,
userAssignable: true userAssignable: true,
canAssign: true
} }
]); ]);
}); });
@ -445,7 +460,8 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
testTags = [ testTags = [
new OC.SystemTags.SystemTagModel({id: '1', name: 'test1'}), new OC.SystemTags.SystemTagModel({id: '1', name: 'test1'}),
new OC.SystemTags.SystemTagModel({id: '2', name: 'test2'}), new OC.SystemTags.SystemTagModel({id: '2', name: 'test2'}),
new OC.SystemTags.SystemTagModel({id: '3', name: 'test3', userAssignable: false}), new OC.SystemTags.SystemTagModel({id: '3', name: 'test3', userAssignable: false, canAssign: false}),
new OC.SystemTags.SystemTagModel({id: '4', name: 'test4', userAssignable: false, canAssign: true})
]; ];
view.render(); view.render();
}); });
@ -454,7 +470,7 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
}); });
it('grabs values from the full collection', function() { it('grabs values from the full collection', function() {
var $el = view.$el.find('input'); var $el = view.$el.find('input');
$el.val('1,3'); $el.val('1,3,4');
var opts = select2Stub.getCall(0).args[0]; var opts = select2Stub.getCall(0).args[0];
var callback = sinon.stub(); var callback = sinon.stub();
opts.initSelection($el, callback); opts.initSelection($el, callback);
@ -465,11 +481,17 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
expect(callback.calledOnce).toEqual(true); expect(callback.calledOnce).toEqual(true);
var models = callback.getCall(0).args[0]; var models = callback.getCall(0).args[0];
expect(models.length).toEqual(2); expect(models.length).toEqual(3);
expect(models[0].id).toEqual('1'); expect(models[0].id).toEqual('1');
expect(models[0].name).toEqual('test1'); expect(models[0].name).toEqual('test1');
expect(models[0].locked).toBeFalsy();
expect(models[1].id).toEqual('3'); expect(models[1].id).toEqual('3');
expect(models[1].name).toEqual('test3'); expect(models[1].name).toEqual('test3');
// not assignable / cannot assign locks the entry
expect(models[1].locked).toEqual(true);
expect(models[2].id).toEqual('4');
expect(models[2].name).toEqual('test4');
expect(models[2].locked).toBeFalsy();
}); });
}); });
describe('autocomplete', function() { describe('autocomplete', function() {
@ -483,8 +505,9 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
view.collection.add([ view.collection.add([
new OC.SystemTags.SystemTagModel({id: '1', name: 'abc'}), new OC.SystemTags.SystemTagModel({id: '1', name: 'abc'}),
new OC.SystemTags.SystemTagModel({id: '2', name: 'def'}), new OC.SystemTags.SystemTagModel({id: '2', name: 'def'}),
new OC.SystemTags.SystemTagModel({id: '3', name: 'abd', userAssignable: false}), new OC.SystemTags.SystemTagModel({id: '3', name: 'abd', userAssignable: false, canAssign: false}),
new OC.SystemTags.SystemTagModel({id: '4', name: 'Deg'}), new OC.SystemTags.SystemTagModel({id: '4', name: 'Deg'}),
new OC.SystemTags.SystemTagModel({id: '5', name: 'abe', userAssignable: false, canAssign: true})
]); ]);
}); });
afterEach(function() { afterEach(function() {
@ -506,7 +529,15 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
id: '1', id: '1',
name: 'abc', name: 'abc',
userVisible: true, userVisible: true,
userAssignable: true userAssignable: true,
canAssign: true
},
{
id: '5',
name: 'abe',
userVisible: true,
userAssignable: false,
canAssign: true
} }
]); ]);
}); });
@ -526,13 +557,15 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
id: '2', id: '2',
name: 'def', name: 'def',
userVisible: true, userVisible: true,
userAssignable: true userAssignable: true,
canAssign: true
}, },
{ {
id: '4', id: '4',
name: 'Deg', name: 'Deg',
userVisible: true, userVisible: true,
userAssignable: true userAssignable: true,
canAssign: true
} }
]); ]);
}); });

2
lib/private/SystemTag/SystemTagManager.php
View File

@ -350,7 +350,7 @@ class SystemTagManager implements ISystemTagManager {
return false; return false;
} }
$groupIds = $this->groupManager->getUserGroupIds($user->getUID()); $groupIds = $this->groupManager->getUserGroupIds($user);
if (!empty($groupIds)) { if (!empty($groupIds)) {
$matchingGroups = array_intersect($groupIds, $this->getTagGroups($tag)); $matchingGroups = array_intersect($groupIds, $this->getTagGroups($tag));
if (!empty($matchingGroups)) { if (!empty($matchingGroups)) {

1
tests/lib/SystemTag/SystemTagManagerTest.php
View File

@ -493,6 +493,7 @@ class SystemTagManagerTest extends TestCase {
->will($this->returnValue($isAdmin)); ->will($this->returnValue($isAdmin));
$this->groupManager->expects($this->any()) $this->groupManager->expects($this->any())
->method('getUserGroupIds') ->method('getUserGroupIds')
->with($user)
->will($this->returnValue($userGroupIds)); ->will($this->returnValue($userGroupIds));
$this->assertEquals($expectedResult, $this->tagManager->canUserAssignTag($tag1, $user)); $this->assertEquals($expectedResult, $this->tagManager->canUserAssignTag($tag1, $user));