Skip to content

Commit

Permalink
UI renovation (#147)
Browse files Browse the repository at this point in the history 
* UI renovation

- use symbols everywhere
- use new button style
- align cells

* adjust class names

* ensure builds are finished

on windows the still running jobs likely cause problems with post
cleanup due to open file handles

* fix spotless

* fix class names

* Set correct icon for internal user/group ambiguous permissions

* Semicolon for consistency with other similar messages

* Also adapt tooltips for anonymous and authenticated

* make svgs theme aware

* ensure colors work with all themes nicely

---------

Co-authored-by: Daniel Beck <daniel-beck@users.noreply.github.com>
  • Loading branch information
@mawinter69 @daniel-beck
mawinter69 and daniel-beck committed Jul 17, 2023
1 parent d149f8d commit 23db88f
Show file tree
Hide file tree
Showing 11 changed files with 249 additions and 108 deletions.
10 changes: 7 additions & 3 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,16 @@
<changelist>-SNAPSHOT</changelist>
<gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo>
<hpi.compatibleSinceVersion>3.0</hpi.compatibleSinceVersion>
<jenkins.version>2.361.4</jenkins.version>
<jenkins.version>2.387.3</jenkins.version>
<spotless.check.skip>false</spotless.check.skip>
</properties>

<dependencyManagement>
<dependencies>
<dependency>
<groupId>io.jenkins.tools.bom</groupId>
<artifactId>bom-2.361.x</artifactId>
<version>1887.vda_d0ddb_c15c4</version>
<artifactId>bom-2.387.x</artifactId>
<version>2230.v0cb_4040cde55</version>
<type>pom</type>
<scope>import</scope>
</dependency>
Expand All @@ -50,6 +50,10 @@
<artifactId>configuration-as-code</artifactId>
<optional>true</optional>
</dependency>
<dependency>
<groupId>io.jenkins.plugins</groupId>
<artifactId>ionicons-api</artifactId>
</dependency>
<!-- optional plugin dependencies -->
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
Expand Down
58 changes: 38 additions & 20 deletions src/main/java/org/jenkinsci/plugins/matrixauth/AuthorizationContainerDescriptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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) {
Expand All @@ -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);
}
Expand Down
144 changes: 106 additions & 38 deletions src/main/java/org/jenkinsci/plugins/matrixauth/ValidationUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,69 +23,123 @@
*/
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, "<span style='text-decoration: line-through;'>" + tooltip + ": " + user + "</span>", tooltip);
"alert", "<span class='mas-table__cell--not-found'>" + user + "</span>", tooltip, warning);
}

static String formatUserGroupValidationResponse(String img, String label, String tooltip) {
if (img == null) {
return String.format("<span title='%s'>%s</span>", 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(
"<span title='%s'><img src='%s%s/images/16x16/%s.png' style='margin-right:0.2em'>%s</span>",
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(
"<span title='%s'><img src='%s%s/images/svgs/%s.svg' width='16' style='margin-right:0.2em'>%s</span>",
tooltip, Stapler.getCurrentRequest().getContextPath(), Jenkins.RESOURCE_PATH, img, label);
"<div tooltip='%s' class='mas-table__cell mas-table__cell-warning'>%s%s%s</div>",
tooltip, warningSymbol, symbol, user);
}
return String.format("<div tooltip='%s' class='mas-table__cell'>%s%s</div>", tooltip, symbol, user);
}

static FormValidation validateGroup(String groupName, SecurityRealm sr, boolean ambiguous) {
String escapedSid = Functions.escape(groupName);
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
Expand All @@ -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);
}
Expand Down

0 comments on commit 23db88f

Please sign in to comment.