Skip to content

Fix SynchronizeProfiles()#716

Closed
TdlQ wants to merge 2 commits intoCombodo:developfrom
TdlQ:develop
Closed

Fix SynchronizeProfiles()#716
TdlQ wants to merge 2 commits intoCombodo:developfrom
TdlQ:develop

Conversation

@TdlQ
Copy link

@TdlQ TdlQ commented May 22, 2025

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

  • I have performed a self-review of my code
  • 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
  • Is the PR clear and detailed enough so anyone can understand digging in the code?

@TdlQ
Copy link
Author

TdlQ commented May 22, 2025

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

@jf-cbd jf-cbd moved this from First review needed to Pending technical review in Combodo PRs dashboard Jun 6, 2025
@odain-cbd
Copy link
Contributor

Thank you for this contribution. Provisioning enhancement has been moved into combodo-hybridauth extension to release it apart from iTop official releases.
Combodo/combodo-hybridauth@7de4b1a

@jf-cbd
Copy link
Member

jf-cbd commented Oct 3, 2025

@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
Copy link
Author

TdlQ commented Oct 3, 2025

@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

@jf-cbd
Copy link
Member

jf-cbd commented Oct 3, 2025

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 jf-cbd closed this Oct 3, 2025
@github-project-automation github-project-automation bot moved this from Pending technical review to Finished in Combodo PRs dashboard Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants