You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by mike-jumper <gi...@git.apache.org> on 2017/05/22 03:18:06 UTC
[GitHub] incubator-guacamole-client pull request #158: GUACAMOLE-292: Restrict editin...
GitHub user mike-jumper opened a pull request:
https://github.com/apache/incubator-guacamole-client/pull/158
GUACAMOLE-292: Restrict editing of attributes based on permissions.
Previously, Guacamole users within the database authentication backend could edit any of their own user attributes so long as they had `UPDATE` permission on their own user object. This change modifies this behavior, restricting editing of attributes related to scheduling to users with `ADMINISTER` permission on the user object, and applies that same logic across all object management pages.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mike-jumper/incubator-guacamole-client restrict-attributes
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-guacamole-client/pull/158.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #158
----
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-client pull request #158: GUACAMOLE-292: Restrict editin...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/158#discussion_r118811901
--- Diff: guacamole/src/main/webapp/app/form/directives/form.js ---
@@ -163,6 +181,59 @@ angular.module('form').directive('guacForm', [function form() {
});
+ /**
+ * Returns whether the given field should be displayed to the
+ * current user.
+ *
+ * @param {Field} field
+ * The field to check.
+ *
+ * @returns {Boolean}
+ * true if the given field should be visible, false otherwise.
+ */
+ $scope.isVisible = function isVisible(field) {
+
+ // Forms with valuesOnly set should display only fields with
+ // associated values in the model object
+ if ($scope.valuesOnly)
--- End diff --
There was in the early days of the user profile development, but I think that all got reverted. I'll double-check.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-client pull request #158: GUACAMOLE-292: Restrict editin...
Posted by jmuehlner <gi...@git.apache.org>.
Github user jmuehlner commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/158#discussion_r118812447
--- Diff: extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/base/ModeledDirectoryObjectService.java ---
@@ -97,9 +97,12 @@
*
* @return
* An object which is backed by the given model object.
+ *
+ * @throws GuacamoleException
+ * If the object instance cannot be created.
*/
protected abstract InternalType getObjectInstance(ModeledAuthenticatedUser currentUser,
- ModelType model);
+ ModelType model) throws GuacamoleException;
--- End diff --
Ok
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-client pull request #158: GUACAMOLE-292: Restrict editin...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/158#discussion_r118812559
--- Diff: guacamole/src/main/webapp/app/manage/controllers/manageUserController.js ---
@@ -261,6 +261,31 @@ angular.module('manage').controller('manageUserController', ['$scope', '$injecto
};
/**
+ * Returns whether the current user can change/set all user attributes for
+ * the user being edited within the given data source, regardless of
+ * whether those attributes are already explicitly associated with that
+ * user.
+ *
+ * @param {String} [dataSource]
--- End diff --
Fair enough, and true of all the past similar `can*()` functions, too. I was going to argue that this is the pattern used by similar things here, but that's silly. It's wrong and we should stop wronging it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-client pull request #158: GUACAMOLE-292: Restrict editin...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/158#discussion_r118812228
--- Diff: guacamole/src/main/webapp/app/manage/controllers/manageUserController.js ---
@@ -261,6 +261,31 @@ angular.module('manage').controller('manageUserController', ['$scope', '$injecto
};
/**
+ * Returns whether the current user can change/set all user attributes for
+ * the user being edited within the given data source, regardless of
+ * whether those attributes are already explicitly associated with that
+ * user.
+ *
+ * @param {String} [dataSource]
--- End diff --
Wait ... it *is* used here. It's not used in the other controllers though. Did you mean those?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-client pull request #158: GUACAMOLE-292: Restrict editin...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/158#discussion_r118812173
--- Diff: guacamole/src/main/webapp/app/form/directives/form.js ---
@@ -163,6 +181,59 @@ angular.module('form').directive('guacForm', [function form() {
});
+ /**
+ * Returns whether the given field should be displayed to the
+ * current user.
+ *
+ * @param {Field} field
+ * The field to check.
+ *
+ * @returns {Boolean}
+ * true if the given field should be visible, false otherwise.
+ */
+ $scope.isVisible = function isVisible(field) {
+
+ // Forms with valuesOnly set should display only fields with
+ // associated values in the model object
+ if ($scope.valuesOnly)
--- End diff --
Yep. It's definitely not needed by these changes, and the changes which *did* need it were ultimately reverted.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-client pull request #158: GUACAMOLE-292: Restrict editin...
Posted by jmuehlner <gi...@git.apache.org>.
Github user jmuehlner commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/158#discussion_r118812426
--- Diff: guacamole/src/main/webapp/app/manage/controllers/manageUserController.js ---
@@ -261,6 +261,31 @@ angular.module('manage').controller('manageUserController', ['$scope', '$injecto
};
/**
+ * Returns whether the current user can change/set all user attributes for
+ * the user being edited within the given data source, regardless of
+ * whether those attributes are already explicitly associated with that
+ * user.
+ *
+ * @param {String} [dataSource]
--- End diff --
I mean that it's never passed in...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-client pull request #158: GUACAMOLE-292: Restrict editin...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-guacamole-client/pull/158
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-client pull request #158: GUACAMOLE-292: Restrict editin...
Posted by jmuehlner <gi...@git.apache.org>.
Github user jmuehlner commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/158#discussion_r118810040
--- Diff: guacamole/src/main/webapp/app/form/directives/form.js ---
@@ -163,6 +181,59 @@ angular.module('form').directive('guacForm', [function form() {
});
+ /**
+ * Returns whether the given field should be displayed to the
+ * current user.
+ *
+ * @param {Field} field
+ * The field to check.
+ *
+ * @returns {Boolean}
+ * true if the given field should be visible, false otherwise.
+ */
+ $scope.isVisible = function isVisible(field) {
+
+ // Forms with valuesOnly set should display only fields with
+ // associated values in the model object
+ if ($scope.valuesOnly)
--- End diff --
I don't see "valuesOnly" ever being used - is there a plan to use it later?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-client pull request #158: GUACAMOLE-292: Restrict editin...
Posted by jmuehlner <gi...@git.apache.org>.
Github user jmuehlner commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/158#discussion_r118809943
--- Diff: extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/base/ModeledDirectoryObjectService.java ---
@@ -97,9 +97,12 @@
*
* @return
* An object which is backed by the given model object.
+ *
+ * @throws GuacamoleException
+ * If the object instance cannot be created.
*/
protected abstract InternalType getObjectInstance(ModeledAuthenticatedUser currentUser,
- ModelType model);
+ ModelType model) throws GuacamoleException;
--- End diff --
Why are these throwing exceptions now?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-client pull request #158: GUACAMOLE-292: Restrict editin...
Posted by jmuehlner <gi...@git.apache.org>.
Github user jmuehlner commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/158#discussion_r118809952
--- Diff: guacamole/src/main/webapp/app/manage/controllers/manageUserController.js ---
@@ -261,6 +261,31 @@ angular.module('manage').controller('manageUserController', ['$scope', '$injecto
};
/**
+ * Returns whether the current user can change/set all user attributes for
+ * the user being edited within the given data source, regardless of
+ * whether those attributes are already explicitly associated with that
+ * user.
+ *
+ * @param {String} [dataSource]
--- End diff --
What's the reason for this dataSource param? I don't see it used anywhere.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-client pull request #158: GUACAMOLE-292: Restrict editin...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/158#discussion_r118811859
--- Diff: extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/base/ModeledDirectoryObjectService.java ---
@@ -97,9 +97,12 @@
*
* @return
* An object which is backed by the given model object.
+ *
+ * @throws GuacamoleException
+ * If the object instance cannot be created.
*/
protected abstract InternalType getObjectInstance(ModeledAuthenticatedUser currentUser,
- ModelType model);
+ ModelType model) throws GuacamoleException;
--- End diff --
Ah - because `getObjectInstance()` now needs to call `hasObjectPermission()`, at least in the user case. See:
https://github.com/mike-jumper/incubator-guacamole-client/blob/232935fe2cd81077c81bed6079707e3a41ceb9fb/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserService.java#L161-L162
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-client pull request #158: GUACAMOLE-292: Restrict editin...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/158#discussion_r118811890
--- Diff: guacamole/src/main/webapp/app/manage/controllers/manageUserController.js ---
@@ -261,6 +261,31 @@ angular.module('manage').controller('manageUserController', ['$scope', '$injecto
};
/**
+ * Returns whether the current user can change/set all user attributes for
+ * the user being edited within the given data source, regardless of
+ * whether those attributes are already explicitly associated with that
+ * user.
+ *
+ * @param {String} [dataSource]
--- End diff --
I'll remove that/those.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-client pull request #158: GUACAMOLE-292: Restrict editin...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/158#discussion_r118812695
--- Diff: guacamole/src/main/webapp/app/manage/controllers/manageUserController.js ---
@@ -261,6 +261,31 @@ angular.module('manage').controller('manageUserController', ['$scope', '$injecto
};
/**
+ * Returns whether the current user can change/set all user attributes for
+ * the user being edited within the given data source, regardless of
+ * whether those attributes are already explicitly associated with that
+ * user.
+ *
+ * @param {String} [dataSource]
--- End diff --
OK - rebased and removed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-client pull request #158: GUACAMOLE-292: Restrict editin...
Posted by jmuehlner <gi...@git.apache.org>.
Github user jmuehlner commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/158#discussion_r118810011
--- Diff: guacamole/src/main/webapp/app/manage/controllers/manageUserController.js ---
@@ -225,8 +225,8 @@ angular.module('manage').controller('manageUserController', ['$scope', '$injecto
};
/**
- * Returns whether the current user can change attributes associated with
- * the user being edited within the given data source.
+ * Returns whether the current user can change attributes explicitle
--- End diff --
Typo here - "explicitle"
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---