Skip to content

Java11 + @angelusGJ Spring Upgrade #1533

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
language: java
jdk:
- oraclejdk8
- oraclejdk11
sudo: false

after_success:
Original file line number Diff line number Diff line change
@@ -22,11 +22,10 @@

import java.util.Map;

import javax.annotation.PostConstruct;

import org.mitre.oauth2.model.RegisteredClient;
import org.mitre.openid.connect.client.service.ClientConfigurationService;
import org.mitre.openid.connect.config.ServerConfiguration;
import org.springframework.beans.factory.InitializingBean;

/**
* Client configuration service that holds a static map from issuer URL to a ClientDetails object to use at that issuer.
@@ -36,7 +35,7 @@
* @author jricher
*
*/
public class StaticClientConfigurationService implements ClientConfigurationService {
public class StaticClientConfigurationService implements ClientConfigurationService, InitializingBean {

// Map of issuer URL -> client configuration information
private Map<String, RegisteredClient> clients;
@@ -66,7 +65,7 @@ public RegisteredClient getClientConfiguration(ServerConfiguration issuer) {
return clients.get(issuer.getIssuer());
}

@PostConstruct
@Override
public void afterPropertiesSet() {
if (clients == null || clients.isEmpty()) {
throw new IllegalArgumentException("Clients map cannot be null or empty");
Original file line number Diff line number Diff line change
@@ -22,18 +22,18 @@

import java.util.Map;

import javax.annotation.PostConstruct;

import org.mitre.openid.connect.client.service.ServerConfigurationService;
import org.mitre.openid.connect.config.ServerConfiguration;
import org.springframework.beans.factory.InitializingBean;

/**
* Statically configured server configuration service that maps issuer URLs to server configurations to use at that issuer.
*
* @author jricher
*
*/
public class StaticServerConfigurationService implements ServerConfigurationService {
public class StaticServerConfigurationService implements ServerConfigurationService, InitializingBean {

// map of issuer url -> server configuration information
private Map<String, ServerConfiguration> servers;
@@ -60,7 +60,7 @@ public ServerConfiguration getServerConfiguration(String issuer) {
return servers.get(issuer);
}

@PostConstruct
@Override
public void afterPropertiesSet() {
if (servers == null || servers.isEmpty()) {
throw new IllegalArgumentException("Servers map cannot be null or empty.");
Original file line number Diff line number Diff line change
@@ -20,19 +20,19 @@
*/
package org.mitre.openid.connect.client.service.impl;

import javax.annotation.PostConstruct;
import javax.servlet.http.HttpServletRequest;

import org.mitre.openid.connect.client.model.IssuerServiceResponse;
import org.mitre.openid.connect.client.service.IssuerService;

import com.google.common.base.Strings;
import org.springframework.beans.factory.InitializingBean;

/**
* @author jricher
*
*/
public class StaticSingleIssuerService implements IssuerService {
public class StaticSingleIssuerService implements IssuerService, InitializingBean {

private String issuer;

@@ -60,7 +60,7 @@ public IssuerServiceResponse getIssuer(HttpServletRequest request) {
return new IssuerServiceResponse(getIssuer(), null, null);
}

@PostConstruct
@Override
public void afterPropertiesSet() {

if (Strings.isNullOrEmpty(issuer)) {
Original file line number Diff line number Diff line change
@@ -24,12 +24,12 @@
import java.util.HashSet;
import java.util.Set;

import javax.annotation.PostConstruct;
import javax.servlet.http.HttpServletRequest;

import org.apache.http.client.utils.URIBuilder;
import org.mitre.openid.connect.client.model.IssuerServiceResponse;
import org.mitre.openid.connect.client.service.IssuerService;
import org.springframework.beans.factory.InitializingBean;
import org.springframework.security.authentication.AuthenticationServiceException;

import com.google.common.base.Strings;
@@ -41,7 +41,7 @@
* @author jricher
*
*/
public class ThirdPartyIssuerService implements IssuerService {
public class ThirdPartyIssuerService implements IssuerService, InitializingBean {

private String accountChooserUrl;

@@ -128,7 +128,7 @@ public void setBlacklist(Set<String> blacklist) {
this.blacklist = blacklist;
}

@PostConstruct
@Override
public void afterPropertiesSet() {
if (Strings.isNullOrEmpty(this.accountChooserUrl)) {
throw new IllegalArgumentException("Account Chooser URL cannot be null or empty");
5 changes: 5 additions & 0 deletions openid-connect-common/pom.xml
Original file line number Diff line number Diff line change
@@ -87,6 +87,11 @@
<groupId>org.bouncycastle</groupId>
<artifactId>bcprov-jdk15on</artifactId>
</dependency>
<dependency>
<groupId>javax.annotation</groupId>
<artifactId>javax.annotation-api</artifactId>
<version>1.3.2</version>
</dependency>
</dependencies>

<packaging>jar</packaging>
Original file line number Diff line number Diff line change
@@ -25,8 +25,6 @@
import java.util.Map;
import java.util.Set;

import javax.annotation.PostConstruct;

import org.mitre.jose.keystore.JWKSetKeyStore;
import org.mitre.jwt.encryption.service.JWTEncryptionAndDecryptionService;
import org.slf4j.Logger;
@@ -50,12 +48,13 @@
import com.nimbusds.jose.jwk.JWK;
import com.nimbusds.jose.jwk.OctetSequenceKey;
import com.nimbusds.jose.jwk.RSAKey;
import org.springframework.beans.factory.InitializingBean;

/**
* @author wkim
*
*/
public class DefaultJWTEncryptionAndDecryptionService implements JWTEncryptionAndDecryptionService {
public class DefaultJWTEncryptionAndDecryptionService implements JWTEncryptionAndDecryptionService, InitializingBean {

/**
* Logger for this class
@@ -116,7 +115,7 @@ public DefaultJWTEncryptionAndDecryptionService(JWKSetKeyStore keyStore) throws
}


@PostConstruct
@Override
public void afterPropertiesSet() {

if (keys == null) {
Original file line number Diff line number Diff line change
@@ -91,7 +91,7 @@ public UserDetails loadUserByUsername(String clientId) throws UsernameNotFoundE
} else {
throw new UsernameNotFoundException("Client not found: " + clientId);
}
} catch (UnsupportedEncodingException | InvalidClientException e) {
} catch (IllegalArgumentException | InvalidClientException e) {
throw new UsernameNotFoundException("Client not found: " + clientId);
}

Original file line number Diff line number Diff line change
@@ -20,11 +20,10 @@
import java.util.List;
import java.util.Locale;

import javax.annotation.PostConstruct;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.BeanCreationException;
import org.springframework.beans.factory.InitializingBean;
import org.springframework.util.StringUtils;

import com.google.common.collect.Lists;
@@ -40,7 +39,7 @@
* @author AANGANES
*
*/
public class ConfigurationPropertiesBean {
public class ConfigurationPropertiesBean implements InitializingBean {

/**
* Logger for this class
@@ -72,14 +71,16 @@ public class ConfigurationPropertiesBean {
private boolean allowCompleteDeviceCodeUri = false;

public ConfigurationPropertiesBean() {

}

@Override
public void afterPropertiesSet() throws Exception {
checkConfigConsistency();
}
/**
* Endpoints protected by TLS must have https scheme in the URI.
* @throws HttpsUrlRequiredException
* @throws BeanCreationException
*/
@PostConstruct
public void checkConfigConsistency() {
if (!StringUtils.startsWithIgnoreCase(issuer, "https")) {
if (this.forceHttps) {
@@ -273,4 +274,6 @@ public boolean isAllowCompleteDeviceCodeUri() {
public void setAllowCompleteDeviceCodeUri(boolean allowCompleteDeviceCodeUri) {
this.allowCompleteDeviceCodeUri = allowCompleteDeviceCodeUri;
}


}

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -30,9 +30,12 @@
http://www.springframework.org/schema/tx http://www.springframework.org/schema/tx/spring-tx-4.3.xsd
http://www.springframework.org/schema/context http://www.springframework.org/schema/context/spring-context-4.3.xsd">

<bean id="noopPasswordEncoder" class="org.springframework.security.crypto.password.NoOpPasswordEncoder"/>

<security:authentication-manager id="authenticationManager">
<security:authentication-provider>
<security:jdbc-user-service data-source-ref="dataSource"/>
<security:password-encoder ref="noopPasswordEncoder" />
</security:authentication-provider>
</security:authentication-manager>

17 changes: 16 additions & 1 deletion openid-connect-server/pom.xml
Original file line number Diff line number Diff line change
@@ -72,11 +72,26 @@
<artifactId>org.eclipse.persistence.jpa</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-io</artifactId>
</dependency>
<dependency>
<groupId>javax.xml.bind</groupId>
<artifactId>jaxb-api</artifactId>
</dependency>
<dependency>
<groupId>com.sun.xml.bind</groupId>
<artifactId>jaxb-core</artifactId>
</dependency>
<dependency>
<groupId>com.sun.xml.bind</groupId>
<artifactId>jaxb-impl</artifactId>
</dependency>
<dependency>
<groupId>javax.activation</groupId>
<artifactId>activation</artifactId>
</dependency>
</dependencies>
<description>OpenID Connect server libraries for Spring and Spring Security.</description>
<url />
Original file line number Diff line number Diff line change
@@ -43,11 +43,6 @@ public class BlacklistAwareRedirectResolver extends DefaultRedirectResolver {
@Autowired
private BlacklistedSiteService blacklistService;

@Autowired
private ConfigurationPropertiesBean config;

private boolean strictMatch = true;

/* (non-Javadoc)
* @see org.springframework.security.oauth2.provider.endpoint.RedirectResolver#resolveRedirect(java.lang.String, org.springframework.security.oauth2.provider.ClientDetails)
*/
@@ -63,43 +58,4 @@ public String resolveRedirect(String requestedRedirect, ClientDetails client) th
}
}

/* (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() {
if (config.isHeartMode()) {
// HEART mode enforces strict matching
return true;
} else {
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;
}



}
Original file line number Diff line number Diff line change
@@ -25,15 +25,15 @@
import org.springframework.format.datetime.DateFormatter;

public abstract class MITREidDataServiceSupport {
public static final String DATE_TIME_ISO = "yyyy-MM-dd'T'HH:mm:ss.SSSZ";
private final DateFormatter dateFormatter;
/**
* Logger for this class
*/
private static final Logger logger = LoggerFactory.getLogger(MITREidDataServiceSupport.class);

public MITREidDataServiceSupport() {
dateFormatter = new DateFormatter();
dateFormatter.setIso(ISO.DATE_TIME);
dateFormatter = new DateFormatter(DATE_TIME_ISO);
}

protected Date utcToDate(String value) {
Original file line number Diff line number Diff line change
@@ -51,6 +51,8 @@
import org.springframework.http.ResponseEntity;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.security.oauth2.common.util.OAuth2Utils;
import org.springframework.stereotype.Controller;
import org.springframework.ui.Model;
@@ -212,6 +214,16 @@ public PKCEAlgorithm deserialize(JsonElement json, Type typeOfT, JsonDeserializa
}
}
})
.registerTypeAdapter(GrantedAuthority.class, new JsonDeserializer<GrantedAuthority>() {
@Override
public GrantedAuthority deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) throws JsonParseException {
if (json.isJsonPrimitive()) {
return new SimpleGrantedAuthority(json.getAsString());
} else {
return null;
}
}
})
.setDateFormat("yyyy-MM-dd'T'HH:mm:ssZ")
.create();

Original file line number Diff line number Diff line change
@@ -246,10 +246,6 @@ public String registerNewClient(@RequestBody String jsonString, Model m) {
m.addAttribute(HttpCodeView.CODE, HttpStatus.CREATED); // http 201

return ClientInformationResponseView.VIEWNAME;
} catch (UnsupportedEncodingException e) {
logger.error("Unsupported encoding", e);
m.addAttribute(HttpCodeView.CODE, HttpStatus.INTERNAL_SERVER_ERROR);
return HttpCodeView.VIEWNAME;
} catch (IllegalArgumentException e) {
logger.error("Couldn't save client", e);

@@ -293,7 +289,7 @@ public String readClientConfiguration(@PathVariable("id") String clientId, Model
m.addAttribute(HttpCodeView.CODE, HttpStatus.OK); // http 200

return ClientInformationResponseView.VIEWNAME;
} catch (UnsupportedEncodingException e) {
} catch (IllegalArgumentException e) {
logger.error("Unsupported encoding", e);
m.addAttribute(HttpCodeView.CODE, HttpStatus.INTERNAL_SERVER_ERROR);
return HttpCodeView.VIEWNAME;
@@ -319,7 +315,7 @@ public String readClientConfiguration(@PathVariable("id") String clientId, Model
*/
@PreAuthorize("hasRole('ROLE_CLIENT') and #oauth2.hasScope('" + SystemScopeService.REGISTRATION_TOKEN_SCOPE + "')")
@RequestMapping(value = "/{id}", method = RequestMethod.PUT, produces = MediaType.APPLICATION_JSON_VALUE, consumes = MediaType.APPLICATION_JSON_VALUE)
public String updateClient(@PathVariable("id") String clientId, @RequestBody String jsonString, Model m, OAuth2Authentication auth) {
public String updateClient(@PathVariable("id") String clientId, @RequestBody String jsonString, Model m, OAuth2Authentication auth) throws UnsupportedEncodingException {


ClientDetailsEntity newClient = null;
@@ -382,10 +378,6 @@ public String updateClient(@PathVariable("id") String clientId, @RequestBody Str
m.addAttribute(HttpCodeView.CODE, HttpStatus.OK); // http 200

return ClientInformationResponseView.VIEWNAME;
} catch (UnsupportedEncodingException e) {
logger.error("Unsupported encoding", e);
m.addAttribute(HttpCodeView.CODE, HttpStatus.INTERNAL_SERVER_ERROR);
return HttpCodeView.VIEWNAME;
} catch (IllegalArgumentException e) {
logger.error("Couldn't save client", e);

Original file line number Diff line number Diff line change
@@ -177,10 +177,6 @@ public String registerNewProtectedResource(@RequestBody String jsonString, Model
m.addAttribute(HttpCodeView.CODE, HttpStatus.CREATED); // http 201

return ClientInformationResponseView.VIEWNAME;
} catch (UnsupportedEncodingException e) {
logger.error("Unsupported encoding", e);
m.addAttribute(HttpCodeView.CODE, HttpStatus.INTERNAL_SERVER_ERROR);
return HttpCodeView.VIEWNAME;
} catch (IllegalArgumentException e) {
logger.error("Couldn't save client", e);

@@ -245,7 +241,7 @@ public String readResourceConfiguration(@PathVariable("id") String clientId, Mod
m.addAttribute(HttpCodeView.CODE, HttpStatus.OK); // http 200

return ClientInformationResponseView.VIEWNAME;
} catch (UnsupportedEncodingException e) {
} catch (IllegalArgumentException e) {
logger.error("Unsupported encoding", e);
m.addAttribute(HttpCodeView.CODE, HttpStatus.INTERNAL_SERVER_ERROR);
return HttpCodeView.VIEWNAME;
@@ -356,10 +352,6 @@ public String updateProtectedResource(@PathVariable("id") String clientId, @Requ
m.addAttribute(HttpCodeView.CODE, HttpStatus.OK); // http 200

return ClientInformationResponseView.VIEWNAME;
} catch (UnsupportedEncodingException e) {
logger.error("Unsupported encoding", e);
m.addAttribute(HttpCodeView.CODE, HttpStatus.INTERNAL_SERVER_ERROR);
return HttpCodeView.VIEWNAME;
} catch (IllegalArgumentException e) {
logger.error("Couldn't save client", e);

Original file line number Diff line number Diff line change
@@ -87,15 +87,6 @@ public void testResolveRedirect_safe() {

assertThat(res1, is(equalTo(goodUri)));

// set the resolver to non-strict and test the path-based redirect resolution

resolver.setStrictMatch(false);

String res2 = resolver.resolveRedirect(pathUri, client);

assertThat(res2, is(equalTo(pathUri)));


}

@Test(expected = InvalidRequestException.class)
@@ -106,52 +97,4 @@ public void testResolveRedirect_blacklisted() {

}

@Test
public void testRedirectMatches_default() {

// 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_nonstrict() {

// set the resolver to non-strict match mode
resolver.setStrictMatch(false);

// 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));

}

@Test
public void testHeartMode() {
when(config.isHeartMode()).thenReturn(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));
}

}
Original file line number Diff line number Diff line change
@@ -130,8 +130,7 @@ public class TestMITREidDataService_1_0 {

@Before
public void prepare() {
formatter = new DateFormatter();
formatter.setIso(ISO.DATE_TIME);
formatter = new DateFormatter(MITREidDataServiceSupport.DATE_TIME_ISO);
Mockito.reset(clientRepository, approvedSiteRepository, authHolderRepository, tokenRepository, sysScopeRepository, wlSiteRepository, blSiteRepository);
}

@@ -967,4 +966,4 @@ public void testExportDisabled() throws IOException {
dataService.exportData(writer);
}

}
}
Original file line number Diff line number Diff line change
@@ -128,8 +128,7 @@ public class TestMITREidDataService_1_1 {

@Before
public void prepare() {
formatter = new DateFormatter();
formatter.setIso(ISO.DATE_TIME);
formatter = new DateFormatter(MITREidDataServiceSupport.DATE_TIME_ISO);

Mockito.reset(clientRepository, approvedSiteRepository, authHolderRepository, tokenRepository, sysScopeRepository, wlSiteRepository, blSiteRepository);
}
Original file line number Diff line number Diff line change
@@ -131,8 +131,7 @@ public class TestMITREidDataService_1_2 {

@Before
public void prepare() {
formatter = new DateFormatter();
formatter.setIso(ISO.DATE_TIME);
formatter = new DateFormatter(MITREidDataServiceSupport.DATE_TIME_ISO);

Mockito.reset(clientRepository, approvedSiteRepository, authHolderRepository, tokenRepository, sysScopeRepository, wlSiteRepository, blSiteRepository);
}
Original file line number Diff line number Diff line change
@@ -142,8 +142,8 @@ public class TestMITREidDataService_1_3 {

@Before
public void prepare() {
formatter = new DateFormatter();
formatter.setIso(ISO.DATE_TIME);
formatter = new DateFormatter(MITREidDataServiceSupport.DATE_TIME_ISO);
//formatter.setIso(ISO.DATE_TIME);

Mockito.reset(clientRepository, approvedSiteRepository, authHolderRepository, tokenRepository, sysScopeRepository, wlSiteRepository, blSiteRepository);
}
252 changes: 152 additions & 100 deletions pom.xml

Large diffs are not rendered by default.