PR for #5073 - When creating stroom users via the screen or command CLI the values should be trimmed

stroom-core-client/src/main/java/stroom/security/client/presenter/CreateMultipleUsersPresenter.java

@@ -66,10 +66,10 @@ public void create(final Consumer<User> consumer,
                                     .filter(user -> !user.isEnabled())
                                     .collect(Collectors.toList());
                             final Optional<User> nextSelection = result.stream().findFirst();
-                            if (disabled.size() > 0) {
+                            if (!disabled.isEmpty()) {
                                 ConfirmEvent.fire(this,
                                         "Some deleted users already exist with the same names, " +
-                                                "would you like to restore them?",
+                                        "would you like to restore them?",
                                         ok -> {
                                             if (ok) {
                                                 enable(disabled, consumer, event, nextSelection, taskMonitorFactory);

stroom-core-shared/src/main/java/stroom/security/shared/User.java

@@ -3,6 +3,7 @@
 
 import stroom.util.shared.HasAuditInfo;
 import stroom.util.shared.HasIntegerId;
+import stroom.util.shared.NullSafe;
 import stroom.util.shared.UserRef;
 import stroom.util.shared.string.CaseType;
 
@@ -77,17 +78,18 @@ public User(@JsonProperty("id") final Integer id,
                 @JsonProperty("displayName") final String displayName,
                 @JsonProperty("fullName") final String fullName,
                 @JsonProperty("enabled") final boolean enabled) {
+        // Ensure we always have trimmed user identity values
         this.id = id;
         this.version = version;
         this.createTimeMs = createTimeMs;
         this.createUser = createUser;
         this.updateTimeMs = updateTimeMs;
         this.updateUser = updateUser;
-        this.subjectId = subjectId;
+        this.subjectId = NullSafe.get(subjectId, String::trim);
         this.uuid = uuid;
         this.group = group;
-        this.displayName = displayName;
-        this.fullName = fullName;
+        this.displayName = NullSafe.get(displayName, String::trim);
+        this.fullName = NullSafe.get(fullName, String::trim);
         this.enabled = enabled;
     }
 
@@ -167,7 +169,7 @@ public String getSubjectId() {
      * See {@link User#getSubjectId()}
      */
     public void setSubjectId(final String subjectId) {
-        this.subjectId = subjectId;
+        this.subjectId = NullSafe.get(subjectId, String::trim);
     }
 
     /**
@@ -185,7 +187,7 @@ public String getDisplayName() {
      * See {@link User#getDisplayName()}
      */
     public void setDisplayName(final String displayName) {
-        this.displayName = displayName;
+        this.displayName = NullSafe.get(displayName, String::trim);
     }
 
     /**
@@ -206,7 +208,7 @@ public String getFullName() {
      * See {@link User#getFullName()}
      */
     public void setFullName(final String fullName) {
-        this.fullName = fullName;
+        this.fullName = NullSafe.get(fullName, String::trim);
     }
 
     /**

stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/ExternalServiceUserFactory.java

@@ -48,7 +48,8 @@ public UserIdentity createServiceUserIdentity() {
         final OpenIdConfiguration openIdConfiguration = openIdConfigProvider.get();
         final UserIdentity serviceUserIdentity = new ServiceUserIdentity(
                 JwtUtil.getUniqueIdentity(openIdConfiguration, jwtClaims),
-                JwtUtil.getUserDisplayName(openIdConfiguration, jwtClaims).orElse(null),
+                JwtUtil.getUserDisplayName(openIdConfiguration, jwtClaims)
+                        .orElse(null),
                 updatableToken);
 
         // Associate the token with the user it is for
@@ -66,7 +67,7 @@ public boolean isServiceUser(final UserIdentity userIdentity,
         // Use instance equality check as there should only ever be one ServiceUserIdentity
         // in this JVM
         final boolean isServiceUserIdentity = userIdentity instanceof ServiceUserIdentity
-                && userIdentity == serviceUserIdentity;
+                                              && userIdentity == serviceUserIdentity;
         LOGGER.debug("isServiceUserIdentity: {}, userIdentity: {}, serviceUserIdentity: {}",
                 isServiceUserIdentity, userIdentity, serviceUserIdentity);
         return isServiceUserIdentity;

stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/JwtUtil.java

@@ -2,7 +2,6 @@
 
 import stroom.security.openid.api.OpenId;
 import stroom.security.openid.api.OpenIdConfiguration;
-import stroom.util.exception.ThrowingFunction;
 import stroom.util.logging.LambdaLogger;
 import stroom.util.logging.LambdaLoggerFactory;
 import stroom.util.logging.LogUtil;
@@ -87,6 +86,7 @@ public static String getUniqueIdentity(final OpenIdConfiguration openIdConfigura
         Objects.requireNonNull(openIdConfiguration);
         Objects.requireNonNull(jwtClaims);
         final String uniqueIdentityClaim = openIdConfiguration.getUniqueIdentityClaim();
+        // Trim so that the value is consistent with the users created in our DB
         final String id = JwtUtil.getClaimValue(jwtClaims, uniqueIdentityClaim)
                 .orElseThrow(() -> new RuntimeException(LogUtil.message(
                         "Expecting claims to contain configured uniqueIdentityClaim '{}' " +
@@ -108,7 +108,9 @@ public static Optional<String> getUserDisplayName(final OpenIdConfiguration open
         Objects.requireNonNull(openIdConfiguration);
         Objects.requireNonNull(jwtClaims);
         final String userDisplayNameClaim = openIdConfiguration.getUserDisplayNameClaim();
-        final Optional<String> userDisplayName = JwtUtil.getClaimValue(jwtClaims, userDisplayNameClaim);
+        // Trim so that the value is consistent with the users created in our DB
+        final Optional<String> userDisplayName = JwtUtil.getClaimValue(jwtClaims, userDisplayNameClaim)
+                .map(String::trim);
 
         LOGGER.debug("userDisplayNameClaim: {}, userDisplayName: {}", userDisplayNameClaim, userDisplayName);
 
@@ -171,25 +173,31 @@ public static String getSubject(final JwtClaims jwtClaims) {
         return subject;
     }
 
+    /**
+     * Get the trimmed value corresponding to claim
+     */
     public static Optional<String> getClaimValue(final JwtContext jwtContext, final String claim) {
-        try {
-            return NullSafe.getAsOptional(
-                    jwtContext,
-                    JwtContext::getJwtClaims,
-                    ThrowingFunction.unchecked(jwtClaims ->
-                            jwtClaims.getClaimValue(claim, String.class)));
-        } catch (final Exception e) {
-            LOGGER.debug(() -> LogUtil.message("Error getting claim {}: {}", claim, e.getMessage()), e);
-            return Optional.empty();
-        }
+        return Optional.ofNullable(jwtContext)
+                .map(JwtContext::getJwtClaims)
+                .flatMap(jwtClaims ->
+                        getClaimValue(jwtClaims, claim));
     }
 
+    /**
+     * Get the trimmed value corresponding to claim
+     */
     public static Optional<String> getClaimValue(final JwtClaims jwtClaims, final String claim) {
+        Objects.requireNonNull(claim);
         try {
-            return NullSafe.getAsOptional(
-                    jwtClaims,
-                    ThrowingFunction.unchecked(jwtClaims2 ->
-                            jwtClaims2.getClaimValue(claim, String.class)));
+            if (jwtClaims != null) {
+                final String value = jwtClaims.getClaimValue(claim, String.class);
+                final String trimmed = NullSafe.trim(value);
+                return !trimmed.isEmpty()
+                        ? Optional.of(trimmed)
+                        : Optional.empty();
+            } else {
+                return Optional.empty();
+            }
         } catch (final Exception e) {
             LOGGER.debug(() -> LogUtil.message("Error getting claim {}: {}", claim, e.getMessage()), e);
             return Optional.empty();

stroom-security/stroom-security-impl-db/src/main/resources/stroom/security/impl/db/migration/V07_10_00_005__trim_user_identities.sql

@@ -0,0 +1,45 @@
+-- ------------------------------------------------------------------------
+-- Copyright 2020 Crown Copyright
+--
+-- Licensed under the Apache License, Version 2.0 (the "License");
+-- you may not use this file except in compliance with the License.
+-- You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+-- ------------------------------------------------------------------------
+
+-- Stop NOTE level warnings about objects (not)? existing
+SET @OLD_SQL_NOTES=@@SQL_NOTES, SQL_NOTES=0;
+
+-- --------------------------------------------------
+
+-- Trim existing user identity values so they are consistent with the app
+-- that now trims these values.
+-- Idempotent.
+
+update stroom_user
+set name = trim(name)
+where name is not null
+and name != trim(name);
+
+update stroom_user
+set display_name = trim(display_name)
+where display_name is not null
+and display_name != trim(display_name);
+
+update stroom_user
+set full_name = trim(full_name)
+where full_name is not null
+and full_name != trim(full_name);
+
+-- --------------------------------------------------
+
+SET SQL_NOTES=@OLD_SQL_NOTES;
+
+-- vim: set tabstop=4 shiftwidth=4 expandtab:

stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/StroomUserIdentityFactory.java

@@ -124,8 +124,8 @@ protected Optional<UserIdentity> mapApiIdentity(final JwtContext jwtContext,
                                                     final HttpServletRequest request) {
 
         final String headerKey = UserIdentityFactory.RUN_AS_USER_HEADER;
-        final String runAsUserUuid = request.getHeader(headerKey);
-        if (!NullSafe.isBlankString(runAsUserUuid)) {
+        final String runAsUserUuid = NullSafe.trim(request.getHeader(headerKey));
+        if (!runAsUserUuid.isEmpty()) {
             // Request is proxying for a user, so it needs to be the processing user that
             // sent the request. Getting the proc user, even though we don't do anything with it will
             // ensure it is authenticated.

stroom-util-shared/src/main/java/stroom/util/shared/UserDesc.java

@@ -27,9 +27,10 @@ public class UserDesc {
     public UserDesc(@JsonProperty("subjectId") final String subjectId,
                     @JsonProperty("displayName") final String displayName,
                     @JsonProperty("fullName") final String fullName) {
-        this.subjectId = subjectId;
-        this.displayName = displayName;
-        this.fullName = fullName;
+        // Ensure there is no spurious leading/trailing space as these values may have come from user input
+        this.subjectId = NullSafe.get(subjectId, String::trim);
+        this.displayName = NullSafe.get(displayName, String::trim);
+        this.fullName = NullSafe.get(fullName, String::trim);
     }
 
     public static UserDesc forSubjectId(final String subjectId) {

unreleased_changes/20250820_141759_212__5073.md

@@ -0,0 +1,60 @@
+* Issue **#5073** : Trim the unique identity, display name and full name values for a user to ensure no leading/trailing spaces are stored. Includes DB migration `V07_10_00_005__trim_user_identities.sql` that trims existing values in the `name`, `display_name` and `full_name` columns of the `stroom_user` table.
+
+
+```sh
+# ********************************************************************************
+# Issue title: When creating stroom users via the screen or command CLI the values should be trimmed
+# Issue link:  https://github.com/gchq/stroom/issues/5073
+# ********************************************************************************
+
+# ONLY the top line will be included as a change entry in the CHANGELOG.
+# The entry should be in GitHub flavour markdown and should be written on a SINGLE
+# line with no hard breaks. You can have multiple change files for a single GitHub issue.
+# The  entry should be written in the imperative mood, i.e. 'Fix nasty bug' rather than
+# 'Fixed nasty bug'.
+#
+# Examples of acceptable entries are:
+#
+#
+# * Issue **123** : Fix bug with an associated GitHub issue in this repository
+#
+# * Issue **namespace/other-repo#456** : Fix bug with an associated GitHub issue in another repository
+#
+# * Fix bug with no associated GitHub issue.
+
+
+# --------------------------------------------------------------------------------
+# The following is random text to make this file unique for git's change detection
+# xDAtLg8YPrT0HXtVDudmLuR5iepbeaCzbUKZQaSYdxODS001a7wbDkXcOvrZ58ECEAZFwG53HfSWUOYC
+# uRTgvfmKxO2jNlWgflZubn25gNVhzjocuPCgz3nIenHlNb7er74tbLK6Xdco1RZdf4lPnOpqIvslGNNJ
+# 6F6Wl0GwuBfC5ZUZTS1XstabasYQ9pBMDGeDbVmLmbqdnGnMBugYktIjnk1yzuTmSMUU8kcuYgmLVKNM
+# HeQazwcqUAQhq2Zrl3eK3uYVixobM4i739aQ5ghfVKttNNbHZk4JWZ843YYLFtB21z4c2EFVXvQ13fCY
+# NOnLvssvuVTNmBlUvTqGXxMcRpZ2UNCfLEvyUsON54q3iFM5JTNBbabz9v0Jzc2XsRKBaFPbCafdT9Tg
+# H3qgGe2ajub6CnRbfMW8NJZvEabA3U3iUSsa0fIxFEXy18hrNNbd72RYCaPjohQ9bTNZqXVIZSiuJSWK
+# HY4m0xanX1JIbeaXcoueVS66exhynKS5eRUhyLpMmAcuiJd7R8i5PLssbRr4xRcy3boWdvDHF2C75hbx
+# O42sbDoGUB81jQXZW22TyeOhPxanHf5Za5g0u6PUeecBjnYmYNu3mLHvqz3SU7JkG1cC6k5aLaMBlOdP
+# dUqzNCgv7or8FCcfT4UcOcFgesGDJ4vq5v9v6FD0v4oZ6x7BHzmXPiHQRQf1U0i67RQwP3vUzqH91G7w
+# Z4DgLKnsFh0deYe69I8wAQS3uYqbmhHwWTSq9bbhtxPaIkpuhJPGx4WgHv1aVEThK9CPJ6s8MWOEOPMp
+# 6eNhxCmftkeoAES0Je7OjRG2dHovW2a3zTYqezw9ARhjUWnnckIyB3L5gMyOLSv0EwHjVJx0HmrxaQ8W
+# XSvK9PRaM4ZxLaDqT977wb8EuLOm4G4nK7WnKUQ0NNcK8Bg9QJjVFQTQuBO2HhutYKxXRy4UuVzddKh8
+# sGjZmj6l7FELg1x0OR0oAOgw86BDXDDnURc5ASXhSPIzP4Rqh4BpB0wdzWWDUBeN7wjnG1GKPHNFf7dC
+# zl4q9yZqS0LTen9vpjO0Up7c5kRchLtW80pp1FnqqXQQBQUvPtagPvLu7elt5QJuuOHLtDrSappOA4Vh
+# TzpbmUAYCUczSqPEbxWtqcVaPcqxNrahZMpIgrMbgMpqy8ryY9sPKUyMPdBRXJBN9QJAw6d7uRVnxmaR
+# kny4IW9Ce8jEB4jv3yky1UgsqOr10Vw3PY87wBdQbF7DFEBODnCzZa9yZAV1ARJio61PlvYXNvJb8Pnt
+# OaxPBR9EoOTJpIYPG27NVuciVxzMG4jZVaHb3k0wY8Cc0NHJxTo9We4HX6psU7TPFAxkdRmTfskEVevv
+# xZE6AHAoiBT20FzDcp700xVKIF5rr4oQS5lofXxqPya92Q3AGVf0iRsFFJ6nXfFY59yRRty2ZuyTUzuN
+# P0MgAt2PpAuyG5JPrSBWr6v7X3OG4N3GlPoxfG0x8LM2egbePGADy7WPiTgBeOaPfsu0cKwKePSCZwmU
+# uAEJj1tG5MhofTGEfNyavpJBy6Tvdj4yFVA2d2QycsiKzDt6e0ht3V2Is22w3GJtmuaIIcDEFeT4RiXD
+# fGXyPcKX9tF9deTuxB3Cd0nM0Ua8QudPioBMaOdZ1TT077tOTIURnG4EiTnZ71Zl9s8kHRHgXRMICBrH
+# spAznsasmWjcrlyuXuT3fs7X9EglyrRBdfyAxq0cYyVEQ8wdUNYb6hVIE0sf3RI2KxumAZsE6k9JuztO
+# 67s2OBljbM0cVA9DEqv2f7EC6qS7GVVe7lXwFlSnFtaNFvhRWt6F3XAYXBGuC5VhQUmLqUD5nf7DsBg1
+# 5lsGxD6g0RYSq2RnqtiueqT1rYY4cxg3DHJeuq2uxP8QnTKb3FmxtKuLf9LL0AIy3S7F7xyKDmLROPBf
+# ozEPZzC5A40xnmoAY04XI0r2mVu9oCSmFDNAbTvFh7fUngkBHqzT7Pka1NwIrtJrTeHwxNxejsFk3ztF
+# FmEqFwZxtPzKJv7XwlBJZRZhIEWTLNGGdqfAAdzhgSSHQku2jdBWzhljLHFrp0JB3P3Mc66XgC5ytOMP
+# PsqAZBJLcwbQvgHbI84oGlm4WYHoEEX5gf7KgyB5Uob72N5PTtHunnKILdKkZhneHB10lKVRTpd1VpUm
+# 8oVDhfIiQtiMODQ5AQ19C3gaImL113hp8xjRo0GNUTuuafDCyuK7RzdCtRooMPmEyYGoJTdwL68mVkX0
+# rak16EAW9RRd2BwWj9rFMMh3cnZsnJH8j63tiAGR7ibEJPLpNZVbMoS52DO8Z6pdxq4vfveHuvZZOgez
+# 5IcHgj8nhSc8bpaHMIybAyB8Z5o8YiwLll8z0DLiQS5OiwQbck9jdud3IcAqt3Befbq4mFzPckzu6HrA
+# --------------------------------------------------------------------------------
+
+```