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.

Sign up for GitHub

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

Jump to bottom

Fixes #24304: Deleted users should change status on file reload #5428

Merged
merged 1 commit into from
Mar 4, 2024
Merged

Fixes #24304: Deleted users should change status on file reload #5428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 24 additions & 12 deletions ...sources/rudder/rudder-core/src/main/scala/com/normation/rudder/users/UserRepository.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,20 +234,22 @@ object UserRepository {
)
}.toMap

// `resurrected` are zombie that get back to live with the new origin.
// `resurrected` are zombie that get back to live with the new origin. For users with same origin, we keep them as is.
// We resurrect users with "active" status because we have nothing to manage "disabled" for now.
val resurrected = zombies.map {
val resurrectedOrSameOrigin = zombies.map {
case (k, v) =>
// check that status is really deleted, just in case
if (v.status == UserStatus.Deleted) {
(
k,
v.modify(_.status)
.setTo(UserStatus.Active)
.modify(_.statusHistory)
.using(StatusHistory(UserStatus.Disabled, trace) :: _)
.modify(_.managedBy)
.setTo(origin)
if (v.managedBy != origin) {
v.modify(_.status)
.setTo(UserStatus.Active)
.modify(_.statusHistory)
.using(StatusHistory(UserStatus.Disabled, trace) :: _)
.modify(_.managedBy)
.setTo(origin)
} else v
)
} else (k, v)
}
Expand All @@ -264,7 +266,7 @@ object UserRepository {
)
}

realNew ++ resurrected ++ deleted
realNew ++ resurrectedOrSameOrigin ++ deleted
}

}
Expand Down Expand Up @@ -634,13 +636,23 @@ class JdbcUserRepository(doobie: Doobie) extends UserRepository {
fr")"

def update(updated: Vector[UserInfo]): ConnectionIO[Int] = {
// never update the user personal information and managedby of an existing user which may have been provided and modified by another origin
// never update the user personal information of an existing user which may have been provided and modified by another origin
val sql =
"""insert into users (id, creationdate, status, managedby, name, email, lastlogin, statushistory, otherinfo)
values (?,?,?,?,? , ?,?,?,?)
on conflict (id) do update
set (creationdate, status, lastlogin, statushistory) =
(EXCLUDED.creationdate, EXCLUDED.status, EXCLUDED.lastlogin, EXCLUDED.statushistory)"""
set (creationdate, status, managedby, lastlogin, statushistory) =
(
EXCLUDED.creationdate,
EXCLUDED.status,
CASE WHEN users.status = 'active'
THEN users.managedby
ELSE EXCLUDED.managedby
END,
EXCLUDED.lastlogin,
EXCLUDED.statushistory
)
"""

Update[UserInfo](sql).updateMany(updated)
}
Expand Down
10 changes: 10 additions & 0 deletions ...ces/rudder/rudder-core/src/test/scala/com/normation/rudder/users/UserRepositoryTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,11 @@ trait UserRepositoryTest extends Specification with Loggable {
}
}

// BOB is rept removed after setting the users again without BOB
val dateBobRemovedIdempotent = DateTime.parse("2023-09-02T02:03:04Z")
val traceBobRemovedIdempotent = EventTrace(actor, dateBobRemovedIdempotent)
val userInfosBobRemovedIdempotent = userInfosBobRemoved

// BOB added back from OIDC
val dateBobOidc = DateTime.parse("2023-09-03T03:03:03Z")
val traceBobOidc = EventTrace(actor, dateBobOidc)
Expand Down Expand Up @@ -182,6 +187,11 @@ trait UserRepositoryTest extends Specification with Loggable {
repo.getAll().runNow.toUTC must containTheSameElementsAs(userInfosBobRemoved)
}

"If an user is reloaded from the same origin, it should be kept as is" >> {
repo.setExistingUsers(AUTH_PLUGIN_NAME_LOCAL, userFileBobRemoved, traceBobRemovedIdempotent).runNow
repo.getAll().runNow.toUTC must containTheSameElementsAs(userInfosBobRemovedIdempotent)
}

"If an user is created by an other module and file reloaded, it's Active and remains so" >> {
repo.setExistingUsers(AUTH_PLUGIN_NAME_REMOTE, List("bob"), traceBobOidc).runNow
repo.setExistingUsers(AUTH_PLUGIN_NAME_LOCAL, userFileBobRemoved, traceReload).runNow
Expand Down