From 7bd8ad4a3a75b0843f268624a27117a7a3d29550 Mon Sep 17 00:00:00 2001 From: Okke Harsta Date: Mon, 23 Oct 2023 12:11:40 +0200 Subject: [PATCH] Restrict the URL used to do delegated authorization See https://www.pivotaltracker.com/story/show/186313207 --- pom.xml | 2 +- .../JWTRequestURIMismatchException.java | 17 +++++++++ .../exceptions/RedirectMismatchException.java | 2 +- src/main/java/oidc/model/OpenIDClient.java | 2 + .../java/oidc/saml/AuthnRequestConverter.java | 13 ++++++- .../saml/AuthnRequestConverterUnitTest.java | 37 ++++++++++++++++--- 6 files changed, 64 insertions(+), 9 deletions(-) create mode 100644 src/main/java/oidc/exceptions/JWTRequestURIMismatchException.java diff --git a/pom.xml b/pom.xml index f4f1c5b4..ecfac0d1 100644 --- a/pom.xml +++ b/pom.xml @@ -21,7 +21,7 @@ org.openconext oidcng - 6.1.7 + 6.1.8-SNAPSHOT oidcng diff --git a/src/main/java/oidc/exceptions/JWTRequestURIMismatchException.java b/src/main/java/oidc/exceptions/JWTRequestURIMismatchException.java new file mode 100644 index 00000000..eeac16e6 --- /dev/null +++ b/src/main/java/oidc/exceptions/JWTRequestURIMismatchException.java @@ -0,0 +1,17 @@ +package oidc.exceptions; + +import org.springframework.http.HttpStatus; +import org.springframework.web.bind.annotation.ResponseStatus; + +@ResponseStatus(value = HttpStatus.BAD_REQUEST) +public class JWTRequestURIMismatchException extends BaseException { + + public JWTRequestURIMismatchException(String message) { + super(message); + } + + @Override + public String getErrorCode() { + return "invalid_request_uri"; + } +} diff --git a/src/main/java/oidc/exceptions/RedirectMismatchException.java b/src/main/java/oidc/exceptions/RedirectMismatchException.java index 3b47e9d7..9a3ab04d 100644 --- a/src/main/java/oidc/exceptions/RedirectMismatchException.java +++ b/src/main/java/oidc/exceptions/RedirectMismatchException.java @@ -16,6 +16,6 @@ protected boolean suppressStackTrace() { @Override public String getErrorCode() { - return "invalid_request_uri"; + return "invalid_redirect_uri"; } } diff --git a/src/main/java/oidc/model/OpenIDClient.java b/src/main/java/oidc/model/OpenIDClient.java index 43203912..5b982a97 100644 --- a/src/main/java/oidc/model/OpenIDClient.java +++ b/src/main/java/oidc/model/OpenIDClient.java @@ -42,6 +42,7 @@ public class OpenIDClient { private List scopes; private List grants; private List allowedResourceServers; + private String jwtRequestUri; private boolean resourceServer; private boolean publicClient; //seconds @@ -82,6 +83,7 @@ public OpenIDClient(Map root) { this.clientSecretJWT = (String) metaDataFields.get("clientSecretJWT"); this.logoUrl = (String) metaDataFields.get("logo:0:url"); this.redirectUrls = (List) metaDataFields.get("redirectUrls"); + this.jwtRequestUri = (String) metaDataFields.get("oidc:jwtRequestUri"); this.grants = (List) metaDataFields.getOrDefault("grants", Collections.singletonList("authorization_code")); this.allowedResourceServers = ((List>) data.getOrDefault("allowedResourceServers", new ArrayList<>())) diff --git a/src/main/java/oidc/saml/AuthnRequestConverter.java b/src/main/java/oidc/saml/AuthnRequestConverter.java index 83cebfa5..51743f68 100644 --- a/src/main/java/oidc/saml/AuthnRequestConverter.java +++ b/src/main/java/oidc/saml/AuthnRequestConverter.java @@ -1,5 +1,7 @@ package oidc.saml; +import com.nimbusds.jose.JOSEException; +import com.nimbusds.jose.proc.BadJOSEException; import com.nimbusds.oauth2.sdk.AuthorizationRequest; import com.nimbusds.oauth2.sdk.ParseException; import com.nimbusds.oauth2.sdk.id.ClientID; @@ -8,6 +10,7 @@ import lombok.SneakyThrows; import oidc.endpoints.AuthorizationEndpoint; import oidc.exceptions.CookiesNotSupportedException; +import oidc.exceptions.JWTRequestURIMismatchException; import oidc.exceptions.UnknownClientException; import oidc.log.MDCContext; import oidc.manage.ServiceProviderTranslation; @@ -32,11 +35,13 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpSession; +import java.io.IOException; import java.io.UnsupportedEncodingException; import java.net.URI; import java.net.URISyntaxException; import java.net.URLDecoder; import java.nio.charset.Charset; +import java.security.cert.CertificateException; import java.time.Instant; import java.time.LocalDateTime; import java.time.ZoneId; @@ -175,6 +180,11 @@ private AuthnRequest enhanceAuthenticationRequest(AuthnRequest authnRequest, if (StringUtils.hasText(requestP) || StringUtils.hasText(requestUrlP)) { OpenIDClient openIDClient = openIDClientRepository.findOptionalByClientId(clientId) .orElseThrow(() -> new UnknownClientException(clientId)); + if (StringUtils.hasText(requestUrlP) && !requestUrlP.equalsIgnoreCase(openIDClient.getJwtRequestUri())) { + throw new JWTRequestURIMismatchException( + String.format("JWT request_uri mismatch for RP %s. Requested %s, configured %s", + openIDClient.getClientId(), requestUrlP, openIDClient.getJwtRequestUri())); + } try { com.nimbusds.openid.connect.sdk.AuthenticationRequest authRequest = com.nimbusds.openid.connect.sdk.AuthenticationRequest.parse(request); @@ -189,7 +199,8 @@ private AuthnRequest enhanceAuthenticationRequest(AuthnRequest authnRequest, if (!authnRequest.isForceAuthn() && prompt != null) { authnRequest.setForceAuthn(prompt.contains("login")); } - } catch (Exception e) { + } catch (CertificateException | JOSEException | IOException | BadJOSEException | + java.text.ParseException | URISyntaxException e) { throw new RuntimeException(e); } } diff --git a/src/test/java/oidc/saml/AuthnRequestConverterUnitTest.java b/src/test/java/oidc/saml/AuthnRequestConverterUnitTest.java index df85e8de..394f33d7 100644 --- a/src/test/java/oidc/saml/AuthnRequestConverterUnitTest.java +++ b/src/test/java/oidc/saml/AuthnRequestConverterUnitTest.java @@ -2,6 +2,7 @@ import com.nimbusds.jwt.SignedJWT; import oidc.exceptions.CookiesNotSupportedException; +import oidc.exceptions.JWTRequestURIMismatchException; import oidc.model.OpenIDClient; import oidc.model.Scope; import oidc.repository.AuthenticationRequestRepository; @@ -15,21 +16,22 @@ import org.springframework.security.web.savedrequest.RequestCache; import javax.servlet.http.HttpServletRequest; - +import java.util.List; +import java.util.Map; import java.util.Optional; import static java.util.Collections.singletonList; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static oidc.model.EntityType.OAUTH_RS; +import static org.junit.Assert.*; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; public class AuthnRequestConverterUnitTest extends AbstractSamlUnitTest implements SignedJWTTest { - private OpenIDClientRepository openIDClientRepository = mock(OpenIDClientRepository.class); - private AuthenticationRequestRepository authenticationRequestRepository = mock(AuthenticationRequestRepository.class); - private RequestCache requestCache = mock(RequestCache.class); + private final OpenIDClientRepository openIDClientRepository = mock(OpenIDClientRepository.class); + private final AuthenticationRequestRepository authenticationRequestRepository = mock(AuthenticationRequestRepository.class); + private final RequestCache requestCache = mock(RequestCache.class); private AuthnRequestConverter subject; @@ -69,6 +71,29 @@ public void testSaml() throws Exception { assertEquals("http://idp", authnRequest.getScoping().getIDPList().getIDPEntrys().get(0).getProviderID()); } + @Test + public void testJWTRequestURIMismatch() throws Exception { + OpenIDClient openIDClient = new OpenIDClient(Map.of( + "type", OAUTH_RS.getType(), + "data", Map.of("entityid", "mock_sp", + "metaDataFields", + Map.of("oidc:jwtRequestUri", "http://valid.url", + "redirectUrls", List.of("http://localhost:8080"))))); + when(openIDClientRepository.findOptionalByClientId("mock_sp")).thenReturn(Optional.of(openIDClient)); + + MockHttpServletRequest request = new MockHttpServletRequest("GET", "http://localhost/oidc/authorize"); + request.addParameter("client_id", "mock_sp"); + request.addParameter("response_type", "code"); + request.addParameter("request_uri", "http://invalid_url"); + + HttpServletRequest servletRequest = new MockHttpServletRequest(); + CustomSaml2AuthenticationRequestContext ctx = new CustomSaml2AuthenticationRequestContext(relyingParty, servletRequest); + + when(requestCache.getRequest(any(HttpServletRequest.class), any())) + .thenReturn(new DefaultSavedRequest(request, portResolver)); + assertThrows(JWTRequestURIMismatchException.class, () -> subject.convert(ctx)); + } + @Test public void testSamlForceAuthn() throws Exception { OpenIDClient openIDClient = new OpenIDClient("clientId", singletonList("http://redirect"), singletonList(new Scope("openid")), singletonList("authorization_code"));