Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CreatedTimestamp on REST import not used #14009

Closed
heddn opened this issue Aug 25, 2022 · 5 comments · Fixed by #23293
Closed

CreatedTimestamp on REST import not used #14009

heddn opened this issue Aug 25, 2022 · 5 comments · Fixed by #23293
Labels
kind/enhancement Categorizes a PR related to an enhancement
Milestone

Comments

@heddn
Copy link
Contributor

heddn commented Aug 25, 2022

Description

The CreatedTimestamp (if set on the user rest creation call) is always discarded in favor of Time.currentTimeMillis(). This shouldn't be the case for scenarios when someone is REST importing from a legacy system.

Discussion

No response

Motivation

Handle graceful import of users from legacy systems.

Details

From MapUserProvider.

image

@heddn heddn added kind/enhancement Categorizes a PR related to an enhancement status/triage labels Aug 25, 2022
@efesler
Copy link

efesler commented Aug 4, 2023

In LegacyExportImportManager.createUser, the createdTimeStamp is set to the timestamp coming from the legacy repository.

// Import users just to user storage. Don't federate
        UserModel user = UserStoragePrivateUtil.userLocalStorage(session).addUser(newRealm, userRep.getId(), userRep.getUsername(), false, false);
        user.setEnabled(userRep.isEnabled() != null && userRep.isEnabled());
        user.setCreatedTimestamp(userRep.getCreatedTimestamp());

Is the issue still relevant ?

@heddn
Copy link
Contributor Author

heddn commented Sep 5, 2023

I'm dusting off the user migration again and will confirm. But LegacyExportImportManager and MapUserProvider are different things. The REST import should pass through this code. And

entity.setCreatedTimestamp(Time.currentTimeMillis());
still has the hard coded timestamp.

@heddn
Copy link
Contributor Author

heddn commented Sep 15, 2023

No, the provided value from the REST import is not used. It still uses the time of import instead of the value provided in the import.

@heddn
Copy link
Contributor Author

heddn commented Sep 15, 2023

I'm looking at org.keycloak.services.resources.admin.UsersResource::createUser, and there is nothing to set the create timestamp.

heddn added a commit to heddn/keycloak that referenced this issue Sep 15, 2023
Fixes keycloak#14009. Untested as I don't really have a good way to compile and test locally, but from looking at the code changes here, this _should_ fix the issue.
@heddn
Copy link
Contributor Author

heddn commented Sep 15, 2023

Just pushed up a PR that should at least start the conversation here.

@ghost ghost removed the status/triage label Sep 27, 2023
mposolda pushed a commit that referenced this issue Sep 27, 2023
@mposolda mposolda added this to the 23.0.0 milestone Sep 27, 2023
srose pushed a commit to srose/keycloak that referenced this issue Dec 20, 2023
kamontat pushed a commit to kamontat/keycloak that referenced this issue Jan 20, 2024
Closes keycloak#14009

Signed-off-by: Kamontat Chantrachirathumrong <14089557+kamontat@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Categorizes a PR related to an enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants