Merge pull request payara#5515 from OndroMih/Community-FISH-5769-Cert…

…ificateIS-ignores-role-mappings FISH-5769 RFC2253 format with Certificate HAM, fix role verification Signed-off-by: JamesHillyard <james.hillyard@payara.fish>
JamesHillyard · Jan 21, 2022 · 1bba58f · 1bba58f
1 parent 333f6a0
commit 1bba58f
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 3 deletions.
diff --git a/appserver/security/core-ee/pom.xml b/appserver/security/core-ee/pom.xml
@@ -236,6 +236,11 @@
         <dependency>
             <groupId>jakarta.authorization</groupId>
             <artifactId>jakarta.authorization-api</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>jakarta.security.enterprise</groupId>
+            <artifactId>jakarta.security.enterprise-api</artifactId>
+            <scope>provided</scope>
         </dependency>
          <dependency>
             <groupId>fish.payara.server.internal.security</groupId>

diff --git a/...y/core-ee/src/main/java/com/sun/enterprise/security/jacc/JaccWebAuthorizationManager.java b/...y/core-ee/src/main/java/com/sun/enterprise/security/jacc/JaccWebAuthorizationManager.java
@@ -99,8 +99,10 @@
 import org.glassfish.security.common.Role;
 
 import static com.sun.enterprise.security.common.AppservAccessController.privilegedException;
+import java.util.HashSet;
 import static java.util.logging.Level.FINE;
 import static java.util.logging.Level.SEVERE;
+import javax.security.enterprise.CallerPrincipal;
 import static org.glassfish.api.web.Constants.ADMIN_VS;
 
 /**
@@ -118,6 +120,7 @@
  *
  * @author Jean-Francois Arcand
  * @author Harpreet Singh.
+ * @author Ondro Mihalyi
  * @todo introduce a new class called AbstractSecurityManager. Move functionality from this class and EJBSecurityManager
  * class and extend this class from AbstractSecurityManager
  */
@@ -344,8 +347,12 @@ public boolean hasResourcePermission(HttpServletRequest servletRequest) {
     public boolean hasRoleRefPermission(String servletName, String role, Principal principal) {
         WebRoleRefPermission requestedPermission = new WebRoleRefPermission(servletName, role);
 
-        boolean isGranted = checkPermission(requestedPermission, getSecurityContext(principal).getPrincipalSet());
-
+        Set<Principal> principalSetFromSecurityContext = getSecurityContext(principal).getPrincipalSet();
+        boolean isGranted = checkPermission(requestedPermission, principalSetFromSecurityContext);
+        if (!isGranted) {
+            isGranted = checkPermissionForModifiedPrincipalSet(principalSetFromSecurityContext, isGranted, requestedPermission);
+        }
+
         if (logger.isLoggable(Level.FINE)) {
             logger.log(FINE, "[Web-Security] hasRoleRef perm: {0}", requestedPermission);
             logger.log(FINE, "[Web-Security] hasRoleRef isGranted: {0}", isGranted);
@@ -354,6 +361,28 @@ public boolean hasRoleRefPermission(String servletName, String role, Principal p
         return isGranted;
     }
 
+    /* If the principal set contains CallerPrincipal, replace it with PrincipalImpl. 
+       This is because CallerPrincipal isn't equal to PrincipalImpl and doesn't imply it.
+       CallerPrincipal doesn't even implement equals method, so 2 CallerPrincipals with the same name are not equal. 
+       Because CallerPrincipal is from Jakarta EE, we can't change it.
+    */
+    private boolean checkPermissionForModifiedPrincipalSet(Set<Principal> principalSetFromSecurityContext, boolean isGranted, WebRoleRefPermission requestedPermission) {
+        boolean principalSetContainsCallerPrincipal = false;
+        Set<Principal> modifiedPrincipalSet = new HashSet<Principal>(principalSetFromSecurityContext.size());
+        for (Principal p : principalSetFromSecurityContext) {
+            if (p instanceof CallerPrincipal) {
+                principalSetContainsCallerPrincipal = true;
+                modifiedPrincipalSet.add(new PrincipalImpl(p.getName()));
+            } else {
+                modifiedPrincipalSet.add(p);
+            }
+        }
+        if (principalSetContainsCallerPrincipal) {
+            isGranted = checkPermission(requestedPermission, modifiedPrincipalSet);
+        }
+        return isGranted;
+    }
+
     /**
      * Analogous to destroy, except does not remove links from Policy Context, and does not remove context_id from role
      * mapper factory. Used to support Policy Changes that occur via ServletContextListener.
@@ -800,3 +829,4 @@ private void logProtectionDomainCreated(Principal[] principals) {
     }
 
 }
+
diff --git a/...rity/realm-stores/src/main/java/fish/payara/security/realm/CertificateCredentialImpl.java b/...rity/realm-stores/src/main/java/fish/payara/security/realm/CertificateCredentialImpl.java
@@ -43,10 +43,12 @@
 import java.security.cert.X509Certificate;
 import jakarta.security.enterprise.CallerPrincipal;
 import jakarta.security.enterprise.credential.AbstractClearableCredential;
+import javax.security.auth.x500.X500Principal;
 
 /**
  *
  * @author Gaurav Gupta
+ * @author Ondro Mihalyi
  */
 public class CertificateCredentialImpl extends AbstractClearableCredential implements CertificateCredential {
 
@@ -66,7 +68,7 @@ public X509Certificate[] getCertificates() {
 
     @Override
     public CallerPrincipal getPrincipal() {
-        return new CallerPrincipal(certificates[0].getIssuerDN().getName());
+        return new CallerPrincipal(certificates[0].getIssuerX500Principal().getName(X500Principal.RFC2253));
     }
 
     @Override