Fix SynchronizeProfiles()
Base information
| Question | Answer |
|---|---|
| Related to a SourceForge thead / Another PR / Combodo ticket? | no |
| Type of change? | Bug fix |
Symptom (bug) / Objective (enhancement)
We use hybridauth with user provisioning enabled, profiles are defined depending on Okta groups of the external user (it requires some patches to https://github.com/Combodo/combodo-hybridauth to work). The SynchronizeProfiles() function works only when the existing profile list is empty, while creation is OK, updates aren't done correctly and this PR fixes this point.
Reproduction procedure (bug)
In current state of iTop, bug can't be triggered because a User object is created once and never updated after that. I've modified HybridAuthLoginExtension.php so it updates the User (and his profiles) everytime.
Cause (bug)
One issue is related to false and 0 being mixed up (return of array_search). Second issue is that Fetch() does not handle modified list very well and a deletion prevent the next item to be properly evaluated.
Proposed solution (bug and enhancement)
See code, fixes are trivial.
Checklist before requesting a review
- [x] I have performed a self-review of my code
- [x] I have tested all changes I made on an iTop instance
- [ ] I have added a unit test, otherwise I have explained why I couldn't
- [x] Is the PR clear and detailed enough so anyone can understand digging in the code?
To make this PR relevant, the modifications of HybridAuthLoginExtension.php are in this other PR: https://github.com/Combodo/combodo-hybridauth/pull/7
Thank you for this contribution. Provisioning enhancement has been moved into combodo-hybridauth extension to release it apart from iTop official releases. https://github.com/Combodo/combodo-hybridauth/commit/7de4b1aa41e6cc6560dd7bb36d276772102386d0
@TdlQ could you please confirm that this PR can be closed, since the edition made in combodo-hybridauth should provide the same functionality ? (If I'm not wrong, you tested it, right ?) Thank you
@TdlQ could you please confirm that this PR can be closed, since the edition made in combodo-hybridauth should provide the same functionality ? (If I'm not wrong, you tested it, right ?) Thank you
Yes, it can be closed. We've been using for a few weeks the full OktaOIDC feature with User provisioning and everything is working properly
That's perfect, thanks for your feedback. Thank you for contribution (on both repos) that helped to fix the issue. We're grateful for that, and we'd like to send you stickers. Would you like some ? If yes, please send us your address at community[at]combodo.com