Merge pull request #38 from arjantijms/fix_warnings

Fix several warnings during building

impl/pom.xml

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!--
 
-    Copyright (c) 2021, 2023 Contributors to Eclipse Foundation.
+    Copyright (c) 2021, 2024 Contributors to Eclipse Foundation.
     Copyright (c) 2010, 2019 Oracle and/or its affiliates. All rights reserved.
 
     This program and the accompanying materials are made available under the
@@ -96,21 +96,21 @@
         <dependency>
             <groupId>jakarta.servlet</groupId>
             <artifactId>jakarta.servlet-api</artifactId>
-            <version>6.1.0-M1</version>
+            <version>6.1.0-M2</version>
             <scope>provided</scope>
             <optional>true</optional>
         </dependency>
         <dependency>
             <groupId>jakarta.authorization</groupId>
             <artifactId>jakarta.authorization-api</artifactId>
-            <version>3.0.0-M1</version>
+            <version>3.0.0-M2</version>
             <scope>provided</scope>
             <optional>true</optional>
         </dependency>
         <dependency>
             <groupId>org.javassist</groupId>
             <artifactId>javassist</artifactId>
-            <version>3.29.2-GA</version>
+            <version>3.30.2-GA</version>
             <scope>provided</scope>
             <optional>true</optional>
         </dependency>
@@ -178,7 +178,7 @@
 
             <plugin>
                 <artifactId>maven-compiler-plugin</artifactId>
-                <version>3.10.1</version>
+                <version>3.12.1</version>
                 <configuration>
                     <release>17</release>
                 </configuration>
@@ -286,7 +286,7 @@
                             </groups>
                             <bottom><![CDATA[
 Comments to: <a href="mailto:exousia-dev@eclipse.org">exousia-dev@eclipse.org</a>.<br>
-Copyright &#169; 2021, 2023 Eclipse Foundation. All rights reserved.<br>
+Copyright &#169; 2021, 2024 Eclipse Foundation. All rights reserved.<br>
 Use is subject to <a href="http://www.eclipse.org/legal/epl-2.0" target="_top">license terms</a>.]]>
                             </bottom>
                         </configuration>
@@ -298,7 +298,7 @@ Use is subject to <a href="http://www.eclipse.org/legal/epl-2.0" target="_top">l
             <plugin>
                 <groupId>org.eclipse.dash</groupId>
                 <artifactId>license-tool-plugin</artifactId>
-                <version>1.0.2</version>
+                <version>1.1.0</version>
                 <executions>
                     <execution>
                         <id>license-check</id>
@@ -313,7 +313,7 @@ Use is subject to <a href="http://www.eclipse.org/legal/epl-2.0" target="_top">l
             <plugin>
                 <groupId>org.codehaus.mojo</groupId>
                 <artifactId>flatten-maven-plugin</artifactId>
-                <version>1.5.0</version>
+                <version>1.6.0</version>
                 <configuration>
                     <flattenMode>ossrh</flattenMode>
                 </configuration>

impl/src/main/java/org/glassfish/exousia/mapping/DefaultPrincipalMapper.java

@@ -40,6 +40,9 @@
 import javax.security.auth.Subject;
 
 /**
+ * TODO: much or all of this should be removed and/or refactored.
+ * The servers supported here via reflection don't support this version of authorization
+ * at all, and have no need to be longer supported here.
  *
  * @author Arjan Tijms
  */
@@ -74,7 +77,8 @@ private static void tryInitGeronimo() {
             // Geronimo 3.0.1 contains a protection mechanism to ensure only a Geronimo policy provider is installed.
             // This protection can be beat by creating an instance of GeronimoPolicyConfigurationFactory once. This instance
             // will statically register itself with an internal Geronimo class
-            geronimoPolicyConfigurationFactoryInstance = Class.forName("org.apache.geronimo.security.jacc.mappingprovider.GeronimoPolicyConfigurationFactory").newInstance();
+            geronimoPolicyConfigurationFactoryInstance =
+                Class.forName("org.apache.geronimo.security.jacc.mappingprovider.GeronimoPolicyConfigurationFactory").getDeclaredConstructor().newInstance();
             geronimoContextToRoleMapping = new ConcurrentHashMap<>();
         } catch (Exception e) {
             // ignore

impl/src/main/java/org/glassfish/exousia/modules/def/DefaultPolicy.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2023 Contributors to the Eclipse Foundation.
+ * Copyright (c) 2023, 2024 Contributors to the Eclipse Foundation.
  * Copyright (c) 2019, 2021 OmniFaces. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
@@ -16,18 +16,17 @@
  */
 package org.glassfish.exousia.modules.def;
 
+import static jakarta.security.jacc.PolicyContext.PRINCIPAL_MAPPER;
 import static java.util.Collections.list;
 
 import jakarta.security.jacc.Policy;
 import jakarta.security.jacc.PolicyConfiguration;
 import jakarta.security.jacc.PolicyConfigurationFactory;
 import jakarta.security.jacc.PolicyContext;
-import jakarta.security.jacc.PolicyContextException;
 import jakarta.security.jacc.PrincipalMapper;
 import java.security.Permission;
 import java.security.PermissionCollection;
 import java.security.Permissions;
-import java.security.Principal;
 import java.util.Map;
 import java.util.Set;
 import javax.security.auth.Subject;
@@ -38,46 +37,55 @@
  */
 public class DefaultPolicy implements Policy {
 
-    @Override
-    public boolean implies(Permission permissionToBeChecked, Subject subject) {
-        PolicyConfiguration policyConfiguration = getPolicyConfigurationFactory().getPolicyConfiguration();
-        PrincipalMapper roleMapper = getRoleMapper();
+    private PolicyConfigurationFactory policyConfigurationFactory;
+    private PrincipalMapper principalMapper;
 
-        if (isExcluded(policyConfiguration.getExcludedPermissions(), permissionToBeChecked)) {
-            // Excluded permissions cannot be accessed by anyone
-            return false;
-        }
+    @Override
+    public boolean isExcluded(Permission permissionToBeChecked) {
+        return isExcluded(
+                getPolicyConfigurationFactory().getPolicyConfiguration().getExcludedPermissions(),
+                permissionToBeChecked);
+    }
 
-        if (isUnchecked(policyConfiguration.getUncheckedPermissions(), permissionToBeChecked)) {
-            // Unchecked permissions are free to be accessed by everyone
-            return true;
-        }
+    @Override
+    public boolean isUnchecked(Permission permissionToBeChecked) {
+        return isUnchecked(
+                getPolicyConfigurationFactory().getPolicyConfiguration().getUncheckedPermissions(),
+                permissionToBeChecked);
+    }
 
+    @Override
+    public boolean impliesByRole(Permission permissionToBeChecked, Subject subject) {
         if (subject == null) {
+            // Without a subject we can't check for roles, so we can shortcut the outcome.
             return false;
         }
 
-        Set<Principal> currentUserPrincipals = subject.getPrincipals();
+        // Get the configuration and mapper instances.
+        // Note that these are obtained for the current (application) context ID, and this policy could potentially
+        // be used for multiple context IDs. Therefore these objects should not be cached as instance data of this policy.
+        PolicyConfiguration policyConfiguration = getPolicyConfigurationFactory().getPolicyConfiguration();
+        PrincipalMapper roleMapper = getRoleMapper();
 
-        if (!roleMapper.isAnyAuthenticatedUserRoleMapped() && !currentUserPrincipals.isEmpty()) {
+        if (!roleMapper.isAnyAuthenticatedUserRoleMapped() && !subject.getPrincipals().isEmpty()) {
             // The "any authenticated user" role is not mapped, so available to anyone and the current
-            // user is assumed to be authenticated (we assume that an unauthenticated user doesn't have any
+            // caller is assumed to be authenticated (we assume that an unauthenticated caller doesn't have any
             // principals whatever they are)
             if (hasAccessViaRole(policyConfiguration.getPerRolePermissions(), "**", permissionToBeChecked)) {
                 // Access is granted purely based/ on the user being authenticated
-                // (the actual roles, if any, the user has it not important)
+                // (the actual roles, if any, the caller has are not important)
                 return true;
             }
         }
 
-        if (hasAccessViaRoles(policyConfiguration.getPerRolePermissions(), roleMapper.getMappedRoles(subject),
-                permissionToBeChecked)) {
-            // Access is granted via role. Note that if this returns false/ it doesn't mean the permission is not granted.
-            // A role can only grant, not take away permissions.
-            return true;
-        }
-
-        return false;
+        // Check to see if access is granted via role.
+        // Note that if this returns false it doesn't necessarily mean the permission is not granted.
+        // A role can only grant, not take away permissions. Other checks (perhaps another custom policy that embeds us)
+        // may still grant the permission.
+        return hasAccessViaRoles(
+                policyConfiguration.getPerRolePermissions(),
+                roleMapper.getMappedRoles(subject),
+                permissionToBeChecked);
     }
 
     @Override
@@ -110,19 +118,19 @@ public PermissionCollection getPermissionCollection(Subject subject) {
     // ### Private methods
 
     private PolicyConfigurationFactory getPolicyConfigurationFactory() {
-        try {
-            return PolicyConfigurationFactory.getPolicyConfigurationFactory();
-        } catch (ClassNotFoundException | PolicyContextException e) {
-            throw new IllegalStateException(e);
+        if (policyConfigurationFactory == null) {
+            policyConfigurationFactory = PolicyConfigurationFactory.get();
         }
+
+        return policyConfigurationFactory;
     }
 
     private PrincipalMapper getRoleMapper() {
-        try {
-            return PolicyContext.getContext(PolicyContext.PRINCIPAL_MAPPER);
-        } catch (PolicyContextException e) {
-            throw new IllegalStateException(e);
+        if (principalMapper == null) {
+            principalMapper = PolicyContext.get(PRINCIPAL_MAPPER);
         }
+
+        return principalMapper;
     }
 
     private boolean isExcluded(PermissionCollection excludedPermissions, Permission permission) {