Project

General

Profile

Actions

Task #1053

closed

Include workflow for AD groups sync to our product

Added by Marcel Poul over 4 years ago. Updated over 4 years ago.

Status:
Closed
Priority:
High
Assignee:
Patrik Stloukal
Category:
Synchronization
Target version:
Start date:
04/04/2018
Due date:
% Done:

100%

Estimated time:

Description

We use this kind of workflow in 2 or 3 projects. This is so common tast, that it should be included in product. Please revise the wf, make it little bit more customable (default values) and merge it into CzechIdM.


Related issues

Related to CzechIdM - Task #1268: Improve the AD group synchronization workflowClosedVít Švanda09/24/2018

Actions
Actions #2

Updated by Marcel Poul over 4 years ago

Also new tutorial should be created for AD - how to synchronize groups

https://wiki.czechidm.com/tutorial/adm/ad_groups_sync

Actions #3

Updated by Luboš Čábelka over 4 years ago

test

Actions #4

Updated by Luboš Čábelka over 4 years ago

  • Status changed from New to In Progress
  • Assignee changed from Ondřej Kopr to Luboš Čábelka
Actions #5

Updated by Marcel Poul over 4 years ago

  • Assignee changed from Luboš Čábelka to Patrik Stloukal
Actions #7

Updated by Patrik Stloukal over 4 years ago

  • Assignee changed from Patrik Stloukal to Vít Švanda

Please could you look on this version of workflow and write feedback? Thanks.

commit: https://github.com/bcvsolutions/CzechIdMng/commit/a91fd55fa98ed2e856c5b8b7376232f362e79fc9

Actions #8

Updated by Vít Švanda over 4 years ago

  • Status changed from In Progress to Needs feedback
Actions #9

Updated by Vít Švanda over 4 years ago

  • Status changed from Needs feedback to In Progress
  • Assignee changed from Vít Švanda to Patrik Stloukal

I started with review and I'm not happy, this is not good:

  • Where are a tests?
  • Where is documentation?
  • We spoke about that only project specific code should be in the WF process. It means static methods should be separate to standard services/managers. Why you defined methods as 'getAccount(UUID accountId, AccAccountService accService)', 'getRoleId(UUID account, AccRoleAccountService accService)' ... again and again in every script?
  • We spoke about that the logic in the WF process should be defined to more activities. Why is in the activity "set attributes" groovy script with 162 rows? In this script are defined methods "getTreeNode (name), resolveCatalogue (distinguishedName), getAttributesForAutomaticRole(name), removeLastItemFromCatalogue()". Everyone from these methods are called only once. May be create an activity for every these method could do workflow more nice and cleary.
  • Missing JavaDoc for many methods.
  • On some places are used comments in the Czech.
  • Dynamic input parameters should be defined in the process, not in the groovy script. Use ' <dataObject>'.
  • Process does not have correct description.
  • Script in the "Missing situations" activity is sets as 'javascript'. This couldn't never work.
  • How purpose has method 'isChangedRoleMappingAttribute'? You use her only for check if exists overrided attribute.
  • Method 'addRoleMappingAttribute', doesn't have the interface and javadoc. This method set attribute allways to MERGE and Entity=false. This is not correct.
    ....
Actions #10

Updated by Marcel Poul over 4 years ago

  • Priority changed from Normal to High
Actions #13

Updated by Patrik Stloukal over 4 years ago

  • Status changed from In Progress to Needs feedback
  • Assignee changed from Patrik Stloukal to Vít Švanda
  • % Done changed from 0 to 90

so i resolved all issues from your last feedback and made a lot of improvements,
there are auite many variables to configure to workflow working fine, so it is quite difficult.

Please look on this workflow and write feedback.

commits: https://github.com/bcvsolutions/CzechIdMng/commit/c03d83fca4ad932470cc27be993d718cef6f1582
https://github.com/bcvsolutions/CzechIdMng/commit/2420141865e9b32f9ea6c58a65734a51c3238adc

i wrote tutorials: https://wiki.czechidm.com/tutorial/adm/ad_groups_sync
https://wiki.czechidm.com/tutorial/dev/ad_groups_sync_workflow

Actions #14

Updated by Vít Švanda over 4 years ago

  • Status changed from Needs feedback to Resolved
  • Assignee changed from Vít Švanda to Patrik Stloukal
  • Target version set to Lapis (8.2.0)
  • % Done changed from 90 to 100

I did review and test.

  • I refactored some method and change name of WF process.
  • I improved logging. Logs added during WF are persisted to SyncItemLog now.
  • I extended input attribute for WFs calls from the sync for "systemId".
  • I created tests for new method in the SysRoleAttributeMappingService.

I sucessfuly completed the green line with create Role with mapping on Identity system with overrided attribute and automatic role by tree. Some usecauses as role catalog and create automatic role by attributes was too hard to configurate (for me). Globaly it works correctly.

I have a dubiety for existing of that WF process in the product, because potencial user have to change process manually and set many of properties. That is good for projects, but not for admin user.

Actions #15

Updated by Marcel Poul over 4 years ago

Vít Švanda wrote:

I have a dubiety for existing of that WF process in the product, because potencial user have to change process manually and set many of properties. That is good for projects, but not for admin user.

I vote for some kind of external configuration (in Script? IdM properties?) so we do not have to fork it on projects.

Actions #16

Updated by Vít Švanda over 4 years ago

  • Status changed from Resolved to Closed
Actions #17

Updated by Marcel Poul almost 4 years ago

  • Related to Task #1268: Improve the AD group synchronization workflow added
Actions

Also available in: Atom PDF