Product management (2nd part), permission system #857
Conversation
| this.set('rightsDiff', rightsDiff); | ||
|
|
||
| // Sort and unique the name list. | ||
| var nameSet = new Set(nameList.users.sort()); |
There was a problem hiding this comment.
Use util.arrayUnique once #847 is merged.
6992e97 to
fc8f179
Compare
fc8f179 to
3da2c62
Compare
1017ba0 to
6a00524
Compare
6a00524 to
b882512
Compare
7aa083a to
d4a2fc8
Compare
| AUTH_OBJECTS.Permission.PRODUCT_STORE, util.createPermissionParams({ | ||
| productID : CURRENT_PRODUCT.id | ||
| })); | ||
| console.log("POSTCREATE", showDelete); |
| //--- Back button to product list ---// | ||
|
|
||
| var productListButton = new Button({ | ||
| class : 'mainMenuButton', |
There was a problem hiding this comment.
Use dash-case class names: main-menu-button
| * given other colour with the given ratio. | ||
| * | ||
| * @param blendColour a variable applicable to the constructor of | ||
| * dojo.Color. It can be a color name, a hex string, on an array of RGB. |
There was a problem hiding this comment.
typo: or an array of RGB
| var adminLevel = layout.get('adminLevel'); | ||
|
|
||
| if (adminLevel) | ||
| adminLevel = 0; |
There was a problem hiding this comment.
adminLevel = adminLevel
? 0 : isAdminOfAnyProduct
? 1 : isSuperuser
? 2 : 0;There was a problem hiding this comment.
(Be careful with this notation if you are talking to a PHP developer, as the tertiary conditional operator is working a lot differently in that context.)
|
|
||
| layout.set('adminLevel', adminLevel); | ||
| listOfProducts.setAdmin(adminLevel); | ||
| this.set('label', |
| } | ||
| }); | ||
|
|
||
| optionsToAdd.sort(function (a, b) { |
There was a problem hiding this comment.
The compare function has the following form:
function compare(a, b) {
if (a is less than b by some ordering criterion) {
return -1;
}
if (a is greater than b by the ordering criterion) {
return 1;
}
// a must be equal to b
return 0;
}For more information see: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort
| if (mode === 'add') { | ||
| this.set('title', "Add new product"); | ||
| this._metadataPane.set('settingsMode', 'add'); | ||
| this._permissionPane.set('disabled', true); |
There was a problem hiding this comment.
It would be better to hide the Permissions tab instead of disabling it. It's annoying that I cannot go to that tab when I create a new product.
www/style/productlist.css
Outdated
| content: "\e012"; | ||
| } | ||
|
|
||
| .customIcon.product-delete { |
There was a problem hiding this comment.
Change the color and the cursor on hover of this item:
.customIcon.product-delete:hover {
color: #ff0000;
cursor: pointer;
}|
|
||
| var isAdminOfAnyProduct = PROD_SERVICE.isAdministratorOfAnyProduct(); | ||
|
|
||
| var menuButton = new Button({ |
There was a problem hiding this comment.
If I'm allow to edit the product list, why don't we show the administration settings (edit, remove icons, create new product button, etc.) by default?
There was a problem hiding this comment.
It's bit of an extra security measure to prevent an accidental permission override or product deletion. A product deletion is a dangerous action as it irrecoverably destroys the access control list of a product. It does not destroy analysis results, but after adding the product back, the permission list will be reset to defaults.
Think of it as sudo on Linux or the UAC introduced with Windows Vista. Even though you are an admin, it's better to put this extra barrier before dangerous actions.
| @@ -72,3 +84,110 @@ span.product-avatar { | |||
| .customIcon.product-error:before { | |||
| content: "\e015"; | |||
| } | |||
|
|
|||
| .customIcon.product-edit { | |||
There was a problem hiding this comment.
Change the color and the cursor on hover of this item:
.customIcon.product-edit:hover {
color: #373c44;
cursor: pointer;
}d4a2fc8 to
0965c45
Compare
0965c45 to
ba69fb2
Compare
api/authentication.thrift
Outdated
| * The following permission scopes exist. | ||
| * | ||
| * SYSTEM: These permissions are global to the running CodeChecker server. | ||
| * In this case, the 'extra_params' field is empty. |
There was a problem hiding this comment.
It can be a bit misleading if we refer to extra_params field here which is not used in the api.
Should this be extraParams?
api/authentication.thrift
Outdated
|
|
||
| // ---------------------------------------------------------------- | ||
| // Refer to the documentation in api/shared.thrift on what data the | ||
| // 'extra_params' field for a particular permission requires. |
docs/permissions.md
Outdated
|
|
||
| Some permissions are *default not granted*, which means that only users whom | ||
| are explicitly given the permission have it. This also means that if the | ||
| server is running with authentication disabled, noone has the permission |
| session.close() | ||
|
|
||
| @timeit | ||
| def getAuthorisedNames(self, permission, extra_params): |
There was a problem hiding this comment.
A short documentation at the api implementations would be nice too.
| @@ -427,7 +473,7 @@ def __get_run_ids_to_query(self, session, cmp_data=None): | |||
|
|
|||
| @timeit | |||
| def getRunData(self, run_name_filter): | |||
|
|
|||
| self.__require_access() | |||
There was a problem hiding this comment.
Permissions are read from the config database for each api call?
There was a problem hiding this comment.
Yes. This is a security measure. It is not a good option security-wise to ever cache any sort of "permission" descriptor.
|
|
||
| added = self._add_perm_impl(auth_name, is_group) | ||
|
|
||
| if added: |
There was a problem hiding this comment.
Shouldn't we print something if adding fails?
|
|
||
| removed = self._rem_perm_impl(auth_name, is_group) | ||
|
|
||
| if removed: |
There was a problem hiding this comment.
Shouldn't we print something if the remove fails?
|
|
||
| session = None |
There was a problem hiding this comment.
Why do we set the session here to None? We will not use it until the line 440.
| logging.debug('Comment cannot be removed by another user') | ||
| with self.assertRaises(RequestFailed): | ||
| self._cc_client_john.removeComment(comments[1].id) | ||
| logging.debug('Comment was be removed by another user') |
ba69fb2 to
5453762
Compare
5c1b87a to
7bef448
Compare
- Add management of the "root" user automatically created at server start - Add permission management code, API bindings, handling - Add permission settings view to the Web GUI for global and product editing [ci skip]
- This commit also introduces the sanity check for product endpoints at the addition/editing of new products. Documentation has been updated for the permission system.
7bef448 to
0816a36
Compare
Product management changes
[A-Za-z][A-Za-z0-9_]*. It's the safest way.Permission system
The permission system has been introduced. Four permissions are defined as of now:
SUPERUSERPRODUCT_ADMINPRODUCT_ACCESS: required to view reports, add comments, filter, etc.PRODUCT_STORE: required to check in and delete runsUsers can now be assigned to groups in dictionary auth, so group authentication can be tested. LDAP groups will come in the follow-up #807.