Skip to content

Commit eab026b

Browse files
jglickjglick
committed
Deprecate UserIdMapper and just use an HMAC
1 parent 78f3e0f commit eab026b

File tree

44 files changed

+172
-1160
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+172
-1160
lines changed

‎core/src/main/java/hudson/model/User.java‎

Lines changed: 107 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,15 @@
5252
import jakarta.servlet.http.HttpServletResponse;
5353
import java.io.File;
5454
import java.io.IOException;
55+
import java.nio.file.Files;
56+
import java.nio.file.StandardCopyOption;
5557
import java.util.ArrayList;
5658
import java.util.Arrays;
5759
import java.util.Collection;
5860
import java.util.Collections;
5961
import java.util.HashSet;
6062
import java.util.List;
63+
import java.util.Locale;
6164
import java.util.Map;
6265
import java.util.Objects;
6366
import java.util.Set;
@@ -67,12 +70,14 @@
6770
import java.util.function.Predicate;
6871
import java.util.logging.Level;
6972
import java.util.logging.Logger;
73+
import java.util.regex.Pattern;
7074
import jenkins.model.IdStrategy;
7175
import jenkins.model.Jenkins;
7276
import jenkins.model.Loadable;
7377
import jenkins.model.ModelObjectWithContextMenu;
7478
import jenkins.scm.RunWithSCM;
7579
import jenkins.search.SearchGroup;
80+
import jenkins.security.HMACConfidentialKey;
7681
import jenkins.security.ImpersonatingUserDetailsService2;
7782
import jenkins.security.LastGrantedAuthoritiesProperty;
7883
import jenkins.security.UserDetailsCache;
@@ -174,7 +179,7 @@ public class User extends AbstractModelObject implements AccessControlled, Descr
174179

175180
@SuppressFBWarnings(value = "SS_SHOULD_BE_STATIC", justification = "Reserved for future use")
176181
private final int version = 10; // Not currently used, but it may be helpful in the future to store a version.
177-
private String id;
182+
String id;
178183
private volatile String fullName;
179184
private volatile String description;
180185

@@ -185,6 +190,8 @@ public class User extends AbstractModelObject implements AccessControlled, Descr
185190
XSTREAM.alias("user", User.class);
186191
}
187192

193+
private User() {}
194+
188195
private User(String id, String fullName) {
189196
this.id = id;
190197
this.fullName = fullName;
@@ -199,6 +206,10 @@ public void load() {
199206
private void load(String userId) {
200207
clearExistingProperties();
201208
loadFromUserConfigFile(userId);
209+
fixUpAfterLoad();
210+
}
211+
212+
private void fixUpAfterLoad() {
202213
removeNullsThatFailedToLoad();
203214
allocateDefaultPropertyInstancesAsNeeded();
204215
setUserToProperties();
@@ -227,7 +238,7 @@ private void removeNullsThatFailedToLoad() {
227238
private void loadFromUserConfigFile(String userId) {
228239
XmlFile config = getConfigFile();
229240
try {
230-
if (config != null && config.exists()) {
241+
if (config.exists()) {
231242
config.unmarshal(this);
232243
this.id = userId;
233244
}
@@ -241,8 +252,7 @@ private void clearExistingProperties() {
241252
}
242253

243254
private XmlFile getConfigFile() {
244-
File existingUserFolder = getExistingUserFolder();
245-
return existingUserFolder == null ? null : new XmlFile(XSTREAM, new File(existingUserFolder, CONFIG_XML));
255+
return new XmlFile(XSTREAM, new File(getUserFolderFor(id), CONFIG_XML));
246256
}
247257

248258
/**
@@ -571,10 +581,10 @@ public void doSubmitDescription(StaplerRequest2 req, StaplerResponse2 rsp) throw
571581
*/
572582
private static @Nullable User getOrCreateById(@NonNull String id, @NonNull String fullName, boolean create) {
573583
User u = AllUsers.get(id);
574-
if (u == null && (create || UserIdMapper.getInstance().isMapped(id))) {
584+
if (u == null && create) {
575585
u = new User(id, fullName);
576586
AllUsers.put(id, u);
577-
if (!id.equals(fullName) && !UserIdMapper.getInstance().isMapped(id)) {
587+
if (!id.equals(fullName)) {
578588
try {
579589
u.save();
580590
} catch (IOException x) {
@@ -691,7 +701,6 @@ public void doSubmitDescription(StaplerRequest2 req, StaplerResponse2 rsp) throw
691701
*/
692702
@Restricted(Beta.class)
693703
public static void reload() throws IOException {
694-
UserIdMapper.getInstance().reload();
695704
AllUsers.reload();
696705
}
697706

@@ -708,6 +717,32 @@ public static void rekey() {
708717
or greater issues in the realm change, could affect currently logged
709718
in users and even the user making the change. */
710719
try {
720+
var subdirectories = getRootDir().listFiles();
721+
if (subdirectories != null) {
722+
for (var oldDirectory : subdirectories) {
723+
var dirName = oldDirectory.getName();
724+
if (!HASHED_DIRNAMES.matcher(dirName).matches()) {
725+
continue;
726+
}
727+
var xml = new XmlFile(XSTREAM, new File(oldDirectory, CONFIG_XML));
728+
if (!xml.exists()) {
729+
continue;
730+
}
731+
try {
732+
var user = (User) xml.read();
733+
if (user.id == null) {
734+
continue;
735+
}
736+
var newDirectory = getUserFolderFor(user.id);
737+
if (!oldDirectory.equals(newDirectory)) {
738+
Files.move(oldDirectory.toPath(), newDirectory.toPath(), StandardCopyOption.REPLACE_EXISTING);
739+
LOGGER.info(() -> "migrated " + oldDirectory + " to " + newDirectory);
740+
}
741+
} catch (Exception x) {
742+
LOGGER.log(Level.WARNING, "failed to migrate " + xml, x);
743+
}
744+
}
745+
}
711746
reload();
712747
} catch (IOException e) {
713748
LOGGER.log(Level.SEVERE, "Failed to perform rekey operation.", e);
@@ -777,17 +812,9 @@ public static void clear() {
777812
if (ExtensionList.lookup(AllUsers.class).isEmpty()) {
778813
return;
779814
}
780-
UserIdMapper.getInstance().clear();
781815
AllUsers.clear();
782816
}
783817

784-
private static File getConfigFileFor(String id) {
785-
return new File(getUserFolderFor(id), "config.xml");
786-
}
787-
788-
private static File getUserFolderFor(String id) {
789-
return new File(getRootDir(), idStrategy().filenameOf(id));
790-
}
791818
/**
792819
* Returns the folder that store all the user information.
793820
* Useful for plugins to save a user-specific file aside the config.xml.
@@ -799,11 +826,8 @@ private static File getUserFolderFor(String id) {
799826
*/
800827

801828
public @CheckForNull File getUserFolder() {
802-
return getExistingUserFolder();
803-
}
804-
805-
private @CheckForNull File getExistingUserFolder() {
806-
return UserIdMapper.getInstance().getDirectory(id);
829+
var d = getUserFolderFor(id);
830+
return d.isDirectory() ? d : null;
807831
}
808832

809833
/**
@@ -813,6 +837,21 @@ static File getRootDir() {
813837
return new File(Jenkins.get().getRootDir(), "users");
814838
}
815839

840+
private static final int PREFIX_MAX = 14;
841+
private static final Pattern DISALLOWED_PREFIX_CHARS = Pattern.compile("[^A-Za-z0-9]");
842+
static final Pattern HASHED_DIRNAMES = Pattern.compile("[a-z0-9]{0," + PREFIX_MAX + "}_[a-f0-9]{64}");
843+
private static final HMACConfidentialKey DIRNAMES = new HMACConfidentialKey(User.class, "DIRNAMES");
844+
845+
private static String getUserFolderNameFor(String id) {
846+
var fullPrefix = DISALLOWED_PREFIX_CHARS.matcher(id).replaceAll("").toLowerCase(Locale.ROOT);
847+
return (fullPrefix.length() > PREFIX_MAX ? fullPrefix.substring(0, PREFIX_MAX) : fullPrefix) + '_' + DIRNAMES.mac(idStrategy().keyFor(id));
848+
}
849+
850+
@SuppressFBWarnings(value = "PATH_TRAVERSAL_IN", justification = "sanitized")
851+
static File getUserFolderFor(String id) {
852+
return new File(getRootDir(), getUserFolderNameFor(id));
853+
}
854+
816855
/**
817856
* Is the ID allowed? Some are prohibited for security reasons. See SECURITY-166.
818857
* <p>
@@ -852,39 +891,23 @@ public synchronized void save() throws IOException {
852891
if (BulkChange.contains(this)) {
853892
return;
854893
}
855-
XmlFile xmlFile = new XmlFile(XSTREAM, constructUserConfigFile());
894+
XmlFile xmlFile = getConfigFile();
856895
xmlFile.write(this);
857896
SaveableListener.fireOnChange(this, xmlFile);
858897
}
859898

860-
private File constructUserConfigFile() throws IOException {
861-
return new File(putUserFolderIfAbsent(), CONFIG_XML);
862-
}
863-
864-
private File putUserFolderIfAbsent() throws IOException {
865-
return UserIdMapper.getInstance().putIfAbsent(id, true);
866-
}
867-
868899
/**
869900
* Deletes the data directory and removes this user from Hudson.
870901
*
871902
* @throws IOException if we fail to delete.
872903
*/
873904
public void delete() throws IOException {
874905
String idKey = idStrategy().keyFor(id);
875-
File existingUserFolder = getExistingUserFolder();
876-
UserIdMapper.getInstance().remove(id);
877906
AllUsers.remove(id);
878-
deleteExistingUserFolder(existingUserFolder);
907+
Util.deleteRecursive(getUserFolderFor(id));
879908
UserDetailsCache.get().invalidate(idKey);
880909
}
881910

882-
private void deleteExistingUserFolder(File existingUserFolder) throws IOException {
883-
if (existingUserFolder != null && existingUserFolder.exists()) {
884-
Util.deleteRecursive(existingUserFolder);
885-
}
886-
}
887-
888911
/**
889912
* Exposed remote API.
890913
*/
@@ -947,7 +970,7 @@ public ACL getACL() {
947970
public boolean canDelete() {
948971
final IdStrategy strategy = idStrategy();
949972
return hasPermission(Jenkins.ADMINISTER) && !strategy.equals(id, Jenkins.getAuthentication2().getName())
950-
&& UserIdMapper.getInstance().isMapped(id);
973+
&& getUserFolder() != null;
951974
}
952975

953976
/**
@@ -1077,11 +1100,50 @@ public static final class AllUsers {
10771100
private final ConcurrentMap<String, User> byName = new ConcurrentHashMap<>();
10781101

10791102
@Initializer(after = InitMilestone.JOB_CONFIG_ADAPTED)
1080-
public static void scanAll() {
1081-
for (String userId : UserIdMapper.getInstance().getConvertedUserIds()) {
1082-
User user = new User(userId, userId);
1083-
getInstance().byName.putIfAbsent(idStrategy().keyFor(userId), user);
1103+
public static void scanAll() throws IOException {
1104+
DIRNAMES.createMac(); // force the key to be saved during startup
1105+
var subdirectories = getRootDir().listFiles();
1106+
if (subdirectories == null) {
1107+
return;
1108+
}
1109+
var byName = getInstance().byName;
1110+
var idStrategy = idStrategy();
1111+
for (var dir : subdirectories) {
1112+
var dirName = dir.getName();
1113+
if (!HASHED_DIRNAMES.matcher(dirName).matches()) {
1114+
LOGGER.fine(() -> "ignoring unrecognized dir " + dir);
1115+
continue;
1116+
}
1117+
var xml = new XmlFile(XSTREAM, new File(dir, CONFIG_XML));
1118+
if (!xml.exists()) {
1119+
LOGGER.fine(() -> "ignoring dir " + dir + " with no " + CONFIG_XML);
1120+
continue;
1121+
}
1122+
var user = new User();
1123+
try {
1124+
xml.unmarshal(user);
1125+
} catch (Exception x) {
1126+
LOGGER.log(Level.WARNING, "failed to load " + xml, x);
1127+
continue;
1128+
}
1129+
if (user.id == null) {
1130+
LOGGER.warning(() -> "ignoring " + xml + " with no <id>");
1131+
continue;
1132+
}
1133+
var expectedFolderName = getUserFolderNameFor(user.id);
1134+
if (!dirName.equals(expectedFolderName)) {
1135+
LOGGER.warning(() -> "ignoring " + xml + " with <id> " + user.id + " expected to be in " + expectedFolderName);
1136+
continue;
1137+
}
1138+
user.fixUpAfterLoad();
1139+
var old = byName.put(idStrategy.keyFor(user.id), user);
1140+
if (old != null) {
1141+
LOGGER.warning(() -> "duplicate entries for " + user.id);
1142+
} else {
1143+
LOGGER.fine(() -> "successfully loaded " + user.id + " from " + xml);
1144+
}
10841145
}
1146+
LOGGER.fine(() -> "loaded " + byName.size() + " entries");
10851147
}
10861148

10871149
/**
@@ -1094,7 +1156,7 @@ private static AllUsers getInstance() {
10941156
return ExtensionList.lookupSingleton(AllUsers.class);
10951157
}
10961158

1097-
private static void reload() {
1159+
private static void reload() throws IOException {
10981160
getInstance().byName.clear();
10991161
UserDetailsCache.get().invalidateAll();
11001162
scanAll();
@@ -1252,7 +1314,7 @@ public String resolveCanonicalId(String idOrFullName, Map<String, ?> context) {
12521314
UserDetails userDetails = UserDetailsCache.get().loadUserByUsername(idOrFullName);
12531315
return userDetails.getUsername();
12541316
} catch (UsernameNotFoundException x) {
1255-
LOGGER.log(Level.FINE, "not sure whether " + idOrFullName + " is a valid username or not", x);
1317+
LOGGER.log(Level.FINER, "not sure whether " + idOrFullName + " is a valid username or not", x);
12561318
} catch (ExecutionException x) {
12571319
LOGGER.log(Level.FINE, "could not look up " + idOrFullName, x);
12581320
} finally {

0 commit comments

Comments
 (0)