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.

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

Fix permission handling for editing/deleting roles #4270

Merged
merged 1 commit into from Oct 19, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -81,10 +81,10 @@ public RolesResource(RoleService roleService) {

@GET
@RequiresPermissions(RestPermissions.ROLES_READ)
@ApiOperation(value = "List all roles", notes = "")
@ApiOperation("List all roles")
public RolesResponse listAll() throws NotFoundException {
final Set<Role> roles = roleService.loadAll();
Set<RoleResponse> roleResponses = Sets.newHashSet();
Set<RoleResponse> roleResponses = Sets.newHashSetWithExpectedSize(roles.size());
for (Role role : roles) {
roleResponses.add(RoleResponse.create(role.getName(), Optional.ofNullable(role.getDescription()), role.getPermissions(), role.isReadOnly()));
}
Expand All @@ -104,7 +104,7 @@ public RoleResponse read(@ApiParam(name = "rolename", required = true) @PathPara

@POST
@RequiresPermissions(RestPermissions.ROLES_CREATE)
@ApiOperation(value = "Create a new role", notes = "")
@ApiOperation("Create a new role")
@AuditEvent(type = AuditEventTypes.ROLE_CREATE)
public Response create(@ApiParam(name = "JSON body", value = "The new role to create", required = true) @Valid @NotNull RoleResponse roleResponse) {
if (roleService.exists(roleResponse.name())) {
Expand All @@ -114,23 +114,23 @@ public Response create(@ApiParam(name = "JSON body", value = "The new role to cr
Role role = new RoleImpl();
role.setName(roleResponse.name());
role.setPermissions(roleResponse.permissions());
role.setDescription(roleResponse.description().orElse(null));
roleResponse.description().ifPresent(role::setDescription);

try {
role = roleService.save(role);
} catch (ValidationException e) {
log.error("Invalid role creation request.");
throw new BadRequestException(e);
log.error("Invalid role creation request", e);
throw new BadRequestException("Couldn't create role \"" + roleResponse.name() + "\"", e);
}

final URI uri = getUriBuilderToSelf().path(RolesResource.class)
.path("{rolename}")
.build(role.getName());

return Response.created(uri).entity(RoleResponse.create(role.getName(),
Optional.ofNullable(role.getDescription()),
role.getPermissions(),
role.isReadOnly())).build();
Optional.ofNullable(role.getDescription()),
role.getPermissions(),
role.isReadOnly())).build();
}

@PUT
Expand All @@ -139,27 +139,31 @@ public Response create(@ApiParam(name = "JSON body", value = "The new role to cr
@AuditEvent(type = AuditEventTypes.ROLE_UPDATE)
public RoleResponse update(
@ApiParam(name = "rolename", required = true) @PathParam("rolename") String name,
@ApiParam(name = "JSON Body", value = "The new representation of the role", required = true) RoleResponse role) throws NotFoundException {
final Role roleToUpdate = roleService.load(name);
@ApiParam(name = "JSON Body", value = "The new representation of the role", required = true) RoleResponse roleRepresentation) throws NotFoundException {
checkPermission(RestPermissions.ROLES_EDIT, name);

if (roleToUpdate.isReadOnly()) {
final Role role = roleService.load(name);
if (role.isReadOnly()) {
throw new BadRequestException("Cannot update read only role " + name);
}
roleToUpdate.setName(role.name());
roleToUpdate.setDescription(role.description().orElse(null));
roleToUpdate.setPermissions(role.permissions());

role.setName(roleRepresentation.name());
role.setPermissions(roleRepresentation.permissions());
roleRepresentation.description().ifPresent(role::setDescription);

try {
roleService.save(roleToUpdate);
roleService.save(role);
} catch (ValidationException e) {
throw new BadRequestException(e);
throw new BadRequestException("Couldn't update role \"" + roleRepresentation.name() + "\"", e);
}
return RoleResponse.create(roleToUpdate.getName(), Optional.ofNullable(roleToUpdate.getDescription()), roleToUpdate.getPermissions(),
role.readOnly());

return RoleResponse.create(role.getName(), Optional.ofNullable(role.getDescription()), role.getPermissions(),
roleRepresentation.readOnly());
}

@DELETE
@Path("{rolename}")
@ApiOperation(value = "Remove the named role and dissociate any users from it")
@ApiOperation("Remove the named role and dissociate any users from it")
@AuditEvent(type = AuditEventTypes.ROLE_DELETE)
public void delete(@ApiParam(name = "rolename", required = true) @PathParam("rolename") String name) throws NotFoundException {
checkPermission(RestPermissions.ROLES_DELETE, name);
Expand All @@ -178,7 +182,7 @@ public void delete(@ApiParam(name = "rolename", required = true) @PathParam("rol
@GET
@Path("{rolename}/members")
@RequiresPermissions({RestPermissions.USERS_LIST, RestPermissions.ROLES_READ})
@ApiOperation(value = "Retrieve the role's members")
@ApiOperation("Retrieve the role's members")
public RoleMembershipResponse getMembers(@ApiParam(name = "rolename", required = true) @PathParam("rolename") String name) throws NotFoundException {
final Role role = roleService.load(name);
final Collection<User> users = userService.loadAllForRole(role);
Expand All @@ -193,7 +197,7 @@ public RoleMembershipResponse getMembers(@ApiParam(name = "rolename", required =
user.getEmail(),
user.getFullName(),
isPermitted(RestPermissions.USERS_PERMISSIONSEDIT,
user.getName()) ? userService.getPermissionsForUser(user) : Collections.<String>emptyList(),
user.getName()) ? userService.getPermissionsForUser(user) : Collections.<String>emptyList(),
user.getPreferences(),
firstNonNull(user.getTimeZone(), DateTimeZone.UTC).getID(),
user.getSessionTimeoutMs(),
Expand All @@ -217,7 +221,8 @@ public RoleMembershipResponse getMembers(@ApiParam(name = "rolename", required =
public Response addMember(@ApiParam(name = "rolename") @PathParam("rolename") String rolename,
@ApiParam(name = "username") @PathParam("username") String username,
@ApiParam(name = "JSON Body", value = "Placeholder because PUT requests should have a body. Set to '{}', the content will be ignored.", defaultValue = "{}") String body) throws NotFoundException {
checkPermission(RestPermissions.ROLES_EDIT, username);
checkPermission(RestPermissions.USERS_EDIT, username);
checkPermission(RestPermissions.ROLES_EDIT, rolename);

final User user = userService.load(username);
if (user == null) {
Expand Down Expand Up @@ -246,7 +251,8 @@ public Response addMember(@ApiParam(name = "rolename") @PathParam("rolename") St
@AuditEvent(type = AuditEventTypes.ROLE_MEMBERSHIP_DELETE)
public Response removeMember(@ApiParam(name = "rolename") @PathParam("rolename") String rolename,
@ApiParam(name = "username") @PathParam("username") String username) throws NotFoundException {
checkPermission(RestPermissions.ROLES_EDIT, username);
checkPermission(RestPermissions.USERS_EDIT, username);
checkPermission(RestPermissions.ROLES_EDIT, rolename);

final User user = userService.load(username);
if (user == null) {
Expand All @@ -266,7 +272,6 @@ public Response removeMember(@ApiParam(name = "rolename") @PathParam("rolename")
throw new BadRequestException("Validation failed", e);
}


return status(Response.Status.NO_CONTENT).build();
}
}
31 changes: 22 additions & 9 deletions graylog2-web-interface/src/components/users/RoleList.jsx
@@ -1,10 +1,17 @@
import React from 'react';
import Reflux from 'reflux';
import Immutable from 'immutable';
import { Button } from 'react-bootstrap';

import StoreProvider from 'injection/StoreProvider';

import PermissionsMixin from 'util/PermissionsMixin';
const CurrentUserStore = StoreProvider.getStore('CurrentUser');

import { DataTable } from 'components/common';

const RoleList = React.createClass({
mixins: [Reflux.connect(CurrentUserStore), PermissionsMixin],
propTypes: {
roles: React.PropTypes.instanceOf(Immutable.Set).isRequired,
showEditRole: React.PropTypes.func.isRequired,
Expand All @@ -15,21 +22,27 @@ const RoleList = React.createClass({
const className = (header === 'Actions' ? 'actions' : '');
return <th className={className}>{header}</th>;
},
_editButton(role) {
if (this.isPermitted(this.state.currentUser.permissions, ['roles:edit:' + role.name]) === false || role.read_only) {
return null;
}
return (<Button key="edit" bsSize="xsmall" bsStyle="info" onClick={() => this.props.showEditRole(role)} title="Edit role">Edit</Button>);
},
_deleteButton(role) {
if (this.isPermitted(this.state.currentUser.permissions, ['roles:delete:' + role.name]) === false || role.read_only) {
return null;
}
return (<Button key="delete" bsSize="xsmall" bsStyle="primary" onClick={() => this.props.deleteRole(role)} title="Delete role">Delete</Button>);
},
_roleInfoFormatter(role) {
const actions = [
<Button key="delete" bsSize="xsmall" bsStyle="primary" onClick={() => this.props.deleteRole(role)}
title="Delete role">Delete</Button>,
<span key="space">&nbsp;</span>,
<Button key="edit" bsSize="xsmall" bsStyle="info" onClick={() => this.props.showEditRole(role)}
title="Edit role">Edit</Button>,
];

return (
<tr key={role.name}>
<td>{role.name}</td>
<td className="limited">{role.description}</td>
<td>
{role.read_only ? null : actions}
{this._editButton(role)}
<span key="space">&nbsp;</span>
{this._deleteButton(role)}
</td>
</tr>
);
Expand Down