[midPoint-git] [Evolveum/midpoint] 1e0a81: Fix change of immutable object caused by preview

[Andrej]

  Branch: refs/heads/bugfix/4.8/10204
  Home:   https://github.com/Evolveum/midpoint
  Commit: 1e0a81b9a2d583e78a39aaae67d0bf1a9f6dd501
      https://github.com/Evolveum/midpoint/commit/1e0a81b9a2d583e78a39aaae67d0bf1a9f6dd501
  Author: Andrej Zan <andrej.zan at evolveum.com>
  Date:   2025-01-07 (Tue, 07 Jan 2025)

  Changed paths:
    M model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/controller/SchemaTransformer.java
    M model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/controller/transformer/DataAccessProcessor.java
    M model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/LensContext.java
    M model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/security/AbstractInitializedSecurityTest.java
    M model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/security/TestSecurityBasic.java
    A model/model-intest/src/test/resources/security/role-user-add-read-some.xml
    M release-notes.adoc

  Log Message:
  -----------
  Fix change of immutable object caused by preview

**What**

Fix an error caused by change of immutable object (in this case
immutable delta), which happens during preview of "create role" changes.
But it could potentially happen during any "create" operation.

**Notes**

This error could be fixed in various ways. Some of considered solutions:
- Add `modifyEachObject` and `modifyEachDelta` methods to `ElementState`
  and to `LensElementContext`. These methods would allow to modify
  internal deltas/objects by internally cloning them if they are
  immutable.
- Cloning whole `LensContext` before passing it to the
  `SchemaTransformer` for application of security constraints.
- Cloning `LensFocusContext` and `LensProjectionContext` before passing
  them to the `DataAccessProcessor`.

It's important to say, that **none** from mentioned solutions is very
good. So I simply chose the one which I see as a lesser evil, the third
one. Here are my arguments:

**Add `modifyEach*` methods**

This solution seemed to me as the most compelling. However, during more
research I have understood, that deltas and objects in the
`ElementState` should not be changed "just like that" without a big
amount of care. Changes of already immutable objects/deltas could cause
that whole `ElementState` would be invalid. It could be a problem
especially, if it was changed before or during clockwork execution. Even
though there already are methods, which allows to modify
primary/secondary deltas, they are commented as a "dangerous". I don't
think, it would be wise to add yet another set of methods, which
somebody could (what means "will") misuse.

**Cloning whole `LensContext`**

To be honest I haven't think about this approach very much. It simply
seems to me as a too big chunk of data to clone without a reason.

**Clonging `LensFocusContext` and `LensProjectionContext`

This method seems to me as a compromise between cloning too much data
and modifying potentially "living" state. It also does not require
addition of any new "modification" methods to the `ElementState` class
which in turn reduce the risk of misuse.

**Proper solution**

Based on my current understanding, the best solution, would be to limit
data which are needed for a preview. If I understand it correctly, the
data which are necessary to display a preview are just deltas. So I
think, the proper solution would be to sent just those to the UI. Of
course if more data are needed, than those could be sent too, but
definitely not whole `LensContext`.

If data sent to UI would be limited, we could simply apply the security
constraints just to those deltas and not bother with whole `LensContext`
which would be "dropped" anyway (since it's just preview).

**Fixes**: MID-10204

(cherry picked from commit a55cbe43936a67ff4fa2f49559e2dcfd96f19f87)



To unsubscribe from these emails, change your notification settings at https://github.com/Evolveum/midpoint/settings/notifications