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: Add SAML2 audit features #9233

Merged
merged 21 commits into from
Sep 14, 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 @@ -332,6 +332,11 @@ public final class Constants {
*/
public static final String PROFILE_LTI = "lti";

/**
* The name of the Spring profile used for activating SAML2 in Artemis, see {@link de.tum.cit.aet.artemis.core.service.connectors.SAML2Service}.
*/
public static final String PROFILE_SAML2 = "saml2";

public static final String PROFILE_SCHEDULING = "scheduling";

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
* Describes the security configuration for SAML2.
*/
@Configuration
@Profile("saml2")
@Profile(Constants.PROFILE_SAML2)
public class SAML2Configuration {

private static final Logger log = LoggerFactory.getLogger(SAML2Configuration.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/
@Profile(PROFILE_CORE)
@Component
@ConfigurationProperties("saml2")
@ConfigurationProperties(Constants.PROFILE_SAML2)
public class SAML2Properties {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,18 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.boot.actuate.audit.AuditEvent;
import org.springframework.boot.actuate.audit.AuditEventRepository;
import org.springframework.context.annotation.Profile;
import org.springframework.core.env.Environment;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.stereotype.Repository;

import de.tum.cit.aet.artemis.core.config.Constants;
import de.tum.cit.aet.artemis.core.config.audit.AuditEventConverter;
import de.tum.cit.aet.artemis.core.domain.PersistentAuditEvent;

Expand All @@ -24,6 +28,10 @@
@Repository
public class CustomAuditEventRepository implements AuditEventRepository {

private final boolean isSaml2Active;

private static final String AUTHENTICATION_SUCCESS = "AUTHENTICATION_SUCCESS";

private static final String AUTHORIZATION_FAILURE = "AUTHORIZATION_FAILURE";

/**
Expand All @@ -37,9 +45,10 @@ public class CustomAuditEventRepository implements AuditEventRepository {

private static final Logger log = LoggerFactory.getLogger(CustomAuditEventRepository.class);

public CustomAuditEventRepository(PersistenceAuditEventRepository persistenceAuditEventRepository, AuditEventConverter auditEventConverter) {
public CustomAuditEventRepository(Environment environment, PersistenceAuditEventRepository persistenceAuditEventRepository, AuditEventConverter auditEventConverter) {
this.persistenceAuditEventRepository = persistenceAuditEventRepository;
this.auditEventConverter = auditEventConverter;
this.isSaml2Active = Set.of(environment.getActiveProfiles()).contains(Constants.PROFILE_SAML2);
}

@Override
Expand All @@ -51,6 +60,11 @@ public List<AuditEvent> find(String principal, Instant after, String type) {
@Override
public void add(AuditEvent event) {
if (!AUTHORIZATION_FAILURE.equals(event.getType())) {
if (isSaml2Active && AUTHENTICATION_SUCCESS.equals(event.getType()) && SecurityContextHolder.getContext().getAuthentication() == null) {
// If authentication is null, Auth is success, and SAML2 profile is active => SAML2 authentication is running.
// Logging is handled manually.
return;
}

PersistentAuditEvent persistentAuditEvent = new PersistentAuditEvent();
persistentAuditEvent.setPrincipal(event.getPrincipal());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
package de.tum.cit.aet.artemis.core.service.connectors;

import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_SAML2;
import static de.tum.cit.aet.artemis.core.config.Constants.SYSTEM_ACCOUNT;

import java.time.Instant;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Optional;
Expand All @@ -12,6 +17,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.actuate.audit.AuditEvent;
import org.springframework.boot.actuate.audit.AuditEventRepository;
import org.springframework.context.annotation.Profile;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.core.Authentication;
Expand All @@ -35,7 +42,7 @@
/**
* This class describes a service for SAML2 authentication.
* <p>
* The main method is {@link #handleAuthentication(Saml2AuthenticatedPrincipal)}. The service extracts the user information
* The main method is {@link #handleAuthentication(Authentication,Saml2AuthenticatedPrincipal)}. The service extracts the user information
* from the {@link Saml2AuthenticatedPrincipal} and creates the user, if it does not exist already.
* <p>
* When the user gets created, the SAML2 attributes can be used to fill in user information. The configuration happens
Expand All @@ -45,9 +52,11 @@
* This is needed, since the client "does not know" that he is already authenticated via SAML2.
*/
@Service
@Profile("saml2")
@Profile(PROFILE_SAML2)
public class SAML2Service {

private final AuditEventRepository auditEventRepository;

@Value("${info.saml2.enable-password:#{null}}")
private Optional<Boolean> saml2EnablePassword;

Expand All @@ -68,12 +77,14 @@ public class SAML2Service {
/**
* Constructs a new instance.
*
* @param userRepository The user repository
* @param properties The properties
* @param userCreationService The user creation service
* @param auditEventRepository The audit event repository
* @param userRepository The user repository
* @param properties The properties
* @param userCreationService The user creation service
*/
public SAML2Service(final UserRepository userRepository, final SAML2Properties properties, final UserCreationService userCreationService, MailService mailService,
UserService userService) {
public SAML2Service(final AuditEventRepository auditEventRepository, final UserRepository userRepository, final SAML2Properties properties,
final UserCreationService userCreationService, MailService mailService, UserService userService) {
this.auditEventRepository = auditEventRepository;
this.userRepository = userRepository;
this.properties = properties;
this.userCreationService = userCreationService;
Expand All @@ -93,10 +104,13 @@ private Map<String, Pattern> generateExtractionPatterns(final SAML2Properties pr
* <p>
* Registers new users and returns a new {@link UsernamePasswordAuthenticationToken} matching the SAML2 user.
*
* @param principal the principal, containing the user information
* @param originalAuth the original authentication with details
* @param principal the principal, containing the user information
* @return a new {@link UsernamePasswordAuthenticationToken} matching the SAML2 user
*/
public Authentication handleAuthentication(final Saml2AuthenticatedPrincipal principal) {
public Authentication handleAuthentication(final Authentication originalAuth, final Saml2AuthenticatedPrincipal principal) {
Map<String, Object> details = originalAuth.getDetails() == null ? Map.of() : Map.of("details", originalAuth.getDetails());

Authentication auth = SecurityContextHolder.getContext().getAuthentication();

log.debug("SAML2 User '{}' logged in, attributes {}", auth.getName(), principal.getAttributes());
Expand All @@ -107,6 +121,9 @@ public Authentication handleAuthentication(final Saml2AuthenticatedPrincipal pri
if (user.isEmpty()) {
// create User if not exists
user = Optional.of(createUser(username, principal));
Map<String, Object> accountCreationDetails = new HashMap<>(details);
accountCreationDetails.put("user", user.get().getLogin());
auditEventRepository.add(new AuditEvent(Instant.now(), SYSTEM_ACCOUNT, "SAML2_ACCOUNT_CREATE", accountCreationDetails));

if (saml2EnablePassword.isPresent() && Boolean.TRUE.equals(saml2EnablePassword.get())) {
log.debug("Sending SAML2 creation mail");
Expand All @@ -125,6 +142,7 @@ public Authentication handleAuthentication(final Saml2AuthenticatedPrincipal pri
}

auth = new UsernamePasswordAuthenticationToken(user.get().getLogin(), user.get().getPassword(), toGrantedAuthorities(user.get().getAuthorities()));
auditEventRepository.add(new AuditEvent(Instant.now(), user.get().getLogin(), "SAML2_AUTHENTICATION_SUCCESS", details));
return auth;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public ResponseEntity<Void> authorizeSAML2(@RequestBody final String body, HttpS
log.debug("SAML2 authentication: {}", authentication);

try {
authentication = saml2Service.get().handleAuthentication(principal);
authentication = saml2Service.get().handleAuthentication(authentication, principal);
SecurityContextHolder.getContext().setAuthentication(authentication);
}
catch (UserNotActivatedException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_ARTEMIS;
import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_CORE;
import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_LTI;
import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_SAML2;
import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_SCHEDULING;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyString;
Expand Down Expand Up @@ -48,7 +49,7 @@

@ResourceLock("AbstractSpringIntegrationGitlabCIGitlabSamlTest")
// NOTE: we use a common set of active profiles to reduce the number of application launches during testing. This significantly saves time and memory!
@ActiveProfiles({ SPRING_PROFILE_TEST, PROFILE_ARTEMIS, PROFILE_CORE, "gitlabci", "gitlab", "saml2", PROFILE_SCHEDULING, PROFILE_LTI })
@ActiveProfiles({ SPRING_PROFILE_TEST, PROFILE_ARTEMIS, PROFILE_CORE, "gitlabci", "gitlab", PROFILE_SAML2, PROFILE_SCHEDULING, PROFILE_LTI })
@TestPropertySource(properties = { "artemis.user-management.use-external=false" })
public abstract class AbstractSpringIntegrationGitlabCIGitlabSamlTest extends AbstractArtemisIntegrationTest {

Expand Down
Loading