Deprecate `UserIdMapper` and just use an HMAC by jglick · Pull Request #10926 · jenkinsci/jenkins

jglick · 2025-08-06T11:28:50Z

Subsumes both #10870 and #10871, following @daniel-beck’s suggestion recorded in #10871 (comment). With this change, the logic of managing Users is simplified in most respects: every id gets a predictable directory name determined by a secure hash of the current IdStrategy and a per-controller secret, so no index is needed. The directory names are also strictly sanitized against dangerous characters (as UserIdMapper did), and it should not be possible to construct an id which would hash to a given directory name.

Testing done

A 2.516.x version passes CloudBees CI PCT & ATH. 2d18199 passed bom PCT. Light interactive testing with mock-security-realm.

Proposed changelog entries

Pick a new name format for subdirectories of $JENKINS_HOME/users/.
Stop creating redundant $JENKINS_HOME/users/users.xml.

Proposed changelog category

/label removed

Proposed upgrade guidelines

No action is necessary when upgrading. Downgrading past this version (without restoring from backup) may lose user records including configuration such as API tokens.

Maintainer checklist

There are at least two (2) approvals for the pull request and no outstanding requests for change.
Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
Proper changelog labels are set so that the changelog can be generated automatically.
If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

jglick · 2025-08-06T11:32:49Z

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

            or greater issues in the realm change, could affect currently logged
            in users and even the user making the change. */


Not sure rekey has any real test coverage.

jenkins/test/src/test/java/hudson/model/UserTest.java

Lines 223 to 266 in eab026b

@Test

void caseInsensitivity() {

j.jenkins.setSecurityRealm(new IdStrategySpecifyingSecurityRealm(new IdStrategy.CaseInsensitive()));

User user = User.get("john smith");

User user2 = User.get("John Smith");

assertSame(user.getId(), user2.getId(), "Users should have the same id.");

}

@Test

void caseSensitivity() {

j.jenkins.setSecurityRealm(new IdStrategySpecifyingSecurityRealm(new IdStrategy.CaseSensitive()));

User user = User.get("john smith");

User user2 = User.get("John Smith");

assertNotSame(user.getId(), user2.getId(), "Users should not have the same id.");

assertEquals("john smith", User.idStrategy().keyFor(user.getId()));

assertEquals("John Smith", User.idStrategy().keyFor(user2.getId()));

}

@Test

void caseSensitivityEmail() {

j.jenkins.setSecurityRealm(new IdStrategySpecifyingSecurityRealm(new IdStrategy.CaseSensitiveEmailAddress()));

User user = User.get("john.smith@acme.org");

User user2 = User.get("John.Smith@acme.org");

assertNotSame(user.getId(), user2.getId(), "Users should not have the same id.");

assertEquals("john.smith@acme.org", User.idStrategy().keyFor(user.getId()));

assertEquals("John.Smith@acme.org", User.idStrategy().keyFor(user2.getId()));

user2 = User.get("john.smith@ACME.ORG");

assertEquals(user.getId(), user2.getId(), "Users should have the same id.");

assertEquals("john.smith@acme.org", User.idStrategy().keyFor(user2.getId()));

}

private static class IdStrategySpecifyingSecurityRealm extends HudsonPrivateSecurityRealm {

private final IdStrategy idStrategy;

IdStrategySpecifyingSecurityRealm(IdStrategy idStrategy) {

super(true, false, null);

this.idStrategy = idStrategy;

}

@Override

public IdStrategy getUserIdStrategy() {

return idStrategy;

}

}

do not change the strategy after user records have been created.

(Interactive testing with mock-security-realm seems fine: switching the id strategy renames the dir of a user whose id contains uppercase letters.)

jglick · 2025-08-06T11:38:47Z

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

        return new File(Jenkins.get().getRootDir(), "users");
    }

+    private static final int PREFIX_MAX = 14;


Original version in UserIdMapper set this to 15 but then subtracted one for the _. I find this a bit clearer. Of course the size is arbitrary; here it allows directory names up to 79 (ASCII) characters, which I think is safe on all operating systems. The bigger concern on Windows is a total path length approaching (IIRC) 254, when not using the UNC \\?\ trick, but if you control the %JENKINS_HOME% length, this should not be too bad: C:\Jenkins\users\quixotichacker_01ba4719c80b6fe911b091a7c05124b64eeece964e09c058ef8f9805daca546b\config.xml is only 107.

The bigger concern on Windows is a total path length approaching (IIRC) 254, when not using the UNC \?\ trick, but if you control the %JENKINS_HOME% length, this should not be too bad:

Java has been using the unicode API with \\?\ for a loooong time.
https://github.com/openjdk/jdk21u-dev/blame/d90297a828dff468afc34e2767439e51379f4f95/src/java.base/windows/classes/sun/nio/fs/WindowsPath.java#L174-L178

OK, I just recall problems with branch-api on Windows.

with branch-api on Windows.

🤔 it should have only been an issue with other tools (like git.exe) and the like, nothing directly inside the JVM (calling cmd.exe from a durable task for example in a long directory would be fun).

an issue with other tools (like git.exe) and the like

Ah, perhaps so, in which case that would indeed not be a concern here.

jglick · 2025-08-06T11:40:29Z

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

                        return userDetails.getUsername();
                    } catch (UsernameNotFoundException x) {
-                        LOGGER.log(Level.FINE, "not sure whether " + idOrFullName + " is a valid username or not", x);
+                        LOGGER.log(Level.FINER, "not sure whether " + idOrFullName + " is a valid username or not", x);


Noise in UserTest.

…er-HMAC

…izes users early

jglick · 2025-08-11T13:16:52Z

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

        XSTREAM.alias("user", User.class);
    }

+    private User() {}


There is a subtle difference between calling the no-arg ctor and unmarshalling into that object, vs. deserializing a new object: whether properties (and, if anyone cares, version) are initialized.

jglick · 2025-08-11T13:18:05Z

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

+        var d = getUserFolderFor(id);
+        return d.isDirectory() ? d : null;


Dubious (normally you would just want to get the File unconditionally whether it currently exists or not), but following the existing nullability annotation and semantics.

jglick · 2025-08-11T13:19:43Z

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

+            if (!migratedUserIdMapper) {
+                try {
+                    UserIdMapper.migrate();


May happen prior to scanAll if JCasC defines users in the built-in realm (mostly useful for demos).

jtnord

Seems reasonable but appears to be lacking any test coverage for the older data migration?

jtnord · 2025-08-11T19:47:56Z

core/src/main/java/hudson/model/UserIdMapper.java

+                        user.id = idKey; // not quite right but hoping for the best
+                        userXml.write(user);
                    }
+                    var newDirectory = User.getUserFolderFor(user.id);


seems weired that the users folder uses the normalized user id, but we don;t actually store that in the User.id here or when creating a new User?

Sorry, I am not following this comment.

not related (directly) to this change.
if you say log in first with "JOE" then your user.id can be JOE (but the keyfor will create this file as it if is for "joe". if you later log in with "joe" (your user will load, but your user.id will be "JOE") (yes its the same user, but there is no normalisation going on).

Indeed the first login will store $JENKINS_HOME/users/joe_deadbeef1234/config.xml with

<id>JOE</id>

and I suppose that will be your id henceforth, even if User.getById("joe", …) is called. I think nothing in this PR is changing that; the previous format would have written the same XML content to $JENKINS_HOME/users/JOE_11772365/config.xml and an entry joe → JOE_11772365 to $JENKINS_HOME/users.xml but otherwise behaved the same.

jtnord · 2025-08-11T19:56:49Z

core/src/main/java/hudson/model/UserIdMapper.java

-    private final int version = 1; // Not currently used, but it may be helpful in the future to store a version.

-    private transient File usersDirectory;
+    // contrary to the name, the keys were actually IdStrategy.keyFor, not necessarily ids


Not sure what this comment is trying to imply, keyFor creates the normalized id. If you are saying that this can also contain Names (when the id is not known and this is from say an SCM commit) then that should not be persisted so should not have a directory Name?

keyFor creates the normalized id

Right, which is not necessarily the id. So the map should really have been named something like idKeyToDirectoryName. That is all.

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

jglick · 2025-08-11T21:04:15Z

lacking any test coverage for the older data migration

No newly written coverage currently, though various cases are in fact tested implicitly via @LocalData and @PresetData.

…er-HMAC

… `UserIdMigratorTest/migrateMultipleUsers/users/foo$002fbar/config.xml`

…er-HMAC

Vlatombe

LGTM

timja · 2025-08-18T14:48:45Z

/label ready-for-merge

This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

Thanks!

A1exKH

LGTM.

NotMyFault

Seams reasonable

PallierJean · 2025-11-06T16:59:48Z

Hello,

I am encountering a problem when using the latest version of Jenkins in my company.

Jenkins is deployed in a k3s cluster as a pod. Each Jenkins pod is customised using the as code plugin.
This allows us to have Jenkins instances configured differently for each team.

We back up the user folder and the jov folder so that we do not lose our data.
In the user folder, we have the users and tokens.
When we redeploy the pod to get a new version of the image or additional customisation, we noticed that a new folder suffixed with an HMAC was created. However, it does not migrate the token correctly.
I see that in the users.xml file, the link between the user and the newly created folder is made.
The problem is that the ID of the jenkins user, which allows us to use user:token to access the jenkins APIs, keeps changing with each redeployment.
With the latest version, it is no longer possible to use the tokens. There is a bug: they are not transferred to the new folder.
We therefore lose the bot or user tokens with each redeployment.

Thanks,

J. Pallier (Team CI from Lectra)

daniel-beck · 2025-11-06T17:23:43Z

Token migration problems from this PR are tracked in https://issues.jenkins.io/browse/JENKINS-76269.

long76 · 2025-12-23T07:47:44Z

please check #16840 (comment)

Deprecate UserIdMapper and just use an HMAC

eab026b

comment-ops-bot bot added the removed This PR removes a feature or a public API label Aug 6, 2025

jglick commented Aug 6, 2025

View reviewed changes

jglick mentioned this pull request Aug 6, 2025

Pretesting User change jenkinsci/bom#5473

Closed

jglick added 2 commits August 7, 2025 15:39

Merge branch 'master' of https://github.com/jenkinsci/jenkins into Us…

1fa89de

…er-HMAC

Run UserIdMapper.migrate when first required, in case JCasC initial…

2d18199

…izes users early

jglick marked this pull request as ready for review August 11, 2025 13:07

This was referenced Aug 11, 2025

Delete UserIdMigrator #10870

Closed

Simplified UserIdMapper #10871

Closed

jglick requested review from a team, Vlatombe and daniel-beck August 11, 2025 13:08

jglick commented Aug 11, 2025

View reviewed changes

jtnord reviewed Aug 11, 2025

View reviewed changes

jglick added 2 commits August 11, 2025 17:13

Enhanced logging for an error case jenkinsci#10926 (comment)

3006ef0

Merge branch 'master' of https://github.com/jenkinsci/jenkins into Us…

a84bf81

…er-HMAC

jtnord approved these changes Aug 12, 2025

View reviewed changes

jglick added 2 commits August 12, 2025 12:07

More compatible migration from ancient $JENKINS_HOME highlighted by…

fdd43c3

… `UserIdMigratorTest/migrateMultipleUsers/users/foo$002fbar/config.xml`

Merge branch 'master' of https://github.com/jenkinsci/jenkins into Us…

d23bbf4

…er-HMAC

jtnord approved these changes Aug 14, 2025

View reviewed changes

Vlatombe approved these changes Aug 18, 2025

View reviewed changes

comment-ops-bot bot added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Aug 18, 2025

A1exKH approved these changes Aug 18, 2025

View reviewed changes

NotMyFault approved these changes Aug 19, 2025

View reviewed changes

NotMyFault merged commit fef9250 into jenkinsci:master Aug 19, 2025
18 checks passed

jglick deleted the User-HMAC branch August 19, 2025 11:39

dduportal mentioned this pull request Nov 5, 2025

[incrementals.jenkins.io/ci.jenkins.io] Outage of incrementals due to releasebot user's API token reset jenkins-infra/helpdesk#4852

Closed

jenkins-infra-bot mentioned this pull request Nov 11, 2025

[JENKINS-76269] Users migration failing since LTS 2.528.1 upgrade causing new API tokens to disappear #16840

Closed

hsuram189 mentioned this pull request Dec 3, 2025

From version 2.524 onwards, Login is not supported due to migration failure of users folder #23898

Closed

		or greater issues in the realm change, could affect currently logged
		in users and even the user making the change. */

	@Test
	void caseInsensitivity() {
	j.jenkins.setSecurityRealm(new IdStrategySpecifyingSecurityRealm(new IdStrategy.CaseInsensitive()));
	User user = User.get("john smith");
	User user2 = User.get("John Smith");
	assertSame(user.getId(), user2.getId(), "Users should have the same id.");
	}

	@Test
	void caseSensitivity() {
	j.jenkins.setSecurityRealm(new IdStrategySpecifyingSecurityRealm(new IdStrategy.CaseSensitive()));
	User user = User.get("john smith");
	User user2 = User.get("John Smith");
	assertNotSame(user.getId(), user2.getId(), "Users should not have the same id.");
	assertEquals("john smith", User.idStrategy().keyFor(user.getId()));
	assertEquals("John Smith", User.idStrategy().keyFor(user2.getId()));
	}

	@Test
	void caseSensitivityEmail() {
	j.jenkins.setSecurityRealm(new IdStrategySpecifyingSecurityRealm(new IdStrategy.CaseSensitiveEmailAddress()));
	User user = User.get("john.smith@acme.org");
	User user2 = User.get("John.Smith@acme.org");
	assertNotSame(user.getId(), user2.getId(), "Users should not have the same id.");
	assertEquals("john.smith@acme.org", User.idStrategy().keyFor(user.getId()));
	assertEquals("John.Smith@acme.org", User.idStrategy().keyFor(user2.getId()));
	user2 = User.get("john.smith@ACME.ORG");
	assertEquals(user.getId(), user2.getId(), "Users should have the same id.");
	assertEquals("john.smith@acme.org", User.idStrategy().keyFor(user2.getId()));
	}

	private static class IdStrategySpecifyingSecurityRealm extends HudsonPrivateSecurityRealm {
	private final IdStrategy idStrategy;

	IdStrategySpecifyingSecurityRealm(IdStrategy idStrategy) {
	super(true, false, null);
	this.idStrategy = idStrategy;
	}

	@Override
	public IdStrategy getUserIdStrategy() {
	return idStrategy;
	}
	}

		var d = getUserFolderFor(id);
		return d.isDirectory() ? d : null;

Uh oh!

Conversation

jglick commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing done

Proposed changelog entries

Proposed changelog category

Proposed upgrade guidelines

Maintainer checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jtnord Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jtnord left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jglick commented Aug 11, 2025

Uh oh!

Vlatombe left a comment

Choose a reason for hiding this comment

Uh oh!

timja commented Aug 18, 2025

Uh oh!

A1exKH left a comment

Choose a reason for hiding this comment

Uh oh!

NotMyFault left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PallierJean commented Nov 6, 2025

Uh oh!

daniel-beck commented Nov 6, 2025

Uh oh!

long76 commented Dec 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

jglick commented Aug 6, 2025 •

edited

Loading

jtnord Aug 11, 2025 •

edited

Loading