Do not manage brokers through the Organization API

Closes #29268

Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>

integration/admin-client-jee/src/main/java/org/keycloak/admin/client/resource/OrganizationIdentityProviderResource.java

@@ -17,10 +17,8 @@
 
 package org.keycloak.admin.client.resource;
 
-import jakarta.ws.rs.Consumes;
 import jakarta.ws.rs.DELETE;
 import jakarta.ws.rs.GET;
-import jakarta.ws.rs.PUT;
 import jakarta.ws.rs.Produces;
 import jakarta.ws.rs.core.MediaType;
 import jakarta.ws.rs.core.Response;
@@ -32,10 +30,6 @@ public interface OrganizationIdentityProviderResource {
     @Produces(MediaType.APPLICATION_JSON)
     IdentityProviderRepresentation toRepresentation();
 
-    @PUT
-    @Consumes(MediaType.APPLICATION_JSON)
-    Response update(IdentityProviderRepresentation idpRepresentation);
-
     @DELETE
     Response delete();
 }

integration/admin-client-jee/src/main/java/org/keycloak/admin/client/resource/OrganizationIdentityProvidersResource.java

@@ -33,7 +33,7 @@ public interface OrganizationIdentityProvidersResource {
 
     @POST
     @Consumes(MediaType.APPLICATION_JSON)
-    Response create(IdentityProviderRepresentation idpRepresentation);
+    Response addIdentityProvider(String id);
 
     @GET
     @Produces(MediaType.APPLICATION_JSON)

model/jpa/src/main/java/org/keycloak/organization/jpa/JpaOrganizationProvider.java

@@ -100,7 +100,7 @@ public class JpaOrganizationProvider implements OrganizationProvider {
         //TODO: won't scale, requires a better mechanism for bulk deleting users
         userProvider.getGroupMembersStream(realm, group).forEach(userModel -> removeMember(organization, userModel));
         groupProvider.removeGroup(realm, group);
-        organization.getIdentityProviders().forEach((model) -> realm.removeIdentityProviderByAlias(model.getAlias()));
+        organization.getIdentityProviders().forEach((model) -> removeIdentityProvider(organization, model));
 
         em.remove(entity);
 
@@ -216,6 +216,13 @@ public class JpaOrganizationProvider implements OrganizationProvider {
         throwExceptionIfObjectIsNull(identityProvider, "Identity provider");
 
         OrganizationEntity organizationEntity = getEntity(organization.getId());
+        String orgId = identityProvider.getOrganizationId();
+
+        if (organizationEntity.getId().equals(orgId)) {
+            return false;
+        } else if (orgId != null) {
+            throw new ModelValidationException("Identity provider already associated with a different organization");
+        }
 
         identityProvider.setOrganizationId(organizationEntity.getId());
         realm.updateIdentityProvider(identityProvider);
@@ -243,7 +250,8 @@ public class JpaOrganizationProvider implements OrganizationProvider {
             return false;
         }
 
-        realm.removeIdentityProviderByAlias(identityProvider.getAlias());
+        identityProvider.setOrganizationId(null);
+        realm.updateIdentityProvider(identityProvider);
 
         return true;
     }

server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java

@@ -60,6 +60,7 @@ import org.keycloak.client.clienttype.ClientType;
 import org.keycloak.client.clienttype.ClientTypeException;
 import org.keycloak.client.clienttype.ClientTypeManager;
 import org.keycloak.common.Profile;
+import org.keycloak.common.Profile.Feature;
 import org.keycloak.common.util.MultivaluedHashMap;
 import org.keycloak.common.util.ObjectUtil;
 import org.keycloak.common.util.UriUtils;
@@ -80,6 +81,8 @@ import org.keycloak.models.IdentityProviderModel;
 import org.keycloak.models.KeycloakContext;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.ModelException;
+import org.keycloak.models.OrganizationDomainModel;
+import org.keycloak.models.OrganizationModel;
 import org.keycloak.models.ProtocolMapperModel;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.RoleModel;
@@ -92,6 +95,7 @@ import org.keycloak.models.credential.PasswordCredentialModel;
 import org.keycloak.models.credential.dto.OTPCredentialData;
 import org.keycloak.models.credential.dto.OTPSecretData;
 import org.keycloak.models.credential.dto.PasswordCredentialData;
+import org.keycloak.organization.OrganizationProvider;
 import org.keycloak.policy.PasswordPolicyNotMetException;
 import org.keycloak.provider.ProviderConfigProperty;
 import org.keycloak.representations.idm.AuthenticationExecutionRepresentation;
@@ -875,6 +879,7 @@ public class RepresentationToModel {
         identityProviderModel.setAuthenticateByDefault(representation.isAuthenticateByDefault());
         identityProviderModel.setStoreToken(representation.isStoreToken());
         identityProviderModel.setAddReadTokenRoleOnCreate(representation.isAddReadTokenRoleOnCreate());
+        updateOrganizationBroker(realm, representation, session);
         identityProviderModel.setConfig(removeEmptyString(representation.getConfig()));
 
         String flowAlias = representation.getFirstBrokerLoginFlowAlias();
@@ -1645,4 +1650,34 @@ public class RepresentationToModel {
                 .map(HashSet::new)
                 .orElse(null);
     }
+
+    private static void updateOrganizationBroker(RealmModel realm, IdentityProviderRepresentation representation, KeycloakSession session) {
+        if (!Profile.isFeatureEnabled(Feature.ORGANIZATION)) {
+            return;
+        }
+
+        IdentityProviderModel existing = realm.getIdentityProvidersStream()
+                .filter(p -> Objects.equals(p.getAlias(), representation.getAlias()) || Objects.equals(p.getInternalId(), representation.getInternalId()))
+                .findFirst().orElse(null);
+        String orgId = existing == null ? representation.getConfig().get(OrganizationModel.ORGANIZATION_ATTRIBUTE) : existing.getOrganizationId();
+
+        if (orgId != null) {
+            OrganizationProvider provider = session.getProvider(OrganizationProvider.class);
+            OrganizationModel org = provider.getById(orgId);
+            String newOrgId = representation.getConfig().get(OrganizationModel.ORGANIZATION_ATTRIBUTE);
+
+            if (org == null || (newOrgId != null && provider.getById(newOrgId) == null)) {
+                throw new IllegalArgumentException("Organization associated with broker does not exist");
+            }
+
+            String domain = representation.getConfig().get(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE);
+
+            if (domain != null && org.getDomains().map(OrganizationDomainModel::getName).noneMatch(domain::equals)) {
+                throw new IllegalArgumentException("Domain does not match any domain from the organization");
+            }
+
+            // make sure the link to an organization does not change
+            representation.getConfig().put(OrganizationModel.ORGANIZATION_ATTRIBUTE, orgId);
+        }
+    }
 }

services/src/main/java/org/keycloak/organization/admin/resource/OrganizationIdentityProvidersResource.java

@@ -18,6 +18,7 @@
 package org.keycloak.organization.admin.resource;
 
 import jakarta.ws.rs.Consumes;
+import jakarta.ws.rs.DELETE;
 import jakarta.ws.rs.GET;
 import jakarta.ws.rs.POST;
 import jakarta.ws.rs.Path;
@@ -28,13 +29,12 @@ import jakarta.ws.rs.core.Response;
 import jakarta.ws.rs.core.Response.Status;
 import jakarta.ws.rs.ext.Provider;
 
-import java.util.List;
+import java.util.Objects;
 import java.util.stream.Stream;
 
 import org.keycloak.models.IdentityProviderModel;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.ModelException;
-import org.keycloak.models.OrganizationDomainModel;
 import org.keycloak.models.OrganizationModel;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.utils.ModelToRepresentation;
@@ -42,19 +42,15 @@ import org.keycloak.organization.OrganizationProvider;
 import org.keycloak.representations.idm.IdentityProviderRepresentation;
 import org.keycloak.services.ErrorResponse;
 import org.keycloak.services.resources.admin.AdminEventBuilder;
-import org.keycloak.services.resources.admin.IdentityProviderResource;
-import org.keycloak.services.resources.admin.IdentityProvidersResource;
 import org.keycloak.services.resources.admin.permissions.AdminPermissionEvaluator;
 
 @Provider
 public class OrganizationIdentityProvidersResource {
 
-    private final KeycloakSession session;
     private final RealmModel realm;
     private final OrganizationProvider organizationProvider;
     private final OrganizationModel organization;
     private final AdminPermissionEvaluator auth;
-    private final AdminEventBuilder adminEvent;
 
     public OrganizationIdentityProvidersResource() {
         // needed for registering to the JAX-RS stack
@@ -62,38 +58,36 @@ public class OrganizationIdentityProvidersResource {
     }
 
     public OrganizationIdentityProvidersResource(KeycloakSession session, OrganizationModel organization, AdminPermissionEvaluator auth, AdminEventBuilder adminEvent) {
-        this.session = session;
         this.realm = session == null ? null : session.getContext().getRealm();
         this.organizationProvider = session == null ? null : session.getProvider(OrganizationProvider.class);
         this.organization = organization;
         this.auth = auth;
-        this.adminEvent = adminEvent;
     }
 
     @POST
     @Consumes(MediaType.APPLICATION_JSON)
-    public Response addIdentityProvider(IdentityProviderRepresentation providerRep) {
+    public Response addIdentityProvider(String id) {
         auth.realm().requireManageRealm();
 
-        Response response = new IdentityProvidersResource(realm, session, auth, adminEvent).create(providerRep);
-
-        if (Status.CREATED.getStatusCode() == response.getStatus()) {
         try {
-                IdentityProviderModel identityProvider = realm.getIdentityProviderByAlias(providerRep.getAlias());
+            IdentityProviderModel identityProvider =  this.realm.getIdentityProvidersStream()
+                    .filter(p -> Objects.equals(p.getAlias(), id) || Objects.equals(p.getInternalId(), id))
+                    .findFirst().orElse(null);
 
-                if (organizationProvider.addIdentityProvider(organization, identityProvider)) {
+            if (identityProvider == null) {
-                    return response;
+                throw ErrorResponse.error("Identity provider not found with the given alias", Status.BAD_REQUEST);
             }
 
-                throw ErrorResponse.error("Identity provider already associated to the organization", Status.BAD_REQUEST);
+            if (organizationProvider.addIdentityProvider(organization, identityProvider)) {
+                return Response.noContent().build();
+            }
+
+            throw ErrorResponse.error("Identity provider already associated to the organization", Status.CONFLICT);
         } catch (ModelException me) {
             throw ErrorResponse.error(me.getMessage(), Status.BAD_REQUEST);
         }
     }
 
-        return response;
-    }
-
     @GET
     @Produces(MediaType.APPLICATION_JSON)
     public Stream<IdentityProviderRepresentation> getIdentityProviders() {
@@ -102,37 +96,40 @@ public class OrganizationIdentityProvidersResource {
     }
 
     @Path("{alias}")
-    public IdentityProviderResource getIdentityProvider(@PathParam("alias") String alias) {
+    @GET
+    @Produces(MediaType.APPLICATION_JSON)
+    public IdentityProviderRepresentation getIdentityProvider(@PathParam("alias") String alias) {
         IdentityProviderModel broker = realm.getIdentityProviderByAlias(alias);
-        return new IdentityProviderResource(auth, realm, session, broker, adminEvent) {
+
-            @Override
+        if (!isOrganizationBroker(broker)) {
-            public Response delete() {
+            throw ErrorResponse.error("Identity provider not associated with the organization", Status.NOT_FOUND);
-                Response response = super.delete();
+        }
+
+        return ModelToRepresentation.toRepresentation(realm, broker);
+    }
+
+    @Path("{alias}")
+    @DELETE
+    @Produces(MediaType.APPLICATION_JSON)
+    public Response delete(@PathParam("alias") String alias) {
+        IdentityProviderModel broker = realm.getIdentityProviderByAlias(alias);
+
+        if (!isOrganizationBroker(broker)) {
+            throw ErrorResponse.error("Identity provider not found with the given alias", Status.NOT_FOUND);
+        }
 
         if (organizationProvider.removeIdentityProvider(organization, broker)) {
-                    return response;
+            return Response.noContent().build();
         }
 
         throw ErrorResponse.error("Identity provider not associated with the organization", Status.BAD_REQUEST);
     }
 
-            @Override
-            public Response update(IdentityProviderRepresentation providerRep) {
-                if (organization.getIdentityProviders().noneMatch(model -> model.getInternalId().equals(providerRep.getInternalId()) || model.getAlias().equals(providerRep.getAlias()))) {
-                    return Response.status(Status.NOT_FOUND).build();
-                }
-                String domain = providerRep.getConfig().get(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE);
-
-                if (domain != null && organization.getDomains().map(OrganizationDomainModel::getName).noneMatch(domain::equals)) {
-                    return Response.status(Status.BAD_REQUEST).build();
-                }
-
-                return super.update(providerRep);
-            }
-        };
-    }
-
     private IdentityProviderRepresentation toRepresentation(IdentityProviderModel idp) {
         return ModelToRepresentation.toRepresentation(realm, idp);
     }
+
+    private boolean isOrganizationBroker(IdentityProviderModel broker) {
+        return broker != null && organization.getId().equals(broker.getOrganizationId());
+    }
 }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractOrganizationTest.java

@@ -30,6 +30,7 @@ import org.jboss.arquillian.graphene.page.Page;
 import org.keycloak.admin.client.resource.OrganizationResource;
 import org.keycloak.representations.idm.ClientRepresentation;
 import org.keycloak.admin.client.resource.UsersResource;
+import org.keycloak.representations.idm.IdentityProviderRepresentation;
 import org.keycloak.representations.idm.OrganizationDomainRepresentation;
 import org.keycloak.representations.idm.OrganizationRepresentation;
 import org.keycloak.representations.idm.RealmRepresentation;
@@ -144,8 +145,10 @@ public abstract class AbstractOrganizationTest extends AbstractAdminTest  {
             assertEquals(Status.CREATED.getStatusCode(), response.getStatus());
             id = ApiUtil.getCreatedId(response);
         }
-
+        IdentityProviderRepresentation broker = brokerConfigFunction.apply(name).setUpIdentityProvider();
-        testRealm().organizations().get(id).identityProviders().create(brokerConfigFunction.apply(name).setUpIdentityProvider()).close();
+        testRealm().identityProviders().create(broker).close();
+        getCleanup().addCleanup(testRealm().identityProviders().get(broker.getAlias())::remove);
+        testRealm().organizations().get(id).identityProviders().addIdentityProvider(broker.getAlias()).close();
         org = testRealm().organizations().get(id).toRepresentation();
         getCleanup().addCleanup(() -> testRealm().organizations().get(id).delete().close());
 

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/BrokerConfigurationWrapper.java

@@ -0,0 +1,130 @@
+/*
+ * Copyright 2024 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.keycloak.testsuite.organization.admin;
+
+import static org.keycloak.testsuite.AbstractTestRealmKeycloakTest.TEST_REALM_NAME;
+
+import java.util.List;
+
+import org.keycloak.models.IdentityProviderSyncMode;
+import org.keycloak.representations.idm.ClientRepresentation;
+import org.keycloak.representations.idm.IdentityProviderRepresentation;
+import org.keycloak.representations.idm.RealmRepresentation;
+import org.keycloak.testsuite.broker.BrokerConfiguration;
+import org.keycloak.testsuite.util.UserBuilder;
+
+public class BrokerConfigurationWrapper implements BrokerConfiguration {
+
+    private final String orgName;
+    private final BrokerConfiguration delegate;
+
+    public BrokerConfigurationWrapper(String orgName, BrokerConfiguration delegate) {
+        this.orgName = orgName;
+        this.delegate = delegate;
+    }
+
+    @Override
+    public String consumerRealmName() {
+        return TEST_REALM_NAME;
+    }
+
+    @Override
+    public RealmRepresentation createProviderRealm() {
+        RealmRepresentation providerRealm = delegate.createProviderRealm();
+
+        providerRealm.setClients(createProviderClients());
+        providerRealm.setUsers(List.of(
+                        UserBuilder.create()
+                                .username(getUserLogin())
+                                .email(getUserEmail())
+                                .password(getUserPassword())
+                                .enabled(true)
+                                .build(),
+                        UserBuilder.create()
+                                .username("external")
+                                .email("external@user.org")
+                                .password("password")
+                                .enabled(true)
+                                .build()
+                )
+        );
+
+        return providerRealm;
+    }
+
+    @Override
+    public String getUserEmail() {
+        return getUserLogin() + "@" + orgName + ".org";
+    }
+
+    @Override
+    public String getIDPAlias() {
+        return orgName + "-identity-provider";
+    }
+
+    @Override
+    public IdentityProviderRepresentation setUpIdentityProvider() {
+        IdentityProviderRepresentation broker = delegate.setUpIdentityProvider();
+        broker.setAlias(getIDPAlias());
+        return broker;
+    }
+
+    @Override
+    public List<ClientRepresentation> createProviderClients() {
+        List<ClientRepresentation> clients = delegate.createProviderClients();
+        clients.get(0).setRedirectUris(List.of("*"));
+        return clients;
+    }
+
+    @Override
+    public RealmRepresentation createConsumerRealm() {
+        return delegate.createConsumerRealm();
+    }
+
+    @Override
+    public List<ClientRepresentation> createConsumerClients() {
+        return delegate.createConsumerClients();
+    }
+
+    @Override
+    public IdentityProviderRepresentation setUpIdentityProvider(IdentityProviderSyncMode force) {
+        IdentityProviderRepresentation broker = delegate.setUpIdentityProvider(force);
+        broker.setAlias(getIDPAlias());
+        return broker;
+    }
+
+    @Override
+    public String providerRealmName() {
+        return delegate.providerRealmName();
+    }
+
+    @Override
+    public String getIDPClientIdInProviderRealm() {
+        return delegate.getIDPClientIdInProviderRealm();
+    }
+
+    @Override
+    public String getUserLogin() {
+        return delegate.getUserLogin();
+    }
+
+    @Override
+    public String getUserPassword() {
+        return delegate.getUserPassword();
+    }
+}

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationAdminPermissionsTest.java

@@ -105,13 +105,15 @@ public class OrganizationAdminPermissionsTest extends AbstractOrganizationTest {
             IdentityProviderRepresentation idpRep = new IdentityProviderRepresentation();
             idpRep.setAlias("dummy");
             idpRep.setProviderId("oidc");
+            realmAdminResource.identityProviders().create(idpRep).close();
+
             //create IdP
             try (
-                    Response userResponse = realmUserResource.organizations().get(orgId).identityProviders().create(idpRep);
+                    Response userResponse = realmUserResource.organizations().get(orgId).identityProviders().addIdentityProvider(idpRep.getAlias());
-                    Response adminResponse = realmAdminResource.organizations().get(orgId).identityProviders().create(idpRep)
+                    Response adminResponse = realmAdminResource.organizations().get(orgId).identityProviders().addIdentityProvider(idpRep.getAlias())
             ) {
                 assertThat(userResponse.getStatus(), equalTo(Status.FORBIDDEN.getStatusCode()));
-                assertThat(adminResponse.getStatus(), equalTo(Status.CREATED.getStatusCode()));
+                assertThat(adminResponse.getStatus(), equalTo(Status.NO_CONTENT.getStatusCode()));
                 getCleanup().addCleanup(() -> testRealm().organizations().get(orgId).identityProviders().get(idpRep.getAlias()).delete().close());
             }
 
@@ -127,11 +129,6 @@ public class OrganizationAdminPermissionsTest extends AbstractOrganizationTest {
             } catch (ForbiddenException expected) {}
             assertThat(realmAdminResource.organizations().get(orgId).identityProviders().get(idpRep.getAlias()).toRepresentation(), Matchers.notNullValue());
 
-            //update IdP
-            try (Response userResponse = realmUserResource.organizations().get(orgId).identityProviders().get(idpRep.getAlias()).update(idpRep)) {
-                assertThat(userResponse.getStatus(), equalTo(Status.FORBIDDEN.getStatusCode()));
-            }
-
             //delete IdP
             try (Response userResponse = realmUserResource.organizations().get(orgId).identityProviders().get(idpRep.getAlias()).delete()) {
                 assertThat(userResponse.getStatus(), equalTo(Status.FORBIDDEN.getStatusCode()));

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationBrokerSelfRegistrationTest.java

@@ -61,7 +61,7 @@ public class OrganizationBrokerSelfRegistrationTest extends AbstractOrganization
         OrganizationResource organization = testRealm().organizations().get(createOrganization().getId());
         IdentityProviderRepresentation idp = organization.identityProviders().get(bc.getIDPAlias()).toRepresentation();
         idp.getConfig().put(IdentityProviderModel.LOGIN_HINT, "true");
-        organization.identityProviders().get(bc.getIDPAlias()).update(idp).close();
+        testRealm().identityProviders().get(bc.getIDPAlias()).update(idp);
 
         oauth.clientId("broker-app");
         loginPage.open(bc.consumerRealmName());
@@ -174,6 +174,7 @@ public class OrganizationBrokerSelfRegistrationTest extends AbstractOrganization
         // logout to force the user to authenticate again
         UserRepresentation account = getUserRepresentation(bc.getUserEmail());
         realmsResouce().realm(bc.consumerRealmName()).users().get(account.getId()).logout();
+        realmsResouce().realm(bc.providerRealmName()).logoutAll();
 
         // login with email only
         loginPage.open(bc.consumerRealmName());
@@ -197,7 +198,7 @@ public class OrganizationBrokerSelfRegistrationTest extends AbstractOrganization
         OrganizationIdentityProviderResource idp = organization.identityProviders().get(bc.getIDPAlias());
         IdentityProviderRepresentation idpRep = idp.toRepresentation();
         idpRep.getConfig().put(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE, "neworg.org");
-        idp.update(idpRep).close();
+        testRealm().identityProviders().get(idpRep.getAlias()).update(idpRep);
 
         // add the member for the first time
         assertBrokerRegistration(organization, bc.getUserEmail());
@@ -248,11 +249,13 @@ public class OrganizationBrokerSelfRegistrationTest extends AbstractOrganization
         OrganizationResource organization = testRealm().organizations().get(createOrganization().getId());
         IdentityProviderRepresentation idp = organization.identityProviders().get(bc.getIDPAlias()).toRepresentation();
         idp.getConfig().put(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE, "neworg.org");
-        organization.identityProviders().get(bc.getIDPAlias()).update(idp).close();
+        testRealm().identityProviders().get(bc.getIDPAlias()).update(idp);
         idp.setAlias("second-idp");
         idp.setInternalId(null);
         idp.getConfig().remove(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE);
-        organization.identityProviders().create(idp).close();
+        testRealm().identityProviders().create(idp).close();
+        getCleanup().addCleanup(testRealm().identityProviders().get("second-idp")::remove);
+        organization.identityProviders().addIdentityProvider(idp.getAlias()).close();
 
         oauth.clientId("broker-app");
         loginPage.open(bc.consumerRealmName());
@@ -290,7 +293,7 @@ public class OrganizationBrokerSelfRegistrationTest extends AbstractOrganization
         IdentityProviderRepresentation idp = organization.identityProviders().get(bc.getIDPAlias()).toRepresentation();
         idp.getConfig().put(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE, "neworg.org");
         // set a domain to the existing broker
-        organization.identityProviders().get(bc.getIDPAlias()).update(idp).close();
+        testRealm().identityProviders().get(bc.getIDPAlias()).update(idp);
 
         idp = bc.setUpIdentityProvider();
         idp.setAlias("second-idp");
@@ -298,7 +301,9 @@ public class OrganizationBrokerSelfRegistrationTest extends AbstractOrganization
         idp.getConfig().remove(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE);
         idp.getConfig().put(OrganizationModel.BROKER_PUBLIC, Boolean.TRUE.toString());
         // create a second broker without a domain set
-        organization.identityProviders().create(idp).close();
+        testRealm().identityProviders().create(idp).close();
+        getCleanup().addCleanup(testRealm().identityProviders().get("second-idp")::remove);
+        organization.identityProviders().addIdentityProvider(idp.getAlias()).close();
         idp = organization.identityProviders().get(idp.getAlias()).toRepresentation();
 
         oauth.clientId("broker-app");
@@ -313,7 +318,7 @@ public class OrganizationBrokerSelfRegistrationTest extends AbstractOrganization
         Assert.assertTrue(loginPage.isSocialButtonPresent(idp.getAlias()));
 
         idp.getConfig().put(OrganizationModel.BROKER_PUBLIC, Boolean.FALSE.toString());
-        organization.identityProviders().get(idp.getAlias()).update(idp).close();
+        testRealm().identityProviders().get(idp.getAlias()).update(idp);
         driver.navigate().refresh();
         Assert.assertTrue(loginPage.isPasswordInputPresent());
         Assert.assertFalse(loginPage.isSocialButtonPresent(bc.getIDPAlias()));
@@ -326,7 +331,7 @@ public class OrganizationBrokerSelfRegistrationTest extends AbstractOrganization
         IdentityProviderRepresentation idp = organization.identityProviders().get(bc.getIDPAlias()).toRepresentation();
         idp.getConfig().put(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE, "neworg.org");
         // set a domain to the existing broker
-        organization.identityProviders().get(bc.getIDPAlias()).update(idp).close();
+        testRealm().identityProviders().get(bc.getIDPAlias()).update(idp);
 
         idp = bc.setUpIdentityProvider();
         idp.setAlias("second-idp");
@@ -334,7 +339,9 @@ public class OrganizationBrokerSelfRegistrationTest extends AbstractOrganization
         idp.getConfig().remove(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE);
         idp.getConfig().put(OrganizationModel.BROKER_PUBLIC, Boolean.TRUE.toString());
         // create a second broker without a domain set
-        organization.identityProviders().create(idp).close();
+        testRealm().identityProviders().create(idp).close();
+        getCleanup().addCleanup(testRealm().identityProviders().get("second-idp")::remove);
+        organization.identityProviders().addIdentityProvider(idp.getAlias()).close();
 
         oauth.clientId("broker-app");
         loginPage.open(bc.consumerRealmName());
@@ -375,7 +382,7 @@ public class OrganizationBrokerSelfRegistrationTest extends AbstractOrganization
         IdentityProviderRepresentation idp = organization.identityProviders().get(bc.getIDPAlias()).toRepresentation();
         idp.getConfig().put(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE, "neworg.org");
         // set a domain to the existing broker
-        organization.identityProviders().get(bc.getIDPAlias()).update(idp).close();
+        testRealm().identityProviders().get(bc.getIDPAlias()).update(idp);
 
         idp = bc.setUpIdentityProvider();
         idp.setAlias("second-idp");
@@ -383,7 +390,9 @@ public class OrganizationBrokerSelfRegistrationTest extends AbstractOrganization
         idp.getConfig().put(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE, "other.org");
         idp.getConfig().put(OrganizationModel.BROKER_PUBLIC, Boolean.TRUE.toString());
         // create a second broker without a domain set
-        organization.identityProviders().create(idp).close();
+        testRealm().identityProviders().create(idp).close();
+        getCleanup().addCleanup(testRealm().identityProviders().get("second-idp")::remove);
+        organization.identityProviders().addIdentityProvider(idp.getAlias()).close();
 
         oauth.clientId("broker-app");
         loginPage.open(bc.consumerRealmName());
@@ -421,14 +430,16 @@ public class OrganizationBrokerSelfRegistrationTest extends AbstractOrganization
         IdentityProviderRepresentation idp = organization.identityProviders().get(bc.getIDPAlias()).toRepresentation();
         idp.getConfig().put(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE, "neworg.org");
         // set a domain to the existing broker
-        organization.identityProviders().get(bc.getIDPAlias()).update(idp).close();
+        testRealm().identityProviders().get(bc.getIDPAlias()).update(idp);
 
         idp = bc.setUpIdentityProvider();
         idp.setAlias("second-idp");
         idp.setInternalId(null);
         idp.getConfig().put(OrganizationModel.BROKER_PUBLIC, Boolean.TRUE.toString());
         // create a second broker without a domain set
-        organization.identityProviders().create(idp).close();
+        testRealm().identityProviders().create(idp).close();
+        getCleanup().addCleanup(testRealm().identityProviders().get("second-idp")::remove);
+        organization.identityProviders().addIdentityProvider(idp.getAlias()).close();
 
         oauth.clientId("broker-app");
         loginPage.open(bc.consumerRealmName());
@@ -498,7 +509,7 @@ public class OrganizationBrokerSelfRegistrationTest extends AbstractOrganization
 
         // make sure the user can select this idp from the organization when authenticating
         idpRep.getConfig().put(OrganizationModel.BROKER_PUBLIC, Boolean.TRUE.toString());
-        organization.identityProviders().get(idpRep.getAlias()).update(idpRep).close();
+        testRealm().identityProviders().get(idpRep.getAlias()).update(idpRep);
 
         // create a user to the provider realm using a email that does not share the same domain as the org
         UserRepresentation user = UserBuilder.create()
@@ -536,7 +547,7 @@ public class OrganizationBrokerSelfRegistrationTest extends AbstractOrganization
 
         // make sure the user can select this idp from the organization when authenticating
         idpRep.getConfig().put(OrganizationModel.BROKER_PUBLIC, Boolean.TRUE.toString());
-        organization.identityProviders().get(idpRep.getAlias()).update(idpRep).close();
+        testRealm().identityProviders().get(idpRep.getAlias()).update(idpRep);
 
         // create a user to the provider realm using a email that does not share the same domain as the org
         UserRepresentation user = UserBuilder.create()
@@ -571,6 +582,7 @@ public class OrganizationBrokerSelfRegistrationTest extends AbstractOrganization
         updateAccountInformationPage.updateAccountInformation(user.getUsername(), user.getEmail(), "Firstname", "Lastname");
         UserRepresentation account = getUserRepresentation(user.getEmail());
         realmsResouce().realm(bc.consumerRealmName()).users().get(account.getId()).logout();
+        realmsResouce().realm(bc.providerRealmName()).logoutAll();
 
         // the flow now changed and the user should be automatically redirected to the origin broker
         loginPage.open(bc.consumerRealmName());

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationIdentityProviderTest.java

@@ -20,11 +20,13 @@ package org.keycloak.testsuite.organization.admin;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
 
+import jakarta.ws.rs.BadRequestException;
 import jakarta.ws.rs.NotFoundException;
 import jakarta.ws.rs.core.Response;
 import jakarta.ws.rs.core.Response.Status;
 import org.junit.Assert;
 import org.junit.Test;
+import org.keycloak.admin.client.resource.IdentityProviderResource;
 import org.keycloak.admin.client.resource.OrganizationIdentityProviderResource;
 import org.keycloak.admin.client.resource.OrganizationResource;
 import org.keycloak.common.Profile.Feature;
@@ -41,28 +43,35 @@ public class OrganizationIdentityProviderTest extends AbstractOrganizationTest {
         OrganizationRepresentation organization = createOrganization();
         OrganizationIdentityProviderResource orgIdPResource = testRealm().organizations().get(organization.getId())
                 .identityProviders().get(bc.getIDPAlias());
-        IdentityProviderRepresentation actual = orgIdPResource.toRepresentation();
+        IdentityProviderRepresentation expected = orgIdPResource.toRepresentation();
-        IdentityProviderRepresentation expected = actual;
-        assertThat(expected.getAlias(), equalTo(bc.getIDPAlias()));
 
-        //update
+        // organization link set
-        expected.setAlias("changed-alias");
+        Assert.assertEquals(expected.getConfig().get(OrganizationModel.ORGANIZATION_ATTRIBUTE), organization.getId());
-        expected.setDisplayName("My Org Broker");
+
-        expected.getConfig().put("test", "value");
+        IdentityProviderResource idpResource = testRealm().identityProviders().get(expected.getAlias());
-        try (Response response = orgIdPResource.update(expected)) {
+        IdentityProviderRepresentation actual = idpResource.toRepresentation();
-            assertThat(response.getStatus(), equalTo(Response.Status.NO_CONTENT.getStatusCode()));
+        Assert.assertEquals(actual.getConfig().get(OrganizationModel.ORGANIZATION_ATTRIBUTE), organization.getId());
-        }
+        actual.getConfig().put(OrganizationModel.ORGANIZATION_ATTRIBUTE, "somethingelse");
         try {
-            orgIdPResource.toRepresentation();
+            idpResource.update(actual);
-            Assert.fail("should fail because the alias changed");
+            Assert.fail("Should fail because it maps to an invalid org");
-        } catch (NotFoundException ignore) {
+        } catch (BadRequestException ignore) {
-
         }
-        orgIdPResource = testRealm().organizations().get(organization.getId()).identityProviders().get(expected.getAlias());
+
-        actual = orgIdPResource.toRepresentation();
+        OrganizationRepresentation secondOrg = createOrganization("secondorg");
-        assertThat(expected.getAlias(), equalTo(actual.getAlias()));
+        actual.getConfig().put(OrganizationModel.ORGANIZATION_ATTRIBUTE, secondOrg.getId());
-        assertThat(expected.getDisplayName(), equalTo(actual.getDisplayName()));
+        idpResource.update(actual);
-        Assert.assertEquals(expected.getConfig().get("test"), actual.getConfig().get("test"));
+        actual = idpResource.toRepresentation();
+        Assert.assertEquals(actual.getConfig().get(OrganizationModel.ORGANIZATION_ATTRIBUTE), organization.getId());
+
+        actual = idpResource.toRepresentation();
+        // the link to the organization should not change
+        Assert.assertEquals(actual.getConfig().get(OrganizationModel.ORGANIZATION_ATTRIBUTE), organization.getId());
+        actual.getConfig().remove(OrganizationModel.ORGANIZATION_ATTRIBUTE);
+        idpResource.update(actual);
+        actual = idpResource.toRepresentation();
+        // the link to the organization should not change
+        Assert.assertEquals(actual.getConfig().get(OrganizationModel.ORGANIZATION_ATTRIBUTE), organization.getId());
     }
 
     @Test
@@ -74,13 +83,15 @@ public class OrganizationIdentityProviderTest extends AbstractOrganizationTest {
         for (int i = 0; i < 5; i++) {
             idpTemplate.setAlias("idp-" + i);
             idpTemplate.setInternalId(null);
-            organization.identityProviders().create(idpTemplate).close();
+            testRealm().identityProviders().create(idpTemplate).close();
+            organization.identityProviders().addIdentityProvider(idpTemplate.getAlias()).close();
         }
 
         Assert.assertEquals(6, organization.identityProviders().getIdentityProviders().size());
 
         for (int i = 0; i < 5; i++) {
-            OrganizationIdentityProviderResource idpResource = organization.identityProviders().get("idp-" + i);
+            String alias = "idp-" + i;
+            OrganizationIdentityProviderResource idpResource = organization.identityProviders().get(alias);
 
             try (Response response = idpResource.delete()) {
                 assertThat(response.getStatus(), equalTo(Response.Status.NO_CONTENT.getStatusCode()));
@@ -91,11 +102,13 @@ public class OrganizationIdentityProviderTest extends AbstractOrganizationTest {
                 Assert.fail("should be removed");
             } catch (NotFoundException expected) {
             }
+
+            // not removed from the realm
+            testRealm().identityProviders().get(alias).toRepresentation();
         }
 
         organization.identityProviders().get(bc.getIDPAlias()).delete().close();
-
+        Assert.assertFalse(testRealm().identityProviders().findAll().isEmpty());
-        Assert.assertTrue(testRealm().identityProviders().findAll().isEmpty());
     }
 
     @Test
@@ -108,20 +121,25 @@ public class OrganizationIdentityProviderTest extends AbstractOrganizationTest {
 
         String alias = idpRepresentation.getAlias();
         idpRepresentation.setAlias("another-idp");
+        testRealm().identityProviders().create(idpRepresentation).close();
 
-        try (Response response = organization.identityProviders().create(idpRepresentation)) {
+        try (Response response = organization.identityProviders().addIdentityProvider(alias)) {
+            // already associated with the org
             assertThat(response.getStatus(), equalTo(Status.CONFLICT.getStatusCode()));
         }
 
         idpRepresentation.setAlias(alias);
         idpRepresentation.setInternalId(null);
 
-        try (Response response = organization.identityProviders().create(idpRepresentation)) {
+        OrganizationResource secondOrg = testRealm().organizations().get(createOrganization("secondorg").getId());
-            assertThat(response.getStatus(), equalTo(Status.CONFLICT.getStatusCode()));
+
+        try (Response response = secondOrg.identityProviders().addIdentityProvider(alias)) {
+            // associated with another org
+            assertThat(response.getStatus(), equalTo(Status.BAD_REQUEST.getStatusCode()));
         }
     }
 
-    @Test(expected = jakarta.ws.rs.NotFoundException.class)
+    @Test
     public void testRemovingOrgShouldRemoveIdP() {
         OrganizationRepresentation orgRep = createOrganization();
         OrganizationResource orgResource = testRealm().organizations().get(orgRep.getId());
@@ -130,8 +148,10 @@ public class OrganizationIdentityProviderTest extends AbstractOrganizationTest {
             assertThat(response.getStatus(), equalTo(Response.Status.NO_CONTENT.getStatusCode()));
         }
 
-        testRealm().identityProviders().get(bc.getIDPAlias()).toRepresentation();
+        // broker not removed from realm
-        Assert.assertTrue(testRealm().identityProviders().findAll().isEmpty());
+        IdentityProviderRepresentation idpRep = testRealm().identityProviders().get(bc.getIDPAlias()).toRepresentation();
+        // broker no longer linked to the org
+        Assert.assertNull(idpRep.getConfig().get(OrganizationModel.ORGANIZATION_ATTRIBUTE));
     }
 
     @Test
@@ -144,10 +164,6 @@ public class OrganizationIdentityProviderTest extends AbstractOrganizationTest {
         //create IdP in realm not bound to Org
         testRealm().identityProviders().create(idpRepresentation).close();
 
-        try (Response response = orgIdPResource.update(idpRepresentation)) {
-            assertThat(response.getStatus(), equalTo(Response.Status.NOT_FOUND.getStatusCode()));
-        }
-
         try (Response response = orgIdPResource.delete()) {
             assertThat(response.getStatus(), equalTo(Status.NO_CONTENT.getStatusCode()));
         }
@@ -157,33 +173,6 @@ public class OrganizationIdentityProviderTest extends AbstractOrganizationTest {
         }
     }
 
-    @Test
-    public void tryUpdateIdPWithValidAliasInvalidInternalId() {
-        OrganizationRepresentation orgRep = createOrganization();
-        OrganizationResource orgResource = testRealm().organizations().get(orgRep.getId());
-
-        OrganizationIdentityProviderResource orgIdPResource = orgResource.identityProviders().get(bc.getIDPAlias());
-
-        IdentityProviderRepresentation idpRepresentation = createRep("some-broker", "oidc");
-        //create IdP in realm not bound to Org and get created internalId
-        testRealm().identityProviders().create(idpRepresentation).close();
-        getCleanup().addCleanup(() -> testRealm().identityProviders().get(idpRepresentation.getAlias()).remove());
-        String internalId = testRealm().identityProviders().get("some-broker").toRepresentation().getInternalId();
-
-        IdentityProviderRepresentation orgIdPRep = orgIdPResource.toRepresentation();
-        orgIdPRep.setInternalId(internalId);
-
-        try (Response response = orgIdPResource.update(orgIdPRep)) {
-            assertThat(response.getStatus(), equalTo(Status.CONFLICT.getStatusCode()));
-        }
-
-        orgIdPRep.setAlias("some-broker-alias");
-
-        try (Response response = orgIdPResource.update(orgIdPRep)) {
-            assertThat(response.getStatus(), equalTo(Response.Status.NOT_FOUND.getStatusCode()));
-        }
-    }
-
     @Test
     public void testAssignDomainNotBoundToOrganization() {
         OrganizationRepresentation orgRep = createOrganization();
@@ -192,8 +181,17 @@ public class OrganizationIdentityProviderTest extends AbstractOrganizationTest {
         IdentityProviderRepresentation idpRep = orgIdPResource.toRepresentation();
         idpRep.getConfig().put(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE, "unknown.org");
 
-        try (Response response = orgIdPResource.update(idpRep)) {
+        try {
-            assertThat(response.getStatus(), equalTo(Status.BAD_REQUEST.getStatusCode()));
+            testRealm().identityProviders().get(idpRep.getAlias()).update(idpRep);
+            Assert.fail("Domain set to broker is invalid");
+        } catch (BadRequestException ignore) {
+
+        }
+
+        idpRep.setAlias("newbroker");
+        idpRep.setInternalId(null);
+        try (Response response = testRealm().identityProviders().create(idpRep)) {
+            Assert.assertEquals(Status.BAD_REQUEST.getStatusCode(), response.getStatus());
         }
     }