Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

General: Allow users to login with their email address #9181

Merged
merged 5 commits into from
Aug 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ public final class Constants {
// Regex for acceptable logins
public static final String LOGIN_REGEX = "^[_'.@A-Za-z0-9-]*$";

public static final String SIMPLE_EMAIL_REGEX = "^[A-Za-z0-9+_.-]+@[A-Za-z0-9.-]+$";

public static final String SYSTEM_ACCOUNT = "system";

public static final String DEFAULT_LANGUAGE = "en";
Expand Down Expand Up @@ -76,7 +78,9 @@ public final class Constants {

public static final String TUM_LDAP_MATRIKEL_NUMBER = "imMatrikelNr";

public static final String TUM_LDAP_EMAIL = "imHauptEMail";
public static final String TUM_LDAP_MAIN_EMAIL = "imHauptEMail";

public static final String TUM_LDAP_EMAILS = "imEmailAdressen";

// NOTE: the following values for programming exercises are hard-coded at the moment
public static final String TEST_REPO_NAME = "tests";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,15 @@ public interface UserRepository extends ArtemisJpaRepository<User, Long>, JpaSpe
@EntityGraph(type = LOAD, attributePaths = { "groups", "authorities" })
Optional<User> findOneWithGroupsAndAuthoritiesByLogin(String login);

@EntityGraph(type = LOAD, attributePaths = { "authorities" })
Optional<User> findOneWithAuthoritiesByLogin(String login);

@EntityGraph(type = LOAD, attributePaths = { "groups", "authorities" })
Optional<User> findOneWithGroupsAndAuthoritiesByEmail(String email);

@EntityGraph(type = LOAD, attributePaths = { "groups", "authorities" })
Optional<User> findOneWithGroupsAndAuthoritiesByLoginAndIsInternal(String login, boolean isInternal);

@EntityGraph(type = LOAD, attributePaths = { "groups", "authorities" })
Optional<User> findOneWithGroupsAndAuthoritiesByEmailAndIsInternal(String email, boolean isInternal);

@EntityGraph(type = LOAD, attributePaths = { "groups", "authorities" })
Optional<User> findOneWithGroupsAndAuthoritiesById(Long id);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,26 @@ public DomainUserDetailsService(UserRepository userRepository) {
}

@Override
public UserDetails loadUserByUsername(final String login) {
log.debug("Authenticating {}", login);
String lowercaseLogin = login.toLowerCase(Locale.ENGLISH);
User user = userRepository.findOneWithGroupsAndAuthoritiesByLoginAndIsInternal(lowercaseLogin, true)
.orElseThrow(() -> new UsernameNotFoundException("User " + lowercaseLogin + " was not found in the database"));
public UserDetails loadUserByUsername(final String loginOrEmail) {
log.debug("Authenticating {}", loginOrEmail);
String lowercaseLoginOrEmail = loginOrEmail.toLowerCase(Locale.ENGLISH);

User user;
if (SecurityUtils.isEmail(lowercaseLoginOrEmail)) {
// It's an email, try to find the user based on the email
user = userRepository.findOneWithGroupsAndAuthoritiesByEmailAndIsInternal(lowercaseLoginOrEmail, true)
.orElseThrow(() -> new UsernameNotFoundException("User " + lowercaseLoginOrEmail + " was not found in the database"));
}
else {
// It's a login, try to find the user based on the login
user = userRepository.findOneWithGroupsAndAuthoritiesByLoginAndIsInternal(lowercaseLoginOrEmail, true)
.orElseThrow(() -> new UsernameNotFoundException("User " + lowercaseLoginOrEmail + " was not found in the database"));
}

if (!user.isInternal()) {
throw new UsernameNotFoundException("User " + lowercaseLogin + " is an external user and thus was not found as an internal user.");
throw new UsernameNotFoundException("User " + lowercaseLoginOrEmail + " is an external user and thus was not found as an internal user.");
}
return createSpringSecurityUser(lowercaseLogin, user);
return createSpringSecurityUser(lowercaseLoginOrEmail, user);
}

private org.springframework.security.core.userdetails.User createSpringSecurityUser(String lowercaseLogin, User user) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static de.tum.in.www1.artemis.config.Constants.PASSWORD_MAX_LENGTH;
import static de.tum.in.www1.artemis.config.Constants.PASSWORD_MIN_LENGTH;
import static de.tum.in.www1.artemis.config.Constants.SIMPLE_EMAIL_REGEX;
import static de.tum.in.www1.artemis.config.Constants.USERNAME_MAX_LENGTH;
import static de.tum.in.www1.artemis.config.Constants.USERNAME_MIN_LENGTH;

Expand Down Expand Up @@ -32,6 +33,10 @@ public final class SecurityUtils {
private SecurityUtils() {
}

public static boolean isEmail(String input) {
return input.matches(SIMPLE_EMAIL_REGEX);
}
krusche marked this conversation as resolved.
Show resolved Hide resolved

/**
* check that the username and password are not null and have the correct length
*
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package de.tum.in.www1.artemis.service.connectors.ldap;

import java.util.HashSet;
import java.util.Locale;
import java.util.Objects;
import java.util.Optional;

import org.slf4j.Logger;
Expand All @@ -20,6 +22,7 @@
import de.tum.in.www1.artemis.repository.UserRepository;
import de.tum.in.www1.artemis.security.ArtemisAuthenticationProvider;
import de.tum.in.www1.artemis.security.ArtemisAuthenticationProviderImpl;
import de.tum.in.www1.artemis.security.SecurityUtils;
import de.tum.in.www1.artemis.service.ldap.LdapUserDto;
import de.tum.in.www1.artemis.service.ldap.LdapUserService;
import de.tum.in.www1.artemis.service.user.AuthorityService;
Expand Down Expand Up @@ -58,41 +61,101 @@ public Authentication authenticate(Authentication authentication) throws Authent
return null;
}

/**
* Get or create a user based on the given authentication. This method will check the password
*
* @param authentication The authentication object
* @return The user object or null if the user is internal
*/
krusche marked this conversation as resolved.
Show resolved Hide resolved
private User getOrCreateUser(Authentication authentication) {
String username = authentication.getName().toLowerCase();
String loginOrEmail = authentication.getName().toLowerCase(Locale.ENGLISH);
String password = authentication.getCredentials().toString();

long start = System.nanoTime();

final var optionalUser = userRepository.findOneWithGroupsAndAuthoritiesByLogin(username);
// distinguish between login and email here by using a simple regex
boolean isEmail = SecurityUtils.isEmail(loginOrEmail);

Optional<User> optionalUser;
if (isEmail) {
// It's an email, try to find the Artemis user in the database based on the email
optionalUser = userRepository.findOneWithGroupsAndAuthoritiesByEmail(loginOrEmail);
}
else {
// It's a login, try to find the Artemis user in the database based on the login
optionalUser = userRepository.findOneWithGroupsAndAuthoritiesByLogin(loginOrEmail);
}
if (optionalUser.isPresent() && optionalUser.get().isInternal()) {
// User found but is internal. Skip external authentication.
return null;
}

log.debug("Finished userRepository.findOneWithGroupsAndAuthoritiesByLogin in {}", TimeLogUtil.formatDurationFrom(start));
log.debug("Finished userRepository.findOneWithGroupsAndAuthoritiesByLoginOrEmail in {}", TimeLogUtil.formatDurationFrom(start));
start = System.nanoTime();

// If the following code is executed, the user is either not yet existent or an external user
final LdapUserDto ldapUserDto;
if (isEmail) {
// It's an email, try to find the LDAP user in the external user management system based on the given email (which must not be the main user email)
ldapUserDto = ldapUserService.findByAnyEmail(loginOrEmail).orElseThrow(() -> new BadCredentialsException("Wrong credentials"));
}
else {
// It's a login, try to find the LDAP user in the external user management system based on the given login
ldapUserDto = ldapUserService.findByLogin(loginOrEmail).orElseThrow(() -> new BadCredentialsException("Wrong credentials"));
}

final LdapUserDto ldapUserDto = ldapUserService.findByUsername(username).orElseThrow(() -> new BadCredentialsException("Wrong credentials"));

log.debug("Finished ldapUserService.findByUsername in {}", TimeLogUtil.formatDurationFrom(start));
if (isEmail && optionalUser.isEmpty()) {
// this is an edge case which could happen when the user email changed or the user has multiple email addresses and used a secondary email to login
// therefore, double check if the Artemis User with the LDAP login (based on the given email) exists. If yes, we will use this user and update the LDAP values below
// without this code a second user would be created in Artemis which is not what we want (additionally this would fail because of unique constraints)
optionalUser = userRepository.findOneWithGroupsAndAuthoritiesByLogin(ldapUserDto.getLogin());
}
log.debug("Finished ldapUserService.findByLogin in {}", TimeLogUtil.formatDurationFrom(start));
start = System.nanoTime();

// We create our own authorization and use the credentials of the user.
// Use the given password to compare it with the LDAP entry (i.e. check the authorization)
byte[] passwordBytes = Utf8.encode(password);
boolean passwordCorrect = ldapTemplate.compare(ldapUserDto.getUid().toString(), "userPassword", passwordBytes);
log.debug("Compare password with LDAP entry for user {} to validate login", username);
// this is the normal case, where the password is validated
log.debug("Compare password with LDAP entry for user {} to validate login", loginOrEmail);
if (!passwordCorrect) {
throw new BadCredentialsException("Wrong credentials");
}

log.debug("Finished ldapTemplate.compare password in {}", TimeLogUtil.formatDurationFrom(start));

return optionalUser.orElseGet(() -> {
User newUser = userCreationService.createUser(ldapUserDto.getUsername(), null, null, ldapUserDto.getFirstName(), ldapUserDto.getLastName(), ldapUserDto.getEmail(),
// update the user details from ldapUserDto (because they might have changed, e.g. when the user changes the name)
if (optionalUser.isPresent()) {
User user = optionalUser.get();
boolean saveNeeded = false;
if (!Objects.equals(user.getLogin(), ldapUserDto.getLogin())) {
user.setLogin(ldapUserDto.getLogin());
saveNeeded = true;
}
if (!Objects.equals(user.getFirstName(), ldapUserDto.getFirstName())) {
user.setFirstName(ldapUserDto.getFirstName());
saveNeeded = true;
}
if (!Objects.equals(user.getLastName(), ldapUserDto.getLastName())) {
user.setLastName(ldapUserDto.getLastName());
saveNeeded = true;
}
if (!Objects.equals(user.getEmail(), ldapUserDto.getEmail())) {
user.setEmail(ldapUserDto.getEmail());
saveNeeded = true;
}
if (!Objects.equals(user.getRegistrationNumber(), ldapUserDto.getRegistrationNumber())) {
user.setRegistrationNumber(ldapUserDto.getRegistrationNumber());
saveNeeded = true;
}
// only save the user in the database in case it has changed
if (saveNeeded) {
user = userRepository.save(user);
}
return user;
krusche marked this conversation as resolved.
Show resolved Hide resolved
}
else {
// this handles the case that the user does not exist in the Artemis database yet (i.e. first time user login)
User newUser = userCreationService.createUser(ldapUserDto.getLogin(), null, null, ldapUserDto.getFirstName(), ldapUserDto.getLastName(), ldapUserDto.getEmail(),
ldapUserDto.getRegistrationNumber(), null, "en", false);

newUser.setGroups(new HashSet<>());
Expand All @@ -103,7 +166,7 @@ private User getOrCreateUser(Authentication authentication) {
newUser.setActivationKey(null);
}
return userCreationService.saveUser(newUser);
});
}
}

@Override
Expand All @@ -119,6 +182,6 @@ public boolean supports(Class<?> authentication) {
*/
@Override
public Optional<String> getUsernameForEmail(String email) {
return ldapUserService.findByEmail(email).map(LdapUserDto::getUsername);
return ldapUserService.findByAnyEmail(email).map(LdapUserDto::getLogin);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,14 @@ public void authenticateLtiUser(String email, String username, String firstName,
}

@NotNull
protected Authentication createNewUserFromLaunchRequest(String email, String username, String firstName, String lastName) {
final var user = userRepository.findOneByLogin(username).orElseGet(() -> {
protected Authentication createNewUserFromLaunchRequest(String email, String login, String firstName, String lastName) {
final var user = userRepository.findOneByLogin(login).orElseGet(() -> {
final User newUser;
final var groups = new HashSet<String>();
groups.add(LTI_GROUP_NAME);

var password = RandomUtil.generatePassword();
newUser = userCreationService.createUser(username, password, groups, firstName, lastName, email, null, null, Constants.DEFAULT_LANGUAGE, true);
newUser = userCreationService.createUser(login, password, groups, firstName, lastName, email, null, null, Constants.DEFAULT_LANGUAGE, true);
newUser.setActivationKey(null);
userRepository.save(newUser);

Expand All @@ -141,7 +141,7 @@ protected Authentication createNewUserFromLaunchRequest(String email, String use

log.info("createNewUserFromLaunchRequest: {}", user);

log.info("Signing in as {}", username);
log.info("Signing in as {}", login);
return new UsernamePasswordAuthenticationToken(user.getLogin(), user.getPassword(), SIMPLE_USER_LIST_AUTHORITY);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
* A Data Transfer Object (DTO) representing a user's role within a course.
*
* <p>
* This DTO contains the user ID, username, and role of a user. It is used to encapsulate and transfer
* This DTO contains the user ID, login, and role of a user. It is used to encapsulate and transfer
* the user role information between different layers of the application.
* </p>
*
Expand All @@ -18,7 +18,7 @@
* </p>
*
* @param userId the unique identifier of the user
* @param username the username of the user
* @param username the login of the user
* @param role the role of the user within the course, represented as a {@link UserRole} enum
*/
@JsonInclude(JsonInclude.Include.NON_EMPTY)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package de.tum.in.www1.artemis.service.ldap;

import static de.tum.in.www1.artemis.config.Constants.TUM_LDAP_EMAIL;
import static de.tum.in.www1.artemis.config.Constants.TUM_LDAP_MAIN_EMAIL;
import static de.tum.in.www1.artemis.config.Constants.TUM_LDAP_MATRIKEL_NUMBER;

import javax.naming.Name;
Expand All @@ -12,13 +12,14 @@

@Entry(base = "ou=users", objectClasses = { "imdPerson" })
@Profile("ldap | ldap-only")
// TODO: double check if we can use a Record here
public final class LdapUserDto {
krusche marked this conversation as resolved.
Show resolved Hide resolved

@Id
private Name uid;

@Attribute(name = "uid")
private String username;
private String login;

@Attribute(name = TUM_LDAP_MATRIKEL_NUMBER)
private String registrationNumber;
Expand All @@ -29,19 +30,19 @@ public final class LdapUserDto {
@Attribute(name = "sn")
private String lastName;

@Attribute(name = TUM_LDAP_EMAIL)
@Attribute(name = TUM_LDAP_MAIN_EMAIL)
private String email;

public String getUsername() {
return username;
public String getLogin() {
return login;
}

public void setUsername(String username) {
this.username = username;
public void setLogin(String login) {
this.login = login;
}

public LdapUserDto username(String username) {
this.username = username;
public LdapUserDto login(String login) {
this.login = login;
return this;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package de.tum.in.www1.artemis.service.ldap;

import static de.tum.in.www1.artemis.config.Constants.TUM_LDAP_EMAIL;
import static de.tum.in.www1.artemis.config.Constants.TUM_LDAP_EMAILS;
import static de.tum.in.www1.artemis.config.Constants.TUM_LDAP_MAIN_EMAIL;
import static de.tum.in.www1.artemis.config.Constants.TUM_LDAP_MATRIKEL_NUMBER;
import static org.springframework.ldap.query.LdapQueryBuilder.query;

Expand Down Expand Up @@ -38,12 +39,16 @@ public LdapUserService(LdapUserRepository ldapUserRepository) {
this.ldapUserRepository = ldapUserRepository;
}

public Optional<LdapUserDto> findByUsername(final String username) {
return ldapUserRepository.findOne(query().base(ldapBase).searchScope(SearchScope.SUBTREE).where("cn").is(username));
public Optional<LdapUserDto> findByLogin(final String login) {
return ldapUserRepository.findOne(query().base(ldapBase).searchScope(SearchScope.SUBTREE).where("cn").is(login));
}

public Optional<LdapUserDto> findByEmail(final String email) {
return ldapUserRepository.findOne(query().base(ldapBase).searchScope(SearchScope.SUBTREE).where(TUM_LDAP_EMAIL).is(email));
public Optional<LdapUserDto> findByMainEmail(final String email) {
return ldapUserRepository.findOne(query().base(ldapBase).searchScope(SearchScope.SUBTREE).where(TUM_LDAP_MAIN_EMAIL).is(email));
}

public Optional<LdapUserDto> findByAnyEmail(final String email) {
return ldapUserRepository.findOne(query().base(ldapBase).searchScope(SearchScope.SUBTREE).where(TUM_LDAP_EMAILS).is(email));
}

public Optional<LdapUserDto> findByRegistrationNumber(final String registrationNumber) {
Expand All @@ -59,10 +64,10 @@ public Optional<LdapUserDto> findByRegistrationNumber(final String registrationN
@Nullable
public LdapUserDto loadUserDetailsFromLdap(@NotNull String login) {
try {
Optional<LdapUserDto> ldapUserOptional = findByUsername(login);
Optional<LdapUserDto> ldapUserOptional = findByLogin(login);
if (ldapUserOptional.isPresent()) {
LdapUserDto ldapUser = ldapUserOptional.get();
log.debug("Ldap User {} has registration number: {}", ldapUser.getUsername(), ldapUser.getRegistrationNumber());
log.debug("Ldap User {} has registration number: {}", ldapUser.getLogin(), ldapUser.getRegistrationNumber());
return ldapUserOptional.get();
}
else {
Expand Down
Loading
Loading