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

Revert "Gracefully handle Google OAuth failures from Maker App" #38838

Merged
merged 1 commit into from Feb 1, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 4 additions & 11 deletions dashboard/app/controllers/maker_controller.rb
Expand Up @@ -122,24 +122,17 @@ def login_code
# renders a page for users to copy and paste a login key
def display_code
# Generate encrypted code to display to user
user_auth = current_user&.authentication_options&.find_by_credential_type(AuthenticationOption::GOOGLE)
if user_auth
@secret_code = Encryption.encrypt_string_utf8(
Time.now.strftime('%Y%m%dT%H%M%S%z') + user_auth.authentication_id + user_auth.credential_type
)
end
user_auth = current_user.authentication_options.find_by_credential_type(AuthenticationOption::GOOGLE)
@secret_code = Encryption.encrypt_string_utf8(
Time.now.strftime('%Y%m%dT%H%M%S%z') + user_auth['authentication_id'] + user_auth['credential_type']
)
end

# GET /maker/confirm_login
# @param [String] params[:user_return_to] Where to redirect the user after they've logged in
# Renders a page to confirm uses want to login via Google OAuth
# This route is need to convert the GET request from the Maker App into
# a POST that can be used to login via Google OAuth
def confirm_login
session[:user_return_to] = params[:user_return_to]
if current_user
redirect_to session[:user_return_to]
end
end

# POST /maker/complete
Expand Down
2 changes: 0 additions & 2 deletions dashboard/app/controllers/omniauth_callbacks_controller.rb
Expand Up @@ -67,8 +67,6 @@ def google_oauth2

if user
sign_in_google_oauth2 user
elsif session[:user_return_to] == maker_display_google_oauth_code_path
redirect_to session[:user_return_to]
else
sign_up_google_oauth2
end
Expand Down
2 changes: 0 additions & 2 deletions dashboard/app/controllers/sessions_controller.rb
Expand Up @@ -8,8 +8,6 @@ class SessionsController < Devise::SessionsController
def new
session[:user_return_to] ||= params[:user_return_to]
if params[:maker]
# Deprecated: this was used by an earlier version of the maker app for google auth.
# Later versions send users directly to maker_google_oauth_confirm_login_path
redirect_to maker_google_oauth_confirm_login_path
return
end
Expand Down
22 changes: 10 additions & 12 deletions dashboard/app/views/maker/display_code.html.haml
@@ -1,16 +1,14 @@
%h1= I18n.t('maker.google_oauth.login_with_google')
- if @secret_code
%p= I18n.t('maker.google_oauth.copy_code')

= text_field_tag :maker_code, @secret_code, readonly: true
= button_tag I18n.t(:copy), id: "btn-copy"
%p= I18n.t('maker.google_oauth.copy_code')

:javascript
$(document).ready(function() {
$('#btn-copy').click(function() {
$('#maker_code').select();
document.execCommand("copy");
});
= text_field_tag :maker_code, @secret_code, readonly: true
= button_tag I18n.t(:copy), id: "btn-copy"

:javascript
$(document).ready(function() {
$('#btn-copy').click(function() {
$('#maker_code').select();
document.execCommand("copy");
});
- else
%p= I18n.t('maker.google_oauth.error_no_linked_google')
});
1 change: 0 additions & 1 deletion dashboard/config/locales/en.yml
Expand Up @@ -1156,7 +1156,6 @@ en:
copy_code: 'Please copy and paste this code into the Maker App to login to Code.org.'
error_token_expired: 'Token has expired. Please log in again for a new token.'
error_wrong_provider: 'Wrong provider. Please make sure you are logging in with Google.'
error_no_linked_google: 'Please go back to the Maker App and use the "Sign in" button in the top right to log in or click the back button to log in with a different Google account'
error_invalid_user: 'Invalid user id. Please try logging in again.'
error_no_code: 'Please enter your login code.'
confirm_login: 'Select "Confirm" to log in with your Google account.'
Expand Down
48 changes: 0 additions & 48 deletions dashboard/test/controllers/maker_controller_test.rb
Expand Up @@ -473,54 +473,6 @@ class MakerControllerTest < ActionController::TestCase
assert_equal 1, CircuitPlaygroundDiscountApplication.where(user_id: @teacher.id).length
end

test "confirm_login redirects to maker_display_google_oauth_code_path when a google user is signed in" do
@student = create :student, :google_sso_provider
CDO.stubs(:properties_encryption_key).returns(SecureRandom.base64(Encryption::KEY_LENGTH / 8))
sign_in @student

get :confirm_login, params: {user_return_to: '/maker/display_google_oauth_code'}
assert_redirected_to '/maker/display_google_oauth_code'
end

test "confirm_login redirects to maker_display_google_oauth_code_path when a non google user is signed in" do
sign_in @student

get :confirm_login, params: {user_return_to: '/maker/display_google_oauth_code'}

assert_redirected_to '/maker/display_google_oauth_code'
end

test "confirm_login does not redirect when no user is signed in" do
get :confirm_login, params: {user_return_to: '/maker/display_google_oauth_code'}
assert_response :success
assert_template :confirm_login
end

test "display_code shows a code when a google user is signed in" do
@student = create :student, :google_sso_provider
CDO.stubs(:properties_encryption_key).returns(SecureRandom.base64(Encryption::KEY_LENGTH / 8))
sign_in @student
get :display_code
assert_response :success
assert_select '#btn-copy'
assert_select '#maker_code' do
assert_select ":match('value', ?)", /.+/
end
end

test "display_code does not show a code when a non google user is signed in" do
sign_in @student
get :display_code
assert_response :success
assert_select '#btn-copy', false
end

test "display_code does not show a code when no user is signed in" do
get :display_code
assert_response :success
assert_select '#btn-copy', false
end

private

def ensure_script(script_name, version_year, is_stable=true)
Expand Down
43 changes: 0 additions & 43 deletions dashboard/test/controllers/omniauth_callbacks_controller_test.rb
Expand Up @@ -786,49 +786,6 @@ class OmniauthCallbacksControllerTest < ActionController::TestCase
assert_equal auth[:credentials][:refresh_token], partial_user.oauth_refresh_token
end

test 'google_oauth2: on successful google login, it redirects to user_return_to if it is present' do
# Given I have a Google-Code.org account
user = create :student, :google_sso_provider

# When I hit the google oauth callback
auth = generate_auth_user_hash \
provider: AuthenticationOption::GOOGLE,
uid: user.primary_contact_info.authentication_id
@request.env['omniauth.auth'] = auth
@request.env['omniauth.params'] = {}
session[:user_return_to] = "/maker/display_google_oauth_code"
CDO.stubs(:properties_encryption_key).returns("thisisafakekeyyyyyyyyyyyyyyyyyyyyy")
assert_does_not_create(User) do
get :google_oauth2
end

# Then I am signed in
user.reload
assert_equal user.id, signed_in_user_id
# Then I go to the display google oauth page to get my code
assert_redirected_to 'http://test.host/maker/display_google_oauth_code'
end

test 'google_oauth2: redirects to maker/display_google_oauth_code if present as user_return_to on unsuccessful google login' do
# Given I do not have a Code.org account
uid = "nonexistent-google-oauth2"

# When I hit the google oauth callback
auth = generate_auth_user_hash \
provider: AuthenticationOption::GOOGLE,
uid: uid,
user_type: '' # Google doesn't provider user_type
@request.env['omniauth.auth'] = auth
@request.env['omniauth.params'] = {}
session[:user_return_to] = "/maker/display_google_oauth_code"
assert_does_not_create(User) do
get :google_oauth2
end

# Then I go to the display google oauth page to see an error message
assert_redirected_to 'http://test.host/maker/display_google_oauth_code'
end

test 'login: google_oauth2 silently takes over unmigrated student with matching email' do
email = 'test@foo.xyz'
uid = '654321'
Expand Down