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

feat: sanity checks on user join and before sending any notification … #586

Merged
merged 1 commit into from
Jan 11, 2025
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
4 changes: 4 additions & 0 deletions agate-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@
<artifactId>postgresql</artifactId>
</dependency>

<dependency>
<groupId>org.owasp.esapi</groupId>
<artifactId>esapi</artifactId>
</dependency>
<dependency>
<groupId>dev.samstevens.totp</groupId>
<artifactId>totp</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import com.google.common.base.Strings;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import org.hibernate.validator.constraints.NotBlank;
import org.obiba.mongodb.domain.AbstractAuditableDocument;
import org.obiba.runtime.Version;
import org.springframework.data.annotation.Transient;
Expand All @@ -28,8 +27,6 @@
@Document
public class Configuration extends AbstractAuditableDocument {

private static final long serialVersionUID = -9020464712632680519L;

public static final String DEFAULT_NAME = "Agate";

public static final Locale DEFAULT_LOCALE = Locale.ENGLISH;
Expand All @@ -40,7 +37,6 @@ public class Configuration extends AbstractAuditableDocument {

public static final int DEFAULT_INACTIVE_TIMEOUT = 365 * 24; // 1 year (in hours)

@NotBlank
private String name = DEFAULT_NAME;

private String domain;
Expand Down
2 changes: 0 additions & 2 deletions agate-core/src/main/java/org/obiba/agate/domain/User.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import com.google.common.base.Strings;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import org.hibernate.validator.constraints.Email;
import org.joda.time.DateTime;
import org.obiba.agate.security.Roles;
import org.obiba.mongodb.domain.AbstractAuditableDocument;
Expand All @@ -41,7 +40,6 @@ public class User extends AbstractAuditableDocument {

private String lastName;

@Email
@Indexed(unique = true)
private String email;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.obiba.agate.web.controller.domain;
package org.obiba.agate.domain;

import org.obiba.agate.domain.User;
import org.owasp.esapi.ESAPI;

import java.util.Map;
Expand Down Expand Up @@ -51,7 +50,7 @@ public String getDisplayName() {
}

public String getPreferredLanguage() {
return user.getPreferredLanguage();
return ESAPI.encoder().encodeForHTML(user.getPreferredLanguage());
}

public boolean getOtpEnabled() {
Expand Down
16 changes: 14 additions & 2 deletions agate-core/src/main/java/org/obiba/agate/service/UserService.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.obiba.agate.repository.UserCredentialsRepository;
import org.obiba.agate.repository.UserRepository;
import org.obiba.agate.service.support.MessageResolverMethod;
import org.obiba.agate.validator.NameValidator;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.BeanUtils;
Expand Down Expand Up @@ -78,6 +79,7 @@ public class UserService {
+ "(?=.*[@#$%^&+=!])" // a special character that must occur at least once
+ "(?=\\S+$).{" + PWD_MINIMUM_LENGTH + "," + PWD_MAXIMUM_LENGTH + "}$");


@Value("${login.otpTimeout:300}")
private int otpTimeout;

Expand Down Expand Up @@ -266,6 +268,10 @@ public void updateUserPassword(@Nonnull User user, @Nonnull String password) {
}

public User createUser(@Nonnull User user, @Nullable String password) {
checkName(user.getName());
checkName(user.getFirstName());
checkName(user.getLastName());

if (Strings.isNullOrEmpty(password)) {
if (user.getRealm() == null) user.setRealm(AgateRealm.AGATE_USER_REALM.getName());
else {
Expand Down Expand Up @@ -455,7 +461,7 @@ public void sendPendingEmail(UserJoinedEvent userJoinedEvent) {
String organization = configurationService.getConfiguration().getName();

Map<String, Object> context = Maps.newHashMap();
context.put("user", user);
context.put("user", new UserProfile(user));

administrators.forEach(u -> sendEmail(u, "[" + organization + "] " + env.getProperty("registration.pendingForReviewSubject"),
"pendingForReviewEmail", context));
Expand All @@ -479,7 +485,7 @@ private void sendEmail(User user, String subject, String templateName, Map<Strin
Locale locale = LocaleUtils.toLocale(user.getPreferredLanguage());
Map<String, Object> ctx = context == null ? Maps.newHashMap() : Maps.newHashMap(context);
if (!ctx.containsKey("user"))
ctx.put("user", user);
ctx.put("user", new UserProfile(user));
ctx.put("organization", configurationService.getConfiguration().getName());
// get user's realm and find if there is a specific agate base url for this realm
Optional<RealmConfig> realmConfig = getRealmConfigs(user).stream().findFirst();
Expand Down Expand Up @@ -711,4 +717,10 @@ private List<RealmConfig> getRealmConfigs(User user) {
return realmConfigRepository.findAll().stream().filter(realmConfig -> user.getRealm().equals(realmConfig.getName())).collect(Collectors.toList());
}

private void checkName(String name) {
if (!NameValidator.isValid(name)) {
throw new BadRequestException("Name contains invalid characters");
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package org.obiba.agate.validator;

import jakarta.mail.internet.InternetAddress;

import java.util.regex.Matcher;
import java.util.regex.Pattern;

import static java.util.regex.Pattern.CASE_INSENSITIVE;

public final class EmailValidator {

public static boolean isValid(String email) {
if (email == null || email.trim().isEmpty()) {
return false;
}

// Check length constraints
if (email.length() > 254) {
return false; // Email too long
}

String[] parts = email.split("@");
if (parts[0].length() > 64) {
return false; // Local part too long
}

// Check for common issues
if (email.contains("..") || email.startsWith(".") || email.endsWith(".")) {
return false; // Invalid dot placement
}

try {
InternetAddress emailAddr = new InternetAddress(email);
emailAddr.validate();
return true;
} catch (Exception e) {
return false;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package org.obiba.agate.validator;

import java.util.regex.Pattern;

public final class NameValidator {

// International pattern supporting accents and other characters
private static final Pattern INTERNATIONAL_NAME_PATTERN = Pattern.compile(
"^[\\p{L}][\\p{L}\\s\\-']{1,49}$");

public static boolean isValid(String name) {
if (name == null || name.isEmpty()) {
return true;
}

return INTERNATIONAL_NAME_PATTERN.matcher(name).matches();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,12 @@
import com.google.common.collect.Sets;
import org.apache.shiro.authc.AuthenticationInfo;
import org.apache.shiro.authc.UsernamePasswordToken;
import org.bson.types.ObjectId;
import org.hibernate.validator.internal.constraintvalidators.hv.EmailValidator;
import org.joda.time.DateTime;
import org.obiba.agate.domain.*;
import org.obiba.agate.security.AgateUserRealm;
import org.obiba.agate.security.Roles;
import org.obiba.agate.service.*;
import org.obiba.agate.validator.EmailValidator;
import org.obiba.agate.web.rest.config.JerseyConfiguration;
import org.obiba.agate.web.rest.security.InvalidApplicationKeyException;
import org.obiba.shiro.authc.HttpAuthorizationToken;
Expand Down Expand Up @@ -185,7 +184,7 @@ public Response create(@Context HttpServletRequest request, MultivaluedMap<Strin
if (email == null || Strings.isNullOrEmpty(email.trim())) throw new BadRequestException("Email cannot be empty");
email = email.trim();

if (!new EmailValidator().isValid(email, null)) throw new BadRequestException("Not a valid email address");
if (!EmailValidator.isValid(email)) throw new BadRequestException("Not a valid email address");

if (!config.getJoinWhitelist().isEmpty()) {
if (config.getJoinWhitelist().stream().noneMatch(email::endsWith)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
import org.apache.shiro.authz.annotation.RequiresRoles;
import org.hibernate.validator.internal.constraintvalidators.hv.EmailValidator;
import org.obiba.agate.domain.AgateRealm;
import org.obiba.agate.domain.User;
import org.obiba.agate.domain.UserStatus;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import org.obiba.agate.service.ConfigurationService;
import org.obiba.agate.service.RealmConfigService;
import org.obiba.agate.service.UserService;
import org.obiba.agate.web.controller.domain.UserProfile;
import org.obiba.agate.domain.UserProfile;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.stereotype.Component;
Expand All @@ -20,8 +20,6 @@
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;

import java.util.stream.Collectors;

@Component
public class SessionInterceptor implements HandlerInterceptor {

Expand Down