From: Tom Clegg <tom@curii.com>
Date: Fri, 29 Sep 2023 14:11:44 +0000 (-0400)
Subject: 20300: Remove obsolete auth code.
X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/eba78b04f0313786bec3f6dfd4c992f69060a8a1

20300: Remove obsolete auth code.

The browser-facing parts are now handled by controller.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>
---

diff --git a/services/api/app/controllers/user_sessions_controller.rb b/services/api/app/controllers/user_sessions_controller.rb
index 1a9cc797fc..c2fc2e1a03 100644
--- a/services/api/app/controllers/user_sessions_controller.rb
+++ b/services/api/app/controllers/user_sessions_controller.rb
@@ -11,24 +11,29 @@ class UserSessionsController < ApplicationController
 
   respond_to :html
 
+  def login
+    return send_error "Legacy code path no longer supported", status: 404
+  end
+
   # create a new session
   def create
-    if !Rails.configuration.Login.LoginCluster.empty? and Rails.configuration.Login.LoginCluster != Rails.configuration.ClusterID
-      raise "Local login disabled when LoginCluster is set"
-    end
-
-    max_expires_at = nil
-    if params[:provider] == 'controller'
-      if request.headers['Authorization'] != 'Bearer ' + Rails.configuration.SystemRootToken
-        return send_error('Invalid authorization header', status: 401)
-      end
-      # arvados-controller verified the user and is passing auth_info
-      # in request params.
-      authinfo = SafeJSON.load(params[:auth_info])
-      max_expires_at = authinfo["expires_at"]
-    else
+    remote, return_to_url = params[:return_to].split(',', 2)
+    if params[:provider] != 'controller' ||
+       return_to_url != 'https://controller.api.client.invalid'
       return send_error "Legacy code path no longer supported", status: 404
     end
+    if request.headers['Authorization'] != 'Bearer ' + Rails.configuration.SystemRootToken
+      return send_error('Invalid authorization header', status: 401)
+    end
+    if remote == ''
+      remote = nil
+    elsif remote !~ /^[0-9a-z]{5}$/
+      return send_error 'Invalid remote cluster id', status: 400
+    end
+    # arvados-controller verified the user and is passing auth_info
+    # in request params.
+    authinfo = SafeJSON.load(params[:auth_info])
+    max_expires_at = authinfo["expires_at"]
 
     if !authinfo['user_uuid'].blank?
       user = User.find_by_uuid(authinfo['user_uuid'])
@@ -49,40 +54,13 @@ class UserSessionsController < ApplicationController
     # For the benefit of functional and integration tests:
     @user = user
 
-    if user.uuid[0..4] != Rails.configuration.ClusterID
-      # Actually a remote user
-      # Send them to their home cluster's login
-      rh = Rails.configuration.RemoteClusters[user.uuid[0..4]]
-      remote, return_to_url = params[:return_to].split(',', 2)
-      @remotehomeurl = "#{rh.Scheme || "https"}://#{rh.Host}/login?remote=#{Rails.configuration.ClusterID}&return_to=#{return_to_url}"
-      render
-      return
-    end
-
     # prevent ArvadosModel#before_create and _update from throwing
     # "unauthorized":
     Thread.current[:user] = user
 
     user.save or raise Exception.new(user.errors.messages)
 
-    # Give the authenticated user a cookie for direct API access
-    session[:user_id] = user.id
-    session[:api_client_uuid] = nil
-    session[:api_client_trusted] = true # full permission to see user's secrets
-
-    @redirect_to = root_path
-    if params.has_key?(:return_to)
-      # return_to param's format is 'remote,return_to_url'. This comes from login()
-      # encoding the remote=zbbbb parameter passed by a client asking for a salted
-      # token.
-      remote, return_to_url = params[:return_to].split(',', 2)
-      if remote !~ /^[0-9a-z]{5}$/ && remote != ""
-        return send_error 'Invalid remote cluster id', status: 400
-      end
-      remote = nil if remote == ''
-      return send_api_token_to(return_to_url, user, remote, max_expires_at)
-    end
-    redirect_to @redirect_to
+    return send_api_token_to(return_to_url, user, remote, max_expires_at)
   end
 
   # Omniauth failure callback
@@ -90,51 +68,6 @@ class UserSessionsController < ApplicationController
     flash[:notice] = params[:message]
   end
 
-  # logout - this gets intercepted by controller, so this is probably
-  # mostly dead code at this point.
-  def logout
-    session[:user_id] = nil
-
-    flash[:notice] = 'You have logged off'
-    return_to = params[:return_to] || root_url
-    redirect_to return_to
-  end
-
-  # login.  Redirect to LoginCluster.
-  def login
-    if params[:remote] !~ /^[0-9a-z]{5}$/ && !params[:remote].nil?
-      return send_error 'Invalid remote cluster id', status: 400
-    end
-    if current_user && params[:return_to] == "https://controller.api.client.invalid"
-      # Already logged in; just need to send a token to the requesting
-      # API client. Note, although this response looks like it's meant
-      # to be sent to a web browser, in fact the only supported use
-      # case is where our client is arvados-controller, giving us the
-      # placeholder URL https://controller.api.client.invalid.
-      return send_api_token_to(params[:return_to], current_user, params[:remote])
-    end
-    p = []
-    p << "auth_provider=#{CGI.escape(params[:auth_provider])}" if params[:auth_provider]
-
-    if !Rails.configuration.Login.LoginCluster.empty? and Rails.configuration.Login.LoginCluster != Rails.configuration.ClusterID
-      host = ApiClientAuthorization.remote_host(uuid_prefix: Rails.configuration.Login.LoginCluster)
-      if not host
-        raise "LoginCluster #{Rails.configuration.Login.LoginCluster} missing from RemoteClusters"
-      end
-      scheme = "https"
-      cluster = Rails.configuration.RemoteClusters[Rails.configuration.Login.LoginCluster]
-      if cluster and cluster['Scheme'] and !cluster['Scheme'].empty?
-        scheme = cluster['Scheme']
-      end
-      login_cluster = "#{scheme}://#{host}"
-      p << "remote=#{CGI.escape(params[:remote])}" if params[:remote]
-      p << "return_to=#{CGI.escape(params[:return_to])}" if params[:return_to]
-      redirect_to "#{login_cluster}/login?#{p.join('&')}"
-    else
-      return send_error "Legacy code path no longer supported", status: 404
-    end
-  end
-
   def send_api_token_to(callback_url, user, remote=nil, token_expiration=nil)
     # Give the API client a token for making API calls on behalf of
     # the authenticated user
diff --git a/services/api/test/functional/user_sessions_controller_test.rb b/services/api/test/functional/user_sessions_controller_test.rb
index 66aff787bd..cf4c6e8b4d 100644
--- a/services/api/test/functional/user_sessions_controller_test.rb
+++ b/services/api/test/functional/user_sessions_controller_test.rb
@@ -6,124 +6,30 @@ require 'test_helper'
 
 class UserSessionsControllerTest < ActionController::TestCase
 
-  test "redirect to joshid" do
-    api_client_page = 'http://client.example.com/home'
-    get :login, params: {return_to: api_client_page}
-    # Not supported any more
-    assert_response 404
-  end
-
-  test "send token when user is already logged in" do
-    authorize_with :inactive
-    api_client_page = 'http://client.example.com/home'
-    get :login, params: {return_to: api_client_page}
-    assert_response :redirect
-    assert_equal(0, @response.redirect_url.index(api_client_page + '?'),
-                 'Redirect url ' + @response.redirect_url +
-                 ' should start with ' + api_client_page + '?')
-    assert_not_nil assigns(:api_client)
-  end
-
-  test "login creates token without expiration by default" do
-    assert_equal Rails.configuration.Login.TokenLifetime, 0
-    authorize_with :inactive
-    api_client_page = 'http://client.example.com/home'
-    get :login, params: {return_to: api_client_page}
-    assert_response :redirect
-    assert_not_nil assigns(:api_client)
-    assert_nil assigns(:api_client_auth).expires_at
-  end
-
-  test "login creates token with configured lifetime" do
-    token_lifetime = 1.hour
-    Rails.configuration.Login.TokenLifetime = token_lifetime
-    authorize_with :inactive
-    api_client_page = 'http://client.example.com/home'
-    get :login, params: {return_to: api_client_page}
-    assert_response :redirect
-    assert_not_nil assigns(:api_client)
-    api_client_auth = assigns(:api_client_auth)
-    assert_in_delta(api_client_auth.expires_at,
-                    api_client_auth.updated_at + token_lifetime,
-                    1.second)
-  end
-
-  [[0, 1.hour, 1.hour],
-  [1.hour, 2.hour, 1.hour],
-  [2.hour, 1.hour, 1.hour],
-  [2.hour, nil, 2.hour],
-  ].each do |config_lifetime, request_lifetime, expect_lifetime|
-    test "login with TokenLifetime=#{config_lifetime} and request has expires_at=#{ request_lifetime.nil? ? "nil" : request_lifetime }" do
-      Rails.configuration.Login.TokenLifetime = config_lifetime
-      expected_expiration_time =  Time.now() + expect_lifetime
-      authorize_with :inactive
-      @request.headers['Authorization'] = 'Bearer '+Rails.configuration.SystemRootToken
-      if request_lifetime.nil?
-        get :create, params: {provider: 'controller', auth_info: {email: "foo@bar.com"}, return_to: ',https://app.example'}
-      else
-        get :create, params: {provider: 'controller', auth_info: {email: "foo@bar.com", expires_at: Time.now() + request_lifetime}, return_to: ',https://app.example'}
-      end
-      assert_response :redirect
-      api_client_auth = assigns(:api_client_auth)
-      assert_not_nil api_client_auth
-      assert_not_nil assigns(:api_client)
-      assert_in_delta(api_client_auth.expires_at,
-                      expected_expiration_time,
-                      1.second)
-    end
-  end
-
-  test "login with remote param returns a salted token" do
-    authorize_with :inactive
-    api_client_page = 'http://client.example.com/home'
-    remote_prefix = 'zbbbb'
-    get :login, params: {return_to: api_client_page, remote: remote_prefix}
-    assert_response :redirect
-    api_client_auth = assigns(:api_client_auth)
-    assert_not_nil api_client_auth
-    assert_includes(@response.redirect_url, 'api_token='+api_client_auth.salted_token(remote: remote_prefix))
+  setup do
+    @allowed_return_to = ",https://controller.api.client.invalid"
   end
 
-  test "login with malformed remote param returns an error" do
-    authorize_with :inactive
-    api_client_page = 'http://client.example.com/home'
-    remote_prefix = 'invalid_cluster_id'
-    get :login, params: {return_to: api_client_page, remote: remote_prefix}
-    assert_response 400
-  end
-
-  test "login to LoginCluster" do
-    Rails.configuration.Login.LoginCluster = 'zbbbb'
-    Rails.configuration.RemoteClusters['zbbbb'] = ConfigLoader.to_OrderedOptions({'Host' => 'zbbbb.example.com'})
-    api_client_page = 'http://client.example.com/home'
-    get :login, params: {return_to: api_client_page}
-    assert_response :redirect
-    assert_equal("https://zbbbb.example.com/login?return_to=http%3A%2F%2Fclient.example.com%2Fhome", @response.redirect_url)
-    assert_nil assigns(:api_client)
-  end
-
-  test "don't go into redirect loop if LoginCluster is self" do
-    Rails.configuration.Login.LoginCluster = 'zzzzz'
-    api_client_page = 'http://client.example.com/home'
-    get :login, params: {return_to: api_client_page}
-    # Doesn't redirect, just fail.
+  test "login route deleted" do
+    @request.headers['Authorization'] = 'Bearer '+Rails.configuration.SystemRootToken
+    get :login, params: {provider: 'controller', return_to: @allowed_return_to}
     assert_response 404
   end
 
   test "controller cannot create session without SystemRootToken" do
-    get :create, params: {provider: 'controller', auth_info: {email: "foo@bar.com"}, return_to: ',https://app.example'}
+    get :create, params: {provider: 'controller', auth_info: {email: "foo@bar.com"}, return_to: @allowed_return_to}
     assert_response 401
   end
 
   test "controller cannot create session with wrong SystemRootToken" do
     @request.headers['Authorization'] = 'Bearer blah'
-    get :create, params: {provider: 'controller', auth_info: {email: "foo@bar.com"}, return_to: ',https://app.example'}
+    get :create, params: {provider: 'controller', auth_info: {email: "foo@bar.com"}, return_to: @allowed_return_to}
     assert_response 401
   end
 
   test "controller can create session using SystemRootToken" do
     @request.headers['Authorization'] = 'Bearer '+Rails.configuration.SystemRootToken
-    get :create, params: {provider: 'controller', auth_info: {email: "foo@bar.com"}, return_to: ',https://app.example'}
+    get :create, params: {provider: 'controller', auth_info: {email: "foo@bar.com"}, return_to: @allowed_return_to}
     assert_response :redirect
     api_client_auth = assigns(:api_client_auth)
     assert_not_nil api_client_auth
diff --git a/services/api/test/integration/user_sessions_test.rb b/services/api/test/integration/user_sessions_test.rb
index 76659f3207..eb49cf832e 100644
--- a/services/api/test/integration/user_sessions_test.rb
+++ b/services/api/test/integration/user_sessions_test.rb
@@ -8,7 +8,7 @@ class UserSessionsApiTest < ActionDispatch::IntegrationTest
   # remote prefix & return url packed into the return_to param passed around
   # between API and SSO provider.
   def client_url(remote: nil)
-    url = ',https://wb.example.com'
+    url = ',https://controller.api.client.invalid'
     url = "#{remote}#{url}" unless remote.nil?
     url
   end