mitreid-connect/OpenID-Connect-Java-Spring-Server — pull request #680 by rafaelweingartner (2014-09-26 17:42:18+00:00)
Title: Small enhancements into client libraries.

Hi,
I am a master student working with privacy into federated identity management systems. My interns and I have been working with your implementation of OpenId Connect protocol, and we have made some changes that we would like you to consider.

We have created a class called “org.mitre.openid.connect.utils.UrlUtils” that has a method “normalizeIssuerURL” which normalizes a given issuer URL with HTTPS schema and ending slash (/).
We felt the need for such method when we used your implementation with Google´s IdP, which claims to have implemented the OpenId Connect Protocol. However we found some discrepancies, such as:
In the process of dynamic server configuration – the issuer field that was supposed to be returned by the IdP was not coming nicely formatted (it comes without HTTPs schema), therefore, we changed methods “org.mitre.openid.connect.client.service.impl.DynamicServerConfigurationService.OpenIDConnectServiceConfigurationFetcher.load(String)”
to use the UrlUtils. normalizeIssuerURL() before the validation that is performed at the line 157.
The above problem was also found on “org.mitre.openid.connect.client.OIDCAuthenticationFilter.handleAuthorizationCodeResponse(HttpServletRequest, HttpServletResponse)” at line 503.
We know that the specs are clear, and that the issuer value should be nicely formatted as can be seen here (http://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata), however, we think that it would not be a pain to add those modifications into your framework which would make the integration with IdPs such as the Googles’ easier.

We also noticed that you always use the parameter nonce when creating an authentication request. However, the specs say that the nonce value is optional, therefore we think that it would be nice to give that opportunity for developer to decide if they want to either use it or not, for instance Googles´ IdP does not use nonce values.
We added a property called useNonce of type boolean into classes:
“org.mitre.openid.connect.client.service.impl.PlainAuthRequestUrlBuilder” and
“org.mitre.openid.connect.client.OIDCAuthenticationFilter”
Then we changed methods:
“org.mitre.openid.connect.client.OIDCAuthenticationFilter.handleAuthorizationCodeResponse(HttpServletRequest, HttpServletResponse)” at lines 547 and 555 to use the newly introduced property that indicates whether or not to use the nonce value.
The same was done in:
“org.mitre.openid.connect.client.service.impl.PlainAuthRequestUrlBuilder.buildAuthRequestUrl(ServerConfiguration, RegisteredClient, String, String, String, Map)” at lines 66 in which we used the useNonce property to decide whether or not to use the nonce value.

In short, your implementation is pretty neat and well documented; we are enjoying working with it. Thus, we would like you to consider ours changes; hence, they are small punctual changes that enhance your framework.