diff --git a/SpringSecurityRestGrailsPlugin.groovy b/SpringSecurityRestGrailsPlugin.groovy index 77aca449a..a0db3f7b8 100644 --- a/SpringSecurityRestGrailsPlugin.groovy +++ b/SpringSecurityRestGrailsPlugin.groovy @@ -5,6 +5,7 @@ import grails.plugin.springsecurity.rest.authentication.DefaultRestAuthenticatio import grails.plugin.springsecurity.rest.authentication.NullRestAuthenticationEventPublisher import grails.plugin.springsecurity.rest.credentials.DefaultJsonPayloadCredentialsExtractor import grails.plugin.springsecurity.rest.credentials.RequestParamsCredentialsExtractor +import grails.plugin.springsecurity.rest.error.DefaultCallbackErrorHandler import grails.plugin.springsecurity.rest.oauth.DefaultOauthUserDetailsService import grails.plugin.springsecurity.rest.token.bearer.BearerTokenAccessDeniedHandler import grails.plugin.springsecurity.rest.token.bearer.BearerTokenAuthenticationEntryPoint @@ -33,7 +34,7 @@ import javax.servlet.http.HttpServletResponse class SpringSecurityRestGrailsPlugin { - String version = "1.5.2" + String version = "1.5.3" String grailsVersion = "2.0 > *" List loadAfter = ['springSecurityCore'] List pluginExcludes = [ @@ -177,6 +178,8 @@ class SpringSecurityRestGrailsPlugin { /* tokenGenerator */ tokenGenerator(SecureRandomTokenGenerator) + callbackErrorHandler(DefaultCallbackErrorHandler) + /* tokenStorageService */ if (conf.rest.token.storage.useMemcached) { conf.rest.token.storage.useJwt = false diff --git a/grails-app/conf/BuildConfig.groovy b/grails-app/conf/BuildConfig.groovy index 588ee3e5a..c24cf991c 100644 --- a/grails-app/conf/BuildConfig.groovy +++ b/grails-app/conf/BuildConfig.groovy @@ -18,7 +18,9 @@ grails.project.dependency.resolution = { compile 'com.google.guava:guava-io:r03' compile 'org.pac4j:pac4j-core:1.6.0' compile 'org.pac4j:pac4j-oauth:1.6.0' - compile 'org.pac4j:pac4j-cas:1.6.0' + compile 'org.pac4j:pac4j-cas:1.6.0', { + exclude "bcprov-jdk15" + } compile 'com.nimbusds:nimbus-jose-jwt:3.9' build("com.lowagie:itext:2.0.8") { excludes "bouncycastle:bcprov-jdk14:138", "org.bouncycastle:bcprov-jdk14:1.38" } diff --git a/grails-app/controllers/grails/plugin/springsecurity/rest/RestOauthController.groovy b/grails-app/controllers/grails/plugin/springsecurity/rest/RestOauthController.groovy index 89263b6ac..66f8529da 100644 --- a/grails-app/controllers/grails/plugin/springsecurity/rest/RestOauthController.groovy +++ b/grails-app/controllers/grails/plugin/springsecurity/rest/RestOauthController.groovy @@ -17,6 +17,7 @@ package grails.plugin.springsecurity.rest import grails.plugin.springsecurity.annotation.Secured +import grails.plugin.springsecurity.rest.error.CallbackErrorHandler import grails.plugin.springsecurity.rest.token.AccessToken import grails.plugin.springsecurity.rest.token.rendering.AccessTokenJsonRenderer import grails.plugin.springsecurity.rest.token.storage.TokenStorageService @@ -39,6 +40,7 @@ class RestOauthController { final String CALLBACK_ATTR = "spring-security-rest-callback" + CallbackErrorHandler callbackErrorHandler RestOauthService restOauthService GrailsApplication grailsApplication @@ -89,36 +91,28 @@ class RestOauthController { try { String tokenValue = restOauthService.storeAuthentication(provider, context) - - if (session[CALLBACK_ATTR]) { - frontendCallbackUrl += tokenValue - session[CALLBACK_ATTR] = null - } else { - frontendCallbackUrl = frontendCallbackUrl.call(tokenValue) - } + frontendCallbackUrl = getCallbackUrl(frontendCallbackUrl, tokenValue) } catch (Exception e) { - String errorParams - - if (e instanceof UsernameNotFoundException) { - errorParams = "&error=403&message=${e.message?.encodeAsURL()?:''}" - } else { - errorParams = "&error=${e.cause?.code?:500}&message=${e.message?.encodeAsURL()?:''}" - } + def errorParams = new StringBuilder() - if (session[CALLBACK_ATTR]) { - frontendCallbackUrl += errorParams - session[CALLBACK_ATTR] = null - } else { - frontendCallbackUrl = frontendCallbackUrl.call(errorParams) + Map params = callbackErrorHandler.convert(e) + params.each { key, value -> + errorParams << "&${key}=${value.encodeAsURL()}" } + frontendCallbackUrl = getCallbackUrl(frontendCallbackUrl, errorParams.toString()) } log.debug "Redirecting to ${frontendCallbackUrl}" redirect url: frontendCallbackUrl } + private String getCallbackUrl(baseUrl, String queryStringSuffix) { + session[CALLBACK_ATTR] = null + baseUrl instanceof Closure ? baseUrl(queryStringSuffix) : baseUrl + queryStringSuffix + } + /** * Generates a new access token given the refresh token passed */ diff --git a/src/docs/guide/introduction.gdoc b/src/docs/guide/introduction.gdoc index c644ecd91..dfadb0e3a 100644 --- a/src/docs/guide/introduction.gdoc +++ b/src/docs/guide/introduction.gdoc @@ -36,6 +36,8 @@ h4. Release History You can view all releases at https://github.com/alvarosanchez/grails-spring-security-rest/releases. +* 20 November 2015 +** [1.5.3|https://github.com/alvarosanchez/grails-spring-security-rest/issues?q=milestone%3A1.5.3+is%3Aclosed] * 19 August 2015 ** [1.5.2|https://github.com/alvarosanchez/grails-spring-security-rest/issues?q=milestone%3A1.5.2+is%3Aclosed] * 6 May 2015 diff --git a/src/docs/guide/oauth.gdoc b/src/docs/guide/oauth.gdoc index 3f651178b..5ffebe18b 100644 --- a/src/docs/guide/oauth.gdoc +++ b/src/docs/guide/oauth.gdoc @@ -41,29 +41,40 @@ can use any OAuth 2.0 provider they support. This includes at the time of writin Note that OAuth 1.0a providers also work, like Twitter. +If your provider is not supported by pac4j, you can write your own. Please refer to the +[pac4j documentation|https://github.com/pac4j/pac4j/wiki/Clients#creating-your-own-client] for that. + The plugin also supports [CAS (Central Authentication Service)|https://www.apereo.org/projects/cas] using the OAuth -authentication flow. See [CAS Authentication|guide:oauth:cas] for details. +authentication flow. See [CAS Authentication|guide:cas] for details. To start the OAuth authentication flow, from your frontend application, generate a link to @/oauth/authenticate/@. The user clicking on that link represents step 4 in the previous diagram. Note that you can define the frontend callback URL in @Config.groovy@ under -@grails.plugin.springsecurity.rest.oauth.frontendCallbackUrl@. You need to define a closure that will be called with -the token value as parameter: +@grails.plugin.springsecurity.rest.oauth.frontendCallbackUrl@. {code} -grails.plugin.springsecurity.rest.oauth.frontendCallbackUrl = { String tokenValue -> "http://my.frontend-app.com/welcome#token=${tokenValue}" } +grails.plugin.springsecurity.rest.oauth.frontendCallbackUrl = "http://my.frontend-app.com/welcome#token=" {code} +The token will be concatenated to the end of the URL. + +{note} +For backwards compatibility with versions 1.5.2 and earlier, the callback URL can also be defined as a closure that will +be called with the token value as a parameter: + + {code} + grails.plugin.springsecurity.rest.oauth.frontendCallbackUrl = { String tokenValue -> "http://my.frontend-app.com/welcome#token=${tokenValue}" } + {code} +{note} + You can also define the URL as a @callback@ parameter in the original link, eg: {code} http://your-grails-api.com/oauth/authenticate/google?callback=http://your-frontend-app.com/auth-success.html?token= {code} -In this case, the token will be *concatenated* to the end of the URL. - Upon successful OAuth authorisation (after step 6.1 in the above diagram), an [OauthUser|http://alvarosanchez.github.io/grails-spring-security-rest/latest/docs/gapi/grails/plugin/springsecurity/rest/oauth/OauthUser.html] will be stored in the security context. This is done by a bean named @oauthUserDetailsService@. The @@ -127,12 +138,38 @@ OauthUser loadUserByUserProfile(OAuth20Profile userProfile, Collection + * + * 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 grails.plugin.springsecurity.rest.error + +interface CallbackErrorHandler { + + /** + * Converts an error that occurs during the callback to a parameter map that will be returned to the frontend + * @param cause + * @return + * + * @author Dónal Murtagh + */ + Map convert(Exception cause) +} \ No newline at end of file diff --git a/src/groovy/grails/plugin/springsecurity/rest/error/DefaultCallbackErrorHandler.groovy b/src/groovy/grails/plugin/springsecurity/rest/error/DefaultCallbackErrorHandler.groovy new file mode 100644 index 000000000..0d04339a7 --- /dev/null +++ b/src/groovy/grails/plugin/springsecurity/rest/error/DefaultCallbackErrorHandler.groovy @@ -0,0 +1,50 @@ +/* + * Copyright 2013-2015 Alvaro Sanchez-Mariscal + * + * 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 grails.plugin.springsecurity.rest.error + +import org.springframework.security.core.userdetails.UsernameNotFoundException + +import static org.springframework.http.HttpStatus.FORBIDDEN +import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR + +/** + * Error handler that's backwardly compatible with the behaviour that was embedded within the callback action of + * RestOauthController up to and including version 1.5.2 + * + * @author Dónal Murtagh + */ +class DefaultCallbackErrorHandler implements CallbackErrorHandler { + + @Override + Map convert(Exception e) { + + Map params = [:] + + if (e instanceof UsernameNotFoundException) { + params.error = FORBIDDEN.value() + + } else { + params.error = e.cause?.hasProperty('code') ? e.cause.code : INTERNAL_SERVER_ERROR.value() + } + + // Add the error message under the keys 'error_description' and 'message' - the former for compatibility with + // the RFC and the latter for backwards compatibility with plugin versions <= 1.5.2 + params.error_description = params.message = e.message ?: '' + params.error_code = e.cause ? e.cause.class.simpleName : e.class.simpleName + params + } +} diff --git a/src/groovy/grails/plugin/springsecurity/rest/token/bearer/BearerTokenReader.groovy b/src/groovy/grails/plugin/springsecurity/rest/token/bearer/BearerTokenReader.groovy index ad218c09d..ffb55adc0 100644 --- a/src/groovy/grails/plugin/springsecurity/rest/token/bearer/BearerTokenReader.groovy +++ b/src/groovy/grails/plugin/springsecurity/rest/token/bearer/BearerTokenReader.groovy @@ -40,11 +40,12 @@ class BearerTokenReader implements TokenReader { AccessToken findToken(HttpServletRequest request) { log.debug "Looking for bearer token in Authorization header, query string or Form-Encoded body parameter" String tokenValue = null + String header = request.getHeader('Authorization') - if (request.getHeader('Authorization')?.startsWith('Bearer')) { + if (header?.startsWith('Bearer') && header.length()>=8) { log.debug "Found bearer token in Authorization header" - tokenValue = request.getHeader('Authorization').substring(7) - } else if (isFormEncoded(request) && request.parts.size() <= 1 && !request.get) { + tokenValue = header.substring(7) + } else if (isFormEncoded(request) && !request.get) { log.debug "Found bearer token in request body" tokenValue = request.parameterMap['access_token']?.first() } else if (request.queryString?.contains('access_token')) { diff --git a/test/unit/grails/plugin/springsecurity/rest/RestOauthControllerSpec.groovy b/test/unit/grails/plugin/springsecurity/rest/RestOauthControllerSpec.groovy new file mode 100644 index 000000000..bc98a6c64 --- /dev/null +++ b/test/unit/grails/plugin/springsecurity/rest/RestOauthControllerSpec.groovy @@ -0,0 +1,134 @@ +/* + * Copyright 2013-2015 Alvaro Sanchez-Mariscal + * + * 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 grails.plugin.springsecurity.rest + +import grails.plugin.springsecurity.rest.error.DefaultCallbackErrorHandler +import grails.test.mixin.TestFor +import groovy.transform.InheritConstructors +import org.springframework.security.core.userdetails.UsernameNotFoundException +import spock.lang.Issue +import spock.lang.Specification + +import static org.springframework.http.HttpStatus.FORBIDDEN +import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR + +@Issue("https://github.com/alvarosanchez/grails-spring-security-rest/issues/237") +@TestFor(RestOauthController) +class RestOauthControllerSpec extends Specification { + + /** + * The frontend callback URL stored in config.groovy + */ + private frontendCallbackBaseUrl = "http://config.com/welcome#token=" + + def setup() { + grailsApplication.config.grails.plugin.springsecurity.rest.oauth.frontendCallbackUrl = { String tokenValue -> + frontendCallbackBaseUrl + tokenValue + } + + params.provider = "google" + } + + private Exception injectDependencies(Exception caughtException) { + def stubRestOauthService = Stub(RestOauthService) + stubRestOauthService.storeAuthentication(*_) >> { throw caughtException } + controller.restOauthService = stubRestOauthService + controller.callbackErrorHandler = new DefaultCallbackErrorHandler() + caughtException + } + + private String getExpectedUrl(Exception exception, expectedErrorValue, String baseUrl = frontendCallbackBaseUrl) { + def message = exception.message + "${baseUrl}&error=${expectedErrorValue}&message=${message}&error_description=${message}&error_code=${exception.class.simpleName}" + } + + def 'UsernameNotFoundException caught within callback action with frontend closure URL in Config.groovy'() { + + def caughtException = injectDependencies(new UsernameNotFoundException('message')) + + when: + controller.callback() + + then: + String expectedUrl = getExpectedUrl(caughtException, FORBIDDEN.value()) + response.redirectedUrl == expectedUrl + } + + def 'UsernameNotFoundException caught within callback action with frontend string URL in Config.groovy'() { + + def caughtException = injectDependencies(new UsernameNotFoundException('message')) + grailsApplication.config.grails.plugin.springsecurity.rest.oauth.frontendCallbackUrl = frontendCallbackBaseUrl + + when: + controller.callback() + + then: + String expectedUrl = getExpectedUrl(caughtException, FORBIDDEN.value()) + response.redirectedUrl == expectedUrl + } + + def 'UsernameNotFoundException caught within callback action with frontend URL in session'() { + + def caughtException = injectDependencies(new UsernameNotFoundException('message')) + def frontendCallbackBaseUrlSession = "http://session.com/welcome#token=" + request.session[controller.CALLBACK_ATTR] = frontendCallbackBaseUrlSession + + when: + controller.callback() + + then: + String expectedUrl = getExpectedUrl(caughtException, FORBIDDEN.value(), frontendCallbackBaseUrlSession) + response.redirectedUrl == expectedUrl + } + + def 'Non-UsernameNotFoundException with cause that has code caught within callback action'() { + + def caughtException = injectDependencies(new ExceptionWithCodedCause('message')) + + when: + controller.callback() + + then: + String expectedUrl = getExpectedUrl(caughtException, 'cause.code') + response.redirectedUrl == expectedUrl + } + + def 'Non-UsernameNotFoundException without cause caught within callback action'() { + + def caughtException = injectDependencies(new Exception('message')) + + when: + controller.callback() + + then: + String expectedUrl = getExpectedUrl(caughtException, INTERNAL_SERVER_ERROR.value()) + response.redirectedUrl == expectedUrl + } +} + +@InheritConstructors +class ExceptionWithCodedCause extends Exception { + + String getCode() { + 'cause.code' + } + + @Override + synchronized Throwable getCause() { + return this + } +} diff --git a/test/unit/grails/plugin/springsecurity/rest/rfc6750/BearerTokenReaderSpec.groovy b/test/unit/grails/plugin/springsecurity/rest/rfc6750/BearerTokenReaderSpec.groovy index 7826e7437..6f753ed64 100644 --- a/test/unit/grails/plugin/springsecurity/rest/rfc6750/BearerTokenReaderSpec.groovy +++ b/test/unit/grails/plugin/springsecurity/rest/rfc6750/BearerTokenReaderSpec.groovy @@ -21,6 +21,7 @@ import grails.plugin.springsecurity.rest.token.bearer.BearerTokenReader import org.codehaus.groovy.grails.plugins.testing.GrailsMockHttpServletRequest import org.springframework.http.MediaType import org.springframework.mock.web.MockHttpServletResponse +import spock.lang.Issue import spock.lang.Specification import spock.lang.Unroll @@ -138,4 +139,16 @@ class BearerTokenReaderSpec extends Specification { !tokenReader.findToken(request) } + @Issue("https://github.com/alvarosanchez/grails-spring-security-rest/issues/235") + def "it doesn't crash if token is missing"() { + given: + request.addHeader('Authorization', 'Bearer') + + when: + tokenReader.findToken(request) + + then: + notThrown(Throwable) + } + }