Skip to content

Commit

Permalink
[KEYCLOAK-4207] SSSD Provider - NullPointerException when mail attrib…
Browse files Browse the repository at this point in the history 
…ute is not filled
  • Loading branch information
Bruno Oliveira committed Jan 13, 2017
1 parent c4cce14 commit 9fb46a7
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 61 deletions.
35 changes: 0 additions & 35 deletions federation/sssd/src/main/java/org/freedesktop/sssd/infopipe/User.java
View file

This file was deleted.

17 changes: 8 additions & 9 deletions federation/sssd/src/main/java/org/keycloak/federation/sssd/SSSDFederationProvider.java
View file
Expand Up @@ -17,13 +17,13 @@

package org.keycloak.federation.sssd;

import org.freedesktop.dbus.Variant;
import org.jboss.logging.Logger;
import org.keycloak.credential.CredentialInput;
import org.keycloak.credential.CredentialInputUpdater;
import org.keycloak.credential.CredentialInputValidator;
import org.keycloak.credential.CredentialModel;
import org.keycloak.federation.sssd.api.Sssd;
import org.keycloak.federation.sssd.api.Sssd.User;
import org.keycloak.federation.sssd.impl.PAMAuthenticator;
import org.keycloak.models.*;
import org.keycloak.models.utils.KeycloakModelUtils;
Expand All @@ -34,7 +34,6 @@

import java.util.Collections;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;

/**
Expand Down Expand Up @@ -112,14 +111,14 @@ protected UserModel findOrCreateAuthenticatedUser(RealmModel realm, String usern

protected UserModel importUserToKeycloak(RealmModel realm, String username) {
Sssd sssd = new Sssd(username);
Map<String, Variant> sssdUser = sssd.getUserAttributes();
User sssdUser = sssd.getUser();
logger.debugf("Creating SSSD user: %s to local Keycloak storage", username);
UserModel user = session.userLocalStorage().addUser(realm, username);
user.setEnabled(true);
user.setEmail(Sssd.getRawAttribute(sssdUser.get("mail")));
user.setFirstName(Sssd.getRawAttribute(sssdUser.get("givenname")));
user.setLastName(Sssd.getRawAttribute(sssdUser.get("sn")));
for (String s : sssd.getUserGroups()) {
user.setEmail(sssdUser.getEmail());
user.setFirstName(sssdUser.getFirstName());
user.setLastName(sssdUser.getLastName());
for (String s : sssd.getGroups()) {
GroupModel group = KeycloakModelUtils.findGroupByPath(realm, "/" + s);
if (group == null) {
group = session.realms().createGroup(realm, s);
Expand Down Expand Up @@ -158,8 +157,8 @@ public void preRemove(RealmModel realm, GroupModel group) {
}

public boolean isValid(RealmModel realm, UserModel local) {
Map<String, Variant> attributes = new Sssd(local.getUsername()).getUserAttributes();
return Sssd.getRawAttribute(attributes.get("mail")).equalsIgnoreCase(local.getEmail());
User user = new Sssd(local.getUsername()).getUser();
return user.equals(local);
}

@Override
Expand Down
82 changes: 68 additions & 14 deletions federation/sssd/src/main/java/org/keycloak/federation/sssd/api/Sssd.java
View file
Expand Up @@ -23,6 +23,7 @@
import org.freedesktop.dbus.exceptions.DBusException;
import org.freedesktop.sssd.infopipe.InfoPipe;
import org.jboss.logging.Logger;
import org.keycloak.models.UserModel;

import java.util.Arrays;
import java.util.List;
Expand Down Expand Up @@ -68,20 +69,7 @@ public static String getRawAttribute(Variant variant) {
return null;
}

public Map<String, Variant> getUserAttributes() {
String[] attr = {"mail", "givenname", "sn", "telephoneNumber"};
Map<String, Variant> attributes = null;
try {
InfoPipe infoPipe = dBusConnection.getRemoteObject(InfoPipe.BUSNAME, InfoPipe.OBJECTPATH, InfoPipe.class);
attributes = infoPipe.getUserAttributes(username, Arrays.asList(attr));
} catch (Exception e) {
throw new SSSDException("Failed to retrieve user's attributes. Check if SSSD service is active.");
}

return attributes;
}

public List<String> getUserGroups() {
public List<String> getGroups() {
List<String> userGroups;
try {
InfoPipe infoPipe = dBusConnection.getRemoteObject(InfoPipe.BUSNAME, InfoPipe.OBJECTPATH, InfoPipe.class);
Expand Down Expand Up @@ -113,4 +101,70 @@ public static boolean isAvailable() {
return sssdAvailable;
}

public User getUser() {

String[] attr = {"mail", "givenname", "sn", "telephoneNumber"};
User user = null;
try {
InfoPipe infoPipe = dBusConnection.getRemoteObject(InfoPipe.BUSNAME, InfoPipe.OBJECTPATH, InfoPipe.class);
user = new User(infoPipe.getUserAttributes(username, Arrays.asList(attr)));
} catch (Exception e) {
throw new SSSDException("Failed to retrieve user's attributes. Check if SSSD service is active.");
}
return user;
}

public class User {

private final String email;
private final String firstName;
private final String lastName;

public User(Map<String, Variant> userAttributes) {
this.email = getRawAttribute(userAttributes.get("mail"));
this.firstName = getRawAttribute(userAttributes.get("givenname"));
this.lastName = getRawAttribute(userAttributes.get("sn"));

}

public String getEmail() {
return email;
}

public String getFirstName() {
return firstName;
}

public String getLastName() {
return lastName;
}

@Override
public boolean equals(Object o) {
if (o == null) return false;

UserModel userModel = (UserModel) o;
if (firstName != null && !firstName.equals(userModel.getFirstName())) {
return false;
}
if (lastName != null && !lastName.equals(userModel.getLastName())) {
return false;
}
if (email != null) {
return email.equals(userModel.getEmail());
}
if (email != userModel.getEmail()) {
return false;
}
return true;
}

@Override
public int hashCode() {
int result = email != null ? email.hashCode() : 0;
result = 31 * result + (firstName != null ? firstName.hashCode() : 0);
result = 31 * result + (lastName != null ? lastName.hashCode() : 0);
return result;
}
}
}
18 changes: 15 additions & 3 deletions ...ation-arquillian/tests/other/sssd/src/test/java/org/keycloak/testsuite/sssd/SSSDTest.java
View file
Expand Up @@ -30,7 +30,9 @@ public class SSSDTest extends AbstractKeycloakTest {
private static final String USERNAME = "emily";
private static final String PASSWORD = "emily123";
private static final String DISABLED_USER = "david";
private static final String DISABLED_USER_PASSWORD = "emily123";
private static final String DISABLED_USER_PASSWORD = "david123";
private static final String NO_EMAIL_USER = "bart";
private static final String NO_EMAIL_USER_PASSWORD = "bart123";

private static final String DEFINITELY_NOT_PASSWORD = "not" + PASSWORD;

Expand Down Expand Up @@ -102,12 +104,12 @@ public void testDisabledUser() {

@Test
public void testAdmin() {
log.debug("Testing wrong password for user " + ADMIN_USERNAME);
log.debug("Testing password for user " + ADMIN_USERNAME);

driver.navigate().to(getAccountUrl());
Assert.assertEquals("Browser should be on login page now", "Log in to " + REALM_NAME, driver.getTitle());
accountLoginPage.login(ADMIN_USERNAME, ADMIN_PASSWORD);
Assert.assertEquals("Unexpected error when handling authentication request to identity provider.", accountLoginPage.getInstruction());
Assert.assertTrue(profilePage.isCurrent());
}

@Test
Expand All @@ -121,6 +123,16 @@ public void testExistingUserLogIn() {
testUserGroups();
}

@Test
public void testExistingUserWithNoEmailLogIn() {
log.debug("Testing correct password, but no e-mail provided");

driver.navigate().to(getAccountUrl());
Assert.assertEquals("Browser should be on login page now", "Log in to " + REALM_NAME, driver.getTitle());
accountLoginPage.login(NO_EMAIL_USER, NO_EMAIL_USER_PASSWORD);
Assert.assertTrue(profilePage.isCurrent());
}

@Test
public void testDeleteSSSDFederationProvider() {
log.debug("Testing correct password");
Expand Down

0 comments on commit 9fb46a7

Please sign in to comment.