Organization identity provider page is broken once a provider is set

p2-inc / keycloak-orgs

Single realm, multi-tenancy for SaaS apps

Other

362 stars 65 forks source link

Organization identity provider page is broken once a provider is set #174

Closed kedare closed 3 months ago

kedare commented 4 months ago

Hello.

Using the quay.io/phasetwo/phasetwo-keycloak:23

Once an IDP has been set on an organization, it is not possible to change the IDP to another one (the list is empty), also the edit link on the page is not doing anything

No error on the console, it's just not doing anything.

Thank you.

xgp commented 4 months ago

The presence of the "Edit" link on the page when no IdP linked is a bug (@pnzrr). Usually that link will go to the IdP tab where you can edit the IdP that is linked.

The list is only populated with unlinked IdPs. In order to link an IdP that is already assigned to an Organization, you must unlink it first, and then go to the Organization you wish to link it to.

kedare commented 4 months ago

Thank you for your fast answers.

Indeed I could associate another IDP once created.

However I may have found new potential issues with the form :

In case of validation error, it would only show a non-explicit error message "IdP failed to update for this org. Please try again." instead of the full error (that can be see in the http response directly when using the browser inspector)
When replacing the IDP by another one, the old one get disabled, not sure this is expected ?
When replacing the IDP by another one, the previous one is still shown in the form until a full refresh of the page if executed

xgp commented 4 months ago

@kedare thanks for the report. I think we can improve the error message and page updating.

Regarding the disable on replacement, yes that is expected. Our assumption is that you have created and IdP for an Organization, and we enforce only one active IdP per organization. However, I can see how this would be confusing.

xgp commented 4 months ago

@kedare We discussed this, and I think there's an better argument not to disable the old one when it is changed from the Admin UI. The argument of "enforcing only one active IdP per organization" makes sense when creating/updating is done by the organization admin (e.g. through the org portal), but not in the Keycloak Admin UI.

We're thinking about a solution, but the current idea is

changing the selection in the Admin UI just removes the old association, but keeps it enabled
the dropdown should show all IdPs available for linking, but give an indication if it is currently owned, and require a confirmation modal if the link is "stealing" it from another organization

Please share any thoughts, and thanks for bringing this up.

kedare commented 4 months ago

I think if we disable the org, we should at least show it in disabled in the drop-down list (disabled html attribute), and when it gets disabled show a notification that says so. Or indeed one of your 2 options (maybe there could be just a check box "Disable previous IdP" to let the admin select the behavior)

pnzrr commented 3 months ago

@kedare We have a whole new version of this UI about to be released

https://github.com/p2-inc/keycloak/pull/244

xgp commented 3 months ago

This is available in >24