Skip to content
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

Pass OAuth 2 errors through to the app as flash[:google_sign_in_error] #31

Merged
merged 2 commits into from
Mar 6, 2019
Merged
Show file tree
Hide file tree
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
10 changes: 7 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ This gem provides a `google_sign_in_button` helper. It generates a button which
```

The `proceed_to` argument is required. After authenticating with Google, the gem redirects to `proceed_to`, providing
a Google ID token in `flash[:google_sign_in_token]`. Your application decides what to do with it:
a Google ID token in `flash[:google_sign_in_token]` or an [OAuth authorizaton code grant error](https://tools.ietf.org/html/rfc6749#section-4.1.2.1)
in `flash[:google_sign_in_error]`. Your application decides what to do with it:

```ruby
# config/routes.rb
Expand Down Expand Up @@ -108,8 +109,11 @@ class LoginsController < ApplicationController

private
def authenticate_with_google
if flash[:google_sign_in_token].present?
User.find_by google_id: GoogleSignIn::Identity.new(flash[:google_sign_in_token]).user_id
if id_token = flash[:google_sign_in_token]
User.find_by google_id: GoogleSignIn::Identity.new(id_token).user_id
elsif error = flash[:google_sign_in_error]
logger.error "Google authentication error: #{error}"
nil
end
end
end
Expand Down
26 changes: 17 additions & 9 deletions app/controllers/google_sign_in/callbacks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,34 @@

class GoogleSignIn::CallbacksController < GoogleSignIn::BaseController
def show
if valid_request?
redirect_to proceed_to_url, flash: { google_sign_in_token: id_token }
else
head :unprocessable_entity
end
redirect_to proceed_to_url, flash: google_sign_in_response
rescue GoogleSignIn::RedirectProtector::Violation => error
logger.error error.message
head :bad_request
end

private
def proceed_to_url
flash[:proceed_to].tap { |url| GoogleSignIn::RedirectProtector.ensure_same_origin(url, request.url) }
end

def google_sign_in_response
if valid_request? && params[:code].present?
{ google_sign_in_token: id_token }
else
{ google_sign_in_error: error_message }
end
end

def valid_request?
flash[:state].present? && params[:state] == flash[:state]
end

def proceed_to_url
flash[:proceed_to].tap { |url| GoogleSignIn::RedirectProtector.ensure_same_origin(url, request.url) }
def id_token
client.auth_code.get_token(params[:code])['id_token']
end

def id_token
client.auth_code.get_token(params.require(:code))['id_token']
def error_message
params[:error].presence_in(GoogleSignIn::OAUTH2_ERRORS) || "invalid_request"
end
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fall back to the invalid_request catch-all.

end
11 changes: 11 additions & 0 deletions lib/google_sign_in.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,17 @@
module GoogleSignIn
mattr_accessor :client_id
mattr_accessor :client_secret

# Authorization Code Grant errors: https://tools.ietf.org/html/rfc6749#section-4.1.2.1
OAUTH2_ERRORS = %w[
invalid_request
unauthorized_client
access_denied
unsupported_response_type
invalid_scope
server_error
temporarily_unavailable
]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could stash these away in e.g. lib/google_sign_in/errors.rb. Exposing so apps can also validate params[:error].presence_in(GoogleSignIn::OAUTH2_ERRORS)

end

require 'google_sign_in/identity'
Expand Down
48 changes: 45 additions & 3 deletions test/controllers/callbacks_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,49 @@ class GoogleSignIn::CallbacksControllerTest < ActionDispatch::IntegrationTest
get google_sign_in.callback_url(code: '4/SgCpHSVW5-Cy', state: flash[:state])
assert_redirected_to 'http://www.example.com/login'
assert_equal 'eyJhbGciOiJSUzI', flash[:google_sign_in_token]
assert_nil flash[:google_sign_in_error]
end

GoogleSignIn::OAUTH2_ERRORS.each do |error|
test "receiving an authorization error: #{error}" do
post google_sign_in.authorization_url, params: { proceed_to: 'http://www.example.com/login' }
assert_response :redirect

get google_sign_in.callback_url(error: error, state: flash[:state])
assert_redirected_to 'http://www.example.com/login'
assert_nil flash[:google_sign_in_token]
assert_equal error, flash[:google_sign_in_error]
end
end

test "receiving an invalid authorization error" do
post google_sign_in.authorization_url, params: { proceed_to: 'http://www.example.com/login' }
assert_response :redirect

get google_sign_in.callback_url(error: 'unknown error code', state: flash[:state])
assert_redirected_to 'http://www.example.com/login'
assert_nil flash[:google_sign_in_token]
assert_equal "invalid_request", flash[:google_sign_in_error]
end

test "receiving neither code nor error" do
post google_sign_in.authorization_url, params: { proceed_to: 'http://www.example.com/login' }
assert_response :redirect

get google_sign_in.callback_url(state: flash[:state])
assert_redirected_to 'http://www.example.com/login'
assert_nil flash[:google_sign_in_token]
assert_equal 'invalid_request', flash[:google_sign_in_error]
end

test "protecting against CSRF without flash state" do
post google_sign_in.authorization_url, params: { proceed_to: 'http://www.example.com/login' }
assert_response :redirect

get google_sign_in.callback_url(code: '4/SgCpHSVW5-Cy', state: 'invalid')
assert_response :unprocessable_entity
assert_redirected_to 'http://www.example.com/login'
assert_nil flash[:google_sign_in_token]
assert_equal 'invalid_request', flash[:google_sign_in_error]
end

test "protecting against CSRF with invalid state" do
Expand All @@ -23,7 +61,9 @@ class GoogleSignIn::CallbacksControllerTest < ActionDispatch::IntegrationTest
assert_not_nil flash[:state]

get google_sign_in.callback_url(code: '4/SgCpHSVW5-Cy', state: 'invalid')
assert_response :unprocessable_entity
assert_redirected_to 'http://www.example.com/login'
assert_nil flash[:google_sign_in_token]
assert_equal 'invalid_request', flash[:google_sign_in_error]
end

test "protecting against CSRF with missing state" do
Expand All @@ -32,7 +72,9 @@ class GoogleSignIn::CallbacksControllerTest < ActionDispatch::IntegrationTest
assert_not_nil flash[:state]

get google_sign_in.callback_url(code: '4/SgCpHSVW5-Cy')
assert_response :unprocessable_entity
assert_redirected_to 'http://www.example.com/login'
assert_nil flash[:google_sign_in_token]
assert_equal 'invalid_request', flash[:google_sign_in_error]
end

test "protecting against open redirects" do
Expand Down