Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[stable10] Improve public link sharing permissions for folders #29413

Merged
merged 2 commits into from
Nov 3, 2017
Merged

[stable10] Improve public link sharing permissions for folders #29413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
32c1b46
Rework permissions in UX friendlier radio-button style, improve modal…
felixheidecke Oct 27, 2017
f97b37d
Update tests specs to meet new feature implementations
felixheidecke Oct 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions apps/files_sharing/css/sharetabview.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,6 @@
}
/* ---------------------------------------------------- PUBLIC LINK MODAL --- */

.public-link-modal {
width: 340px;
}
.public-link-modal--item {
margin-bottom: 20px;
}
Expand Down
23 changes: 19 additions & 4 deletions core/css/jquery.ocdialog.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,28 @@
-webkit-box-sizing: border-box;
-moz-box-sizing: border-box;
box-sizing: border-box;
min-width: 200px;
min-width: 340px;
}

@media (max-width: 512px) {
.oc-dialog {
position: absolute;
top: 55px;
right: 10px;
left: 10px;
}
}

@media (min-width: 513px) {
/* Center positioning */
left: 50%;
top: 50%;
transform: translate(-50%,-50%);
.oc-dialog {
position: fixed;
left: 50%;
top: 50%;
transform: translate(-50%,-50%);
}
}

.oc-dialog-title {
background: white;
font-weight: bold;
Expand Down
38 changes: 0 additions & 38 deletions core/js/jquery.ocdialog.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,6 @@
_create: function() {
var self = this;

this.originalCss = {
display: this.element[0].style.display,
width: this.element[0].style.width,
height: this.element[0].style.height
};

this.originalTitle = this.element.attr('title');
this.options.title = this.options.title || this.originalTitle;

Expand All @@ -29,11 +23,6 @@
this.$dialog.append(this.element.detach());
this.element.removeAttr('title').addClass('oc-dialog-content').appendTo(this.$dialog);

this.$dialog.css({
display: 'inline-block',
position: 'fixed'
});

$(document).on('keydown keyup', function(event) {
if (
event.target !== self.$dialog.get(0) &&
Expand Down Expand Up @@ -138,12 +127,6 @@
this.$dialog.find('.oc-dialog-close').remove();
}
break;
case 'width':
this.$dialog.css('width', value);
break;
case 'height':
this.$dialog.css('height', value);
break;
case 'close':
this.closeCB = value;
break;
Expand All @@ -155,27 +138,6 @@
//this._super(options);
$.Widget.prototype._setOptions.apply(this, arguments);
},
_setSizes: function() {
// var content_height = this.$dialog.height();
// if(this.$title) {
// content_height -= this.$title.outerHeight(true);
// }
// if(this.$buttonrow) {
// content_height -= this.$buttonrow.outerHeight(true);
// }
// this.parent = this.$dialog.parent().length > 0 ? this.$dialog.parent() : $('body');
// content_height = Math.min(content_height, this.parent.height()-20);
// if (content_height> 0) {
// this.element.css({
// height: content_height + 'px',
// width: this.$dialog.innerWidth()-20 + 'px'
// });
// } else {
// this.element.css({
// width : this.$dialog.innerWidth() - 20 + 'px'
// });
// }
},
_createOverlay: function() {
if(!this.options.modal) {
return;
Expand Down
90 changes: 37 additions & 53 deletions core/js/sharedialoglinkshareview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,21 @@
'<label class="public-link-modal--label">Link Name</label>' +
'<input class="public-link-modal--input" type="text" name="linkName" placeholder="{{namePlaceholder}}" value="{{name}}" maxlength="64" />' +
'</div>' +
'<div id="allowPublicRead-{{cid}}" class="public-link-modal--item">' +
'<input type="radio" value="{{publicReadValue}}" name="publicPermissions" id="sharingDialogAllowPublicRead-{{cid}}" class="checkbox publicPermissions" {{#if publicReadSelected}}checked{{/if}} />' +
'<label class="bold" for="sharingDialogAllowPublicRead-{{cid}}">{{publicReadLabel}}</label>' +
'<p>{{publicReadDescription}}</p>' +
'</div>' +
'{{#if publicUploadPossible}}' +
'<div id="allowPublicUploadWrapper-{{cid}}" class="public-link-modal--item">' +
'<input type="checkbox" value="1" name="allowPublicUpload" id="sharingDialogAllowPublicUpload-{{cid}}" class="checkbox publicUploadCheckbox" {{#if publicUploadChecked}}checked="checked"{{/if}} />' +
'<label for="sharingDialogAllowPublicUpload-{{cid}}">{{publicUploadLabel}}</label>' +
'<div id="allowPublicReadWrite-{{cid}}" class="public-link-modal--item">' +
'<input type="radio" value="{{publicReadWriteValue}}" name="publicPermissions" id="sharingDialogAllowPublicReadWrite-{{cid}}" class="checkbox publicPermissions" {{#if publicReadWriteSelected}}checked{{/if}} />' +
'<label class="bold" for="sharingDialogAllowPublicReadWrite-{{cid}}">{{publicReadWriteLabel}}</label>' +
'<p>{{publicReadWriteDescription}}</p>' +
'</div>' +
'<div id="showListingWrapper-{{cid}}" class="public-link-modal--item">' +
'<input type="checkbox" value="1" name="showListing" id="sharingDialogShowListing-{{cid}}" class="checkbox showListingCheckbox" {{#if showListingChecked}}checked="checked"{{/if}} />' +
'<label for="sharingDialogShowListing-{{cid}}">{{showListingLabel}}</label>' +
'<div id="allowPublicUploadWrapper-{{cid}}" class="public-link-modal--item">' +
'<input type="radio" value="{{publicUploadValue}}" name="publicPermissions" id="sharingDialogAllowPublicUpload-{{cid}}" class="checkbox publicPermissions" {{#if publicUploadSelected}}checked{{/if}} />' +
'<label class="bold" for="sharingDialogAllowPublicUpload-{{cid}}">{{publicUploadLabel}}</label>' +
'<p>{{publicUploadDescription}}</p>' +
'</div>' +
'{{/if}}' +
'<div id="linkPass-{{cid}}" class="public-link-modal--item linkPass">' +
Expand Down Expand Up @@ -69,10 +76,6 @@
/** @type {Function} **/
_template: undefined,

events: {
'click .publicUploadCheckbox': '_updateCheckboxes'
},

initialize: function (options) {
if (!_.isUndefined(options.itemModel)) {
this.itemModel = options.itemModel;
Expand All @@ -89,47 +92,16 @@
OC.Plugins.attach('OCA.Share.ShareDialogLinkShareView', this);
},

_updateCheckboxes: function() {
var publicUploadAllowed = this.$('.publicUploadCheckbox').is(':checked');
if (!publicUploadAllowed) {
this.$('.showListingCheckbox').prop('checked', true);
this.$('.showListingCheckbox').prop('disabled', true);
} else {
this.$('.showListingCheckbox').prop('disabled', false);
}
},

/**
* Returns the selected permissions as read from the checkboxes or
* the absence thereof.
*
* @return {int} permissions
*/
_getPermissions: function() {
var $showListingCheckbox = this.$('.showListingCheckbox');
var $publicUploadCheckbox = this.$('.publicUploadCheckbox');
var allowListing = (!$showListingCheckbox.length || $showListingCheckbox.is(':checked'));
var permissions = 0;

// if the checkbox is missing, default to checked
if (allowListing) {
permissions |= OC.PERMISSION_READ;
}
var permissions = this.$('input[name="publicPermissions"]:checked').val();

// if the checkbox is missing it is the equivalent of unchecked
if ($publicUploadCheckbox.is(':checked')) {
if (allowListing) {
permissions |= OC.PERMISSION_UPDATE | OC.PERMISSION_CREATE | OC.PERMISSION_DELETE;
} else {
// without listing only file creation is allowed, no overwrite nor delete
permissions |= OC.PERMISSION_CREATE;
}
} else {
// ignore listing perm, allow reading
permissions |= OC.PERMISSION_READ;
}

return permissions;
return (permissions) ? permissions : OC.PERMISSION_READ;
},

_save: function () {
Expand Down Expand Up @@ -248,18 +220,32 @@

this.$el.html(this.template({
cid: this.cid,
fileNameLabel : t('core', 'Filename'),
passwordLabel: t('core', 'Password'),
passwordPlaceholder: isPasswordSet ? PASSWORD_PLACEHOLDER_STARS : PASSWORD_PLACEHOLDER_MESSAGE,
isPasswordRequired: this.configModel.get('enforcePasswordForPublicLink'),
namePlaceholder: t('core', 'Name'),
name: this.model.get('name'),
isPasswordSet: isPasswordSet,
publicUploadPossible: this._isPublicUploadPossible(),
publicUploadChecked: this.model.canCreate(),
publicUploadLabel: t('core', 'Allow editing'),
showListingChecked: this.model.canRead(),
showListingLabel: t('core', 'Show file listing'),

fileNameLabel : t('core', 'Filename'),
passwordLabel : t('core', 'Password'),

publicUploadPossible : this._isPublicUploadPossible(),

publicUploadLabel : t('core', 'Upload only (File Drop)'),
publicUploadDescription : t('core', 'Receive files from others without revealing the contents of the folder.'),
publicUploadValue : OC.PERMISSION_CREATE,
publicUploadSelected : this.model.get('permissions') === OC.PERMISSION_CREATE,

publicReadLabel : t('core', 'Read only'),
publicReadDescription : t('core', 'Users can view and download contents.'),
publicReadValue : OC.PERMISSION_READ,
publicReadSelected : this.model.get('permissions') === OC.PERMISSION_READ,

publicReadWriteLabel : t('core', 'Read & Write'),
publicReadWriteDescription : t('core', 'Users can view, download, edit and upload contents.'),
publicReadWriteValue : OC.PERMISSION_READ | OC.PERMISSION_UPDATE | OC.PERMISSION_CREATE | OC.PERMISSION_DELETE,
publicReadWriteSelected : this.model.get('permissions') >= (OC.PERMISSION_READ | OC.PERMISSION_UPDATE | OC.PERMISSION_CREATE | OC.PERMISSION_DELETE),

isMailEnabled: showEmailField
}));

Expand All @@ -280,8 +266,6 @@
this.expirationView.render();
this.$('.expirationDateContainer').append(this.expirationView.$el);

this._updateCheckboxes();

this.delegateEvents();

return this;
Expand Down Expand Up @@ -368,7 +352,7 @@
});
}

});
});

OC.Share.ShareDialogLinkShareView = ShareDialogLinkShareView;

Expand Down
62 changes: 16 additions & 46 deletions core/js/tests/specs/sharedialoglinkshareviewSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,18 +135,18 @@ describe('OC.Share.ShareDialogLinkShareView', function() {
publicUploadConfigStub.returns(true);
view.render();
expect(view.$('[name=linkName]').val()).toEqual('first link');
expect(view.$('.publicUploadCheckbox').prop('checked')).toEqual(false);
expect(view.$('.publicPermissions').prop('checked')).toEqual(true);
expect(view.$('.linkPassText').val()).toEqual('');
expect(view.$('.expirationDate').val()).toEqual('');

model.set({
password: 'set',
expireDate: '2017-10-12',
permissions: OC.PERMISSION_ALL
permissions: OC.PERMISSION_CREATE
});
view.render();

expect(view.$('.publicUploadCheckbox').prop('checked')).toEqual(true);
expect(parseInt(view.$('.publicPermissions:checked').val())).toBe(OC.PERMISSION_CREATE);
expect(view.$('.linkPassText').val()).toEqual('');
expect(view.$('.expirationDate').val()).toEqual('12-10-2017');
});
Expand Down Expand Up @@ -208,37 +208,15 @@ describe('OC.Share.ShareDialogLinkShareView', function() {
permissions: OC.PERMISSION_READ | OC.PERMISSION_CREATE
});
view.render();
expect(view.$('.showListingCheckbox').length).toEqual(1);
expect(view.$('.showListingCheckbox').is(':checked')).toEqual(true);
expect(view.$('.showListingCheckbox').is(':disabled')).toEqual(false);
expect(view.$('.publicPermissions').length).toEqual(3);
});
it('renders listing checkbox disabled when public upload is disallowed by user', function() {
it('renders checkbox disabled when public upload is disallowed by user', function() {
publicUploadConfigStub.returns(true);
model.set({
permissions: OC.PERMISSION_READ
});
view.render();
expect(view.$('.showListingCheckbox').length).toEqual(1);
expect(view.$('.showListingCheckbox').is(':checked')).toEqual(true);
expect(view.$('.showListingCheckbox').is(':disabled')).toEqual(true);
});
it('disables listing checkbox when ticking public upload', function() {
publicUploadConfigStub.returns(true);
model.set({
permissions: OC.PERMISSION_CREATE
});
view.render();

expect(view.$('.showListingCheckbox').length).toEqual(1);
expect(view.$('.showListingCheckbox').is(':checked')).toEqual(false);
expect(view.$('.showListingCheckbox').is(':disabled')).toEqual(false);

expect(view.$('.publicUploadCheckbox').length).toEqual(1);
expect(view.$('.publicUploadCheckbox').is(':checked')).toEqual(true);
view.$('.publicUploadCheckbox').trigger(new $.Event('click'));
expect(view.$('.publicUploadCheckbox').is(':checked')).toEqual(false);
expect(view.$('.showListingCheckbox').is(':checked')).toEqual(true);
expect(view.$('.showListingCheckbox').is(':disabled')).toEqual(true);
expect(view.$('.showListingCheckbox').length).toEqual(0);
});
});
describe('password logic', function() {
Expand Down Expand Up @@ -293,7 +271,7 @@ describe('OC.Share.ShareDialogLinkShareView', function() {
name: 'first link',
expireDate: '',
password: 'newpassword',
permissions: OC.PERMISSION_READ,
permissions: OC.PERMISSION_READ.toString(),
shareType: OC.Share.SHARE_TYPE_LINK
});
});
Expand All @@ -306,7 +284,7 @@ describe('OC.Share.ShareDialogLinkShareView', function() {
expect(saveStub.getCall(0).args[0]).toEqual({
name: 'first link',
expireDate: '',
permissions: OC.PERMISSION_READ,
permissions: OC.PERMISSION_READ.toString(),
shareType: OC.Share.SHARE_TYPE_LINK
});
});
Expand Down Expand Up @@ -394,30 +372,22 @@ describe('OC.Share.ShareDialogLinkShareView', function() {
});

var dataProvider = [
// globally enabled
[true, true, true, OC.PERMISSION_READ | OC.PERMISSION_CREATE | OC.PERMISSION_UPDATE | OC.PERMISSION_DELETE],
[true, true, false, OC.PERMISSION_CREATE],
[true, false, true, OC.PERMISSION_READ],
[true, false, false, OC.PERMISSION_READ],

// globally disabled, permission stays regardless
[false, false, false, OC.PERMISSION_READ],
[false, true, false, OC.PERMISSION_READ],
[false, true, false, OC.PERMISSION_READ],
[false, true, true, OC.PERMISSION_READ],
[true, OC.PERMISSION_READ | OC.PERMISSION_CREATE | OC.PERMISSION_UPDATE | OC.PERMISSION_DELETE],
[true, OC.PERMISSION_CREATE],
[true, OC.PERMISSION_READ],
[false, OC.PERMISSION_READ], // globally disabled, permission stays regardless
];

function testPermissions(globalEnabled, uploadChecked, listingChecked, expectedPerms) {
function testPermissions(globalEnabled, expectedPerms) {
expectedPerms = expectedPerms.toString();
it('sets permissions to ' + expectedPerms +
' if global enabled is ' + globalEnabled +
' and public upload checkbox is ' + uploadChecked +
' and listing checkbox is ' + listingChecked, function() {
' and corresponding radiobutton is checked', function() {

publicUploadConfigStub.returns(globalEnabled);
view.render();

view.$('.publicUploadCheckbox').prop('checked', uploadChecked);
view.$('.showListingCheckbox').prop('checked', listingChecked);
view.$('input[name="publicPermissions"]:checked').val(expectedPerms);

view._save();

Expand Down