From 22ca049c5b82711b4aa24841a35b8595a590a916 Mon Sep 17 00:00:00 2001 From: Jens-Otto Larsen <46576810+jolarsen@users.noreply.github.com> Date: Fri, 8 Dec 2023 19:33:22 +0100 Subject: [PATCH] =?UTF-8?q?Ny=20konvensjon=20for=20=C3=A5=20svare=20401=20?= =?UTF-8?q?fra=20ServerAuthModule=20(#1337)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Støtte for å sette / utlede databasedialekt * Nytt pattern for response 401 * Spacefix --- .../jaxrs/AuthenticationFilterDelegate.java | 16 +++-- .../AuthenticationFilterDelegateTest.java | 35 ++++++---- .../sikkerhet/jaspic/OidcAuthModule.java | 13 ++-- .../sikkerhet/jaspic/OidcAuthModuleTest.java | 68 ++++++++++--------- 4 files changed, 75 insertions(+), 57 deletions(-) diff --git a/felles/auth-filter/src/main/java/no/nav/vedtak/sikkerhet/jaxrs/AuthenticationFilterDelegate.java b/felles/auth-filter/src/main/java/no/nav/vedtak/sikkerhet/jaxrs/AuthenticationFilterDelegate.java index 8710d3443..d8b2b74f6 100644 --- a/felles/auth-filter/src/main/java/no/nav/vedtak/sikkerhet/jaxrs/AuthenticationFilterDelegate.java +++ b/felles/auth-filter/src/main/java/no/nav/vedtak/sikkerhet/jaxrs/AuthenticationFilterDelegate.java @@ -4,16 +4,16 @@ import java.time.Instant; import java.util.Optional; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.slf4j.MDC; + import jakarta.ws.rs.WebApplicationException; import jakarta.ws.rs.container.ContainerRequestContext; import jakarta.ws.rs.container.ResourceInfo; import jakarta.ws.rs.core.Cookie; import jakarta.ws.rs.core.HttpHeaders; import jakarta.ws.rs.core.Response; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import no.nav.vedtak.exception.TekniskException; import no.nav.vedtak.log.mdc.MDCOperations; import no.nav.vedtak.sikkerhet.abac.BeskyttetRessurs; @@ -57,8 +57,13 @@ public static void validerSettKontekst(ResourceInfo resourceInfo, ContainerReque var beskyttetRessurs = method.getAnnotation(BeskyttetRessurs.class); var åpenRessurs = method.getAnnotation(ÅpenRessurs.class); var metodenavn = method.getName(); - LOG.trace("{} i klasse {}", metodenavn, method.getDeclaringClass()); + if (KontekstHolder.harKontekst()) { + LOG.info("Kall til {} hadde kontekst {}", metodenavn, KontekstHolder.getKontekst().getKompaktUid()); + KontekstHolder.fjernKontekst(); + } + MDC.clear(); setCallAndConsumerId(ctx); + LOG.trace("{} i klasse {}", metodenavn, method.getDeclaringClass()); if (beskyttetRessurs == null && (åpenRessurs != null || method.getDeclaringClass().getName().startsWith("io.swagger"))) { KontekstHolder.setKontekst(BasisKontekst.ikkeAutentisertRequest(MDCOperations.getConsumerId())); LOG.trace("{} er whitelisted", metodenavn); @@ -83,6 +88,7 @@ public static void fjernKontekst() { if (KontekstHolder.harKontekst()) { KontekstHolder.fjernKontekst(); } + MDC.clear(); } private static void setCallAndConsumerId(ContainerRequestContext request) { diff --git a/felles/auth-filter/src/test/java/no/nav/vedtak/sikkerhet/jaxrs/AuthenticationFilterDelegateTest.java b/felles/auth-filter/src/test/java/no/nav/vedtak/sikkerhet/jaxrs/AuthenticationFilterDelegateTest.java index 6904ce1b3..e1e7e02f1 100644 --- a/felles/auth-filter/src/test/java/no/nav/vedtak/sikkerhet/jaxrs/AuthenticationFilterDelegateTest.java +++ b/felles/auth-filter/src/test/java/no/nav/vedtak/sikkerhet/jaxrs/AuthenticationFilterDelegateTest.java @@ -7,12 +7,6 @@ import java.lang.reflect.Method; import java.util.Map; -import javax.security.auth.Subject; -import jakarta.ws.rs.Path; -import jakarta.ws.rs.WebApplicationException; -import jakarta.ws.rs.container.ContainerRequestContext; -import jakarta.ws.rs.container.ResourceInfo; - import org.jose4j.json.JsonUtil; import org.jose4j.jwt.NumericDate; import org.junit.jupiter.api.AfterEach; @@ -20,6 +14,10 @@ import org.junit.jupiter.api.Test; import org.mockito.Mockito; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.WebApplicationException; +import jakarta.ws.rs.container.ContainerRequestContext; +import jakarta.ws.rs.container.ResourceInfo; import no.nav.vedtak.sikkerhet.abac.BeskyttetRessurs; import no.nav.vedtak.sikkerhet.abac.ÅpenRessurs; import no.nav.vedtak.sikkerhet.kontekst.IdentType; @@ -36,14 +34,11 @@ class AuthenticationFilterDelegateTest { - private OidcTokenValidator tokenValidator = Mockito.mock(OidcTokenValidator.class); + private final OidcTokenValidator tokenValidator = Mockito.mock(OidcTokenValidator.class); - private ContainerRequestContext request = Mockito.mock(ContainerRequestContext.class); + private final ContainerRequestContext request = Mockito.mock(ContainerRequestContext.class); - private Subject subject = new Subject(); - private Subject serviceSubject = new Subject(); - - public void setupAll() throws Exception { + public void setupAll() { System.setProperty(AzureProperty.AZURE_APP_WELL_KNOWN_URL.name(), OidcTokenGenerator.ISSUER + "/" + WellKnownConfigurationHelper.STANDARD_WELL_KNOWN_PATH); @@ -60,13 +55,13 @@ public void setupAll() throws Exception { } @BeforeEach - public void setUp() throws Exception { + public void setUp() { WellKnownConfigurationHelper.unsetWellKnownConfig(); setupAll(); } @AfterEach - public void teardown() throws Exception { + public void teardown() { System.clearProperty(AzureProperty.AZURE_APP_WELL_KNOWN_URL.name()); System.clearProperty(AzureProperty.AZURE_APP_CLIENT_ID.name()); System.clearProperty(AzureProperty.AZURE_OPENID_CONFIG_ISSUER.name()); @@ -94,6 +89,11 @@ public void teardown() throws Exception { when(request.getHeaderString("Authorization")).thenReturn(null); assertThrows(WebApplicationException.class, () -> AuthenticationFilterDelegate.validerSettKontekst(ri, request)); + try { + AuthenticationFilterDelegate.validerSettKontekst(ri, request); + } catch (WebApplicationException e) { + assertThat(e.getResponse().getStatus()).isEqualTo(403); + } } @@ -103,11 +103,16 @@ public void teardown() throws Exception { Method method = RestClass.class.getMethod("beskyttet"); ResourceInfo ri = new TestInvocationContext(method, RestClass.class); - when(request.getHeaderString("Authorization")).thenReturn(OpenIDToken.OIDC_DEFAULT_TOKEN_TYPE + utløptIdToken); + when(request.getHeaderString("Authorization")).thenReturn(OpenIDToken.OIDC_DEFAULT_TOKEN_TYPE + utløptIdToken.token()); when(tokenValidator.validate(utløptIdToken)).thenReturn(OidcTokenValidatorResult.invalid("expired")); assertThrows(WebApplicationException.class, () -> AuthenticationFilterDelegate.validerSettKontekst(ri, request)); + try { + AuthenticationFilterDelegate.validerSettKontekst(ri, request); + } catch (WebApplicationException e) { + assertThat(e.getResponse().getStatus()).isEqualTo(401); + } } diff --git a/felles/sikkerhet/src/main/java/no/nav/vedtak/sikkerhet/jaspic/OidcAuthModule.java b/felles/sikkerhet/src/main/java/no/nav/vedtak/sikkerhet/jaspic/OidcAuthModule.java index ed6cf6364..458b0a651 100644 --- a/felles/sikkerhet/src/main/java/no/nav/vedtak/sikkerhet/jaspic/OidcAuthModule.java +++ b/felles/sikkerhet/src/main/java/no/nav/vedtak/sikkerhet/jaspic/OidcAuthModule.java @@ -19,6 +19,8 @@ import javax.security.auth.login.LoginContext; import javax.security.auth.login.LoginException; +import org.eclipse.jetty.ee10.security.jaspi.JaspiMessageInfo; +import org.eclipse.jetty.server.Response; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.slf4j.MDC; @@ -98,13 +100,16 @@ public AuthStatus validateRequest(MessageInfo messageInfo, Subject clientSubject if (FAILURE.equals(authStatus)) { // Vurder om trengs whitelisting av oppslag mot isAlive/isReady/metrics gitt oppførsel i prod - app må registrere unprotected // Sjekk av whitelist/unprotected mot HttpServletRequest / getPathInfo må foregå før valg av protected/unprotected - HttpServletResponse response = (HttpServletResponse) messageInfo.getResponseMessage(); try { - response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Resource is protected, but id token is missing or invalid."); - } catch (IOException e) { + if (messageInfo instanceof JaspiMessageInfo mi) { + Response.writeError(mi.getBaseRequest(), mi.getBaseResponse(), mi.getCallback(), 401); + } else { + return FAILURE; // Vil gi 403 + } + } catch (Exception e) { throw new TekniskException("F-396795", "Klarte ikke å sende respons", e); } - return SEND_CONTINUE; // TODO - skal man returnere SEND_FAILURE, FAILURE? SEND_CONTINUE virker mest relevant for redirect to login + return SEND_CONTINUE; // TODO - skal man returnere SEND_FAILURE? SEND_CONTINUE virker mest relevant for redirect to login } if (SUCCESS.equals(authStatus)) { diff --git a/felles/sikkerhet/src/test/java/no/nav/vedtak/sikkerhet/jaspic/OidcAuthModuleTest.java b/felles/sikkerhet/src/test/java/no/nav/vedtak/sikkerhet/jaspic/OidcAuthModuleTest.java index cdf5108d2..0f0356d14 100644 --- a/felles/sikkerhet/src/test/java/no/nav/vedtak/sikkerhet/jaspic/OidcAuthModuleTest.java +++ b/felles/sikkerhet/src/test/java/no/nav/vedtak/sikkerhet/jaspic/OidcAuthModuleTest.java @@ -3,7 +3,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import java.util.HashMap; @@ -14,6 +13,13 @@ import javax.security.auth.callback.CallbackHandler; import javax.security.auth.login.Configuration; +import org.eclipse.jetty.ee10.security.jaspi.JaspiMessageInfo; +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpVersion; +import org.eclipse.jetty.server.ConnectionMetaData; +import org.eclipse.jetty.server.Context; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Response; import org.jose4j.json.JsonUtil; import org.jose4j.jwt.NumericDate; import org.junit.jupiter.api.BeforeEach; @@ -37,19 +43,26 @@ class OidcAuthModuleTest { - private OidcTokenValidator tokenValidator = Mockito.mock(OidcTokenValidator.class); - private TokenLocator tokenLocator = Mockito.mock(TokenLocator.class); - private CallbackHandler callbackHandler = Mockito.mock(CallbackHandler.class); + private final OidcTokenValidator tokenValidator = Mockito.mock(OidcTokenValidator.class); + private final TokenLocator tokenLocator = Mockito.mock(TokenLocator.class); + private final CallbackHandler callbackHandler = Mockito.mock(CallbackHandler.class); private final Configuration configuration = new LoginContextConfiguration(); - private OidcAuthModule authModule = new OidcAuthModule(tokenLocator, configuration); - private HttpServletRequest request = Mockito.mock(HttpServletRequest.class); - private HttpServletResponse response = Mockito.mock(HttpServletResponse.class); + private final OidcAuthModule authModule = new OidcAuthModule(tokenLocator, configuration); + private final HttpServletRequest request = Mockito.mock(HttpServletRequest.class); + private final HttpServletResponse response = Mockito.mock(HttpServletResponse.class); + + private final Request requestBase = Mockito.mock(Request.class); + private final Response responseBase = Mockito.mock(Response.class); + private final ConnectionMetaData connMetaData = Mockito.mock(ConnectionMetaData.class); + private final Context context = Mockito.mock(Context.class); private Subject subject = new Subject(); private Subject serviceSubject = new Subject(); - public void setupAll() throws Exception { + @BeforeEach + public void setUp() { + WellKnownConfigurationHelper.unsetWellKnownConfig(); authModule.initialize(null, null, callbackHandler, null); System.setProperty(AzureProperty.AZURE_APP_WELL_KNOWN_URL.name(), @@ -66,12 +79,6 @@ public void setupAll() throws Exception { OidcTokenValidatorConfig.addValidator(OpenIDProvider.AZUREAD, tokenValidator); } - @BeforeEach - public void setUp() throws Exception { - WellKnownConfigurationHelper.unsetWellKnownConfig(); - setupAll(); - } - @Test void skal_ikke_slippe_gjennom_forespørsel_men_svare_med_401_etter_beskyttet_ressurs_når_forespørselen_ikke_har_med_id_token() throws Exception { when(request.getHeader("Accept")).thenReturn("application/json"); @@ -81,8 +88,7 @@ public void setUp() throws Exception { AuthStatus result = authModule.validateRequest(request, subject, serviceSubject); assertThat(result).isEqualTo(AuthStatus.SEND_CONTINUE); - verify(response).sendError(401, "Resource is protected, but id token is missing or invalid."); - verifyNoMoreInteractions(response); + verify(responseBase).setStatus(401); } @Test @@ -99,8 +105,7 @@ void skal_sende_401_for_ugyldig_Authorization_header() throws Exception { AuthStatus result = authModule.validateRequest(request, subject, serviceSubject); assertThat(result).isEqualTo(AuthStatus.SEND_CONTINUE); - verify(response).sendError(401, "Resource is protected, but id token is missing or invalid."); - verifyNoMoreInteractions(response); + verify(responseBase).setStatus(401); } @Test @@ -112,7 +117,7 @@ void skal_sende_401_for_ugyldig_Authorization_header() throws Exception { AuthStatus result = authModule.validateRequest(request, subject, serviceSubject); assertThat(result).isEqualTo(AuthStatus.SEND_CONTINUE); - verify(response).sendError(401, "Resource is protected, but id token is missing or invalid."); + verify(responseBase).setStatus(401); } @Test @@ -138,8 +143,7 @@ void skal_sende_401_for_ugyldig_Authorization_header() throws Exception { AuthStatus result = authModule.validateRequest(request, subject, serviceSubject); assertThat(result).isEqualTo(AuthStatus.SEND_CONTINUE); - verify(response).sendError(HttpServletResponse.SC_UNAUTHORIZED, "Resource is protected, but id token is missing or invalid."); - verifyNoMoreInteractions(response); + verify(responseBase).setStatus(401); } @Test @@ -152,7 +156,7 @@ void skal_sende_401_for_ugyldig_Authorization_header() throws Exception { AuthStatus result = authModule.validateRequest(request, subject, serviceSubject); assertThat(result).isEqualTo(AuthStatus.SEND_CONTINUE); - verify(response).sendError(401, "Resource is protected, but id token is missing or invalid."); + verify(responseBase).setStatus(401); } @Test @@ -166,7 +170,7 @@ void skal_sende_401_for_ugyldig_Authorization_header() throws Exception { AuthStatus result = authModule.validateRequest(request, subject, serviceSubject); assertThat(result).isEqualTo(AuthStatus.SEND_CONTINUE); - verify(response).sendError(401, "Resource is protected, but id token is missing or invalid."); + verify(responseBase).setStatus(401); } @@ -202,20 +206,18 @@ void skal_sende_401_for_ugyldig_Authorization_header() throws Exception { } private MessageInfo createRequestForProtectedResource() { - return createRequestForResource(true); - } - - private MessageInfo createRequestForUnprotectedResource() { - return createRequestForResource(false); - } - - private MessageInfo createRequestForResource(boolean isProtected) { - MessageInfo messageInfo = Mockito.mock(MessageInfo.class); + when(requestBase.getConnectionMetaData()).thenReturn(connMetaData); + when(connMetaData.getHttpVersion()).thenReturn(HttpVersion.HTTP_2); + when(requestBase.getContext()).thenReturn(context); + when(responseBase.getHeaders()).thenReturn(HttpFields.build()); + var messageInfo = Mockito.mock(JaspiMessageInfo.class); Map properties = new HashMap<>(); - properties.put("jakarta.security.auth.message.MessagePolicy.isMandatory", Boolean.toString(isProtected)); when(messageInfo.getMap()).thenReturn(properties); when(messageInfo.getRequestMessage()).thenReturn(request); when(messageInfo.getResponseMessage()).thenReturn(response); + when(messageInfo.getBaseResponse()).thenReturn(responseBase); + when(messageInfo.getBaseRequest()).thenReturn(requestBase); + when(messageInfo.getCallback()).thenReturn(null); return messageInfo; }