ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

pass through delegated auth redirect _after_ login, not before 

refs FOO-639

this also makes login trailer partials unnecessary

Change-Id: I93a1b04addb0d1d355661fd589fb43d5e374b7a5
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/247509
Reviewed-by: Simon Williams <simon@instructure.com>
Tested-by: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
This commit is contained in:
Cody Cutrer 2020-09-11 15:17:25 -06:00
parent 91398a26af
commit f106c87a68
16 changed files with 15 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
app/controllers/communication_channels_controller.rb
View File

@ -263,7 +263,7 @@ class CommunicationChannelsController < ApplicationController
@user.touch
flash[:notice] = t 'notices.registration_confirmed', "Registration confirmed!"
return respond_to do |format|
format.html { redirect_back_or_default(user_profile_url(@current_user)) }
format.html { redirect_to redirect_back_or_default(user_profile_url(@current_user)) }
format.json { render :json => cc.as_json(:except => [:confirmation_code] ) }
end
end
@ -476,7 +476,7 @@ class CommunicationChannelsController < ApplicationController
flash[:notice] = t 'notices.registration_confirmed', "Registration confirmed!"
@current_user ||= @user # since dashboard_url may need it
respond_to do |format|
format.html { @enrollment ? redirect_to(course_url(@course)) : redirect_back_or_default(dashboard_url) }
format.html { redirect_to(@enrollment ? course_url(@course) : redirect_back_or_default(dashboard_url)) }
format.json { render :json => {:url => @enrollment ? course_url(@course) : dashboard_url} }
end
end

2
app/controllers/login/cas_controller.rb
View File

@ -32,7 +32,7 @@ class Login::CasController < ApplicationController
# CAS sends a GET with a ticket when it's doing a login
return create if params[:ticket]
redirect_to delegated_auth_redirect_uri(client.add_service_to_login_url(cas_login_url))
redirect_to client.add_service_to_login_url(cas_login_url)
end
def create

2
app/controllers/login/oauth2_controller.rb
View File

@ -31,7 +31,7 @@ class Login::Oauth2Controller < Login::OauthBaseController
@aac.debug_set(:authorize_url, authorize_url)
end
redirect_to delegated_auth_redirect_uri(authorize_url)
redirect_to authorize_url
end
def create

2
app/controllers/login/oauth_controller.rb
View File

@ -29,7 +29,7 @@ class Login::OauthController < Login::OauthBaseController
}
opts = {}
opts[oauth_callback: callback_uri] unless request_token.callback_confirmed?
redirect_to delegated_auth_redirect_uri(request_token.authorize_url(opts))
redirect_to request_token.authorize_url(opts)
end
end

9
app/controllers/login/saml_controller.rb
View File

@ -26,9 +26,9 @@ class Login::SamlController < ApplicationController
before_action :fix_ms_office_redirects, only: :new
def new
redirect_to delegated_auth_redirect_uri(aac.generate_authn_request_redirect(host: request.host_with_port,
parent_registration: session[:parent_registration],
relay_state: Rails.env.development? && params[:RelayState]))
redirect_to aac.generate_authn_request_redirect(host: request.host_with_port,
parent_registration: session[:parent_registration],
relay_state: Rails.env.development? && params[:RelayState])
end
def create
@ -327,10 +327,9 @@ class Login::SamlController < ApplicationController
end
def observee_validation
redirect_to delegated_auth_redirect_uri(
redirect_to
@domain_root_account.parent_registration_aac.generate_authn_request_redirect(host: request.host_with_port,
parent_registration: session[:parent_registration])
)
end
protected

2
app/controllers/login/shared.rb
View File

@ -100,7 +100,7 @@ module Login::Shared
# assumed that if that URL is found rather than using the default,
# they must have cookies enabled and we don't need to worry about
# adding the :login_success param to it.
format.html { redirect_back_or_default(dashboard_url(:login_success => '1')) }
format.html { redirect_to delegated_auth_redirect_uri(redirect_back_or_default(dashboard_url(:login_success => '1'))) }
end
format.json { render :json => pseudonym.as_json(:methods => :user_code), :status => :ok }
end

3
app/views/login/canvas/mobile_login.html.erb
View File

@ -135,6 +135,3 @@
</div>
<% end %>
<% end %>
<%= # for plugin use
render "shared/login_trailer" %>

3
app/views/login/canvas/new.html.erb
View File

@ -27,9 +27,6 @@
<% add_body_class "full-width ic-Login-Body" %>
<%= render "login/canvas/new_login_content" %>
<%= # for plugin use
render "shared/login_trailer" %>
<% if flash[:logged_out] && HostUrl.file_host(@domain_root_account, request.host_with_port) %>
<img class="hidden-readable" src="//<%= HostUrl.file_host(@domain_root_account, request.host_with_port) %>/file_session/clear"/>
<% end %>

17
app/views/shared/_login_trailer.html.erb
View File

@ -1,17 +0,0 @@
<%
# Copyright (C) 2011 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
%>

17
app/views/users/_dashboard_trailer.html.erb
View File

@ -1,17 +0,0 @@
<%
# Copyright (C) 2020 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
%>

2
app/views/users/new.html.erb
View File

@ -61,5 +61,3 @@
</div>
</div>
</div>
<%= # for plugin use
render "shared/login_trailer" %>

3
app/views/users/user_dashboard.html.erb
View File

@ -55,6 +55,3 @@
<%= render :partial => 'shared/dashboard_card' %>
</div>
<%= # for plugin use
render "dashboard_trailer" %>

3
lib/authentication_methods.rb
View File

@ -329,8 +329,7 @@ module AuthenticationMethods
protected :store_location
def redirect_back_or_default(default)
redirect_to(session[:return_to] || default)
session.delete(:return_to)
session.delete(:return_to) || default
end
protected :redirect_back_or_default

2
spec/apis/auth_spec.rb
View File

@ -228,7 +228,7 @@ describe "API Authentication", type: :request do
follow_redirect!
expect(response).to redirect_to("/login/cas")
follow_redirect!
expect(response).to redirect_to(controller.delegated_auth_redirect_uri(cas.add_service_to_login_url(url_for(controller: 'login/cas', action: :new))))
expect(response).to redirect_to(cas.add_service_to_login_url(url_for(controller: 'login/cas', action: :new)))
get "/login/cas", params: {:ticket => 'ST-abcd'}
expect(response).to be_redirect

14
spec/controllers/login/oauth2_controller_spec.rb
View File

@ -23,13 +23,6 @@ describe Login::Oauth2Controller do
before do
aac
allow(Canvas::Plugin.find(:facebook)).to receive(:settings).and_return({})
# replace on just this instance. this allows the tests to look directly at
# response.location independent of any implementation plugins may add for
# this method.
def @controller.delegated_auth_redirect_uri(uri)
uri
end
end
describe "#new" do
@ -39,13 +32,6 @@ describe Login::Oauth2Controller do
expect(response.location).to match(%r{^https://www.facebook.com/dialog/oauth\?})
expect(session[:oauth2_nonce]).to_not be_blank
end
it "wraps redirect in delegated_auth_redirect_uri" do
# needs the `returns` or it returns nil and causes a 500
expect(@controller).to receive(:delegated_auth_redirect_uri).once.and_return('/')
get :new, params: {auth_type: 'facebook'}
expect(response).to be_redirect
end
end
describe "#create" do

6
spec/controllers/login/saml_controller_spec.rb
View File

@ -448,15 +448,15 @@ describe Login::SamlController do
it "should redirect to default login" do
get_new
expect(response.location.starts_with?(controller.delegated_auth_redirect_uri(@aac1.log_in_url))).to be_truthy
expect(response.location.starts_with?(@aac1.log_in_url)).to be_truthy
end
it "should use the specified AAC" do
get_new("#{@aac1.id}")
expect(response.location.starts_with?(controller.delegated_auth_redirect_uri(@aac1.log_in_url))).to be_truthy
expect(response.location.starts_with?(@aac1.log_in_url)).to be_truthy
controller.instance_variable_set(:@aac, nil)
get_new("#{@aac2.id}")
expect(response.location.starts_with?(controller.delegated_auth_redirect_uri(@aac2.log_in_url))).to be_truthy
expect(response.location.starts_with?(@aac2.log_in_url)).to be_truthy
end
it "reject unknown specified AAC" do