Skip to content

Commit

Permalink
Merge pull request #38838 from code-dot-org/revert-38245-jk-maker-app…
Browse files Browse the repository at this point in the history
…-oauth-failure

Revert "Gracefully handle Google OAuth failures from Maker App"
  • Loading branch information
Madelyn Kasula committed Feb 1, 2021
2 parents 862b985 + 3bc27a2 commit daba5fa
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 119 deletions.
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

0 comments on commit daba5fa

Please sign in to comment.