security policy merging improvement

Evolveum · Nov 29, 2022 · ef9a45f · ef9a45f
1 parent 5683b6f
commit ef9a45f
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 13 deletions.
diff --git a/...evolveum/midpoint/authentication/impl/factory/module/AbstractCredentialModuleFactory.java b/...evolveum/midpoint/authentication/impl/factory/module/AbstractCredentialModuleFactory.java
@@ -113,6 +113,10 @@ protected AuthenticationProvider getProvider(
         return getObjectObjectPostProcessor().postProcess(createProvider(usedPolicy));
     }
 
+    private String getCredentialAuthModuleIdentifier(AbstractCredentialAuthenticationModuleType module) {
+        return StringUtils.isNotEmpty(module.getIdentifier()) ? module.getIdentifier() : module.getName();
+    }
+
     protected abstract ModuleAuthenticationImpl createEmptyModuleAuthentication(
             AbstractAuthenticationModuleType moduleType, C configuration, AuthenticationSequenceModuleType sequenceModule);
 

diff --git a/...java/com/evolveum/midpoint/authentication/impl/factory/module/AuthModuleRegistryImpl.java b/...java/com/evolveum/midpoint/authentication/impl/factory/module/AuthModuleRegistryImpl.java
@@ -57,7 +57,7 @@ public void addToRegistry(AbstractModuleFactory factory) {
 
     }
 
-    public AbstractModuleFactory findModelFactory(AbstractAuthenticationModuleType configuration, AuthenticationChannel authenticationChannel) {
+    public AbstractModuleFactory findModuleFactory(AbstractAuthenticationModuleType configuration, AuthenticationChannel authenticationChannel) {
 
         Optional<AbstractModuleFactory> opt = moduleFactories.stream().filter(f -> f.match(configuration, authenticationChannel)).findFirst();
         if (opt.isEmpty()) {

diff --git a/...pl/src/main/java/com/evolveum/midpoint/authentication/impl/filter/MidpointAuthFilter.java b/...pl/src/main/java/com/evolveum/midpoint/authentication/impl/filter/MidpointAuthFilter.java
@@ -157,7 +157,7 @@ private void doFilterInternal(ServletRequest request, ServletResponse response,
                     LOGGER.debug(UrlUtils.buildRequestUrl(httpRequest)
                             + "has no authentication module");
                 }
-                throw new AuthenticationServiceException("Couldn't find authentication module for sequence " + authWrapper.sequence.getName());
+                throw new AuthenticationServiceException("Couldn't find authentication module for sequence " + AuthSequenceUtil.getAuthSequenceIdentifier(authWrapper.sequence));
             }
             resolveErrorWithMoreModules(mpAuthentication, httpRequest);
 

diff --git a/...n-impl/src/main/java/com/evolveum/midpoint/authentication/impl/util/AuthSequenceUtil.java b/...n-impl/src/main/java/com/evolveum/midpoint/authentication/impl/util/AuthSequenceUtil.java
@@ -299,7 +299,7 @@ public static List<AuthModule> buildModuleFilters(AuthModuleRegistryImpl authReg
                 if (module == null) {
                     module = getModuleByName(sequenceModule.getName(), authenticationModulesType);  //just to support old config with name attribute
                 }
-                AbstractModuleFactory moduleFactory = authRegistry.findModelFactory(module, authenticationChannel);
+                AbstractModuleFactory moduleFactory = authRegistry.findModuleFactory(module, authenticationChannel);
                 AuthModule authModule = moduleFactory.createModuleFilter(module, sequence.getChannel().getUrlSuffix(), request,
                         sharedObjects, authenticationModulesType, credentialPolicy, authenticationChannel, sequenceModule);
                 authModules.add(authModule);
@@ -394,7 +394,7 @@ private static AbstractAuthenticationModuleType getModuleByIdentifier(String ide
         });
 
         for (AbstractAuthenticationModuleType module : modules) {
-            if (module.getIdentifier().equals(identifier)) {
+            if (StringUtils.equals(module.getIdentifier(), identifier)) {
                 return module;
             }
         }

diff --git a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/security/SecurityHelper.java b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/security/SecurityHelper.java
@@ -146,12 +146,11 @@ private void storeConnectionEnvironment(AuditEventRecord record, ConnectionEnvir
     public <F extends FocusType> SecurityPolicyType locateSecurityPolicy(PrismObject<F> focus, PrismObject<SystemConfigurationType> systemConfiguration,
             Task task, OperationResult result) throws SchemaException, CommunicationException, ConfigurationException, SecurityViolationException, ExpressionEvaluationException {
 
+        SecurityPolicyType globalSecurityPolicy = locateGlobalSecurityPolicy(focus, systemConfiguration, task, result);
         SecurityPolicyType securityPolicyFromOrgs = locateFocusSecurityPolicyFromOrgs(focus, task, result);
+        SecurityPolicyType mergedSecurityPolicy = mergeSecurityPolicies(securityPolicyFromOrgs, globalSecurityPolicy);  //sec policy from org overrides global sec policy
         SecurityPolicyType securityPolicyFromArchetypes = locateFocusSecurityPolicyFromArchetypes(focus, task, result);
-        SecurityPolicyType mergedSecurityPolicy = mergeSecurityPolicies(securityPolicyFromArchetypes, securityPolicyFromOrgs);  //sec policy from archetypes overrides sec policy from org
-
-        SecurityPolicyType globalSecurityPolicy = locateGlobalSecurityPolicy(focus, systemConfiguration, task, result);
-        mergedSecurityPolicy = mergeSecurityPolicies(mergedSecurityPolicy, globalSecurityPolicy);
+        mergedSecurityPolicy = mergeSecurityPolicies(securityPolicyFromArchetypes, mergedSecurityPolicy);  //sec policy from archetypes overrides sec policy from org
 
         if (mergedSecurityPolicy != null) {
             traceSecurityPolicy(mergedSecurityPolicy, focus);
@@ -338,7 +337,7 @@ private <AM extends AbstractAuthenticationModuleType> void mergeAuthenticationMo
             return;
         }
         if (CollectionUtils.isEmpty(mergedList)) {
-            mergedList.addAll(listToProcess);
+            listToProcess.forEach(i -> mergedList.add((AM) i.clone()));
             return;
         }
         listToProcess.forEach(itemToProcess -> {
@@ -360,7 +359,7 @@ private void mergeSequences(AuthenticationsPolicyType mergedAuthentication, List
             return;
         }
         if (CollectionUtils.isEmpty(mergedAuthentication.getSequence())) {
-            mergedAuthentication.getSequence().addAll(sequences);
+            sequences.forEach(s -> mergedAuthentication.getSequence().add(s.clone()));
             return;
         }
         sequences.forEach(sequenceToProcess -> {
@@ -373,7 +372,7 @@ private void mergeSequences(AuthenticationsPolicyType mergedAuthentication, List
                 }
             }
             if (!exist) {
-                mergedAuthentication.getSequence().add(sequenceToProcess);
+                mergedAuthentication.getSequence().add(sequenceToProcess.clone());
             }
         });
     }
@@ -391,11 +390,11 @@ private void mergeSequence(AuthenticationSequenceType sequence, AuthenticationSe
         }
         if (CollectionUtils.isNotEmpty(sequenceToProcess.getModule())) {
             if (CollectionUtils.isEmpty(sequence.getModule())) {
-                sequence.getModule().addAll(sequenceToProcess.getModule());
+                sequenceToProcess.getModule().forEach(m -> sequence.getModule().add(m.clone()));
             } else {
                 sequenceToProcess.getModule().forEach(sequenceModule -> {
                     if (findSequenceModuleByIdentifier(sequence.getModule(), sequenceModule) != null) {
-                        sequence.getModule().add(sequenceModule);
+                        sequence.getModule().add(sequenceModule.clone());
                     }
                 });
             }