From e155cdc282aa682d54af1924f93ddd5bdde3ac2e Mon Sep 17 00:00:00 2001 From: Justin Richer Date: Sat, 9 May 2015 16:36:08 -0400 Subject: [PATCH] added strict URI matching option to redirect resolver (off by default) --- .../impl/BlacklistAwareRedirectResolver.java | 44 +++++- .../TestBlacklistAwareRedirectResolver.java | 132 ++++++++++++++++++ 2 files changed, 175 insertions(+), 1 deletion(-) create mode 100644 openid-connect-server/src/test/java/org/mitre/oauth2/service/impl/TestBlacklistAwareRedirectResolver.java diff --git a/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/BlacklistAwareRedirectResolver.java b/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/BlacklistAwareRedirectResolver.java index 3a276c37d..a4adf7195 100644 --- a/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/BlacklistAwareRedirectResolver.java +++ b/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/BlacklistAwareRedirectResolver.java @@ -27,7 +27,13 @@ import org.springframework.security.oauth2.provider.ClientDetails; import org.springframework.security.oauth2.provider.endpoint.DefaultRedirectResolver; import org.springframework.stereotype.Component; +import com.google.common.base.Strings; + /** + * + * A redirect resolver that knows how to check against the blacklisted URIs + * for forbidden values. Can be configured to do strict string matching also. + * * @author jricher * */ @@ -36,7 +42,9 @@ public class BlacklistAwareRedirectResolver extends DefaultRedirectResolver { @Autowired private BlacklistedSiteService blacklistService; - + + private boolean strictMatch = false; + /* (non-Javadoc) * @see org.springframework.security.oauth2.provider.endpoint.RedirectResolver#resolveRedirect(java.lang.String, org.springframework.security.oauth2.provider.ClientDetails) */ @@ -52,4 +60,38 @@ public class BlacklistAwareRedirectResolver extends DefaultRedirectResolver { } } + /* (non-Javadoc) + * @see org.springframework.security.oauth2.provider.endpoint.DefaultRedirectResolver#redirectMatches(java.lang.String, java.lang.String) + */ + @Override + protected boolean redirectMatches(String requestedRedirect, String redirectUri) { + + if (isStrictMatch()) { + // we're doing a strict string match for all clients + return Strings.nullToEmpty(requestedRedirect).equals(redirectUri); + } else { + // otherwise do the prefix-match from the library + return super.redirectMatches(requestedRedirect, redirectUri); + } + + } + + /** + * @return the strictMatch + */ + public boolean isStrictMatch() { + return strictMatch; + } + + /** + * Set this to true to require exact string matches for all redirect URIs. (Default is false) + * + * @param strictMatch the strictMatch to set + */ + public void setStrictMatch(boolean strictMatch) { + this.strictMatch = strictMatch; + } + + + } diff --git a/openid-connect-server/src/test/java/org/mitre/oauth2/service/impl/TestBlacklistAwareRedirectResolver.java b/openid-connect-server/src/test/java/org/mitre/oauth2/service/impl/TestBlacklistAwareRedirectResolver.java new file mode 100644 index 000000000..8f2954e23 --- /dev/null +++ b/openid-connect-server/src/test/java/org/mitre/oauth2/service/impl/TestBlacklistAwareRedirectResolver.java @@ -0,0 +1,132 @@ +/******************************************************************************* + * Copyright 2015 The MITRE Corporation + * and the MIT Kerberos and Internet Trust Consortium + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + *******************************************************************************/ + +package org.mitre.oauth2.service.impl; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mitre.openid.connect.service.BlacklistedSiteService; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.security.oauth2.common.exceptions.InvalidRequestException; +import org.springframework.security.oauth2.provider.ClientDetails; + +import com.google.common.collect.ImmutableSet; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.is; + +import static org.mockito.Matchers.anyString; + +import static org.mockito.Mockito.when; + +import static org.junit.Assert.assertThat; + +/** + * @author jricher + * + */ +@RunWith(MockitoJUnitRunner.class) +public class TestBlacklistAwareRedirectResolver { + + @Mock + private BlacklistedSiteService blacklistService; + + @Mock + private ClientDetails client; + + @InjectMocks + private BlacklistAwareRedirectResolver resolver; + + private String blacklistedUri = "https://evil.example.com/"; + + private String goodUri = "https://good.example.com/"; + + private String pathUri = "https://good.example.com/with/path"; + + /** + * @throws java.lang.Exception + */ + @Before + public void setUp() throws Exception { + + when(blacklistService.isBlacklisted(anyString())).thenReturn(false); + when(blacklistService.isBlacklisted(blacklistedUri)).thenReturn(true); + + when(client.getAuthorizedGrantTypes()).thenReturn(ImmutableSet.of("authorization_code")); + when(client.getRegisteredRedirectUri()).thenReturn(ImmutableSet.of(goodUri, blacklistedUri)); + + } + + @Test + public void testResolveRedirect_safe() { + + // default uses prefix matching, both of these should work + + String res1 = resolver.resolveRedirect(goodUri, client); + + assertThat(res1, is(equalTo(goodUri))); + + String res2 = resolver.resolveRedirect(pathUri, client); + + assertThat(res2, is(equalTo(pathUri))); + + + } + + @Test(expected = InvalidRequestException.class) + public void testResolveRedirect_blacklisted() { + + // this should fail with an error + resolver.resolveRedirect(blacklistedUri, client); + + } + + @Test + public void testRedirectMatches_strict() { + resolver.setStrictMatch(true); + + // this is not an exact match + boolean res1 = resolver.redirectMatches(pathUri, goodUri); + + assertThat(res1, is(false)); + + // this is an exact match + boolean res2 = resolver.redirectMatches(goodUri, goodUri); + + assertThat(res2, is(true)); + + } + + @Test + public void testRedirectMatches_default() { + + // this is not an exact match (but that's OK) + boolean res1 = resolver.redirectMatches(pathUri, goodUri); + + assertThat(res1, is(true)); + + // this is an exact match + boolean res2 = resolver.redirectMatches(goodUri, goodUri); + + assertThat(res2, is(true)); + + } + +}