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 REST API Node Loading Permission Issue #1104

Open
Jotschi opened this issue Aug 5, 2020 · 0 comments
Open

Fix REST API Node Loading Permission Issue #1104

Jotschi opened this issue Aug 5, 2020 · 0 comments

Comments

@Jotschi
Copy link
Contributor

Jotschi commented Aug 5, 2020

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:

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant