Anonymous access to test fails due to hooks

[rvansa]

Retrieving hooks for given test

Horreum/src/main/java/io/hyperfoil/tools/horreum/api/HookService.java

Lines 230 to 241 in 31b1071

	@RolesAllowed(Roles.ADMIN)
	@GET
	@Path("test/{id}")
	public List<Hook> variables(@PathParam("id") Integer testId) {
	try (@SuppressWarnings("unused") CloseMe closeMe = sqlService.withRoles(em, identity)) {
	if (testId != null) {
	return Hook.list("target", testId);
	} else {
	return Variable.listAll();
	}
	}
	}

is limited to ADMIN role. Is that intended @johnaohara ? Alternatively, do we want a custom priviledge to set that?

The current problem is that this opening test by anonymous user results in an error message being shown rather than hiding/graying out the webhooks section.

Btw. it seems that the code was copied from elsewhere but not fully finished (method name, else branch returning variables...)

[rvansa]

@johnaohara Any comments?

[johnaohara]

I don't think that we would want an anon user to see the webhooks defined for a test and should be restricted as to who can view/edit the webhooks.

I am assuming that the endpoint returns a 401 error when an anon user tries to access this endpoint? Is that not a valid reponse? Should we handle that in the UI rather than creating a custom priviledge?

Yes, sry looks like it was copied from AlertingService and I didn't finsh the impl.

[rvansa]

The other settings in the test are readable even by an anonymous user - if it's sensitive you can make the test 'protected' (readable only by authenticated users) or 'private' (viewable only by the owner). Do you want to make webhooks an exception from that policy?
Also you've set this to admin-only: that's even more strict as owner of the test could not set up webhooks. That could be sensible setting because it'd allow anyone with access to some test hit some internal infrastructure. However it would limit the usefulness of webhooks for an user (there should be few admins). We could limit the splash radius by setting a white-list of domains in the admin screen.

[rvansa]

This was fixed in f316f44