iTop icon indicating copy to clipboard operation
iTop copied to clipboard

Fix SynchronizeProfiles()

Open TdlQ opened this issue 7 months ago • 1 comments

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?

TdlQ avatar May 22 '25 08:05 TdlQ

To make this PR relevant, the modifications of HybridAuthLoginExtension.php are in this other PR: https://github.com/Combodo/combodo-hybridauth/pull/7

TdlQ avatar May 22 '25 09:05 TdlQ

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

odain-cbd avatar Sep 22 '25 12:09 odain-cbd

@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

jf-cbd avatar Oct 03 '25 07:10 jf-cbd

@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

TdlQ avatar Oct 03 '25 09:10 TdlQ

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

jf-cbd avatar Oct 03 '25 09:10 jf-cbd