Skip to content

Commit

Permalink
Prevent users from renaming an object without write permission
Browse files Browse the repository at this point in the history
Also improve messaging when there is a sysMeta 401 error

Relates to #1418
  • Loading branch information
robyngit committed Dec 3, 2020
1 parent db938f3 commit adf3da7
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 35 deletions.
8 changes: 7 additions & 1 deletion src/css/metacatui-common.css
Original file line number Diff line number Diff line change
Expand Up @@ -6701,8 +6701,14 @@ ul.side-nav-items {
}

.data-package-item td.name div {
padding: 8px 0 8px 0;
padding: 8px;
width: fit-content;
cursor: not-allowed;
}
.data-package-item td.canRename div {
cursor: text;
}

.data-package-item td.error > [contenteditable]{
border: 2px solid red;
}
Expand Down
52 changes: 35 additions & 17 deletions src/js/models/DataONEObject.js
Original file line number Diff line number Diff line change
Expand Up @@ -765,43 +765,58 @@ define(['jquery', 'underscore', 'backbone', 'uuid', 'he', 'collections/AccessPol
},

/**
* Check if the current user is authorized to perform an action on this object
* @param {string} action - The action (read, write, or changePermission) to check
* if the current user has authorization to perform. This function doesn't return
* Check if the current user is authorized to perform an action on this object. This function doesn't return
* the result of the check, but it sends an XHR, updates this model, and triggers a change event.
* @param {string} [action=changePermission] - The action (read, write, or changePermission) to check
* if the current user has authorization to perform. By default checks for the highest level of permission.
* @param {object} [options] Additional options for this function. See the properties below.
* @property {function} options.onSuccess - A function to execute when the checkAuthority API is successfully completed
* @property {function} options.onError - A function to execute when the checkAuthority API returns an error
* @property {function} options.onError - A function to execute when the checkAuthority API returns an error, or when no PID or SID can be found for this object.
* @return {boolean}
*/
checkAuthority: function(action, options){

// return false - if neither PID nor SID is present to check the authority
if ( (this.get("id") == null) && (this.get("seriesId") == null) ) {
return false;
}
checkAuthority: function(action = "changePermission", options){

if( typeof options == "undefined" ){
var options = {};
}

// If PID is not present - check authority with seriesId
// If onError or onSuccess options were provided by the user,
// check that they are functions first, so we don't try to use
// some other type of variable as a function later on.
["onError", "onSuccess"].forEach(function(userFunction){
if(typeof options[userFunction] !== "function"){
options[userFunction] = null;
}
});

// The Object's PID
var identifier = this.get("id");
// The URL for the authentication service API
var authServiceUrl = MetacatUI.appModel.get('authServiceUrl');

// If PID is not present - check authority with seriesId
if ( (identifier == null) ) {
identifier = this.get("seriesId");
}

if(!action) var action = "changePermission";

var authServiceUrl = MetacatUI.appModel.get('authServiceUrl');
if(!authServiceUrl)
// If neither PID nor SID is present to check the authority,
// or if we don't have a URL for the API, return false.
if ( identifier == null || !authServiceUrl) {
if(options.onError){
options.onError();
}
return false;
}

var onSuccess = options.onSuccess || function(data, textStatus, xhr) {
var onSuccess = function(data, textStatus, xhr) {
model.set("isAuthorized_" + action, true);
model.set("isAuthorized", true);
model.trigger("change:isAuthorized");
if(options.onSuccess){
options.onSuccess(data, textStatus, xhr)
}
},
onError = options.onError || function(xhr, textStatus, errorThrown){
onError = function(xhr, textStatus, errorThrown){
if(errorThrown == 404){
model.set("notFound", true);
model.trigger("notFound");
Expand All @@ -810,6 +825,9 @@ define(['jquery', 'underscore', 'backbone', 'uuid', 'he', 'collections/AccessPol
model.set("isAuthorized_" + action, false);
model.set("isAuthorized", false);
}
if(options.onError){
options.onError(data, textStatus, xhr)
}
};

var model = this;
Expand Down
16 changes: 13 additions & 3 deletions src/js/templates/dataItem.html
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,19 @@

<!-- Files column -->

<td class="name" title="Click to rename" >
<% var fileClasses = "name" %>
<% var filenameEditable = "false" %>
<% var filenameTitle = "You are not authorized to rename this file" %>

<% if (canWrite) { %>
<% fileClasses = fileClasses + " canRename" %>
<% var filenameEditable = "true" %>
<% var filenameTitle = "Click to rename" %>
<% } %>

<td class="<%= fileClasses %>" >
<% if(uploadStatus != "l" && uploadStatus != "p"){ %>
<div contenteditable="true">
<div contenteditable="<%= filenameEditable %>" title="<%= filenameTitle %>">
<% } else { %>
<div>
<% } %>
Expand Down Expand Up @@ -112,7 +122,7 @@
<ul class="dropdown-menu">
<input class="file-replace" type="file" style="display:none; height: 0px; width: 0px" />

<% if (canReplace) { %>
<% if (canWrite) { %>
<li><a href="" class="replace replaceFile" title="Replace this file with another">Replace</a></li>
<% } else { %>
<li><a href="" class="replace replaceFile muted disabled">Replace</a></li>
Expand Down
61 changes: 47 additions & 14 deletions src/js/views/DataItemView.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ define(['underscore', 'jquery', 'backbone', 'models/DataONEObject',

/* Events this view listens to */
events: {
"focusout .name" : "updateName",
"click .name" : "emptyName",
"focusout .name.canRename" : "updateName",
"click .name.canRename" : "emptyName",
"click .duplicate" : "duplicate", // Edit dropdown, duplicate scimeta/rdf
"click .addFolder" : "handleAddFolder", // Edit dropdown, add nested scimeta/rdf
"click .addFiles" : "handleAddFiles", // Edit dropdown, open file picker dialog
Expand All @@ -54,14 +54,14 @@ define(['underscore', 'jquery', 'backbone', 'models/DataONEObject',

this.model = options.model || new DataONEObject();
this.id = this.model.get("id");
this.canReplace = false; // Default. Updated in render()
this.canWrite = false; // Default. Updated in render()
},

/* Render the template into the DOM */
render: function(model) {

//Prevent duplicate listeners
this.stopListening();
//Prevent duplicate listeners
this.stopListening();

// Set the data-id for identifying events to model ids
this.$el.attr("data-id", this.model.get("id"));
Expand All @@ -83,12 +83,13 @@ define(['underscore', 'jquery', 'backbone', 'models/DataONEObject',

// Restrict item replacement depending on access
//
// Note: .canReplace is set here (at render) instead of at init
// Note: .canWrite is set here (at render) instead of at init
// because render will get called a few times during page load
// as the app updates what it knows about the object
this.canReplace = this.model.get("accessPolicy") &&
this.canWrite = this.model.get("accessPolicy") &&
this.model.get("accessPolicy").isAuthorized("write");
attributes.canReplace = this.canReplace; // Copy to template

attributes.canWrite = this.canWrite; // Copy to template

//Get the number of attributes for this item
if(this.model.type != "EML"){
Expand Down Expand Up @@ -226,8 +227,28 @@ define(['underscore', 'jquery', 'backbone', 'models/DataONEObject',
});
}

if( !accessPolicy.isAuthorized("write") ){

this.$(".name > div").tooltip({
placement: "top",
container: this.el,
trigger: "hover",
delay: { show: 800 }
});
}
else{
this.$(".name > div").tooltip({
placement: "top",
container: this.el,
trigger: "hover",
delay: { show: 800 }
});
}

}



// Add tooltip to a disabled Replace link
$(this.$el).find(".replace.disabled").tooltip({
title: "You don't have sufficient privileges to replace this item.",
Expand All @@ -246,7 +267,16 @@ define(['underscore', 'jquery', 'backbone', 'models/DataONEObject',

// Use a friendlier message for 401 errors (the one returned is a little hard to understand)
if(this.model.get("sysMetaErrorCode") == 401){
errorMessage = "You do not have permission to update the system metadata for this file."
var accessPolicy = this.model.get("accessPolicy");

// If the user at least has edit permission, assume the error was related to changing the access rules
if(accessPolicy && accessPolicy.isAuthorized("edit")){
errorMessage = "You don't have permission to change access rules, so we updated the object but did not change the access rules."
// Otherwise, assume they only have read access
} else {
errorMessage = "We updated the object but could not update the access rules, or re-name or replace this file, because you're not authorized to make these changes."
}

}

// When there's an error or a warninig
Expand Down Expand Up @@ -572,7 +602,7 @@ define(['underscore', 'jquery', 'backbone', 'models/DataONEObject',
event.stopPropagation();

// Stop immediately if we know the user doesn't have privs
if (!this.canReplace) {
if (!this.canWrite) {
event.preventDefault();
return;
}
Expand Down Expand Up @@ -613,7 +643,7 @@ define(['underscore', 'jquery', 'backbone', 'models/DataONEObject',
event.stopPropagation();
event.preventDefault();

if (!this.canReplace) {
if (!this.canWrite) {
return;
}

Expand Down Expand Up @@ -953,14 +983,17 @@ define(['underscore', 'jquery', 'backbone', 'models/DataONEObject',
previewRemove: function(){
this.$el.toggleClass("remove-preview");
},

/**
* Clears the text in the cell if the text was the default. We add
* an 'empty' class, and remove it when the user focuses back out.
*
*/
emptyName: function(e){

var editableCell = this.$(".name [contenteditable]");
var editableCell = this.$(".canRename [contenteditable]");

editableCell.tooltip('hide');

if(editableCell.text().indexOf("Untitled") > -1){
editableCell.attr("data-original-text", editableCell.text().trim())
Expand Down Expand Up @@ -1047,15 +1080,15 @@ define(['underscore', 'jquery', 'backbone', 'models/DataONEObject',
if(this.model.get("type") != "Metadata")
this.$(".controls").prepend($(document.createElement("div")).addClass("disable-layer"));

this.$(".name > div").prop("contenteditable", false);
this.$(".canRename > div").prop("contenteditable", false);
},

hideSaving: function(){
this.$("button").prop("disabled", false);
this.$(".disable-layer").remove();

//Make the name cell editable again
this.$(".name > div").prop("contenteditable", true);
this.$(".canRename > div").prop("contenteditable", true);

this.$el.removeClass("error-saving");
},
Expand Down

0 comments on commit adf3da7

Please sign in to comment.