Fix REST API Node Loading Permission Issue
- Dominant language
- Java
- Stars
- 593
- Forks
- 123
- Avg merge
- 2d 18h
- Merged PRs (30d)
- 4
Description
Loading a node by uuid with specific version number does not correctly check permissions. The current perm check uses the READ_PERM instead of the READ_PUBLISHED perm when specifying the published version by number.
The perm check within `NodeRootImpl#loadObjectByUuid` should be adapted.
Potential fix:
```java
if (!ac.getUser().hasPermissionForId(element.id(), READ_PUBLISHED_PERM)) {
throw error(FORBIDDEN, "error_missing_perm", element.getUuid(),
"published".equals(requestedVersion)
? READ_PUBLISHED_PERM.getRestPerm().getName()
: READ_PERM.getRestPerm().getName());
}
ac.getUser().failOnNoReadPermission(fieldContainer, branch.getUuid(), versioiningParameters.getVersion());
```
Additionally the `NodeCrudHandler#handleRead` method needs to be updated. It should not pre-determine the needed permission by inspecting the `version` parameter. The code must always use the `READ_PUBLISHED` perm in order to ensure that the `NodeRootImpl#loadObjectByUuid` method checks the content read permissions.
Adding the needed permission check will however change the current behaviour.
## Current Behaviour
When loading a node using `?version=draft` the empty node response will be returned (containing no fields).
When using `?version=published` and loading a node with a non-existing language an error will be returned that the field could not be found in the current branch.
We should thus adapt this behaviour and ensure that it works the same for `?version=draft` and `?version=published`.
This change may however affect existing implementations.
## Tests
* `NodeEndpointTest#testReadPublishedNodeNoPermission3` asserts the expected behaviour.
* `NodeEndpointTest#testReadPublishedNodesNoPermission2` contains additional assertions.
Contributor guide
Assessment
This issue has not been assessed yet.