Skip to content

Commit

Permalink
Restrict the URL used to do delegated authorization
Browse files Browse the repository at this point in the history
  • Loading branch information
oharsta committed Oct 23, 2023
1 parent 00789fc commit 7bd8ad4
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 9 deletions.
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

<groupId>org.openconext</groupId>
<artifactId>oidcng</artifactId>
<version>6.1.7</version>
<version>6.1.8-SNAPSHOT</version>
<name>oidcng</name>

<dependencyManagement>
Expand Down
17 changes: 17 additions & 0 deletions src/main/java/oidc/exceptions/JWTRequestURIMismatchException.java
Original file line number Diff line number Diff line change
@@ -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";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ protected boolean suppressStackTrace() {

@Override
public String getErrorCode() {
return "invalid_request_uri";
return "invalid_redirect_uri";
}
}
2 changes: 2 additions & 0 deletions src/main/java/oidc/model/OpenIDClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public class OpenIDClient {
private List<Scope> scopes;
private List<String> grants;
private List<String> allowedResourceServers;
private String jwtRequestUri;
private boolean resourceServer;
private boolean publicClient;
//seconds
Expand Down Expand Up @@ -82,6 +83,7 @@ public OpenIDClient(Map<String, Object> 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<Map<String, String>>) data.getOrDefault("allowedResourceServers", new ArrayList<>()))
Expand Down
13 changes: 12 additions & 1 deletion src/main/java/oidc/saml/AuthnRequestConverter.java
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}
}
Expand Down
37 changes: 31 additions & 6 deletions src/test/java/oidc/saml/AuthnRequestConverterUnitTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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"));
Expand Down

0 comments on commit 7bd8ad4

Please sign in to comment.