Project

General

Profile

Actions
Copy link

Defect #3045

closed

Removed automatic roles by attributes when validity of contract was extended on the last day

Added by Alena Peterová over 2 years ago. Updated almost 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Roman Kučera
Category:
Automatic roles
Target version:
12.2.0
Start date:
01/26/2022
Due date:
% Done:

100%

Estimated time:
Affected versions:
10.8.3, 11.2.1
Owner:

Description

Tested on 10.8.3 and 11.2.1

Situation:
  • Contract is valid till 26.1.2022, so assigned automatic roles are also valid till 26.1.2022
  • The validity of contract is extended in HR on the last day, new validity is 30.1.2022
  • Synchronization of contracts processes the contract on 27.1.2022 in the morning. (It runs with both settings: After end, start the HR processes; After end, start the automatic role recalculation)
  • The validity of the contract and of the automatic roles by org. structure is extended, but the validity of automatic roles by attributes is not changed
  • When IdentityRoleExpirationTaskExecutor runs that day, it removes the roles

Note: Tested for manual changes in GUI and synchronization of contracts, I don't know the behavior for contracts slices.
Note: For easier testing, screenshots come from appliance with the date set 1 day in the future (date --set='+1 day'). The line with the LDAP_CAS would be otherwise grey.

Files

Download all files
contracts_roles_after.png (50.6 KB) contracts_roles_after.png Alena Peterová, 01/26/2022 03:40 PM
contracts_roles_before.png (50.6 KB) contracts_roles_before.png Alena Peterová, 01/26/2022 03:40 PM
automatic_role_attr_validity.png (120 KB) automatic_role_attr_validity.png Alena Peterová, 01/26/2022 03:40 PM
Actions
Copy link
 #1

Updated by Alena Peterová over 2 years ago

I believe that workaround is similar to https://redmine.czechidm.com/issues/2640#note-6:
  • unschedule IdentityRoleExpirationTaskExecutor to avoid removal of roles
One-time fix:
  • automatic roles with wrong validity
    • manually move the "valid till" of the contract to some different date and back (don't move it to the past, that would remove all roles!)
  • completely missing automatic roles
    • run ProcessAllAutomaticRoleByAttributeTaskExecutor
Actions
Copy link
 #3

Updated by Peter Štrunc about 2 years ago

  • Target version set to 12.2.0
Actions
Copy link
 #4

Updated by Roman Kučera about 2 years ago

  • Sprint set to Sprint 12.2-2 (Mar 02 - Mar 16)
Actions
Copy link
 #5

Updated by Tomáš Doischer about 2 years ago

  • Status changed from New to In Progress
  • Assignee changed from Peter Štrunc to Tomáš Doischer
Actions
Copy link
 #6

Updated by Tomáš Doischer about 2 years ago

  • % Done changed from 0 to 10

ProcessAllAutomaticRoleByAttributeTaskExecutor doesn't really touch automatic role "in the middle", i. e., newly assigned ones or newly removed ones, in terms of validity. This is done by the IdentityContractUpdateByAutomaticRoleProcessor which is run after the contract is updated. I will investigate it, it's not covered by tests so I will add them and try to replicate the issue reported. But of course, the issue may be located elsewhere.

Actions
Copy link
 #7

Updated by Alena Peterová about 2 years ago

  • Subject changed from Removed automatic roles by attributes when contract was extended on the last day to Removed automatic roles by attributes when validity of contract was extended on the last day
Actions
Copy link
 #8

Updated by Tomáš Doischer about 2 years ago

  • % Done changed from 10 to 40

I've abandoned the attempts to use Clock to mock time. This would be useful but it would also be a massive rework of fairly critical parts of the application.

I was, however, able to write a test that fails even though it shouldn't. I have a user with a valid contract, create an automatic role, then change the contract's validity to the past (using saveInteral to skip all processors), then set the contract validity to the future (via an event, the same way a synchronization does). In the end, the user should have the role with the last validity but instead, they have the role with the validity of the original contract.

The culprit seems to be in the IdentityContractUpdateByAutomaticRoleProcessor. The way I tested this was fairly desperate - I put a breakpoint in every class which can set the role validity, then ran the code. This was the only class where role validity was set...

Actions
Copy link
 #9

Updated by Tomáš Doischer about 2 years ago

  • Status changed from In Progress to Needs feedback
  • Assignee changed from Tomáš Doischer to Roman Kučera
  • % Done changed from 40 to 80

Implemented. I ended up using a fairly careful approach that cannot break anything, even though a deeper refactoring would be great. Instead, I just ensured that the code would run the way it was intended.

I tried to run the code in the appliance from a WAR file but, in the end, I gave up. The appliance started with the correct version but didn't use the correct configuration. At this moment, I don't suggest anybody try this. But it would be useful and we could quite easily add this to CzechIdM Docker image for development purposes.

To test this, follow these steps:
  1. have a user with a valid contract from synchronization
  2. set the system time to yesterday (-1 day from today) - in Ubuntu, a simple GUI change in settings does that
  3. change the user's contract end of validity to yesterday
  4. set the system time to today
  5. synchronize the contract with HR processes enabled
  6. check that the validity of the assigned role was updated based on the contract's validity

This didn't work in the past but it works now. If you don't want to or can't follow the testing steps, please let me know, and I will show you it's working.

@kucerar, can you please give me feedback?

PR: https://github.com/bcvsolutions/CzechIdMng/pull/198

In the end, this ticket could have been much easier to solve but I tried fairly complicated steps to replicate the issue. It can be replicated quite easily.

Actions
Copy link
 #10

Updated by Roman Kučera about 2 years ago

  • Status changed from Needs feedback to Resolved
  • % Done changed from 80 to 100

LGTM, thx. Merged to develop

Actions
Copy link
 #11

Updated by Roman Kučera almost 2 years ago

  • Status changed from Resolved to Closed
Actions
Copy link

Also available in: Atom PDF