fix: add hyphen to permission pattern (#3882)

Author: thoniTUB

backend/src/main/java/com/bakdata/conquery/util/validation/ValidConqueryPermission.java

@@ -14,11 +14,12 @@
  * Validate Shiro permissions
  */
 @NotBlank
-@Pattern(regexp="^[\\w,*]+(?::[\\w,*@]+){0,2}$", message="Provide a valid shiro permission string with 3 parts at most.")
+@Pattern(regexp=ValidConqueryPermission.PERMISSION_PATTERN, message="Provide a valid shiro permission string with 3 parts at most, following this pattern: " + ValidConqueryPermission.PERMISSION_PATTERN)
 @Target({ElementType.FIELD, ElementType.PARAMETER})
 @Retention(RetentionPolicy.RUNTIME)
 @Constraint(validatedBy = {}) // No custom validator needed if only using built-ins
 public @interface ValidConqueryPermission {
+	String PERMISSION_PATTERN = "^[\\w,*-]+(?::[\\w,*@-]+){0,2}$";
 	String message() default "Invalid permission";
 	Class<?>[] groups() default {};
 	Class<? extends Payload>[] payload() default {};

backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/templates/permissionCreator.html.ftl

@@ -1,7 +1,7 @@
 <#macro permissionCreator ownerId permissionTemplateMap>
     <#local INSTANCES = "instances_">
 	<#local ABILITIES = "abilities_">
-	<div class"table-responsive">
+	<div>
     <table class="table table-sm table-striped">
         <thead>
             <tr>
@@ -68,43 +68,64 @@
 
 
 		let abilitySelector = document.getElementById("${ABILITIES}"+domain);
-		abilityJoin = null;
+		let abilityJoin = null;
+		let abilities = [];
 		if (abilitySelector != null) {
-			let abilities = [];
 			let abilityCheckboxes =abilitySelector.getElementsByTagName('input');
 			for(let cb of abilityCheckboxes ) {
 				if(cb.checked){
 					abilities.push(cb.value.trim())
 				}
 			}
-			if(abilityCheckboxes.length > 0 && abilities.length == 0 ) {
+			if(abilityCheckboxes.length > 0 && abilities.length === 0 ) {
 				alert("No abilities for permission specified");
 				return;
 			}
 			abilityJoin = abilities.join(",");
 		}
 
-        // Collection only only as a member if a permission supports abilities and thus targets
+        // Collection only as a member if a permission supports abilities and thus targets
 		let instanceInput = document.getElementById("${INSTANCES}"+domain).getElementsByTagName("ul");
 
 
-		if(instanceInput.length != 0) {
+		let instances = null;
+		if(instanceInput.length !== 0) {
+			if (abilities.length === 0) {
+				alert("Abilities are required if instances are set");
+				return;
+			}
 		    // Permission can have targets
 		    instances = Array.from(instanceInput[0].children,c => c.innerHTML).join(",");
 		}
-		else {
-		    // Permission does not support abilities and targets (e.g. super permission)
-		    instances = null;
-		}
-		
-		permission = [domain, abilityJoin, instances].join(":")
+
+
+		let permission = [domain, abilityJoin, instances].filter(x => !!x).join(":");
 		console.log("Sending Permission: " + permission);
-		await rest('/admin/permissions/${ownerId}',
+		let response = await rest('/admin/permissions/${ownerId}',
 			{
 				method: 'post',
 				body: permission
-			});
-		location.reload();
+			}
+		);
+		if (!response.ok) {
+			showToastMessage(
+					ToastTypes.ERROR,
+					"Error",
+					"The send request came back with the following error: " + (await response.json()).message,
+					"Status " + response.status,
+			);
+		} else {
+			showToastMessage(
+					ToastTypes.SUCCESS,
+					"Permission Created",
+					`Permission was created`,
+					"Status " + response.status,
+			);
+			setTimeout(function(){
+				location.reload();
+			}, 500);
+
+		}
 	}
 	</script>
 </#macro>

backend/src/test/java/com/bakdata/conquery/integration/tests/PermissionResourceTest.java

@@ -35,12 +35,20 @@ public class PermissionResourceTest {
 	static Stream<Arguments> testParams() {
 		return Stream.of(
 				Arguments.of("domain", 204),
+				Arguments.of("domain:", 422),
 				Arguments.of("domain:operation", 204),
+				Arguments.of("domain:operation:", 422),
 				Arguments.of("domain:operation:instance", 204),
 				Arguments.of("domain:operation1,:instance", 204),
+				Arguments.of("domain-hyphen:operation,:instance@at", 204),
+				Arguments.of("domain-hyphen:operation,:instance_underscore", 204),
 				Arguments.of("domain:operation1,operation2:instance", 204),
+				Arguments.of("*", 204),
+				Arguments.of("*:", 422),
 				Arguments.of("", 422),
-				Arguments.of("domain:operation:instance:too_much", 422),
+				Arguments.of(":", 422),
+				Arguments.of("domain::instance", 422),
+				Arguments.of("domain:operation:instance:too_many_parts", 422),
 				Arguments.of("domain:,", 400)
 
 		);