From a55691e43cc6698c5f3799df3d21708cf7473e35 Mon Sep 17 00:00:00 2001
From: Tim Jacomb <timjacomb1+github@gmail.com>
Date: Fri, 15 Jul 2022 23:34:33 +0100
Subject: [PATCH] Use object ID consistently

---
 .run/azure-ad [hpi_run].run.xml               |   2 +-
 pom.xml                                       |   2 +-
 .../AzureAdMatrixAuthorizationStrategy.java   |  89 +++++++++++++
 .../jenkins/azuread/AzureAdUser.java          |   2 +-
 .../azuread/AzureAuthenticationToken.java     |   4 +-
 .../jenkins/azuread/AzureSecurityRealm.java   |   9 +-
 .../jenkins/azuread/utils/ValidationUtil.java | 126 ++++++++++++++++++
 .../com/microsoft/jenkins/azuread/table.js    |   2 +-
 8 files changed, 226 insertions(+), 10 deletions(-)
 create mode 100644 src/main/java/com/microsoft/jenkins/azuread/utils/ValidationUtil.java

diff --git a/.run/azure-ad [hpi_run].run.xml b/.run/azure-ad [hpi_run].run.xml
index 8259292b..abe5a993 100644
--- a/.run/azure-ad [hpi_run].run.xml	
+++ b/.run/azure-ad [hpi_run].run.xml	
@@ -12,7 +12,7 @@
           <option name="mavenProperties">
             <map>
               <entry key="java.awt.headless" value="true" />
-              <entry key="jenkins.version" value="2.347" />
+              <entry key="jenkins.version" value="2.355" />
               <entry key="port" value="6322" />
               <entry key="skip.npm" value="true" />
             </map>
diff --git a/pom.xml b/pom.xml
index 1f6381c9..45a3ac15 100644
--- a/pom.xml
+++ b/pom.xml
@@ -49,7 +49,7 @@
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
         <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
 
-        <jenkins.version>2.289.3</jenkins.version>
+        <jenkins.version>2.355</jenkins.version>
         <node.version>16.13.2</node.version>
         <npm.version>8.4.0</npm.version>
     </properties>
diff --git a/src/main/java/com/microsoft/jenkins/azuread/AzureAdMatrixAuthorizationStrategy.java b/src/main/java/com/microsoft/jenkins/azuread/AzureAdMatrixAuthorizationStrategy.java
index 2d5c249a..def9a4db 100644
--- a/src/main/java/com/microsoft/jenkins/azuread/AzureAdMatrixAuthorizationStrategy.java
+++ b/src/main/java/com/microsoft/jenkins/azuread/AzureAdMatrixAuthorizationStrategy.java
@@ -14,8 +14,10 @@
 import com.microsoft.graph.requests.GraphServiceClient;
 import com.microsoft.graph.requests.GroupCollectionPage;
 import com.microsoft.graph.requests.UserCollectionPage;
+import com.microsoft.jenkins.azuread.utils.ValidationUtil;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.Extension;
+import hudson.Functions;
 import hudson.init.InitMilestone;
 import hudson.init.Initializer;
 import hudson.model.AbstractItem;
@@ -25,14 +27,17 @@
 import hudson.model.Job;
 import hudson.model.Node;
 import hudson.security.ACL;
+import hudson.security.AccessControlled;
 import hudson.security.GlobalMatrixAuthorizationStrategy;
 import hudson.security.Permission;
 import hudson.security.SecurityRealm;
 import hudson.security.SidACL;
+import hudson.util.FormValidation;
 import jenkins.model.Jenkins;
 import okhttp3.Request;
 import org.apache.commons.lang.StringUtils;
 import org.jenkinsci.plugins.matrixauth.AuthorizationContainer;
+import org.jenkinsci.plugins.matrixauth.AuthorizationType;
 import org.jenkinsci.plugins.matrixauth.PermissionEntry;
 import org.jenkinsci.plugins.matrixauth.AuthorizationMatrixNodeProperty;
 import org.kohsuke.accmod.Restricted;
@@ -49,6 +54,10 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import static com.microsoft.jenkins.azuread.utils.ValidationUtil.formatNonExistentUserGroupValidationResponse;
+import static com.microsoft.jenkins.azuread.utils.ValidationUtil.formatUserGroupValidationResponse;
+
+
 public class AzureAdMatrixAuthorizationStrategy extends GlobalMatrixAuthorizationStrategy {
 
     private static final Logger LOGGER = Logger.getLogger(AzureAdMatrixAuthorizationStrategy.class.getName());
@@ -257,6 +266,86 @@ public boolean isDisableGraphIntegration() {
 
             return true;
         }
+
+        @Override
+        public FormValidation doCheckName(String value) {
+            final String unbracketedValue = value.substring(1, value.length() - 1); // remove leading [ and trailing ]
+            AccessControlled subject = Jenkins.get();
+            Permission permission = Jenkins.ADMINISTER;
+            
+            final int splitIndex = unbracketedValue.indexOf(':');
+            if (splitIndex < 0) {
+                return FormValidation.error("No type prefix: " + unbracketedValue);
+            }
+            final String typeString = unbracketedValue.substring(0, splitIndex);
+            final AuthorizationType type;
+            try {
+                type = AuthorizationType.valueOf(typeString);
+            } catch (Exception ex) {
+                return FormValidation.error("Invalid type prefix: " + unbracketedValue);
+            }
+            String sid = unbracketedValue.substring(splitIndex + 1);
+
+            String escapedSid = Functions.escape(sid);
+
+            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"));
+                }
+                if (type == AuthorizationType.GROUP) {
+                    return FormValidation.okWithMarkup(formatUserGroupValidationResponse("user", 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"));
+            }
+
+            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"));
+            }
+
+            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"));
+            }
+
+            try {
+                FormValidation groupValidation;
+                FormValidation userValidation;
+                switch (type) {
+                    case GROUP:
+                        groupValidation = ValidationUtil.validateGroup(sid, sr, false);
+                        if (groupValidation != null) {
+                            return groupValidation;
+                        }
+                        return FormValidation.errorWithMarkup(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)
+                    case EITHER:
+                        userValidation = ValidationUtil.validateUser(sid, sr, true);
+                        if (userValidation != null) {
+                            return userValidation;
+                        }
+                        groupValidation = ValidationUtil.validateGroup(sid, sr, true);
+                        if (groupValidation != null) {
+                            return groupValidation;
+                        }
+                        return FormValidation.errorWithMarkup(formatNonExistentUserGroupValidationResponse(escapedSid, "User or group not found")); // TODO i18n (after 3.0)
+                    default:
+                        return FormValidation.error("Unexpected type: " + type);
+                }
+            } catch (Exception e) {
+                // if the check fails miserably, we still want the user to be able to see the name of the user,
+                // so use 'escapedSid' as the message
+                return FormValidation.error(e,escapedSid);
+            }
+        }
     }
 
     @Restricted(DoNotUse.class)
diff --git a/src/main/java/com/microsoft/jenkins/azuread/AzureAdUser.java b/src/main/java/com/microsoft/jenkins/azuread/AzureAdUser.java
index 8bd6ddf2..75f3efe6 100644
--- a/src/main/java/com/microsoft/jenkins/azuread/AzureAdUser.java
+++ b/src/main/java/com/microsoft/jenkins/azuread/AzureAdUser.java
@@ -189,7 +189,7 @@ public String getPassword() {
 
     @Override
     public String getUsername() {
-        return getUniqueName();
+        return getObjectID();
     }
 
     @Override
diff --git a/src/main/java/com/microsoft/jenkins/azuread/AzureAuthenticationToken.java b/src/main/java/com/microsoft/jenkins/azuread/AzureAuthenticationToken.java
index 0e527452..c020c4cf 100644
--- a/src/main/java/com/microsoft/jenkins/azuread/AzureAuthenticationToken.java
+++ b/src/main/java/com/microsoft/jenkins/azuread/AzureAuthenticationToken.java
@@ -48,7 +48,7 @@ private String getDisplayName() {
 
     @Override
     public Object getPrincipal() {
-        return String.format("%s (%s)", getDisplayName(), getObjectId());
+        return getObjectId();
     }
 
     @Override
@@ -63,7 +63,7 @@ public void setAuthenticated(boolean isAuthenticated) throws IllegalArgumentExce
 
     @Override
     public String getName() {
-        return (azureAdUser != null ? azureAdUser.getUniqueName() : null);
+        return (azureAdUser != null ? azureAdUser.getObjectID() : null);
     }
 
     public AzureAdUser getAzureAdUser() {
diff --git a/src/main/java/com/microsoft/jenkins/azuread/AzureSecurityRealm.java b/src/main/java/com/microsoft/jenkins/azuread/AzureSecurityRealm.java
index 911e56fc..f8958479 100644
--- a/src/main/java/com/microsoft/jenkins/azuread/AzureSecurityRealm.java
+++ b/src/main/java/com/microsoft/jenkins/azuread/AzureSecurityRealm.java
@@ -71,6 +71,7 @@
 import org.springframework.security.authentication.BadCredentialsException;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.security.core.userdetails.UserDetails;
 import org.springframework.security.core.userdetails.UsernameNotFoundException;
 
 import javax.servlet.FilterChain;
@@ -388,7 +389,7 @@ public HttpResponse doFinishLogin(StaplerRequest request)
             }
             // validate the nonce to avoid CSRF
             final JwtClaims claims = validateIdToken(expectedNonce, idToken);
-            String key = (String) claims.getClaimValue("preferred_username");
+            String key = (String) claims.getClaimValue("oid");
 
             AzureAdUser userDetails = caches.get(key, (cacheKey) -> {
                 final AzureAdUser user;
@@ -498,6 +499,7 @@ public SecurityComponents createSecurityComponents() {
                 // Currently triggers annoying log spam if the user is a group, but there's no way to tell currently
                 // as we look up by object id we don't know if it's a user or a group :(
                 try {
+                    // TODO try https://docs.microsoft.com/en-us/answers/questions/42697/how-to-get-a-particular-azure-ad-guest-user-from-h.html
                     com.microsoft.graph.models.User activeDirectoryUser = azureClient.users(userId).buildRequest()
                             .get();
 
@@ -550,7 +552,7 @@ public GroupDetails loadGroupByGroupname2(String groupName, boolean fetchMembers
         String groupId = ObjId2FullSidMap.extractObjectId(groupName);
 
         if (groupId == null) {
-            // just an object id on it's own?
+            // just an object id on its own?
             groupId = groupName;
         }
 
@@ -797,8 +799,7 @@ private String generateDescription(Authentication auth) {
                     + "\nUnique Principal Name: " + user.getUniqueName()
                     + "\nEmail: " + user.getEmail()
                     + "\nObject ID: " + user.getObjectID()
-                    + "\nTenant ID: " + user.getTenantID()
-                    + "\nGroups: " + user.getGroupOIDs() + "\n";
+                    + "\nTenant ID: " + user.getTenantID();
         }
         return "";
     }
diff --git a/src/main/java/com/microsoft/jenkins/azuread/utils/ValidationUtil.java b/src/main/java/com/microsoft/jenkins/azuread/utils/ValidationUtil.java
new file mode 100644
index 00000000..7608a928
--- /dev/null
+++ b/src/main/java/com/microsoft/jenkins/azuread/utils/ValidationUtil.java
@@ -0,0 +1,126 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2004-2017 Sun Microsystems, Inc., Kohsuke Kawaguchi, Yahoo! Inc., Peter Hayes, Tom Huybrechts, Daniel Beck
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package com.microsoft.jenkins.azuread.utils;
+
+import com.microsoft.jenkins.azuread.AzureAdUser;
+import hudson.Functions;
+import hudson.Util;
+import hudson.model.User;
+import hudson.security.GroupDetails;
+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.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.UserDetails;
+import org.springframework.security.core.userdetails.UsernameNotFoundException;
+
+@Restricted(NoExternalUse.class)
+public class ValidationUtil {
+    private ValidationUtil() {
+        // do not use
+    }
+
+    private static final VersionNumber jenkinsVersion = Jenkins.getVersion();
+
+    public static String formatNonExistentUserGroupValidationResponse(String user, String tooltip) {
+        return formatUserGroupValidationResponse(null, "<span style='text-decoration: line-through;'>" + tooltip + ": " + user + "</span>", tooltip);
+    }
+
+    public static String formatUserGroupValidationResponse(String img, String label, String tooltip) {
+        if (img == null) {
+            return String.format("<span title='%s'>%s</span>", tooltip, label);
+        }
+
+        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 {
+            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);
+        }
+    }
+
+    public static FormValidation validateGroup(String groupName, SecurityRealm sr, boolean ambiguous) {
+        String escapedSid = Functions.escape(groupName);
+        try {
+            GroupDetails groupDetails = sr.loadGroupByGroupname2(groupName, false);
+            if (ambiguous) {
+                return FormValidation.warningWithMarkup(formatUserGroupValidationResponse("user", groupDetails.getDisplayName(), "Group found; but permissions would also be granted to a user of this name"));
+            } else {
+                return FormValidation.okWithMarkup(formatUserGroupValidationResponse("user", groupDetails.getDisplayName(), "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"));
+            } else {
+                return FormValidation.ok(groupName);
+            }
+        } catch (UsernameNotFoundException e) {
+            // fall through next
+        } catch (AuthenticationException e) {
+            // other seemingly unexpected error.
+            return FormValidation.error(e,"Failed to test the validity of the group name " + groupName);
+        }
+        return null;
+    }
+
+    public static FormValidation validateUser(String userName, SecurityRealm sr, boolean ambiguous) {
+        String escapedSid = Functions.escape(userName);
+        try {
+            AzureAdUser userDetails = (AzureAdUser) sr.loadUserByUsername2(userName);
+            User u = User.getById(userName, true);
+            if (userName.equals(u.getFullName())) {
+                // Sid and full name are identical, no need for tooltip
+                if (ambiguous) {
+                    return FormValidation.warningWithMarkup(formatUserGroupValidationResponse("person", userDetails.getUniqueName(), "User found; but permissions would also be granted to a group of this name"));
+                } else {
+                    return FormValidation.okWithMarkup(formatUserGroupValidationResponse("person", userDetails.getUniqueName(), "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"));
+            } else {
+                return FormValidation.okWithMarkup(formatUserGroupValidationResponse("person", 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"));
+            } else {
+                return FormValidation.ok(userName);
+            }
+        } catch (UsernameNotFoundException e) {
+            // fall through next
+        } catch (AuthenticationException e) {
+            // other seemingly unexpected error.
+            return FormValidation.error(e,"Failed to test the validity of the user name " + userName);
+        }
+        return null;
+    }
+}
diff --git a/src/main/resources/com/microsoft/jenkins/azuread/table.js b/src/main/resources/com/microsoft/jenkins/azuread/table.js
index ab35de83..198c1bd7 100644
--- a/src/main/resources/com/microsoft/jenkins/azuread/table.js
+++ b/src/main/resources/com/microsoft/jenkins/azuread/table.js
@@ -36,7 +36,7 @@ Behaviour.specify(".azure-ad-add-button", 'AzureAdMatrixAuthorizationStrategy',
                     type = "GROUP"
                     typeLabel = dataReference.getAttribute('data-type-group-label')
                 } else {
-                    name = person.userPrincipalName + " (" + person.id + ")"
+                    name = person.id
                     type = "USER"
                     typeLabel = dataReference.getAttribute('data-type-user-label')
                 }