Skip to content

Commit

Permalink
SONAR-24023 Rewrite tests for SamlAuthenticator
Browse files Browse the repository at this point in the history
  • Loading branch information
aurelien-poscia-sonarsource authored and sonartech committed Dec 20, 2024
1 parent c770ccb commit 1fce3f5
Show file tree
Hide file tree
Showing 11 changed files with 214 additions and 95 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,17 @@
import java.util.Base64;
import java.util.Map;
import org.json.JSONObject;
import org.sonar.api.server.ServerSide;
import org.sonar.api.server.http.HttpRequest;

public final class SamlAuthStatusPageGenerator {
@ServerSide
final class SamlAuthStatusPageGenerator {

private static final String WEB_CONTEXT = "WEB_CONTEXT";
private static final String SAML_AUTHENTICATION_STATUS = "%SAML_AUTHENTICATION_STATUS%";
private static final String HTML_TEMPLATE_NAME = "samlAuthResult.html";

private SamlAuthStatusPageGenerator() {
throw new IllegalStateException("This Utility class cannot be instantiated");
}

public static String getSamlAuthStatusHtml(HttpRequest request, SamlAuthenticationStatus samlAuthenticationStatus) {
public String getSamlAuthStatusHtml(HttpRequest request, SamlAuthenticationStatus samlAuthenticationStatus) {
Map<String, String> substitutionsMap = getSubstitutionsMap(request, samlAuthenticationStatus);
String htmlTemplate = getPlainTemplate();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.sonar.auth.saml;

import java.io.IOException;
import java.io.UncheckedIOException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.sonar.api.server.ServerSide;
Expand All @@ -30,35 +31,35 @@
import org.springframework.security.saml2.provider.service.authentication.Saml2AuthenticatedPrincipal;
import org.springframework.security.saml2.provider.service.authentication.Saml2AuthenticationException;

import static org.sonar.auth.saml.SamlAuthStatusPageGenerator.getSamlAuthStatusHtml;
import static org.sonar.auth.saml.SamlStatusChecker.getSamlAuthenticationStatus;

@ServerSide
public class SamlAuthenticator {

private static final Logger LOGGER = LoggerFactory.getLogger(SamlAuthenticator.class);

private static final String STATE_REQUEST_PARAMETER = "RelayState";

private final SamlSettings samlSettings;
private final RedirectToUrlProvider redirectToUrlProvider;
private final SamlResponseAuthenticator samlResponseAuthenticator;
private final PrincipalToUserIdentityConverter principalToUserIdentityConverter;
private final SamlStatusChecker samlStatusChecker;
private final SamlAuthStatusPageGenerator samlAuthStatusPageGenerator;

public SamlAuthenticator(SamlSettings samlSettings, RedirectToUrlProvider redirectToUrlProvider,
SamlResponseAuthenticator samlResponseAuthenticator, PrincipalToUserIdentityConverter principalToUserIdentityConverter) {
this.samlSettings = samlSettings;
public SamlAuthenticator(RedirectToUrlProvider redirectToUrlProvider,
SamlResponseAuthenticator samlResponseAuthenticator, PrincipalToUserIdentityConverter principalToUserIdentityConverter, SamlStatusChecker samlStatusChecker,
SamlAuthStatusPageGenerator samlAuthStatusPageGenerator) {
this.redirectToUrlProvider = redirectToUrlProvider;
this.samlResponseAuthenticator = samlResponseAuthenticator;
this.principalToUserIdentityConverter = principalToUserIdentityConverter;
this.samlStatusChecker = samlStatusChecker;
this.samlAuthStatusPageGenerator = samlAuthStatusPageGenerator;
}

public void initLogin(String callbackUrl, String relayState, HttpRequest request, HttpResponse response) {
String redirectToUrl = redirectToUrlProvider.getRedirectToUrl(request, callbackUrl, relayState);
try {
response.sendRedirect(redirectToUrl);
} catch (IOException e) {
throw new RuntimeException(e);
throw new UncheckedIOException(e);
}
}

Expand All @@ -71,15 +72,18 @@ public UserIdentity onCallback(OAuth2IdentityProvider.CallbackContext context, H
return principalToUserIdentityConverter.convertToUserIdentity(principal);
}

public String getAuthenticationStatusPage(HttpRequest request, HttpResponse response) {
public String getAuthenticationStatusPage(HttpRequest request) {
try {
Saml2AuthenticatedPrincipal principal = samlResponseAuthenticator.authenticate(request, request.getRequestURL());
String samlResponse = request.getParameter("SAMLResponse");
return getSamlAuthStatusHtml(request, getSamlAuthenticationStatus(samlResponse, principal, samlSettings));
SamlAuthenticationStatus samlAuthenticationStatus = samlStatusChecker.getSamlAuthenticationStatus(samlResponse, principal);
return samlAuthStatusPageGenerator.getSamlAuthStatusHtml(request, samlAuthenticationStatus);
} catch (Saml2AuthenticationException e) {
return getSamlAuthStatusHtml(request, getSamlAuthenticationStatus(e.getMessage()));
SamlAuthenticationStatus samlAuthenticationStatus = samlStatusChecker.getSamlAuthenticationStatus(e.getMessage());
return samlAuthStatusPageGenerator.getSamlAuthStatusHtml(request, samlAuthenticationStatus);
} catch (IllegalStateException e) {
return getSamlAuthStatusHtml(request, getSamlAuthenticationStatus(String.format("%s due to: %s", e.getMessage(), e.getCause().getMessage())));
SamlAuthenticationStatus samlAuthenticationStatus = samlStatusChecker.getSamlAuthenticationStatus(String.format("%s due to: %s", e.getMessage(), e.getCause().getMessage()));
return samlAuthStatusPageGenerator.getSamlAuthStatusHtml(request, samlAuthenticationStatus);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,18 @@ public class SamlModule extends Module {
protected void configureModule() {
add(
PrincipalToUserIdentityConverter.class,
RedirectToUrlProvider.class,
RelyingPartyRegistrationRepositoryProvider.class,
SamlAuthenticator.class,
SamlAuthStatusPageGenerator.class,
SamlConfiguration.class,
SamlCertificateConverter.class,
SamlIdentityProvider.class,
SamlMessageIdChecker.class,
SamlPrivateKeyConverter.class,
SamlResponseAuthenticator.class,
SamlSettings.class,
RedirectToUrlProvider.class,
SamlStatusChecker.class,
SonarqubeSaml2ResponseValidator.class
);
List<PropertyDefinition> definitions = SamlSettings.definitions();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,22 +31,25 @@
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
import org.sonar.api.server.ServerSide;
import org.springframework.security.saml2.provider.service.authentication.Saml2AuthenticatedPrincipal;

import static org.sonar.auth.saml.SamlSettings.GROUP_NAME_ATTRIBUTE;
import static org.sonar.auth.saml.SamlSettings.USER_EMAIL_ATTRIBUTE;
import static org.sonar.auth.saml.SamlSettings.USER_LOGIN_ATTRIBUTE;
import static org.sonar.auth.saml.SamlSettings.USER_NAME_ATTRIBUTE;

public final class SamlStatusChecker {
@ServerSide
final class SamlStatusChecker {
private final SamlSettings samlSettings;

private static final Pattern encryptedAssertionPattern = Pattern.compile("<saml:EncryptedAssertion|<EncryptedAssertion");

private SamlStatusChecker() {
throw new IllegalStateException("This Utility class cannot be instantiated");
SamlStatusChecker(SamlSettings samlSettings) {
this.samlSettings = samlSettings;
}

public static SamlAuthenticationStatus getSamlAuthenticationStatus(String samlResponse, Saml2AuthenticatedPrincipal principal, SamlSettings samlSettings) {
public SamlAuthenticationStatus getSamlAuthenticationStatus(String samlResponse, Saml2AuthenticatedPrincipal principal) {

SamlAuthenticationStatus samlAuthenticationStatus = new SamlAuthenticationStatus();

Expand All @@ -70,7 +73,7 @@ public static SamlAuthenticationStatus getSamlAuthenticationStatus(String samlRe

}

public static SamlAuthenticationStatus getSamlAuthenticationStatus(String errorMessage) {
public SamlAuthenticationStatus getSamlAuthenticationStatus(String errorMessage) {
SamlAuthenticationStatus samlAuthenticationStatus = new SamlAuthenticationStatus();
samlAuthenticationStatus.getErrors().add(errorMessage);
samlAuthenticationStatus.setStatus("error");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,23 @@
import java.util.HashMap;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.junit.Test;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.junit.jupiter.MockitoExtension;
import org.sonar.api.server.http.HttpRequest;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.sonar.auth.saml.SamlAuthStatusPageGenerator.getSamlAuthStatusHtml;

@ExtendWith(MockitoExtension.class)
public class SamlAuthStatusPageGeneratorTest {

@InjectMocks
private SamlAuthStatusPageGenerator samlAuthStatusPageGenerator;


@Test
public void getSamlAuthStatusHtml_whenCalled_shouldGeneratePageWithData() {
SamlAuthenticationStatus samlAuthenticationStatus = mock(SamlAuthenticationStatus.class);
Expand All @@ -49,7 +56,7 @@ public void getSamlAuthStatusHtml_whenCalled_shouldGeneratePageWithData() {
when(samlAuthenticationStatus.isSignatureEnabled()).thenReturn(false);
when(request.getContextPath()).thenReturn("context");

String decodedDataResponse = getDecodedDataResponse(getSamlAuthStatusHtml(request, samlAuthenticationStatus));
String decodedDataResponse = getDecodedDataResponse(samlAuthStatusPageGenerator.getSamlAuthStatusHtml(request, samlAuthenticationStatus));

assertThat(decodedDataResponse).contains(
"\"encryptionEnabled\":false",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,78 +19,144 @@
*/
package org.sonar.auth.saml;

import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.junit.Test;
import org.sonar.api.config.PropertyDefinitions;
import org.sonar.api.config.internal.MapSettings;
import org.sonar.api.server.http.HttpRequest;
import org.sonar.api.server.http.HttpResponse;
import org.sonar.api.utils.System2;
import java.io.IOException;
import java.io.UncheckedIOException;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.sonar.api.server.authentication.OAuth2IdentityProvider;
import org.sonar.api.server.authentication.UserIdentity;
import org.sonar.server.http.JakartaHttpRequest;
import org.sonar.server.http.JakartaHttpResponse;

import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
import static org.junit.Assert.assertFalse;
import org.springframework.security.saml2.core.Saml2Error;
import org.springframework.security.saml2.provider.service.authentication.Saml2AuthenticatedPrincipal;
import org.springframework.security.saml2.provider.service.authentication.Saml2AuthenticationException;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatException;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

@ExtendWith(MockitoExtension.class)
public class SamlAuthenticatorTest {

private MapSettings settings = new MapSettings(new PropertyDefinitions(System2.INSTANCE, SamlSettings.definitions()));
public static final String CALLBACK_URL = "callbackUrl";
public static final String RELAY_STATE = "relayState";
public static final String REDIRECT_URL = "redirectUrl";

@Mock
private RedirectToUrlProvider redirectToUrlProvider;
@Mock
private SamlResponseAuthenticator samlResponseAuthenticator;
@Mock
private PrincipalToUserIdentityConverter principalToUserIdentityConverter;
@Mock
private SamlStatusChecker samlStatusChecker;
@Mock
private SamlAuthStatusPageGenerator samlAuthStatusPageGenerator;
@Mock
private SamlAuthenticationStatus samlAuthenticationStatus;

@InjectMocks
private SamlAuthenticator samlAuthenticator;

@Mock
private JakartaHttpRequest request;
@Mock
private JakartaHttpResponse response;

@Test
void initLogin_generatesUrlAndSendsRedirect() throws IOException {
when(redirectToUrlProvider.getRedirectToUrl(request, CALLBACK_URL, RELAY_STATE)).thenReturn(REDIRECT_URL);

private SamlSettings samlSettings = new SamlSettings(settings.asConfig());
samlAuthenticator.initLogin(CALLBACK_URL, RELAY_STATE, request, response);

private final SamlAuthenticator underTest = new SamlAuthenticator(samlSettings, null, null, null); //TODO
verify(response).sendRedirect(REDIRECT_URL);
}

@Test
public void authentication_status_with_errors_returned_when_init_fails() {
HttpRequest request = new JakartaHttpRequest(mock(HttpServletRequest.class));
HttpResponse response = new JakartaHttpResponse(mock(HttpServletResponse.class));
when(request.getContextPath()).thenReturn("context");
void initLogin_whenIoException_convertsToRuntimeException() throws IOException {
when(redirectToUrlProvider.getRedirectToUrl(request, CALLBACK_URL, RELAY_STATE)).thenReturn(REDIRECT_URL);

IOException ioException = new IOException();
doThrow(ioException).when(response).sendRedirect(REDIRECT_URL);

assertThatException()
.isThrownBy(() -> samlAuthenticator.initLogin(CALLBACK_URL, RELAY_STATE, request, response))
.isInstanceOf(UncheckedIOException.class)
.withCause(ioException);
}

@Test
void onCallback_verifiesCsrfStateAndAuthenticates() {
OAuth2IdentityProvider.CallbackContext context = mock();
when(context.getCallbackUrl()).thenReturn(CALLBACK_URL);

Saml2AuthenticatedPrincipal principal = mock();
when(samlResponseAuthenticator.authenticate(request, CALLBACK_URL)).thenReturn(principal);

UserIdentity userIdentity = mock();
when(principalToUserIdentityConverter.convertToUserIdentity(principal)).thenReturn(userIdentity);

String authenticationStatus = underTest.getAuthenticationStatusPage(request, response);
UserIdentity actualUserIdentity = samlAuthenticator.onCallback(context, request);

assertFalse(authenticationStatus.isEmpty());
verify(context).verifyCsrfState("RelayState");
verify(samlResponseAuthenticator).authenticate(request, CALLBACK_URL);
assertThat(actualUserIdentity).isEqualTo(userIdentity);
}

@Test
public void givenPrivateKeyIsNotPkcs8Encrypted_whenInitializingTheAuthentication_thenExceptionIsThrown() {
initBasicSamlSettings();

settings.setProperty("sonar.auth.saml.signature.enabled", true);
settings.setProperty("sonar.auth.saml.sp.certificate.secured", "CERTIFICATE");
settings.setProperty("sonar.auth.saml.sp.privateKey.secured", "Not a PKCS8 key");

assertThatIllegalStateException()
.isThrownBy(() -> underTest.initLogin("", "", mock(JakartaHttpRequest.class), mock(JakartaHttpResponse.class)))
.withMessage("Failed to create a SAML Auth")
.havingCause()
.withMessage("Error in parsing service provider private key, please make sure that it is in PKCS 8 format.");
void getAuthenticationStatusPage_returnsHtml() {
String samlResponse = "samlResponse";
when(request.getRequestURL()).thenReturn("url");
when(request.getParameter("SAMLResponse")).thenReturn(samlResponse);

Saml2AuthenticatedPrincipal principal = mock();
when(samlResponseAuthenticator.authenticate(request, request.getRequestURL())).thenReturn(principal);

when(samlStatusChecker.getSamlAuthenticationStatus(samlResponse, principal)).thenReturn(samlAuthenticationStatus);
when(samlAuthStatusPageGenerator.getSamlAuthStatusHtml(request, samlAuthenticationStatus)).thenReturn("html");

String authenticationStatusPage = samlAuthenticator.getAuthenticationStatusPage(request);

assertThat(authenticationStatusPage).isEqualTo("html");
}

@Test
public void givenMissingSpCertificate_whenInitializingTheAuthentication_thenExceptionIsThrown() {
initBasicSamlSettings();
void getAuthenticationStatusPage_whenSaml2AuthenticationException_returnsHtml() {
when(request.getRequestURL()).thenReturn("url");

String errorMessage = "error";
when(samlResponseAuthenticator.authenticate(request, request.getRequestURL())).thenThrow(new Saml2AuthenticationException(new Saml2Error("erorCode", errorMessage)));

when(samlStatusChecker.getSamlAuthenticationStatus(errorMessage)).thenReturn(samlAuthenticationStatus);
when(samlAuthStatusPageGenerator.getSamlAuthStatusHtml(request, samlAuthenticationStatus)).thenReturn("html");

settings.setProperty("sonar.auth.saml.signature.enabled", true);
settings.setProperty("sonar.auth.saml.sp.privateKey.secured", "PRIVATE_KEY");
String authenticationStatusPage = samlAuthenticator.getAuthenticationStatusPage(request);

assertThatIllegalStateException()
.isThrownBy(() -> underTest.initLogin("", "", mock(JakartaHttpRequest.class), mock(JakartaHttpResponse.class)))
.withMessage("Failed to create a SAML Auth")
.havingCause()
.withMessage("Service provider certificate is missing");
assertThat(authenticationStatusPage).isEqualTo("html");
}

private void initBasicSamlSettings() {
settings.setProperty("sonar.auth.saml.applicationId", "MyApp");
settings.setProperty("sonar.auth.saml.providerId", "http://localhost:8080/auth/realms/sonarqube");
settings.setProperty("sonar.auth.saml.loginUrl", "http://localhost:8080/auth/realms/sonarqube/protocol/saml");
settings.setProperty("sonar.auth.saml.certificate.secured", "ABCDEFG");
settings.setProperty("sonar.auth.saml.user.login", "login");
settings.setProperty("sonar.auth.saml.user.name", "name");
settings.setProperty("sonar.auth.saml.enabled", true);
@Test
void getAuthenticationStatusPage_whenIllegalStateException_returnsHtml() {
when(request.getRequestURL()).thenReturn("url");

RuntimeException runtimeException = new RuntimeException("error");
IllegalStateException illegalStateException = new IllegalStateException(runtimeException);
when(samlResponseAuthenticator.authenticate(request, request.getRequestURL())).thenThrow(illegalStateException);

when(samlStatusChecker.getSamlAuthenticationStatus(any())).thenReturn(samlAuthenticationStatus);
when(samlAuthStatusPageGenerator.getSamlAuthStatusHtml(request, samlAuthenticationStatus)).thenReturn("html");

String authenticationStatusPage = samlAuthenticator.getAuthenticationStatusPage(request);

assertThat(authenticationStatusPage).isEqualTo("html");
verify(samlAuthStatusPageGenerator).getSamlAuthStatusHtml(request, samlAuthenticationStatus);
}

}
Loading

0 comments on commit 1fce3f5

Please sign in to comment.