bug4006: more on user merge, updated Wiki doc

java/com.sap.sse.security.common/src/com/sap/sse/security/shared/SecurityUser.java

@@ -14,9 +14,8 @@ import com.sap.sse.common.WithID;
 public interface SecurityUser<RD extends RoleDefinition, R extends AbstractRole<RD, G, ?>, G extends SecurityUserGroup<?>>
         extends UserReference, WithQualifiedObjectIdentifier {
     /**
-     * Returns the "raw" permissions explicitly set for this user. This does not include permissions
-     * inferred by any {@link PermissionsForRoleProvider} for the {@link #getRoles() roles} that this
-     * user has. Use {@link #getAllPermissions(PermissionsForRoleProvider)} for that.
+     * Returns the "raw" permissions explicitly set for this user. This does not include permissions inferred from the
+     * {@link #getRoles() roles} that this user has. See also {@link PermissionChecker}.
      */
     Iterable<WildcardPermission> getPermissions();
 

java/com.sap.sse.security.storemerging/src/com/sap/sse/security/storemerging/SecurityStoreMerger.java

@@ -5,6 +5,7 @@ import java.util.HashSet;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
+import java.util.function.Consumer;
 import java.util.logging.Logger;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -389,6 +390,7 @@ public class SecurityStoreMerger {
                 targetUserStore.addPermissionForUser(targetUser.getName(), permission);
             }
         }
+        boolean updated = false;
         if (!targetUser.isEmailValidated() && sourceUser.isEmailValidated()) {
             final String validationSecret;
             if (targetUser.getValidationSecret() == null) {
@@ -400,10 +402,34 @@ public class SecurityStoreMerger {
             logger.info("Validating e-mail address "+targetUser.getEmail()+" of target user "+targetUser.getName()+
                     " because it was validated successfully on the source side");
             targetUser.validate(validationSecret);
+            updated = true;
+        }
+        updated = copyNonNullValue(sourceUser.getCompany(), targetUser.getCompany(), targetUser::setCompany);
+        updated = copyNonNullValue(sourceUser.getFullName(), targetUser.getFullName(), targetUser::setFullName);
+        updated = copyNonNullValue(sourceUser.getLocale(), targetUser.getLocale(), targetUser::setLocale);
+        if (updated) {
             targetUserStore.updateUser(targetUser);
         }
     }
 
+    /**
+     * If the {@code sourceValue} is a non-{@code null} value, and {@code targetValue} is a {@code null}
+     * value, the {@code setterOnTargetUser} is used to copy the {@code sourceValue} to the target user.
+     * In this case, {@code true} is returned.
+     * 
+     * @return {@code true} if and only if the {@code setter} was called to update the {@code targetUser}
+     */
+    private <T> boolean copyNonNullValue(T sourceValue, T targetValue, Consumer<T> setterOnTargetUser) {
+        final boolean updated;
+        if (sourceValue != null && targetValue == null) {
+            setterOnTargetUser.accept(sourceValue);
+            updated = true;
+        } else {
+            updated = false;
+        }
+        return updated;
+    }
+
     private void mergePreferences(UserStore sourceUserStore, Map<User, User> userMap) {
         for (final Entry<User, User> e : userMap.entrySet()) {
             for (final Entry<String, String> preference : sourceUserStore.getAllPreferences(e.getKey().getName()).entrySet()) {

wiki/projects/consolidating-user-stores.md

@@ -28,7 +28,7 @@ There are a few dedicated server instances with their DBs:
 * ARCHIVE, DB ``dbserver.internal.sapsailing.com:10201/winddb``
 * AST (Australian Sailing Team), DB ``ast.sapsailing.com:27017/AST`` backed up to ``dbserver.internal.sapsailing.com:10202/AST``
 * The DBs of all instances running on "SL Multi-Instance Kiel" (we should check if we could take this opportunity to shut down those that are still entirely empty): bartlomiejkapusta benjaminalhadef ditlevleth emilienpochet filip frederiksivertsen guentherpachschwoell gustavhultgren hinnerksiemsen janekbalster jassiskogman joanameyerduro jyrikuivalainen konradlipski lukasgrallert mandy marcusbaur martinmaursundrishovd oriolmahiques paulbakker peterwagner rolandregnemer tamarafischer thorbennowak thorechristiansen
-* The DBs of all instances running on "SL Multi-Instance Sailing Server NVMe" (we should check if we could take this opportunity to shut down those that are still entirely empty): 49er AARHUSSEJLKLUB abeam-training alarie505 ASVIA baldeneysee BYC d-labs DUTCH_FEDERATION ess-team-training formula18 galvestaure HNV hsc-womensteam irishdbscdw3 ISR JOJOPOLGAR jonaswitt KBSC KJSCS KSSS kyc LYC mdf my NOR oakcliff phoenix PMYC RDSAILING rheinwoche2019 SAILCANADA SAILINGACADEMY sailracer Sailtracks schadewaldt schanzenberg schwielochsee seascape SEGELZENTRUM SINGAPORESAILING SITGES SRN SRV SSC SSV STARLAB TracTracTest ubilabs-test USSAILING VSAW YCL
+* The DBs of all instances running on "SL Multi-Instance Sailing Server NVMe" (we should check if we could take this opportunity to shut down those that are still entirely empty): AARHUSSEJLKLUB abeam-training alarie505 ASVIA baldeneysee BYC d-labs DUTCH_FEDERATION ess-team-training formula18 galvestaure HNV hsc-womensteam irishdbscdw3 ISR JOJOPOLGAR jonaswitt KBSC KJSCS KSSS kyc LYC mdf my NOR oakcliff phoenix PMYC RDSAILING rheinwoche2019 SAILCANADA SAILINGACADEMY sailracer Sailtracks schadewaldt schanzenberg schwielochsee seascape SEGELZENTRUM SINGAPORESAILING SITGES SRN SRV SSC SSV STARLAB TracTracTest ubilabs-test USSAILING VSAW YCL
 
 bartlomiejkapusta is empty. Removing. benjaminalhadef is largely empty. Removing. mandy is basically empty. Removing. filip is empty. Removing. thorbennowak empty. Removing. lukasgrallert basically empty. Removing. konradlipski only imported old stuff. Removing. gustavhultgren was not even started and had only one old test event. Removing. oriolmahiques has only empty events. Removing. thorechristiansen empty other than Bundesliga imports. Removing. tamarafischer basically empty. Removing. emilienpochet contains only empty events. Removing. paulbakker empty, removing. guentherpachschwoell only has league imports. Removing. rolandregnemer empty; removing. ditlevleth empty; removing. frederiksivertsen empty; removing. jassiskogman empty; removing. jyrikuivalainen empty; removing. martinmaursundrishovd empty; removing. The janekbalster and peterwagner DBs have remained, and their server instances have now been moved to the common multi-instance server. The "SL Multi-Instance Kiel" server has been terminated by now. YCL is completely empty. Removed.
 
@@ -121,7 +121,7 @@ The ``PERMISSIONS`` field has entries for a significant number of users, mostly
 The query
 
 ```
-for i in *_USERS.json; do output=$( cat $i | jq -r 'select(.NAME != "<all>" and .PERMISSIONS != [] and .PERMISSIONS != null) | ( .NAME+":", "  "+(.PERMISSIONS[] | select(match("DATA_MINING"))) )'); if [ \! -z "$output" ]; then echo " ====== $i ====="; echo "$output"; fi; done|
+for i in *_USERS.json; do output=$( cat $i | jq -r 'select(.NAME != "<all>" and .PERMISSIONS != [] and .PERMISSIONS != null) | ( .NAME+":", "  "+(.PERMISSIONS[] | select(match("DATA_MINING"))) )'); if [ \! -z "$output" ]; then echo " ====== $i ====="; echo "$output"; fi; done
 ```
 reveals that there are a number of unqualified ``DATA_MINING`` permissions, one on bundesliga1-2019 which is just about to be archived, one on a recently archived server hwcs2020. And then there is a user "Chelsie2112" with unqualified ``SERVER:DATA_MINING`` access on the archive server. This is being changed now to a qualified ``SERVER:DATA_MINING:ARCHIVE``.
 
@@ -320,7 +320,7 @@ All objects marked to be added are copied from their source store to the target
 
 For ``UserGroup`` objects the set of role definitions will be merged, and the set of users will be merged. When merging the role definitions, if source and target group refer to the same RoleDefinition (equal IDs) and one role is granted for all users, the other only for members of the group, the merge result will grant the role for all users.
 
-For ``User`` objects the roles are merged in a "set" logic, avoiding duplicates. Comparison includes the group/user qualifications. Unqualified roles from the source object are ignored. The same applies for the permissions. Proper permission qualification particularly for ``SERVER`` operations needs to be ensured in a preparatory step. The default creation groups are merged by server name to which they apply. If both objects have a default creation group for the same server name, the importing side "wins." The e-mail validation status is set to ``true`` if at least one of the two objects has a validated e-mail address. Furthermore, a role referenced in the user's roles will only be merged if the role's ID exists in the target's role definitions  (which should be the case by the assumption above that no extensions were made anywhere to the ``ROLES`` collection). According to the research above, only ``user``, ``admin`` and ``moderator`` assignments seem to exist and those are pre-defined roles that will exist in the target with the same IDs. During a merge we won't touch the account and password information. The importing side's account data remains unchanged. Users affected by a merge will therefore end up with the username/password combination they have on the ARCHIVE server. Should that be different and should they have forgotten, they can still recover the password.
+For ``User`` objects the roles are merged in a "set" logic, avoiding duplicates. Comparison includes the group/user qualifications. Unqualified roles from the source object are ignored. The same applies for the permissions. Proper permission qualification particularly for ``SERVER`` operations needs to be ensured in a preparatory step. The default creation groups are merged by server name to which they apply. If both objects have a default creation group for the same server name, the importing side "wins." The e-mail validation status is set to ``true`` if at least one of the two objects has a validated e-mail address. Furthermore, a role referenced in the user's roles will only be merged if the role's ID exists in the target's role definitions  (which should be the case by the assumption above that no extensions were made anywhere to the ``ROLES`` collection). According to the research above, only ``user``, ``admin`` and ``moderator`` assignments seem to exist and those are pre-defined roles that will exist in the target with the same IDs. During a merge we won't touch the account and password information. The importing side's account data remains unchanged. Users affected by a merge will therefore end up with the username/password combination they have on the ARCHIVE server. Should that be different and should they have forgotten, they can still recover the password. If the importing side lacks optional field values for the User object for which valid values exist on the imported side, the valid values are used (company, full name, locale).
 
 The ``UserStore.getPreference(...)`` data can be merged based on user name for all users in the source store. If a preference key for a user already exists in the importing (target) side, the target wins, and the source key's value is dropped, and the situation is logged.