Permalink
Browse files

A more verbose error message if your session isn't the one you expect…

… (different initiation and callback domain)
  • Loading branch information...
1 parent babcb5e commit 90f6f047a1a3d28dad8cbf24fc639cb37baff041 Antony Jones committed May 3, 2012
View
@@ -3,8 +3,10 @@
.classpath
.project
settings.xml
+.settings
*.iml
*.iws
+*.ipr
plugin.xml
*.zip
target
@@ -1,6 +1,6 @@
class OauthGrailsPlugin {
- def version = "2.0.1"
+ def version = "2.0.2"
def grailsVersion = "1.3.1 > *"
View
@@ -1 +1 @@
-This file was created by IntelliJ IDEA (Nika) IU-108.1333 for binding GitHub repository
+Please see the documentation at http://aiten.github.com/grails-oauth-scribe/ for information on using this plugin.
@@ -3,6 +3,7 @@ package uk.co.desirableobjects.oauth.scribe
import org.scribe.model.Verifier
import org.scribe.model.Token
import org.codehaus.groovy.grails.web.servlet.mvc.GrailsParameterMap
+import uk.co.desirableobjects.oauth.scribe.exception.MissingRequestTokenException
class OauthController {
@@ -22,6 +23,11 @@ class OauthController {
}
Token requestToken = (Token) session[oauthService.findSessionKeyForRequestToken(providerName)]
+
+ if (!requestToken) {
+ throw new MissingRequestTokenException(providerName)
+ }
@pledbrook
pledbrook Jul 31, 2012 Contributor

This breaks on Oauth 2 because the request token is null!

@antony
antony Jul 31, 2012 Owner

What version of OAuth does Facebook use? Thought I had this working with Oauth2 previously. I think we could change this to only happen if the provider was an OAuth1a provider.

@pledbrook
pledbrook Jul 31, 2012 Contributor

Facebook is OAuth 2 (or at least some draft thereof). But you only have to look at the code: the authenticate action only puts a non-null request token in the session if the provider supports OAuth 1. The callback then assumes that a non-null token is in the session. Perhaps it would be better to check that the callback domain and port match those in grails.serverURL config option? Or request.forwardURI if grails.serverURL isn't set.

@antony
antony Jul 31, 2012 Owner

Even if the callback domain and port are present, you can still visit the app locally on any url which maps to the container (say 127.0.0.1) and when your callback is hit (http://localhost.mycompany.com) you will have a different session - that is the problem I'm trying to solve here.

I guess when I tested this against a provider, my provider wasn't version 2, or at least the version of version 2 I expected! Perhaps for Oauth2 we could check if the verifier is present?

@pledbrook
pledbrook Jul 31, 2012 Contributor

I suggest creating a proper empty Token and storing that in the session. That's what I have done locally. Similar to Collections.EMPTY_LIST, etc.

@antony
antony Jul 31, 2012 Owner

That seems sensible. What provider are you testing this against? I should probably also test the plugin against that provider when I complete this work.

@pledbrook
pledbrook Jul 31, 2012 Contributor

Cloud Foundry UAA (User authentication and authorization service) :)

+
Token accessToken = oauthService.getAccessToken(providerName, requestToken, verifier)
session[oauthService.findSessionKeyForAccessToken(providerName)] = accessToken
@@ -32,11 +38,8 @@ class OauthController {
}
private Verifier extractVerifier(OauthProvider provider, GrailsParameterMap params) {
-
- String verifierKey = 'oauth_verifier'
- if (SupportedOauthVersion.TWO == provider.oauthVersion) {
- verifierKey = 'code'
- }
+
+ String verifierKey = determineVerifierKey(provider)
if (!params[verifierKey]) {
log.error("Cannot authenticate with oauth: Could not find oauth verifier in ${params}.")
@@ -48,6 +51,12 @@ class OauthController {
}
+ private String determineVerifierKey(OauthProvider provider) {
+
+ return SupportedOauthVersion.TWO == provider.oauthVersion ? 'code' : 'oauth_verifier'
+
+ }
+
def authenticate = {
String providerName = params.provider
@@ -0,0 +1,11 @@
+package uk.co.desirableobjects.oauth.scribe.exception
+
+class MissingRequestTokenException extends RuntimeException {
+
+ MissingRequestTokenException(String providerName) {
+
+ super("We couldn't find a request token for ${providerName} in the session. A common cause of this is that you have been given a new session by the servlet container because your callback domain is different to the domain you are authenticating from. Check that the domain name in the URL bar of your browser matches the domain name of your callback URL")
+
+ }
+
+}
@@ -7,8 +7,8 @@ import org.scribe.oauth.OAuthService
import grails.test.mixin.TestFor
import spock.lang.Specification
import spock.lang.Shared
+import uk.co.desirableobjects.oauth.scribe.exception.MissingRequestTokenException
-//@Mixin(GMockAddon)
@TestFor(OauthController)
class OauthControllerSpec extends Specification {
@@ -137,7 +137,24 @@ class OauthControllerSpec extends Specification {
}
- def 'Default success and failure URLs are sane'() {
+ def 'Oauth callback is hit but there is no request token in the session (bad callback domain)'() {
+
+ given:
+ controller.params.provider = PROVIDER_NAME
+ controller.params.oauth_verifier = 'oauth-verifier'
+ controller.params.code = 'verifier-key'
+
+ when:
+ controller.callback()
+
+ then:
+ controller.oauthService.findSessionKeyForRequestToken(PROVIDER_NAME) >> { return REQUEST_TOKEN_SESSION_KEY }
+ controller.oauthService.findProviderConfiguration(PROVIDER_NAME) >> { return provider }
+ provider.service.version >> { return '2.0' }
+
+ and:
+ def exception = thrown MissingRequestTokenException
+ exception.message == "We couldn't find a request token for twitter in the session. A common cause of this is that you have been given a new session by the servlet container because your callback domain is different to the domain you are authenticating from. Check that the domain name in the URL bar of your browser matches the domain name of your callback URL"
}
@@ -10,6 +10,7 @@ import org.scribe.builder.api.TwitterApi
import spock.lang.Unroll
import org.scribe.builder.ServiceBuilder
+import org.codehaus.groovy.grails.web.servlet.mvc.exceptions.InvalidUriException
class OauthServiceSpec extends UnitSpec {
@@ -304,7 +305,6 @@ class OauthServiceSpec extends UnitSpec {
mockConfig '''
import org.scribe.builder.api.TwitterApi
- import org.scribe.model.SignatureType
oauth {
providers {
@@ -336,7 +336,6 @@ class OauthServiceSpec extends UnitSpec {
mockConfig '''
import org.scribe.builder.api.TwitterApi
- import org.scribe.model.SignatureType
oauth {
providers {
@@ -368,7 +367,6 @@ class OauthServiceSpec extends UnitSpec {
given:
mockConfig '''
import org.scribe.builder.api.TwitterApi
- import org.scribe.model.SignatureType
oauth {
providers {
@@ -398,7 +396,6 @@ class OauthServiceSpec extends UnitSpec {
given:
mockConfig """
- import org.scribe.model.SignatureType
oauth {
providers {

0 comments on commit 90f6f04

Please sign in to comment.