Skip to content

Commit fee1e61

Browse files
at055612at055612
committed
gh-5073 Trim user identity values
1 parent 8983614 commit fee1e61

File tree

7 files changed

+96
-26
lines changed

7 files changed

+96
-26
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,10 @@ public void create(final Consumer<User> consumer,
6666
.filter(user -> !user.isEnabled())
6767
.collect(Collectors.toList());
6868
final Optional<User> nextSelection = result.stream().findFirst();
69-
if (disabled.size() > 0) {
69+
if (!disabled.isEmpty()) {
7070
ConfirmEvent.fire(this,
7171
"Some deleted users already exist with the same names, " +
72-
"would you like to restore them?",
72+
"would you like to restore them?",
7373
ok -> {
7474
if (ok) {
7575
enable(disabled, consumer, event, nextSelection, taskMonitorFactory);

‎stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/AbstractUserIdentityFactory.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ protected Optional<String> getUserFullName(final OpenIdConfiguration openIdConfi
424424
() -> {
425425
if (NullSafe.isNonBlankString(claim)) {
426426
return JwtUtil.getClaimValue(jwtClaims, claim)
427-
.filter(NullSafe::isNonBlankString)
427+
.map(NullSafe::trim)
428428
.orElse("");
429429
} else {
430430
return "";

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ public UserIdentity createServiceUserIdentity() {
4848
final OpenIdConfiguration openIdConfiguration = openIdConfigProvider.get();
4949
final UserIdentity serviceUserIdentity = new ServiceUserIdentity(
5050
JwtUtil.getUniqueIdentity(openIdConfiguration, jwtClaims),
51-
JwtUtil.getUserDisplayName(openIdConfiguration, jwtClaims).orElse(null),
51+
JwtUtil.getUserDisplayName(openIdConfiguration, jwtClaims)
52+
.orElse(null),
5253
updatableToken);
5354

5455
// Associate the token with the user it is for
@@ -66,7 +67,7 @@ public boolean isServiceUser(final UserIdentity userIdentity,
6667
// Use instance equality check as there should only ever be one ServiceUserIdentity
6768
// in this JVM
6869
final boolean isServiceUserIdentity = userIdentity instanceof ServiceUserIdentity
69-
&& userIdentity == serviceUserIdentity;
70+
&& userIdentity == serviceUserIdentity;
7071
LOGGER.debug("isServiceUserIdentity: {}, userIdentity: {}, serviceUserIdentity: {}",
7172
isServiceUserIdentity, userIdentity, serviceUserIdentity);
7273
return isServiceUserIdentity;

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

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import stroom.security.openid.api.OpenId;
44
import stroom.security.openid.api.OpenIdConfiguration;
5-
import stroom.util.exception.ThrowingFunction;
65
import stroom.util.logging.LambdaLogger;
76
import stroom.util.logging.LambdaLoggerFactory;
87
import stroom.util.logging.LogUtil;
@@ -87,6 +86,7 @@ public static String getUniqueIdentity(final OpenIdConfiguration openIdConfigura
8786
Objects.requireNonNull(openIdConfiguration);
8887
Objects.requireNonNull(jwtClaims);
8988
final String uniqueIdentityClaim = openIdConfiguration.getUniqueIdentityClaim();
89+
// Trim so that the value is consistent with the users created in our DB
9090
final String id = JwtUtil.getClaimValue(jwtClaims, uniqueIdentityClaim)
9191
.orElseThrow(() -> new RuntimeException(LogUtil.message(
9292
"Expecting claims to contain configured uniqueIdentityClaim '{}' " +
@@ -108,7 +108,9 @@ public static Optional<String> getUserDisplayName(final OpenIdConfiguration open
108108
Objects.requireNonNull(openIdConfiguration);
109109
Objects.requireNonNull(jwtClaims);
110110
final String userDisplayNameClaim = openIdConfiguration.getUserDisplayNameClaim();
111-
final Optional<String> userDisplayName = JwtUtil.getClaimValue(jwtClaims, userDisplayNameClaim);
111+
// Trim so that the value is consistent with the users created in our DB
112+
final Optional<String> userDisplayName = JwtUtil.getClaimValue(jwtClaims, userDisplayNameClaim)
113+
.map(String::trim);
112114

113115
LOGGER.debug("userDisplayNameClaim: {}, userDisplayName: {}", userDisplayNameClaim, userDisplayName);
114116

@@ -171,25 +173,31 @@ public static String getSubject(final JwtClaims jwtClaims) {
171173
return subject;
172174
}
173175

176+
/**
177+
* Get the trimmed value corresponding to claim
178+
*/
174179
public static Optional<String> getClaimValue(final JwtContext jwtContext, final String claim) {
175-
try {
176-
return NullSafe.getAsOptional(
177-
jwtContext,
178-
JwtContext::getJwtClaims,
179-
ThrowingFunction.unchecked(jwtClaims ->
180-
jwtClaims.getClaimValue(claim, String.class)));
181-
} catch (final Exception e) {
182-
LOGGER.debug(() -> LogUtil.message("Error getting claim {}: {}", claim, e.getMessage()), e);
183-
return Optional.empty();
184-
}
180+
return Optional.ofNullable(jwtContext)
181+
.map(JwtContext::getJwtClaims)
182+
.flatMap(jwtClaims ->
183+
getClaimValue(jwtClaims, claim));
185184
}
186185

186+
/**
187+
* Get the trimmed value corresponding to claim
188+
*/
187189
public static Optional<String> getClaimValue(final JwtClaims jwtClaims, final String claim) {
190+
Objects.requireNonNull(claim);
188191
try {
189-
return NullSafe.getAsOptional(
190-
jwtClaims,
191-
ThrowingFunction.unchecked(jwtClaims2 ->
192-
jwtClaims2.getClaimValue(claim, String.class)));
192+
if (jwtClaims != null) {
193+
final String value = jwtClaims.getClaimValue(claim, String.class);
194+
final String trimmed = NullSafe.trim(value);
195+
return !trimmed.isEmpty()
196+
? Optional.of(trimmed)
197+
: Optional.empty();
198+
} else {
199+
return Optional.empty();
200+
}
193201
} catch (final Exception e) {
194202
LOGGER.debug(() -> LogUtil.message("Error getting claim {}: {}", claim, e.getMessage()), e);
195203
return Optional.empty();

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,8 @@ protected Optional<UserIdentity> mapApiIdentity(final JwtContext jwtContext,
124124
final HttpServletRequest request) {
125125

126126
final String headerKey = UserIdentityFactory.RUN_AS_USER_HEADER;
127-
final String runAsUserUuid = request.getHeader(headerKey);
128-
if (!NullSafe.isBlankString(runAsUserUuid)) {
127+
final String runAsUserUuid = NullSafe.trim(request.getHeader(headerKey));
128+
if (!runAsUserUuid.isEmpty()) {
129129
// Request is proxying for a user, so it needs to be the processing user that
130130
// sent the request. Getting the proc user, even though we don't do anything with it will
131131
// ensure it is authenticated.

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,10 @@ public class UserDesc {
2727
public UserDesc(@JsonProperty("subjectId") final String subjectId,
2828
@JsonProperty("displayName") final String displayName,
2929
@JsonProperty("fullName") final String fullName) {
30-
this.subjectId = subjectId;
31-
this.displayName = displayName;
32-
this.fullName = fullName;
30+
// Ensure there is no spurious leading/trailing space as these values may have come from user input
31+
this.subjectId = NullSafe.get(subjectId, String::trim);
32+
this.displayName = NullSafe.get(displayName, String::trim);
33+
this.fullName = NullSafe.get(fullName, String::trim);
3334
}
3435

3536
public static UserDesc forSubjectId(final String subjectId) {

‎unreleased_changes/20250820_141759_212__5073.md‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
* Issue **#5073** : Trim the unique identity, display name and full name values for a user to ensure no leading/trailing spaces are stored.
2+
3+
4+
```sh
5+
# ********************************************************************************
6+
# Issue title: When creating stroom users via the screen or command CLI the values should be trimmed
7+
# Issue link: https://github.com/gchq/stroom/issues/5073
8+
# ********************************************************************************
9+
10+
# ONLY the top line will be included as a change entry in the CHANGELOG.
11+
# The entry should be in GitHub flavour markdown and should be written on a SINGLE
12+
# line with no hard breaks. You can have multiple change files for a single GitHub issue.
13+
# The entry should be written in the imperative mood, i.e. 'Fix nasty bug' rather than
14+
# 'Fixed nasty bug'.
15+
#
16+
# Examples of acceptable entries are:
17+
#
18+
#
19+
# * Issue **123** : Fix bug with an associated GitHub issue in this repository
20+
#
21+
# * Issue **namespace/other-repo#456** : Fix bug with an associated GitHub issue in another repository
22+
#
23+
# * Fix bug with no associated GitHub issue.
24+
25+
26+
# --------------------------------------------------------------------------------
27+
# The following is random text to make this file unique for git's change detection
28+
# xDAtLg8YPrT0HXtVDudmLuR5iepbeaCzbUKZQaSYdxODS001a7wbDkXcOvrZ58ECEAZFwG53HfSWUOYC
29+
# uRTgvfmKxO2jNlWgflZubn25gNVhzjocuPCgz3nIenHlNb7er74tbLK6Xdco1RZdf4lPnOpqIvslGNNJ
30+
# 6F6Wl0GwuBfC5ZUZTS1XstabasYQ9pBMDGeDbVmLmbqdnGnMBugYktIjnk1yzuTmSMUU8kcuYgmLVKNM
31+
# HeQazwcqUAQhq2Zrl3eK3uYVixobM4i739aQ5ghfVKttNNbHZk4JWZ843YYLFtB21z4c2EFVXvQ13fCY
32+
# NOnLvssvuVTNmBlUvTqGXxMcRpZ2UNCfLEvyUsON54q3iFM5JTNBbabz9v0Jzc2XsRKBaFPbCafdT9Tg
33+
# H3qgGe2ajub6CnRbfMW8NJZvEabA3U3iUSsa0fIxFEXy18hrNNbd72RYCaPjohQ9bTNZqXVIZSiuJSWK
34+
# HY4m0xanX1JIbeaXcoueVS66exhynKS5eRUhyLpMmAcuiJd7R8i5PLssbRr4xRcy3boWdvDHF2C75hbx
35+
# O42sbDoGUB81jQXZW22TyeOhPxanHf5Za5g0u6PUeecBjnYmYNu3mLHvqz3SU7JkG1cC6k5aLaMBlOdP
36+
# dUqzNCgv7or8FCcfT4UcOcFgesGDJ4vq5v9v6FD0v4oZ6x7BHzmXPiHQRQf1U0i67RQwP3vUzqH91G7w
37+
# Z4DgLKnsFh0deYe69I8wAQS3uYqbmhHwWTSq9bbhtxPaIkpuhJPGx4WgHv1aVEThK9CPJ6s8MWOEOPMp
38+
# 6eNhxCmftkeoAES0Je7OjRG2dHovW2a3zTYqezw9ARhjUWnnckIyB3L5gMyOLSv0EwHjVJx0HmrxaQ8W
39+
# XSvK9PRaM4ZxLaDqT977wb8EuLOm4G4nK7WnKUQ0NNcK8Bg9QJjVFQTQuBO2HhutYKxXRy4UuVzddKh8
40+
# sGjZmj6l7FELg1x0OR0oAOgw86BDXDDnURc5ASXhSPIzP4Rqh4BpB0wdzWWDUBeN7wjnG1GKPHNFf7dC
41+
# zl4q9yZqS0LTen9vpjO0Up7c5kRchLtW80pp1FnqqXQQBQUvPtagPvLu7elt5QJuuOHLtDrSappOA4Vh
42+
# TzpbmUAYCUczSqPEbxWtqcVaPcqxNrahZMpIgrMbgMpqy8ryY9sPKUyMPdBRXJBN9QJAw6d7uRVnxmaR
43+
# kny4IW9Ce8jEB4jv3yky1UgsqOr10Vw3PY87wBdQbF7DFEBODnCzZa9yZAV1ARJio61PlvYXNvJb8Pnt
44+
# OaxPBR9EoOTJpIYPG27NVuciVxzMG4jZVaHb3k0wY8Cc0NHJxTo9We4HX6psU7TPFAxkdRmTfskEVevv
45+
# xZE6AHAoiBT20FzDcp700xVKIF5rr4oQS5lofXxqPya92Q3AGVf0iRsFFJ6nXfFY59yRRty2ZuyTUzuN
46+
# P0MgAt2PpAuyG5JPrSBWr6v7X3OG4N3GlPoxfG0x8LM2egbePGADy7WPiTgBeOaPfsu0cKwKePSCZwmU
47+
# uAEJj1tG5MhofTGEfNyavpJBy6Tvdj4yFVA2d2QycsiKzDt6e0ht3V2Is22w3GJtmuaIIcDEFeT4RiXD
48+
# fGXyPcKX9tF9deTuxB3Cd0nM0Ua8QudPioBMaOdZ1TT077tOTIURnG4EiTnZ71Zl9s8kHRHgXRMICBrH
49+
# spAznsasmWjcrlyuXuT3fs7X9EglyrRBdfyAxq0cYyVEQ8wdUNYb6hVIE0sf3RI2KxumAZsE6k9JuztO
50+
# 67s2OBljbM0cVA9DEqv2f7EC6qS7GVVe7lXwFlSnFtaNFvhRWt6F3XAYXBGuC5VhQUmLqUD5nf7DsBg1
51+
# 5lsGxD6g0RYSq2RnqtiueqT1rYY4cxg3DHJeuq2uxP8QnTKb3FmxtKuLf9LL0AIy3S7F7xyKDmLROPBf
52+
# ozEPZzC5A40xnmoAY04XI0r2mVu9oCSmFDNAbTvFh7fUngkBHqzT7Pka1NwIrtJrTeHwxNxejsFk3ztF
53+
# FmEqFwZxtPzKJv7XwlBJZRZhIEWTLNGGdqfAAdzhgSSHQku2jdBWzhljLHFrp0JB3P3Mc66XgC5ytOMP
54+
# PsqAZBJLcwbQvgHbI84oGlm4WYHoEEX5gf7KgyB5Uob72N5PTtHunnKILdKkZhneHB10lKVRTpd1VpUm
55+
# 8oVDhfIiQtiMODQ5AQ19C3gaImL113hp8xjRo0GNUTuuafDCyuK7RzdCtRooMPmEyYGoJTdwL68mVkX0
56+
# rak16EAW9RRd2BwWj9rFMMh3cnZsnJH8j63tiAGR7ibEJPLpNZVbMoS52DO8Z6pdxq4vfveHuvZZOgez
57+
# 5IcHgj8nhSc8bpaHMIybAyB8Z5o8YiwLll8z0DLiQS5OiwQbck9jdud3IcAqt3Befbq4mFzPckzu6HrA
58+
# --------------------------------------------------------------------------------
59+
60+
```

0 commit comments

Comments
 (0)