Task #1053
closedInclude workflow for AD groups sync to our product
100%
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
Updated by Marcel Poul over 6 years ago
Also new tutorial should be created for AD - how to synchronize groups
Updated by Luboš Čábelka over 6 years ago
- Status changed from New to In Progress
- Assignee changed from Ondřej Kopr to Luboš Čábelka
Updated by Marcel Poul over 6 years ago
- Assignee changed from Luboš Čábelka to Patrik Stloukal
Updated by Patrik Stloukal over 6 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
Updated by Vít Švanda over 6 years ago
- Status changed from In Progress to Needs feedback
Updated by Vít Švanda over 6 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.
....
Updated by Patrik Stloukal over 6 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
Updated by Vít Švanda over 6 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.
Updated by Marcel Poul over 6 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.
Updated by Marcel Poul about 6 years ago
- Related to Task #1268: Improve the AD group synchronization workflow added