Improve error management in the github provider

Closes https://github.com/keycloak/keycloak/issues/9429
This commit is contained in:
rmartinc 2023-06-20 15:14:53 +02:00 committed by Pedro Igor
parent 165c36f9ff
commit 13870f3a69
2 changed files with 44 additions and 20 deletions

View file

@ -18,7 +18,7 @@
package org.keycloak.social.github; package org.keycloak.social.github;
import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ArrayNode; import jakarta.ws.rs.core.Response;
import java.util.Iterator; import java.util.Iterator;
import org.keycloak.broker.oidc.AbstractOAuth2IdentityProvider; import org.keycloak.broker.oidc.AbstractOAuth2IdentityProvider;
import org.keycloak.broker.oidc.OAuth2IdentityProviderConfig; import org.keycloak.broker.oidc.OAuth2IdentityProviderConfig;
@ -137,36 +137,58 @@ public class GitHubIdentityProvider extends AbstractOAuth2IdentityProvider imple
@Override @Override
protected BrokeredIdentityContext doGetFederatedIdentity(String accessToken) { protected BrokeredIdentityContext doGetFederatedIdentity(String accessToken) {
try { try (SimpleHttp.Response response = SimpleHttp.doGet(profileUrl, session)
JsonNode profile = SimpleHttp.doGet(profileUrl, session).header("Authorization", "Bearer " + accessToken).asJson(); .header("Authorization", "Bearer " + accessToken)
.header("Accept", "application/json")
.asResponse()) {
BrokeredIdentityContext user = extractIdentityFromProfile(null, profile); if (Response.Status.fromStatusCode(response.getStatus()).getFamily() != Response.Status.Family.SUCCESSFUL) {
logger.warnf("Profile endpoint returned an error (%d): %s", response.getStatus(), response.asString());
throw new IdentityBrokerException("Profile could not be retrieved from the github endpoint");
}
if (user.getEmail() == null) { JsonNode profile = response.asJson();
user.setEmail(searchEmail(accessToken)); logger.tracef("profile retrieved from github: %s", profile);
} BrokeredIdentityContext user = extractIdentityFromProfile(null, profile);
return user; if (user.getEmail() == null) {
user.setEmail(searchEmail(accessToken));
}
return user;
} catch (Exception e) { } catch (Exception e) {
throw new IdentityBrokerException("Could not obtain user profile from github.", e); throw new IdentityBrokerException("Profile could not be retrieved from the github endpoint", e);
} }
} }
private String searchEmail(String accessToken) { private String searchEmail(String accessToken) {
try { try (SimpleHttp.Response response = SimpleHttp.doGet(emailUrl, session)
ArrayNode emails = (ArrayNode) SimpleHttp.doGet(emailUrl, session).header("Authorization", "Bearer " + accessToken).asJson(); .header("Authorization", "Bearer " + accessToken)
.header("Accept", "application/json")
.asResponse()) {
Iterator<JsonNode> loop = emails.elements(); if (Response.Status.fromStatusCode(response.getStatus()).getFamily() != Response.Status.Family.SUCCESSFUL) {
while (loop.hasNext()) { logger.warnf("Primary email endpoint returned an error (%d): %s", response.getStatus(), response.asString());
JsonNode mail = loop.next(); throw new IdentityBrokerException("Primary email could not be retrieved from the github endpoint");
if (mail.get("primary").asBoolean()) { }
return getJsonProperty(mail, "email");
} JsonNode emails = response.asJson();
} logger.tracef("emails retrieved from github: %s", emails);
if (emails.isArray()) {
Iterator<JsonNode> loop = emails.elements();
while (loop.hasNext()) {
JsonNode mail = loop.next();
JsonNode primary = mail.get("primary");
if (primary != null && primary.asBoolean()) {
return getJsonProperty(mail, "email");
}
}
}
throw new IdentityBrokerException("Primary email from github is not found in the user's email list.");
} catch (Exception e) { } catch (Exception e) {
throw new IdentityBrokerException("Could not obtain user email from github.", e); throw new IdentityBrokerException("Primary email could not be retrieved from the github endpoint", e);
} }
throw new IdentityBrokerException("Primary email from github is not found.");
} }
@Override @Override

View file

@ -29,6 +29,7 @@ import org.keycloak.representations.idm.authorization.DecisionStrategy;
import org.keycloak.services.resources.admin.permissions.AdminPermissionManagement; import org.keycloak.services.resources.admin.permissions.AdminPermissionManagement;
import org.keycloak.services.resources.admin.permissions.AdminPermissions; import org.keycloak.services.resources.admin.permissions.AdminPermissions;
import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.AbstractKeycloakTest;
import org.keycloak.testsuite.arquillian.annotation.EnableFeature;
import org.keycloak.testsuite.arquillian.annotation.UncaughtServerErrorExpected; import org.keycloak.testsuite.arquillian.annotation.UncaughtServerErrorExpected;
import org.keycloak.testsuite.auth.page.login.UpdateAccount; import org.keycloak.testsuite.auth.page.login.UpdateAccount;
import org.keycloak.testsuite.pages.AppPage; import org.keycloak.testsuite.pages.AppPage;
@ -97,6 +98,7 @@ import com.google.common.collect.ImmutableMap;
* @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a> * @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
* @author Vaclav Muzikar <vmuzikar@redhat.com> * @author Vaclav Muzikar <vmuzikar@redhat.com>
*/ */
@EnableFeature(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ)
public class SocialLoginTest extends AbstractKeycloakTest { public class SocialLoginTest extends AbstractKeycloakTest {
public static final String SOCIAL_CONFIG = "social.config"; public static final String SOCIAL_CONFIG = "social.config";