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

[mederly]

  Branch: refs/heads/master
  Home:   https://github.com/Evolveum/midpoint
  Commit: a55cbe43936a67ff4fa2f49559e2dcfd96f19f87
      https://github.com/Evolveum/midpoint/commit/a55cbe43936a67ff4fa2f49559e2dcfd96f19f87
  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

  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


  Commit: abb4b22ab616f6c85fd47dcd97a22a96b3d08aca
      https://github.com/Evolveum/midpoint/commit/abb4b22ab616f6c85fd47dcd97a22a96b3d08aca
  Author: Andrej Zan <andrej.zan at evolveum.com>
  Date:   2025-01-08 (Wed, 08 Jan 2025)

  Changed paths:
    M model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/controller/SchemaTransformer.java

  Log Message:
  -----------
  Remove unnecessary call to get read constraints

**What**

Read constraints needed for securing "eveluated assignments" are
"calculated" from the focus object. Same constraints are also needed to
securing the focus itself. Before this change, constraints were
"calculated" two times with the same focus object. Now it's calculated
only once.

**Why**

Even though this version is a bit less "clean" (maybe subjective), the
benefit is in reducing excesive logging, which is generated by the
constraints "calculation".


  Commit: a414ea691fee229df8a2f54988ab8d9f2280640d
      https://github.com/Evolveum/midpoint/commit/a414ea691fee229df8a2f54988ab8d9f2280640d
  Author: Andrej Zan <andrej.zan at evolveum.com>
  Date:   2025-01-08 (Wed, 08 Jan 2025)

  Changed paths:
    M release-notes.adoc

  Log Message:
  -----------
  Add fix of MID-1024 to release notes


  Commit: 7874e995d1eb9d3f52a3e5e4b93ab43292faf354
      https://github.com/Evolveum/midpoint/commit/7874e995d1eb9d3f52a3e5e4b93ab43292faf354
  Author: mederly <mederly at evolveum.com>
  Date:   2025-01-08 (Wed, 08 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:
  -----------
  Merge pull request #246 from Evolveum/bugfix/10204

Fix change of immutable object caused by preview


Compare: https://github.com/Evolveum/midpoint/compare/781058072fea...7874e995d1eb

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