Conversation
server/src/main/java/keywhiz/service/permissions/OwnershipPermissionCheck.java
Outdated
Show resolved
Hide resolved
server/src/main/java/keywhiz/service/permissions/OwnershipPermissionCheck.java
Outdated
Show resolved
Hide resolved
public boolean isAllowed(Object source, String action, Object target) { | ||
boolean hasPermission = false; | ||
|
||
if (isClient(source) && isSecret(target)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, super readable.
|
||
if (isClient(source) && isSecret(target)) { | ||
Set<Group> clientGroups = aclDAO.getGroupsFor((Client) source); | ||
String secretOwner =((Secret) target).getOwner(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should really get Checkstyle going for this project, but in the meantime make sure you always put a space on both sides of an equal sign.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is. just realized that the style guide warned me about the if statement simplification for the help functions. but not the spacing
server/src/main/java/keywhiz/service/permissions/OwnershipPermissionCheck.java
Outdated
Show resolved
Hide resolved
public void testIsAllowedWhenSourceIsNotAClient() { | ||
boolean permitted = ownershipPermissionCheck.isAllowed(USER, Action.CREATE, SECRET_WITH_OWNER); | ||
|
||
assertThat(permitted).isEqualTo(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had to make this explicitly equal to false because it appears that assertThat(permitted)
always returns true
35fa9ae
to
f6fc1e6
Compare
No description provided.