diff --git a/server-spi-private/src/main/java/org/keycloak/broker/provider/BrokeredIdentityContext.java b/server-spi-private/src/main/java/org/keycloak/broker/provider/BrokeredIdentityContext.java index c78a4002b5..a4dfa6f612 100755 --- a/server-spi-private/src/main/java/org/keycloak/broker/provider/BrokeredIdentityContext.java +++ b/server-spi-private/src/main/java/org/keycloak/broker/provider/BrokeredIdentityContext.java @@ -34,6 +34,7 @@ import java.util.Map; public class BrokeredIdentityContext { private String id; + private String legacyId; private String username; private String modelUsername; private String email; @@ -64,6 +65,19 @@ public class BrokeredIdentityContext { this.id = id; } + /** + * ID from older API version. For API migrations. + * + * @return legacy ID + */ + public String getLegacyId() { + return legacyId; + } + + public void setLegacyId(String legacyId) { + this.legacyId = legacyId; + } + /** * Username in remote idp * diff --git a/server-spi/src/main/java/org/keycloak/models/FederatedIdentityModel.java b/server-spi/src/main/java/org/keycloak/models/FederatedIdentityModel.java index e7e4ee63e9..443c2c254f 100755 --- a/server-spi/src/main/java/org/keycloak/models/FederatedIdentityModel.java +++ b/server-spi/src/main/java/org/keycloak/models/FederatedIdentityModel.java @@ -38,6 +38,13 @@ public class FederatedIdentityModel { this.token = token; } + public FederatedIdentityModel(FederatedIdentityModel originalIdentity, String userId) { + identityProvider = originalIdentity.getIdentityProvider(); + this.userId = userId; + userName = originalIdentity.getUserName(); + token = originalIdentity.getToken(); + } + public String getUserId() { return userId; } diff --git a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java index 6cfd2961fe..fe59cba4d1 100755 --- a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java +++ b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java @@ -546,6 +546,13 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal .detail(Details.IDENTITY_PROVIDER_USERNAME, context.getUsername()); UserModel federatedUser = this.session.users().getUserByFederatedIdentity(federatedIdentityModel, this.realmModel); + boolean shouldMigrateId = false; + // try to find the user using legacy ID + if (federatedUser == null && context.getLegacyId() != null) { + federatedIdentityModel = new FederatedIdentityModel(federatedIdentityModel, context.getLegacyId()); + federatedUser = this.session.users().getUserByFederatedIdentity(federatedIdentityModel, this.realmModel); + shouldMigrateId = true; + } // Check if federatedUser is already authenticated (this means linking social into existing federatedUser account) UserSessionModel userSession = new AuthenticationSessionManager(session).getUserSession(authenticationSession); @@ -608,6 +615,9 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal } updateFederatedIdentity(context, federatedUser); + if (shouldMigrateId) { + migrateFederatedIdentityId(context, federatedUser); + } authenticationSession.setAuthenticatedUser(federatedUser); return finishOrRedirectToPostBrokerLogin(authenticationSession, context, false, parsedCode.clientSessionCode); @@ -1006,6 +1016,16 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal } + private void migrateFederatedIdentityId(BrokeredIdentityContext context, UserModel federatedUser) { + FederatedIdentityModel identityModel = this.session.users().getFederatedIdentity(federatedUser, context.getIdpConfig().getAlias(), this.realmModel); + FederatedIdentityModel migratedIdentityModel = new FederatedIdentityModel(identityModel, context.getId()); + + // since ID is a partial key we need to recreate the identity + session.users().removeFederatedIdentity(realmModel, federatedUser, identityModel.getIdentityProvider()); + session.users().addFederatedIdentity(realmModel, federatedUser, migratedIdentityModel); + logger.debugf("Federated user ID was migrated from %s to %s", identityModel.getUserId(), migratedIdentityModel.getUserId()); + } + private void updateToken(BrokeredIdentityContext context, UserModel federatedUser, FederatedIdentityModel federatedIdentityModel) { if (context.getIdpConfig().isStoreToken() && !ObjectUtil.isEqualOrBothNull(context.getToken(), federatedIdentityModel.getToken())) { federatedIdentityModel.setToken(context.getToken()); diff --git a/services/src/main/java/org/keycloak/social/instagram/InstagramIdentityProvider.java b/services/src/main/java/org/keycloak/social/instagram/InstagramIdentityProvider.java index 759de42a14..f460678263 100755 --- a/services/src/main/java/org/keycloak/social/instagram/InstagramIdentityProvider.java +++ b/services/src/main/java/org/keycloak/social/instagram/InstagramIdentityProvider.java @@ -27,6 +27,8 @@ import org.keycloak.broker.provider.IdentityBrokerException; import org.keycloak.broker.social.SocialIdentityProvider; import org.keycloak.models.KeycloakSession; +import java.io.IOException; + /** * @author Stian Thorgersen */ @@ -34,9 +36,11 @@ public class InstagramIdentityProvider extends AbstractOAuth2IdentityProvider im public static final String AUTH_URL = "https://api.instagram.com/oauth/authorize"; public static final String TOKEN_URL = "https://api.instagram.com/oauth/access_token"; - public static final String PROFILE_URL = "https://api.instagram.com/v1/users/self"; - public static final String DEFAULT_SCOPE = "basic"; - + public static final String PROFILE_URL = "https://graph.instagram.com/me"; + public static final String PROFILE_FIELDS = "id,username"; + public static final String DEFAULT_SCOPE = "user_profile"; + public static final String LEGACY_ID_FIELD = "ig_id"; + public InstagramIdentityProvider(KeycloakSession session, OAuth2IdentityProviderConfig config) { super(session, config); config.setAuthorizationUrl(AUTH_URL); @@ -46,25 +50,29 @@ public class InstagramIdentityProvider extends AbstractOAuth2IdentityProvider im protected BrokeredIdentityContext doGetFederatedIdentity(String accessToken) { try { - JsonNode raw = SimpleHttp.doGet(PROFILE_URL,session).param("access_token", accessToken).asJson(); - - JsonNode profile = raw.get("data"); + // try to get the profile incl. legacy Instagram ID to allow existing users to log in + JsonNode profile = fetchUserProfile(accessToken, true); + // ig_id field will get deprecated in the future and eventually might stop working (returning error) + if (!profile.has("id")) { + logger.debugf("Could not fetch user profile from instagram. Trying without %s.", LEGACY_ID_FIELD); + profile = fetchUserProfile(accessToken, false); + } logger.debug(profile.toString()); - String id = getJsonProperty(profile, "id"); + // it's not documented whether the new ID system can or cannot have conflicts with the legacy system, therefore + // we're using a custom prefix just to be sure + String id = "graph_" + getJsonProperty(profile, "id"); + String username = getJsonProperty(profile, "username"); + String legacyId = getJsonProperty(profile, LEGACY_ID_FIELD); BrokeredIdentityContext user = new BrokeredIdentityContext(id); - - String username = getJsonProperty(profile, "username"); - user.setUsername(username); - - String full_name = getJsonProperty(profile, "full_name"); - - user.setName(full_name); user.setIdpConfig(getConfig()); user.setIdp(this); + if (legacyId != null && !legacyId.isEmpty()) { + user.setLegacyId(legacyId); + } AbstractJsonUserAttributeMapper.storeUserProfileForMapper(user, profile, getConfig().getAlias()); @@ -74,6 +82,18 @@ public class InstagramIdentityProvider extends AbstractOAuth2IdentityProvider im } } + protected JsonNode fetchUserProfile(String accessToken, boolean includeIgId) throws IOException { + String fields = PROFILE_FIELDS; + if (includeIgId) { + fields += "," + LEGACY_ID_FIELD; + } + + return SimpleHttp.doGet(PROFILE_URL,session) + .param("access_token", accessToken) + .param("fields", fields) + .asJson(); + } + @Override protected String getDefaultScopes() { return DEFAULT_SCOPE; diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/broker/oidc/LegacyIdIdentityProvider.java b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/broker/oidc/LegacyIdIdentityProvider.java new file mode 100644 index 0000000000..7cd069a945 --- /dev/null +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/broker/oidc/LegacyIdIdentityProvider.java @@ -0,0 +1,41 @@ +/* + * Copyright 2020 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.broker.oidc; + +import org.keycloak.broker.oidc.KeycloakOIDCIdentityProvider; +import org.keycloak.broker.oidc.OIDCIdentityProviderConfig; +import org.keycloak.broker.provider.BrokeredIdentityContext; +import org.keycloak.models.KeycloakSession; + +/** + * @author Vaclav Muzikar + */ +public class LegacyIdIdentityProvider extends KeycloakOIDCIdentityProvider { + public static final String LEGACY_ID = "3.14159265359"; + + public LegacyIdIdentityProvider(KeycloakSession session, OIDCIdentityProviderConfig config) { + super(session, config); + } + + @Override + public BrokeredIdentityContext getFederatedIdentity(String response) { + BrokeredIdentityContext user = super.getFederatedIdentity(response); + user.setLegacyId(LEGACY_ID); + return user; + } +} diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/broker/oidc/LegacyIdIdentityProviderFactory.java b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/broker/oidc/LegacyIdIdentityProviderFactory.java new file mode 100644 index 0000000000..d4bc2ff54b --- /dev/null +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/broker/oidc/LegacyIdIdentityProviderFactory.java @@ -0,0 +1,47 @@ +/* + * Copyright 2020 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.broker.oidc; + +import org.keycloak.broker.oidc.KeycloakOIDCIdentityProvider; +import org.keycloak.broker.oidc.OIDCIdentityProviderConfig; +import org.keycloak.broker.oidc.OIDCIdentityProviderFactory; +import org.keycloak.models.IdentityProviderModel; +import org.keycloak.models.KeycloakSession; + +/** + * @author Vaclav Muzikar + */ +public class LegacyIdIdentityProviderFactory extends OIDCIdentityProviderFactory { + + public static final String PROVIDER_ID = "legacy-id-idp"; + + @Override + public String getName() { + return PROVIDER_ID; + } + + @Override + public KeycloakOIDCIdentityProvider create(KeycloakSession session, IdentityProviderModel model) { + return new LegacyIdIdentityProvider(session, new OIDCIdentityProviderConfig(model)); + } + + @Override + public String getId() { + return PROVIDER_ID; + } +} \ No newline at end of file diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/resources/META-INF/services/org.keycloak.broker.provider.IdentityProviderFactory b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/resources/META-INF/services/org.keycloak.broker.provider.IdentityProviderFactory new file mode 100644 index 0000000000..436e93e367 --- /dev/null +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/resources/META-INF/services/org.keycloak.broker.provider.IdentityProviderFactory @@ -0,0 +1,18 @@ +# +# Copyright 2020 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. +# + +org.keycloak.testsuite.broker.oidc.LegacyIdIdentityProviderFactory \ No newline at end of file diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/social/InstagramLoginPage.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/social/InstagramLoginPage.java index 401728a2d6..a57cc6bb57 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/social/InstagramLoginPage.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/social/InstagramLoginPage.java @@ -18,9 +18,12 @@ package org.keycloak.testsuite.pages.social; import org.openqa.selenium.Keys; +import org.openqa.selenium.NoSuchElementException; import org.openqa.selenium.WebElement; import org.openqa.selenium.support.FindBy; +import static org.keycloak.testsuite.util.WaitUtils.pause; + /** * @author Vaclav Muzikar */ @@ -31,11 +34,42 @@ public class InstagramLoginPage extends AbstractSocialLoginPage { @FindBy(name = "password") private WebElement passwordInput; + @FindBy(xpath = "//button[text()='Save Info']") + private WebElement saveInfoBtn; + + @FindBy(xpath = "//button[text()='Authorize']") + private WebElement authorizeBtn; + + @FindBy(xpath = "//button[text()='Continue']") + private WebElement continueBtn; + @Override public void login(String user, String password) { - usernameInput.clear(); - usernameInput.sendKeys(user); - passwordInput.sendKeys(password); - passwordInput.sendKeys(Keys.RETURN); + try { + usernameInput.clear(); + usernameInput.sendKeys(user); + passwordInput.sendKeys(password); + passwordInput.sendKeys(Keys.RETURN); + pause(2000); // wait for the login screen a bit + + try { + saveInfoBtn.click(); + } + catch (NoSuchElementException e) { + log.info("'Save Info' button not found, ignoring"); + pause(2000); // wait for the login screen a bit + } + } + catch (NoSuchElementException e) { + log.info("Instagram is already logged in, just confirmation is expected"); + } + + try { + continueBtn.click(); + } + catch (NoSuchElementException e) { + log.info("'Continue' button not found, trying 'Authorize'..."); + authorizeBtn.click(); + } } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java index 4327288a6b..9d050bf0b1 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java @@ -316,10 +316,14 @@ public abstract class AbstractBaseBrokerTest extends AbstractKeycloakTest { protected void assertLoggedInAccountManagement() { + assertLoggedInAccountManagement(bc.getUserLogin(), bc.getUserEmail()); + } + + protected void assertLoggedInAccountManagement(String username, String email) { waitForAccountManagementTitle(); Assert.assertTrue(accountUpdateProfilePage.isCurrent()); - Assert.assertEquals(accountUpdateProfilePage.getUsername(), bc.getUserLogin()); - Assert.assertEquals(accountUpdateProfilePage.getEmail(), bc.getUserEmail()); + Assert.assertEquals(accountUpdateProfilePage.getUsername(), username); + Assert.assertEquals(accountUpdateProfilePage.getEmail(), email); } protected void waitForAccountManagementTitle() { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/BrokerWithLegacyIdTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/BrokerWithLegacyIdTest.java new file mode 100644 index 0000000000..3bf0eda5fb --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/BrokerWithLegacyIdTest.java @@ -0,0 +1,105 @@ +/* + * Copyright 2020 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.broker; + +import org.junit.Test; +import org.keycloak.admin.client.resource.RealmResource; +import org.keycloak.admin.client.resource.UserResource; +import org.keycloak.models.IdentityProviderSyncMode; +import org.keycloak.representations.idm.FederatedIdentityRepresentation; +import org.keycloak.representations.idm.IdentityProviderRepresentation; +import org.keycloak.representations.idm.UserRepresentation; +import org.keycloak.testsuite.broker.oidc.LegacyIdIdentityProviderFactory; +import org.keycloak.testsuite.util.FederatedIdentityBuilder; +import org.keycloak.testsuite.util.UserBuilder; + +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.keycloak.testsuite.admin.ApiUtil.createUserWithAdminClient; +import static org.keycloak.testsuite.broker.BrokerTestConstants.IDP_OIDC_ALIAS; +import static org.keycloak.testsuite.broker.BrokerTestTools.getConsumerRoot; +import static org.keycloak.testsuite.broker.BrokerTestTools.getProviderRoot; +import static org.keycloak.testsuite.broker.oidc.LegacyIdIdentityProvider.LEGACY_ID; + +/** + * @author Vaclav Muzikar + */ +public class BrokerWithLegacyIdTest extends AbstractInitializedBaseBrokerTest { + private static final UserRepresentation consumerUser = UserBuilder.create() + .username("anakin") + .firstName("Darth") + .lastName("Vader") + .email("anakin@skywalker.tatooine") + .password("Come to the Dark Side. We have cookies") + .build(); + private UserResource consumerUserResource; + + @Override + protected BrokerConfiguration getBrokerConfiguration() { + return new KcOidcBrokerConfiguration() { + @Override + public IdentityProviderRepresentation setUpIdentityProvider(IdentityProviderSyncMode syncMode) { + IdentityProviderRepresentation idp = super.setUpIdentityProvider(syncMode); + idp.setProviderId(LegacyIdIdentityProviderFactory.PROVIDER_ID); + return idp; + } + }; + } + + @Override + public void beforeBrokerTest() { + super.beforeBrokerTest(); + RealmResource consumerRealm = realmsResouce().realm(bc.consumerRealmName()); + + String consumerUserId = createUserWithAdminClient(consumerRealm, consumerUser); + + FederatedIdentityRepresentation identity = FederatedIdentityBuilder.create() + .userId(LEGACY_ID) + .userName(bc.getUserLogin()) + .identityProvider(IDP_OIDC_ALIAS) + .build(); + + consumerUserResource = consumerRealm.users().get(consumerUserId); + consumerUserResource.addFederatedIdentity(IDP_OIDC_ALIAS, identity); + } + + @Test + public void loginWithLegacyId() { + assertEquals(LEGACY_ID, getFederatedIdentity().getUserId()); + // login as existing user with legacy ID (from e.g. a deprecated API) + logInAsUserInIDP(); + // id should be migrated to new one + assertEquals(userId, getFederatedIdentity().getUserId()); + assertLoggedInAccountManagement(consumerUser.getUsername(), consumerUser.getEmail()); + + logoutFromRealm(getProviderRoot(), bc.providerRealmName()); + logoutFromRealm(getConsumerRoot(), bc.consumerRealmName()); + + // try to login again to double check the new ID works + logInAsUserInIDP(); + assertEquals(userId, getFederatedIdentity().getUserId()); + assertLoggedInAccountManagement(consumerUser.getUsername(), consumerUser.getEmail()); + } + + private FederatedIdentityRepresentation getFederatedIdentity() { + List identities = consumerUserResource.getFederatedIdentity(); + assertEquals(1, identities.size()); + return identities.get(0); + } +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/SocialLoginTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/SocialLoginTest.java index 18f698a1a1..464eb1f7ef 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/SocialLoginTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/SocialLoginTest.java @@ -333,7 +333,7 @@ public class SocialLoginTest extends AbstractKeycloakTest { public void instagramLogin() throws InterruptedException { setTestProvider(INSTAGRAM); performLogin(); - assertUpdateProfile(false, false, true); + assertUpdateProfile(true, true, true); assertAccount(); }