Skip to content

fix(control-plane): resolve organization on OIDC login and set default org on first membership#174

Closed
gozen59 wants to merge 6 commits into
reshaprio:mainfrom
gozen59:branch_fix_idc-user-lifecycle
Closed

fix(control-plane): resolve organization on OIDC login and set default org on first membership#174
gozen59 wants to merge 6 commits into
reshaprio:mainfrom
gozen59:branch_fix_idc-user-lifecycle

Conversation

@gozen59

@gozen59 gozen59 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes two issues when authenticating users via an external OIDC provider (Keycloak):

  1. NullPointerException on OIDC callback — JWT generation failed when defaultOrganization was unset, even though the user already belonged to one or more organizations.
  2. Missing default organization — users assigned to their first organization (via admin API or onboarding) did not always get defaultOrganization set, leaving them in an inconsistent state for subsequent logins.

Also improves the local Keycloak startup script for OIDC development.

Changes

Control plane

  • Add User.ensureDefaultOrganization() and call it whenever a membership is added or reassigned (UserResource, OrganizationResource, OnboardingService)
  • On OIDC callback, resolve the login organization from defaultOrganization or fall back to the first membership; return 403 if the user has no organization

Dev tooling

  • Rewrite dev/start-keycloak-docker.sh: detached container, health wait, sslRequired=NONE on master realm, clearer local dev instructions

Test plan

  • Start Keycloak with dev/start-keycloak-docker.sh and confirm it becomes ready on port 8888
  • Log in via OIDC with a user who has organization memberships but no defaultOrganization — login should succeed and a JWT should be issued
  • Assign a user to their first organization via POST /admin/users/{id}/organizations/{name} and verify defaultOrganization is set automatically
  • Reassign organizations via PUT /admin/users/{id}/organizations and verify defaultOrganization is preserved or set from the new list
  • Attempt OIDC login for a user with no organization membership — expect 403 User has no organization

@gozen59 gozen59 requested a review from lbroudoux as a code owner June 5, 2026 21:20
@gozen59 gozen59 force-pushed the branch_fix_idc-user-lifecycle branch from 537c378 to 4c5fe63 Compare June 5, 2026 21:23
gozen59 added 6 commits June 5, 2026 23:24
… to the list of discovered services/API) and Experimental screens

Signed-off-by: Thierry MATHIAS <thierry.mathias@disney.com>
Signed-off-by: Thierry MATHIAS <thierry.mathias@disney.com>
Signed-off-by: Thierry MATHIAS <thierry.mathias@disney.com>
Run Keycloak detached with health wait, master sslRequired=NONE, and clearer
dev instructions for the 3rdparty realm.

Signed-off-by: Thierry MATHIAS <thierry.mathias@disney.com>
Add User.ensureDefaultOrganization() and call it whenever a user is assigned
their first organization, including via admin memberships API.

Signed-off-by: Thierry MATHIAS <thierry.mathias@disney.com>
Use defaultOrganization or the first membership when generating the JWT after
OIDC callback, instead of failing with a null pointer.

Signed-off-by: Thierry MATHIAS <thierry.mathias@disney.com>
@gozen59 gozen59 force-pushed the branch_fix_idc-user-lifecycle branch from 4c5fe63 to c13e10e Compare June 5, 2026 21:24
@lbroudoux

Copy link
Copy Markdown
Member

Hi @gozen59

Thanks a lot for pushing this one but unfortunately your branch also embeds a lot of changes that are not related to these bug fixes. First, I'm gonna create the issue to track the bugs you mentioned; then I'm going to propose that you reopen the PR with just the related changes (I'm also going to merge #170 in the meantime).

@gozen59

gozen59 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Hi @lbroudoux ,

I based these changes on the branch currently under review in PR 170. Logically, once the merge is complete, the changes will be less extensive.
But OK, we’ll do as you say :)

Regards.

@lbroudoux

Copy link
Copy Markdown
Member

Sorry @gozen59 but #183 has been merged as it was easier to address fewer scoped changes.

@lbroudoux lbroudoux closed this Jun 9, 2026
@lbroudoux lbroudoux added the duplicate This issue or pull request already exists label Jun 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

duplicate This issue or pull request already exists

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants