diff --git a/pom.xml b/pom.xml index 45a0aeec..b24eb710 100644 --- a/pom.xml +++ b/pom.xml @@ -28,7 +28,7 @@ -SNAPSHOT jenkinsci/${project.artifactId}-plugin 3.0 - 2.361.4 + 2.387.3 false @@ -36,8 +36,8 @@ io.jenkins.tools.bom - bom-2.361.x - 1887.vda_d0ddb_c15c4 + bom-2.387.x + 2230.v0cb_4040cde55 pom import @@ -50,6 +50,10 @@ configuration-as-code true + + io.jenkins.plugins + ionicons-api + org.jenkins-ci.plugins diff --git a/src/main/java/org/jenkinsci/plugins/matrixauth/AuthorizationContainerDescriptor.java b/src/main/java/org/jenkinsci/plugins/matrixauth/AuthorizationContainerDescriptor.java index 00f443dc..4598fb2d 100644 --- a/src/main/java/org/jenkinsci/plugins/matrixauth/AuthorizationContainerDescriptor.java +++ b/src/main/java/org/jenkinsci/plugins/matrixauth/AuthorizationContainerDescriptor.java @@ -161,33 +161,45 @@ default FormValidation doCheckName_( if (!subject.hasPermission(permission)) { // Lacking permissions, so respond based on input only if (type == AuthorizationType.USER) { - return FormValidation.okWithMarkup( - formatUserGroupValidationResponse("person", escapedSid, "User may or may not exist")); + return FormValidation.respond( + FormValidation.Kind.OK, + formatUserGroupValidationResponse( + AuthorizationType.USER, escapedSid, "User may or may not exist")); } if (type == AuthorizationType.GROUP) { - return FormValidation.okWithMarkup( - formatUserGroupValidationResponse("user", escapedSid, "Group may or may not exist")); + return FormValidation.respond( + FormValidation.Kind.OK, + formatUserGroupValidationResponse( + AuthorizationType.GROUP, escapedSid, "Group may or may not exist")); } - return FormValidation.warningWithMarkup(formatUserGroupValidationResponse( - null, escapedSid, "Permissions would be granted to a user or group of this name")); + return FormValidation.respond( + FormValidation.Kind.OK, + formatUserGroupValidationResponse( + "", escapedSid, "Permissions would be granted to a user or group of this name", true)); } SecurityRealm sr = Jenkins.get().getSecurityRealm(); if (sid.equals("authenticated") && type == AuthorizationType.EITHER) { // system reserved group - return FormValidation.warningWithMarkup(formatUserGroupValidationResponse( - "user", - escapedSid, - "Internal group found; but permissions would also be granted to a user of this name")); + return FormValidation.respond( + FormValidation.Kind.OK, + formatUserGroupValidationResponse( + AuthorizationType.GROUP, + escapedSid, + "Internal group found; but permissions would also be granted to a user of this name", + true)); } if (sid.equals("anonymous") && type == AuthorizationType.EITHER) { // system reserved user - return FormValidation.warningWithMarkup(formatUserGroupValidationResponse( - "person", - escapedSid, - "Internal user found; but permissions would also be granted to a group of this name")); + return FormValidation.respond( + FormValidation.Kind.OK, + formatUserGroupValidationResponse( + AuthorizationType.USER, + escapedSid, + "Internal user found; but permissions would also be granted to a group of this name", + true)); } try { @@ -199,15 +211,19 @@ default FormValidation doCheckName_( if (groupValidation != null) { return groupValidation; } - return FormValidation.errorWithMarkup(formatNonExistentUserGroupValidationResponse( - escapedSid, "Group not found")); // TODO i18n (after 3.0) + return FormValidation.respond( + FormValidation.Kind.OK, + formatNonExistentUserGroupValidationResponse( + escapedSid, "Group not found")); // TODO i18n (after 3.0) case USER: userValidation = ValidationUtil.validateUser(sid, sr, false); if (userValidation != null) { return userValidation; } - return FormValidation.errorWithMarkup(formatNonExistentUserGroupValidationResponse( - escapedSid, "User not found")); // TODO i18n (after 3.0) + return FormValidation.respond( + FormValidation.Kind.OK, + formatNonExistentUserGroupValidationResponse( + escapedSid, "User not found")); // TODO i18n (after 3.0) case EITHER: userValidation = ValidationUtil.validateUser(sid, sr, true); if (userValidation != null) { @@ -217,8 +233,10 @@ default FormValidation doCheckName_( if (groupValidation != null) { return groupValidation; } - return FormValidation.errorWithMarkup(formatNonExistentUserGroupValidationResponse( - escapedSid, "User or group not found")); // TODO i18n (after 3.0) + return FormValidation.respond( + FormValidation.Kind.OK, + formatNonExistentUserGroupValidationResponse( + escapedSid, "User or group not found", true)); // TODO i18n (after 3.0) default: return FormValidation.error("Unexpected type: " + type); } diff --git a/src/main/java/org/jenkinsci/plugins/matrixauth/ValidationUtil.java b/src/main/java/org/jenkinsci/plugins/matrixauth/ValidationUtil.java index 047df49a..767e819f 100644 --- a/src/main/java/org/jenkinsci/plugins/matrixauth/ValidationUtil.java +++ b/src/main/java/org/jenkinsci/plugins/matrixauth/ValidationUtil.java @@ -23,48 +23,93 @@ */ package org.jenkinsci.plugins.matrixauth; +import static org.jenkinsci.plugins.matrixauth.AuthorizationType.EITHER; +import static org.jenkinsci.plugins.matrixauth.AuthorizationType.GROUP; +import static org.jenkinsci.plugins.matrixauth.AuthorizationType.USER; + +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.Functions; import hudson.Util; import hudson.model.User; import hudson.security.SecurityRealm; import hudson.security.UserMayOrMayNotExistException2; import hudson.util.FormValidation; -import hudson.util.VersionNumber; -import jenkins.model.Jenkins; import org.apache.commons.lang.StringUtils; +import org.jenkins.ui.symbol.Symbol; +import org.jenkins.ui.symbol.SymbolRequest; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; -import org.kohsuke.stapler.Stapler; import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.userdetails.UsernameNotFoundException; @Restricted(NoExternalUse.class) class ValidationUtil { + private static final String userSymbol; + private static final String groupSymbol; + private static final String warningSymbol; + private static final String alertSymbol; + private ValidationUtil() { // do not use } - private static final VersionNumber jenkinsVersion = Jenkins.getVersion(); + static { + userSymbol = getSymbol("person", "icon-sm"); + groupSymbol = getSymbol("people", "icon-sm"); + alertSymbol = getSymbol("alert-circle", "icon-md mas-table__icon-alert"); + warningSymbol = getSymbol("warning", "icon-md mas-table__icon-warning"); + } + + private static String getSymbol(String symbol, String classes) { + SymbolRequest.Builder builder = new SymbolRequest.Builder(); + + return Symbol.get(builder.withRaw("symbol-" + symbol + "-outline plugin-ionicons-api") + .withClasses(classes) + .build()); + } static String formatNonExistentUserGroupValidationResponse(String user, String tooltip) { + return formatNonExistentUserGroupValidationResponse(user, tooltip, false); + } + + static String formatNonExistentUserGroupValidationResponse(String user, String tooltip, boolean warning) { return formatUserGroupValidationResponse( - null, "" + tooltip + ": " + user + "", tooltip); + "alert", "" + user + "", tooltip, warning); } - static String formatUserGroupValidationResponse(String img, String label, String tooltip) { - if (img == null) { - return String.format("%s", tooltip, label); - } + static String formatUserGroupValidationResponse(@NonNull AuthorizationType type, String user, String tooltip) { + return formatUserGroupValidationResponse(type.toString(), user, tooltip, false); + } - if (jenkinsVersion.isOlderThan(new VersionNumber("2.308"))) { - return String.format( - "%s", - tooltip, Stapler.getCurrentRequest().getContextPath(), Jenkins.RESOURCE_PATH, img, label); - } else { + static String formatUserGroupValidationResponse( + @NonNull AuthorizationType type, String user, String tooltip, boolean warning) { + return formatUserGroupValidationResponse(type.toString(), user, tooltip, warning); + } + + static String formatUserGroupValidationResponse( + @NonNull String type, String user, String tooltip, boolean warning) { + String symbol; + switch (type) { + case "GROUP": + symbol = groupSymbol; + break; + case "alert": + symbol = alertSymbol; + break; + case "USER": + symbol = userSymbol; + break; + case "EITHER": + default: + symbol = ""; + break; + } + if (warning) { return String.format( - "%s", - tooltip, Stapler.getCurrentRequest().getContextPath(), Jenkins.RESOURCE_PATH, img, label); + "
%s%s%s
", + tooltip, warningSymbol, symbol, user); } + return String.format("
%s%s
", tooltip, symbol, user); } static FormValidation validateGroup(String groupName, SecurityRealm sr, boolean ambiguous) { @@ -72,20 +117,29 @@ static FormValidation validateGroup(String groupName, SecurityRealm sr, boolean try { sr.loadGroupByGroupname2(groupName, false); if (ambiguous) { - return FormValidation.warningWithMarkup(formatUserGroupValidationResponse( - "user", - escapedSid, - "Group found; but permissions would also be granted to a user of this name")); + return FormValidation.respond( + FormValidation.Kind.WARNING, + formatUserGroupValidationResponse( + GROUP, + escapedSid, + "Group found; but permissions would also be granted to a user of this name", + true)); } else { - return FormValidation.okWithMarkup(formatUserGroupValidationResponse("user", escapedSid, "Group")); + return FormValidation.respond( + FormValidation.Kind.OK, formatUserGroupValidationResponse(GROUP, escapedSid, "Group")); } } catch (UserMayOrMayNotExistException2 e) { // undecidable, meaning the group may exist if (ambiguous) { - return FormValidation.warningWithMarkup(formatUserGroupValidationResponse( - "user", escapedSid, "Permissions would also be granted to a user or group of this name")); + return FormValidation.respond( + FormValidation.Kind.WARNING, + formatUserGroupValidationResponse( + GROUP, + escapedSid, + "Permissions would also be granted to a user or group of this name", + true)); } else { - return FormValidation.ok(groupName); + return FormValidation.ok(escapedSid); } } catch (UsernameNotFoundException e) { // fall through next @@ -104,29 +158,43 @@ static FormValidation validateUser(String userName, SecurityRealm sr, boolean am if (userName.equals(u.getFullName())) { // Sid and full name are identical, no need for tooltip if (ambiguous) { - return FormValidation.warningWithMarkup(formatUserGroupValidationResponse( - "person", - escapedSid, - "User found; but permissions would also be granted to a group of this name")); + return FormValidation.respond( + FormValidation.Kind.WARNING, + formatUserGroupValidationResponse( + USER, + escapedSid, + "User found; but permissions would also be granted to a group of this name", + true)); } else { - return FormValidation.okWithMarkup(formatUserGroupValidationResponse("person", escapedSid, "User")); + return FormValidation.respond( + FormValidation.Kind.OK, formatUserGroupValidationResponse(USER, escapedSid, "User")); } } if (ambiguous) { - return FormValidation.warningWithMarkup(formatUserGroupValidationResponse( - "person", - Util.escape(StringUtils.abbreviate(u.getFullName(), 50)), - "User " + escapedSid - + " found, but permissions would also be granted to a group of this name")); + return FormValidation.respond( + FormValidation.Kind.WARNING, + formatUserGroupValidationResponse( + USER, + Util.escape(StringUtils.abbreviate(u.getFullName(), 50)), + "User " + escapedSid + + " found; but permissions would also be granted to a group of this name", + true)); } else { - return FormValidation.okWithMarkup(formatUserGroupValidationResponse( - "person", Util.escape(StringUtils.abbreviate(u.getFullName(), 50)), "User " + escapedSid)); + return FormValidation.respond( + FormValidation.Kind.OK, + formatUserGroupValidationResponse( + USER, Util.escape(StringUtils.abbreviate(u.getFullName(), 50)), "User " + escapedSid)); } } catch (UserMayOrMayNotExistException2 e) { // undecidable, meaning the user may exist if (ambiguous) { - return FormValidation.warningWithMarkup(formatUserGroupValidationResponse( - "person", escapedSid, "Permissions would also be granted to a user or group of this name")); + return FormValidation.respond( + FormValidation.Kind.WARNING, + formatUserGroupValidationResponse( + EITHER, + escapedSid, + "Permissions would also be granted to a user or group of this name", + true)); } else { return FormValidation.ok(userName); } diff --git a/src/main/resources/hudson/security/GlobalMatrixAuthorizationStrategy/config.jelly b/src/main/resources/hudson/security/GlobalMatrixAuthorizationStrategy/config.jelly index 845c52d7..a054cdd3 100644 --- a/src/main/resources/hudson/security/GlobalMatrixAuthorizationStrategy/config.jelly +++ b/src/main/resources/hudson/security/GlobalMatrixAuthorizationStrategy/config.jelly @@ -60,16 +60,16 @@ THE SOFTWARE. - - ${%Authenticated Users} - +
+ ${%Authenticated Users} +
- - ${%Anonymous} - +
+ ${%Anonymous} +
@@ -80,7 +80,7 @@ THE SOFTWARE. - - - - ${%Select all} - - - ${%Unselect all} - - - - + +
+ + - - - - - - - - + + - + + + + + + + + + + + + + + +
@@ -182,28 +184,26 @@ THE SOFTWARE. -
- + + -
+ -
+
${%ambiguous}
diff --git a/src/main/resources/hudson/security/table.css b/src/main/resources/hudson/security/table.css index 72580c20..91cf283a 100644 --- a/src/main/resources/hudson/security/table.css +++ b/src/main/resources/hudson/security/table.css @@ -38,7 +38,7 @@ .global-matrix-authorization-strategy-table .caption-row TH { font-weight: lighter; - padding: 0; + border: 1px solid #D3D7CF; } .global-matrix-authorization-strategy-table .caption-row TH span { @@ -47,6 +47,8 @@ .global-matrix-authorization-strategy-table TD { border: 1px solid #D3D7CF; + padding: 3px; + vertical-align: middle; } .global-matrix-authorization-strategy-table TD.left-most { @@ -75,3 +77,47 @@ -moz-animation: highlightentry 5s; animation: highlightentry 5s; } + +.global-matrix-authorization-strategy-table .jenkins-checkbox { + vertical-align: middle; +} + +.mas-table__cell { + display: flex; + align-items: center; + gap: 3px; +} + +.mas-table__cell svg { + min-width: 16px; +} + +.mas-table__cell a { + height: 16px; + color: var(--text-color) +} + +.mas-table__cell a svg { + vertical-align: initial!important; +} + +.mas-table__cell--not-found { + text-decoration: line-through; + color: var(--danger); + font-weight: 600; +} + +.mas-table__cell-warning { + color: var(--warning); + font-weight: 600; +} + +.mas-table__icon-warning { + color: var(--warning); + min-width: 24px!important; +} + +.mas-table__icon-alert { + color: var(--danger); + min-width: 24px!important; +} diff --git a/src/main/resources/hudson/security/table.js b/src/main/resources/hudson/security/table.js index 6d3c831c..06fd164b 100644 --- a/src/main/resources/hudson/security/table.js +++ b/src/main/resources/hudson/security/table.js @@ -2,7 +2,7 @@ * This handles the addition of new users/groups to the list. */ Behaviour.specify(".matrix-auth-add-button", 'GlobalMatrixAuthorizationStrategy', 0, function(e) { - makeButton(e, function (e) { + e.onclick = function (e) { var dataReference = e.target; var master = document.getElementById(dataReference.getAttribute('data-table-id')); var table = master.parentNode; @@ -58,7 +58,7 @@ Behaviour.specify(".matrix-auth-add-button", 'GlobalMatrixAuthorizationStrategy' }); table.appendChild(copy); Behaviour.applySubtree(findAncestor(table,"TABLE"),true); - }); + } }); /* @@ -160,7 +160,7 @@ Behaviour.specify(".global-matrix-authorization-strategy-table TD.stop A.migrate tr.removeAttribute('data-checked'); // remove migration buttons from updated row - var buttonContainer = findAncestor(this, "TD"); + var buttonContainer = findAncestor(this, "DIV"); var migrateButtons = buttonContainer.getElementsByClassName('migrate'); for (var i = migrateButtons.length - 1; i >= 0; i--) { buttonContainer.removeChild(migrateButtons[i]); diff --git a/src/main/webapp/images/select-all.svg b/src/main/resources/images/symbols/select-all.svg similarity index 90% rename from src/main/webapp/images/select-all.svg rename to src/main/resources/images/symbols/select-all.svg index 352d4021..6b712244 100644 --- a/src/main/webapp/images/select-all.svg +++ b/src/main/resources/images/symbols/select-all.svg @@ -5,7 +5,7 @@ - - + + diff --git a/src/main/webapp/images/unselect-all.svg b/src/main/resources/images/symbols/unselect-all.svg similarity index 94% rename from src/main/webapp/images/unselect-all.svg rename to src/main/resources/images/symbols/unselect-all.svg index bc1aeb8d..0ac2bc7c 100644 --- a/src/main/webapp/images/unselect-all.svg +++ b/src/main/resources/images/symbols/unselect-all.svg @@ -5,6 +5,6 @@ - + diff --git a/src/test/java/org/jenkinsci/plugins/matrixauth/Jenkins57313Test.java b/src/test/java/org/jenkinsci/plugins/matrixauth/Jenkins57313Test.java index dc036354..5218d407 100644 --- a/src/test/java/org/jenkinsci/plugins/matrixauth/Jenkins57313Test.java +++ b/src/test/java/org/jenkinsci/plugins/matrixauth/Jenkins57313Test.java @@ -25,6 +25,6 @@ public void testFormValidation() throws Exception { Assert.assertEquals(200, page.getWebResponse().getStatusCode()); String responseText = page.getWebResponse().getContentAsString(); Assert.assertTrue(responseText.contains("alice")); - Assert.assertTrue(responseText.contains("person")); + Assert.assertTrue(responseText.contains("User")); } } diff --git a/src/test/java/org/jenkinsci/plugins/matrixauth/ReadOnlyTest.java b/src/test/java/org/jenkinsci/plugins/matrixauth/ReadOnlyTest.java index 4a161de1..f0c01378 100644 --- a/src/test/java/org/jenkinsci/plugins/matrixauth/ReadOnlyTest.java +++ b/src/test/java/org/jenkinsci/plugins/matrixauth/ReadOnlyTest.java @@ -55,7 +55,7 @@ private void assertPresentAndEditable(String configurationUrl) throws IOExceptio "should contain add group/user button", hasTagWithClassInPage( page, - "span", + "button", "matrix-auth-add-button")); // Behavior.specify / makeButton converts input to button and wraps // it in span } @@ -66,7 +66,7 @@ private void assertPresentAndReadOnly(String configurationUrl) throws IOExceptio "should not contain add group/user button", hasTagWithClassInPage( page, - "span", + "button", "matrix-auth-add-button")); // Behavior.specify / makeButton converts input to button and wraps // it in span } diff --git a/src/test/java/org/jenkinsci/plugins/matrixauth/Security2180Test.java b/src/test/java/org/jenkinsci/plugins/matrixauth/Security2180Test.java index 0db95be8..aae33421 100644 --- a/src/test/java/org/jenkinsci/plugins/matrixauth/Security2180Test.java +++ b/src/test/java/org/jenkinsci/plugins/matrixauth/Security2180Test.java @@ -15,6 +15,7 @@ import hudson.model.Item; import hudson.model.TopLevelItem; import hudson.model.TopLevelItemDescriptor; +import hudson.model.queue.QueueTaskFuture; import hudson.security.ACL; import hudson.security.AuthorizationMatrixProperty; import hudson.security.Permission; @@ -101,8 +102,10 @@ private void assertJobVisibility(FreeStyleProject job, boolean visibleWithFix, b throws Exception { final String jobUrl = job.getUrl(); // TODO robustness: check queue contents / executor status before scheduling - job.scheduleBuild2(0, new Cause.UserIdCause("admin")).waitForStart(); // schedule one build now - job.scheduleBuild2(0, new Cause.UserIdCause("admin")); // schedule an additional queue item + FreeStyleBuild build = + job.scheduleBuild2(0, new Cause.UserIdCause("admin")).waitForStart(); // schedule one build now + QueueTaskFuture future = + job.scheduleBuild2(0, new Cause.UserIdCause("admin")); // schedule an additional queue item Assert.assertEquals(1, Jenkins.get().getQueue().getItems().length); // expect there to be one queue item final JenkinsRule.WebClient webClient = j.createWebClient().withThrowExceptionOnFailingStatusCode(false); @@ -140,6 +143,8 @@ private void assertJobVisibility(FreeStyleProject job, boolean visibleWithFix, b } } } finally { + future.cancel(true); + build.doStop(); System.clearProperty(propertyName); } }