Author: orudyy
Date: Tue Jul 19 13:45:19 2016
New Revision: 1753388
URL: http://svn.apache.org/viewvc?rev=1753388&view=rev
Log:
QPID-7247: Prevent modification of other user preference
Modified:
qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/model/preferences/UserPreferencesImpl.java
qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/singleton/PreferencesTest.java
qpid/java/trunk/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java
qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/js/qpid/management/query/QueryWidget.js
qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/query/QueryWidget.html
qpid/java/trunk/systests/src/test/java/org/apache/qpid/systest/rest/UserPreferencesRestTest.java
Modified:
qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/model/preferences/UserPreferencesImpl.java
URL:
http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/model/preferences/UserPreferencesImpl.java?rev=1753388&r1=1753387&r2=1753388&view=diff
==============================================================================
---
qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/model/preferences/UserPreferencesImpl.java
(original)
+++
qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/model/preferences/UserPreferencesImpl.java
Tue Jul 19 13:45:19 2016
@@ -442,6 +442,18 @@ public class UserPreferencesImpl impleme
throw new SecurityException(String.format("Preference '%s' not
owned by current user.",
preference.getId().toString()));
}
+
+ if (preference.getId() != null)
+ {
+ Preference oldPreference =
_preferences.get(preference.getId());
+ if (oldPreference != null &&
!principalsEqual(oldPreference.getOwner(), preference.getOwner()))
+ {
+ throw new SecurityException(String.format(
+ "Ownership of other user preference having id '%s'
and name '%s' cannot be changed to current user",
+ preference.getId().toString(),
+ preference.getName()));
+ }
+ }
}
}
Modified:
qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/singleton/PreferencesTest.java
URL:
http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/singleton/PreferencesTest.java?rev=1753388&r1=1753387&r2=1753388&view=diff
==============================================================================
---
qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/singleton/PreferencesTest.java
(original)
+++
qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/singleton/PreferencesTest.java
Tue Jul 19 13:45:19 2016
@@ -1255,6 +1255,37 @@ public class PreferencesTest extends Qpi
assertEquals("Unexpected preference attributes", expectedAttributes,
p.getAttributes());
}
+ public void testSavingOtherUserPreference() throws Exception
+ {
+ final String testGroupName = "testGroup";
+ Subject user1Subject =
TestPrincipalUtils.createTestSubject(TEST_USERNAME, testGroupName);
+
+ Map<String, Object> preferenceAttributes =
PreferenceTestHelper.createPreferenceAttributes(
+ _testObject.getId(),
+ UUID.randomUUID(),
+ "X-PREF",
+ "prefname",
+ null,
+ TEST_USERNAME,
+ Collections.singleton(testGroupName),
+ Collections.<String,Object>emptyMap());
+ updateOrAppendAs(user1Subject, PreferenceFactory.recover(_testObject,
preferenceAttributes));
+
+ Subject user2Subject =
TestPrincipalUtils.createTestSubject(TEST_USERNAME2, testGroupName);
+ preferenceAttributes.put(Preference.OWNER_ATTRIBUTE, TEST_USERNAME2);
+ Preference stolenPreference = PreferenceFactory.recover(_testObject,
preferenceAttributes);
+
+ try
+ {
+ updateOrAppendAs(user2Subject, stolenPreference);
+ fail("Steeling of other user preferences should not be allowed");
+ }
+ catch (SecurityException e)
+ {
+ // pass
+ }
+ }
+
private void updateOrAppendAs(final Subject testSubject, final
Preference... testUserPreference)
{
Subject.doAs(testSubject, new PrivilegedAction<Void>()
Modified:
qpid/java/trunk/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java
URL:
http://svn.apache.org/viewvc/qpid/java/trunk/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java?rev=1753388&r1=1753387&r2=1753388&view=diff
==============================================================================
---
qpid/java/trunk/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java
(original)
+++
qpid/java/trunk/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java
Tue Jul 19 13:45:19 2016
@@ -560,7 +560,7 @@ public class RestServlet extends Abstrac
{
super.service(request, response);
}
- catch (IllegalArgumentException | IllegalConfigurationException |
IllegalStateException | AccessControlException
+ catch (IllegalArgumentException | IllegalConfigurationException |
IllegalStateException | SecurityException
| IntegrityViolationException |
IllegalStateTransitionException | NoClassDefFoundError e)
{
setResponseStatus(request, response, e);
@@ -1101,9 +1101,9 @@ public class RestServlet extends Abstrac
private void setResponseStatus(HttpServletRequest request,
HttpServletResponse response, Throwable e)
throws IOException
{
- if (e instanceof AccessControlException)
+ if (e instanceof SecurityException)
{
- LOGGER.debug("AccessControlException, sending {}",
HttpServletResponse.SC_FORBIDDEN, e);
+ LOGGER.debug("{}, sending {}", e.getClass().getName(),
HttpServletResponse.SC_FORBIDDEN, e);
response.setStatus(HttpServletResponse.SC_FORBIDDEN);
}
else
Modified:
qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/js/qpid/management/query/QueryWidget.js
URL:
http://svn.apache.org/viewvc/qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/js/qpid/management/query/QueryWidget.js?rev=1753388&r1=1753387&r2=1753388&view=diff
==============================================================================
---
qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/js/qpid/management/query/QueryWidget.js
(original)
+++
qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/js/qpid/management/query/QueryWidget.js
Tue Jul 19 13:45:19 2016
@@ -293,6 +293,9 @@ define(["dojo/_base/declare",
saveButton: null,
cloneButton: null,
deleteButton: null,
+ saveButtonTooltip: null,
+ cloneButtonTooltip: null,
+ deleteButtonTooltip: null,
/**
* constructor parameter
@@ -310,6 +313,7 @@ define(["dojo/_base/declare",
_lastStandardModeSelect: null,
_lastHeaders: null,
_queryCloneDialogForm: null,
+ _ownQuery: false,
postCreate: function ()
{
@@ -351,6 +355,20 @@ define(["dojo/_base/declare",
this.cloneButton.on("click", lang.hitch(this,
this._cloneQuery));
this.deleteButton.on("click", lang.hitch(this,
this._deleteQuery));
+ this._ownQuery = !this.preference ||
!this.preference.owner || this.preference.owner ===
this.management.getAuthenticatedUser();
+ this.saveButton.set("disabled", !this._ownQuery);
+ this.deleteButton.set("disabled", !this._ownQuery ||
!(this.preference && this.preference.id));
+
+ if (!this._ownQuery)
+ {
+ this.saveButtonTooltip.set("label", "Shared query
owned by someone else cannot be saved!"
+ + "<br/>"
+ + "Please clone
query to make your own one.");
+ this.deleteButtonTooltip.set("label", "Shared query
owned by someone else cannot be deleted!"
+ + "<br/>"
+ + "Please clone
query to make your own one.");
+ }
+
// advanced mode widgets
this.advancedSelect.on("change", lang.hitch(this,
this._advancedModeSelectChanged));
this.advancedWhere.on("change", lang.hitch(this,
this._advancedModeWhereChanged));
@@ -394,9 +412,6 @@ define(["dojo/_base/declare",
{
this._toggleSearchButton(true);
}
-
- // if the preference has an id, then we know it is in the
store
- this.deleteButton.set("disabled", this.preference != null
&& this.preference.id != null ? false : true);
},
search: function ()
{
@@ -1045,6 +1060,10 @@ define(["dojo/_base/declare",
{
delete preference.id;
}
+ if (preference.owner)
+ {
+ delete preference.owner;
+ }
preference.value = this._getQuery();
this._queryCloneDialog.hide();
this.emit("clone", {preference: preference, parentObject:
e.parentObject});
@@ -1085,10 +1104,13 @@ define(["dojo/_base/declare",
},
_queryChanged: function(query)
{
- var queryParameters = this._getQuery(query);
- var pref = lang.clone(this.preference);
- pref.value = queryParameters;
- this.emit("change", {preference: pref});
+ if (this._ownQuery)
+ {
+ var queryParameters = this._getQuery(query);
+ var pref = lang.clone(this.preference);
+ pref.value = queryParameters;
+ this.emit("change", {preference: pref});
+ }
}
});
Modified:
qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/query/QueryWidget.html
URL:
http://svn.apache.org/viewvc/qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/query/QueryWidget.html?rev=1753388&r1=1753387&r2=1753388&view=diff
==============================================================================
---
qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/query/QueryWidget.html
(original)
+++
qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/query/QueryWidget.html
Tue Jul 19 13:45:19 2016
@@ -19,15 +19,27 @@
<div>
<div data-dojo-type="dijit/Toolbar">
- <div data-dojo-type="dijit/form/Button"
+ <div id="saveButton_${id}"
+ data-dojo-type="dijit/form/Button"
data-dojo-attach-point="saveButton"
data-dojo-props="iconClass: 'dijitIconSave'">Save</div>
- <div data-dojo-type="dijit/form/Button"
+ <div data-dojo-attach-point="saveButtonTooltip"
+ data-dojo-type="dijit/Tooltip"
+
data-dojo-props="connectId:'saveButton_${id}',position:['below']">Save and
share query</div>
+ <div id="cloneButton_${id}"
+ data-dojo-type="dijit/form/Button"
data-dojo-attach-point="cloneButton"
data-dojo-props="iconClass: 'dijitIconCopy'">Clone</div>
- <div data-dojo-type="dijit/form/Button"
+ <div data-dojo-attach-point="cloneButtonTooltip"
+ data-dojo-type="dijit/Tooltip"
+
data-dojo-props="connectId:'cloneButton_${id}',position:['below']">Clone query
into new one<br/>Note, group information is not cloned into new query.</div>
+ <div id="deleteButton_${id}"
+ data-dojo-type="dijit/form/Button"
data-dojo-attach-point="deleteButton"
data-dojo-props="iconClass: 'trashIcon ui-icon'">Delete</div>
+ <div data-dojo-attach-point="deleteButtonTooltip"
+ data-dojo-type="dijit/Tooltip"
+
data-dojo-props="connectId:'deleteButton_${id}',position:['below']">Delete
query from preferences and close the tab</div>
<div data-dojo-type="dijit/form/Button"
data-dojo-attach-point="modeButton"
data-dojo-props="iconClass: 'advancedViewIcon ui-icon',
title:'Switch to \'Advanced View\' search using SQL-like expressions'"
Modified:
qpid/java/trunk/systests/src/test/java/org/apache/qpid/systest/rest/UserPreferencesRestTest.java
URL:
http://svn.apache.org/viewvc/qpid/java/trunk/systests/src/test/java/org/apache/qpid/systest/rest/UserPreferencesRestTest.java?rev=1753388&r1=1753387&r2=1753388&view=diff
==============================================================================
---
qpid/java/trunk/systests/src/test/java/org/apache/qpid/systest/rest/UserPreferencesRestTest.java
(original)
+++
qpid/java/trunk/systests/src/test/java/org/apache/qpid/systest/rest/UserPreferencesRestTest.java
Tue Jul 19 13:45:19 2016
@@ -25,6 +25,7 @@ import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.UUID;
import javax.servlet.http.HttpServletResponse;
@@ -33,7 +34,9 @@ import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import
org.apache.qpid.server.management.plugin.preferences.QueryPreferenceValue;
+import org.apache.qpid.server.model.GroupProvider;
import org.apache.qpid.server.model.preferences.Preference;
+import org.apache.qpid.server.security.group.GroupProviderImpl;
public class UserPreferencesRestTest extends QpidRestTestCase
{
@@ -78,6 +81,33 @@ public class UserPreferencesRestTest ext
assertEquals("Unexpected preference returned from all url",
prefDetails, prefs.get(0));
}
+ public void testSavingOtherUserPreference() throws Exception
+ {
+ getRestTestHelper().submitRequest("groupprovider/temp", "PUT",
Collections.singletonMap(GroupProvider.TYPE, GroupProviderImpl.CONFIG_TYPE),
HttpServletResponse.SC_CREATED);
+ getRestTestHelper().createGroup("test", "temp");
+ getRestTestHelper().createNewGroupMember("temp", "test", "webadmin");
+ getRestTestHelper().createNewGroupMember("temp", "test", "admin");
+
+ String preferenceType = "X-Type-" + getTestName();
+ String preferenceName = getTestName();
+ Map<String, Object> preferenceAttributes = new HashMap<>();
+ preferenceAttributes.put(Preference.ID_ATTRIBUTE, UUID.randomUUID());
+ preferenceAttributes.put(Preference.TYPE_ATTRIBUTE, preferenceType);
+ preferenceAttributes.put(Preference.NAME_ATTRIBUTE, preferenceName);
+ preferenceAttributes.put(Preference.DESCRIPTION_ATTRIBUTE, "");
+ preferenceAttributes.put(Preference.OWNER_ATTRIBUTE, "webadmin");
+ preferenceAttributes.put(Preference.VISIBILITY_LIST_ATTRIBUTE,
Collections.singletonList("test"));
+ preferenceAttributes.put(Preference.VALUE_ATTRIBUTE,
Collections.emptyMap());
+
+ String fullUrl = String.format("broker/userpreferences/%s/%s",
preferenceType, preferenceName);
+ getRestTestHelper().submitRequest(fullUrl, "PUT",
preferenceAttributes, HttpServletResponse.SC_OK);
+
+ getRestTestHelper().setUsernameAndPassword("admin", "admin");
+
+ preferenceAttributes.put(Preference.OWNER_ATTRIBUTE, "admin");
+ getRestTestHelper().submitRequest(fullUrl, "PUT",
preferenceAttributes, HttpServletResponse.SC_FORBIDDEN);
+ }
+
public void testPutQueryPreferenceRoundTrip() throws Exception
{
final String prefName = "myquery";
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
