diff --git a/app/controllers/ajax_controller.rb b/app/controllers/ajax_controller.rb index 1f7157accd..0932892796 100644 --- a/app/controllers/ajax_controller.rb +++ b/app/controllers/ajax_controller.rb @@ -6,7 +6,7 @@ class AjaxController < ::ScopeController include Rescue - authentication_required domain: lambda { |c| + api_authentication_required domain: lambda { |c| c.instance_variable_get(:@scoped_domain_id) }, domain_name: lambda { |c| diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index d45751bca3..a8640cfdaa 100644 --- a/app/controllers/dashboard_controller.rb +++ b/app/controllers/dashboard_controller.rb @@ -1,139 +1,25 @@ -# frozen_string_literal: true - -# This class guarantees that the user is logged in and his token is rescoped. -# All subclasses which require a logged in user should inherit from this class. class DashboardController < ::ScopeController include UrlHelper include AvatarHelper include Rescue + prepend_before_action :define_after_login_url - prepend_before_action do - requested_url = request.env['REQUEST_URI'] - referer_url = request.referer - referer_url = - begin - "#{URI(referer_url).path}?#{URI(referer_url).query}" - rescue StandardError - nil - end + authentication_required( + domain: lambda { |c| c.instance_variable_get(:@scoped_domain_id)}, + domain_name: lambda { |c| c.instance_variable_get(:@scoped_domain_name)}, + project: lambda { |c| c.instance_variable_get(:@scoped_project_id)}, + two_factor: :two_factor_required? + ) - unless params[:after_login] - params[:after_login] = if requested_url =~ /(\?|&)modal=true/ && - referer_url =~ /(\?|&)overlay=.+/ - referer_url - else - requested_url - end - end - end - - before_action :load_help_text - - # authenticate user -> current_user is available - # throws only errors - # api_authentication_required domain: ->(c) { c.instance_variable_get(:@scoped_domain_id) }, - # domain_name: ->(c) { c.instance_variable_get(:@scoped_domain_name) }, - # project: ->(c) { c.instance_variable_get(:@scoped_project_id) }, - # rescope: false, - # two_factor: :two_factor_required?, - # except: :terms_of_use - - # with redirect - authentication_required domain: lambda { |c| - c.instance_variable_get(:@scoped_domain_id) - }, - domain_name: lambda { |c| - c.instance_variable_get(:@scoped_domain_name) - }, - project: lambda { |c| - c.instance_variable_get(:@scoped_project_id) - }, - rescope: false, - two_factor: :two_factor_required?, - except: :terms_of_use - - # after_login is used by monsoon_openstack_auth gem. - # After the authentication process has finished the - # after_login can be removed. - before_action { params.delete(:after_login) } - - # check if user has accepted terms of use. - # Otherwise it is a new, unboarded user. - before_action :check_terms_of_use, - except: %i[accept_terms_of_use terms_of_use] - # rescope token - before_action :rescope_token, except: [:terms_of_use] + before_action { params.delete(:after_login) } + before_action :check_terms_of_use, except: %i[accept_terms_of_use terms_of_use] before_action :raven_context, except: [:terms_of_use] - before_action :load_active_project, - :load_webcli_endpoint, - except: %i[terms_of_use] - before_action :set_mailer_host - - # this method checks if user has permissions for the new scope and if so - # it rescopes the token. - def rescope_token - if @scoped_project_id - # @scoped_project_id exists -> check if friendly id for this project - # also exists. The scope controller runs bevore this controller and - # updates the friendlyId entry if project exists. - unless FriendlyIdEntry.find_project(@scoped_domain_id, @scoped_project_id) - # friendly id entry is nil -> reset @can_access_project, render project - # not found page and return. - @can_access_project = false - return render(template: 'application/exceptions/project_not_found') - end + before_action :load_active_project, except: %i[terms_of_use] + before_action :set_mailer_host, except: %i[terms_of_use] - # NOTE: LEAVE this here because for better review - # we do not need extra permissions check for project and domains because elektron and monsoon_openstack_auth - # are doing the job. If the user has no access with his token monsoon_openstack_auth will trow an NotAuthorized - # error that we will catch and handle to show 'application/exceptions/unauthorized' - # - # if no access this is handled in rescue from above - # did not return -> check if user projects include the requested project. - # has_project_access = services.identity.has_project_access( - # @scoped_project_id - # ) - - # unless has_project_access - # # user has no permissions for requested project -> reset - # # @can_access_project, render unauthorized page and return. - # @can_access_project = false - # return render(template: 'application/exceptions/unauthorized') - # end - elsif @scoped_domain_id - # NOTE: LEAVE hit here because for better review - # @scoped_project_id is nil and @scoped_domain_id exists -> check if - # user can access the requested domain. - - # check if user has access to current domain, add rescue nil for cases where the token scope inexplicably contains a deleted project - # without the rescue this call leads to an error message and the user can't see the domain page - # has_domain_access = services.identity.has_domain_access(@scoped_domain_id) rescue nil - - # unless has_domain_access - # # this can happen if the user is using a link to some domain and project - # # user has no permissions for the new domain -> rescope to - # # unscoped token and return this will be the startpoint to rescope again - # return authentication_rescope_token(domain: nil, project: nil) - # end - else - # both @scoped_project_id and @scoped_domain_id are nil - # -> render unauthorized page and return. - @can_access_project = false - return render(template: 'application/exceptions/unauthorized') - end - # did not return yet -> rescope token to the 'new' scope. - begin - authentication_rescope_token - rescue MonsoonOpenstackAuth::Authentication::NotAuthorized => e - if e.message =~ /has no access to the requested scope/ - if @scoped_project_id.present? - render(template: 'application/exceptions/unauthorized') - elsif @scoped_domain_id.present? - authentication_rescope_token(domain: nil, project: nil) - end - end - # All other NotAuthorized Errors handled by "rescue_and_render_exception_page" - end + # In your Rescue module (application_controller.rb or concern) + rescue_from MonsoonOpenstackAuth::Authentication::NotAuthorized do |exception| + render template: 'application/exceptions/unauthorized', status: :unauthorized end def check_terms_of_use @@ -159,8 +45,6 @@ def accept_terms_of_use tou_version: Settings.send(@domain_config&.terms_of_use_name).version, domain_id: current_user.user_domain_id ) - - reset_last_request_cache # redirect to original path, this is the case after the TOU view if params[:orginal_url] redirect_to params[:orginal_url] @@ -186,25 +70,51 @@ def terms_of_use render action: :terms_of_use end - def two_factor_required? - if ENV['TWO_FACTOR_AUTH_DOMAINS'] - @two_factor_required = - ENV['TWO_FACTOR_AUTH_DOMAINS'] - .gsub(/\s+/, '') - .split(',') - .include?(@scoped_domain_name) - return @two_factor_required - end - false + private + + def load_active_project + return unless @scoped_project_id + + @active_project ||= + services.identity.find_project( + @scoped_project_id, + subtree_as_ids: true, + parents_as_ids: true + ) end - protected + def define_after_login_url + requested_url = request.env['REQUEST_URI'] + referer_url = request.referer + referer_url = + begin + "#{URI(referer_url).path}?#{URI(referer_url).query}" + rescue StandardError + nil + end + + unless params[:after_login] + params[:after_login] = if requested_url =~ /(\?|&)modal=true/ && + referer_url =~ /(\?|&)overlay=.+/ + referer_url + else + requested_url + end + end + end - def show_beta? - params[:betafeatures] == 'showme' + def tou_accepted? + UserProfile.tou_accepted?( + current_user.id, + current_user.user_domain_id, + Settings.send(@domain_config&.terms_of_use_name).version + ) end - helper_method :show_beta? + def set_mailer_host + ActionMailer::Base.default_url_options[:host] = request.host_with_port + ActionMailer::Base.default_url_options[:protocol] = request.protocol + end def raven_context @sentry_user_context = @@ -233,74 +143,7 @@ def raven_context end @sentry_tags_context = tags Raven.tags_context(tags) - end - - def load_active_project - return unless @scoped_project_id - - # load active project. Try first from ObjectCache and then from API - - cached_active_project = ObjectCache.where(id: @scoped_project_id).first - @active_project = if cached_active_project - Identity::Project.new(services.identity, cached_active_project.payload) - else - service_user.identity.find_project(@scoped_project_id) - end - - return if @active_project && @active_project.name == @scoped_project_name - - @active_project = - services.identity.find_project( - @scoped_project_id, - subtree_as_ids: true, - parents_as_ids: true - ) - FriendlyIdEntry.update_project_entry(@active_project) - end - - def load_webcli_endpoint - @webcli_endpoint = current_user.service_url('webcli') - end - - def tou_accepted? - # Consider that every plugin controller inhertis from dashboard controller - # and check_terms_of_use method is called on every request. - # In order to reduce api calls we cache the result of new_user? - # in the session for 5 minutes. - is_cache_expired = - current_user.id != session[:last_user_id] || - session[:last_request_timestamp].nil? || - (session[:last_request_timestamp] < Time.now - 5.minute) - if is_cache_expired - session[:last_request_timestamp] = Time.now - session[:last_user_id] = current_user.id - session[:tou_accepted] = UserProfile.tou_accepted?( - current_user.id, - current_user.user_domain_id, - Settings.send(@domain_config&.terms_of_use_name).version - ) - end - - session[:tou_accepted] - end - - def reset_last_request_cache - session[:last_request_timestamp] = nil - session[:last_user_id] = nil - end - - def set_mailer_host - ActionMailer::Base.default_url_options[:host] = request.host_with_port - ActionMailer::Base.default_url_options[:protocol] = request.protocol - end - - def project_id_required - - return unless params[:project_id].blank? - - raise Core::Error::ProjectNotFound, - 'The project you have requested was not found.' - end + end def load_help_text # Different types of help files are supported: @@ -372,4 +215,4 @@ def load_help_text @plugin_help_links = @plugin_help_links.gsub('#{@sap_docu_url}', sap_url_for('documentation')) end end -end +end \ No newline at end of file diff --git a/app/controllers/scope_controller.rb b/app/controllers/scope_controller.rb index 8ec564a6c9..3463693082 100644 --- a/app/controllers/scope_controller.rb +++ b/app/controllers/scope_controller.rb @@ -22,17 +22,15 @@ def hidden_plugin_redirect def load_scoped_objects # initialize scoped domain's and project's friendly id # use existing, user's or default domain - domain_id = - params[:domain_id] || Rails.application.config.service_user_domain_name + domain_id = params[:domain_id] #|| Rails.application.config.service_user_domain_name project_id = params[:project_id] - + @scoped_domain_fid = @scoped_domain_id = domain_id @scoped_project_fid = @scoped_project_id = project_id - + # try to find or create friendly_id entry for domain rescoping_service = Dashboard::RescopingService.new(service_user) - domain_friendly_id = - rescoping_service.domain_friendly_id(@scoped_domain_fid) + domain_friendly_id = rescoping_service.domain_friendly_id(@scoped_domain_fid) raise Core::Error::DomainNotFound, "Domain #{domain_id} not found!" unless domain_friendly_id # set scoped domain parameters @@ -56,10 +54,6 @@ def load_scoped_objects @scoped_project_name = project_friendly_id.name end - # puts "SCOPED CONTROLLER" - # puts @scoped_domain_id - # puts @scoped_project_id - # url_for does not work for plugins. Use path instead! if (domain_id != @scoped_domain_fid || project_id != @scoped_project_fid) && @scoped_domain_id # replace domain_id with domain friendly id diff --git a/plugins/compute/app/controllers/compute/instances_controller.rb b/plugins/compute/app/controllers/compute/instances_controller.rb index 863816c238..a6d5117d90 100644 --- a/plugins/compute/app/controllers/compute/instances_controller.rb +++ b/plugins/compute/app/controllers/compute/instances_controller.rb @@ -748,14 +748,6 @@ def hard_reset execute_instance_action("reboot", "HARD") end - def two_factor_required? - if action_name == "console" - true - else - super - end - end - def edit_securitygroups @action_from_show = params[:action_from_show] == "true" || false @instance = services.compute.find_server(params[:id]) diff --git a/plugins/identity/app/controllers/identity/projects_controller.rb b/plugins/identity/app/controllers/identity/projects_controller.rb index 9a5077b31a..32f294ec97 100644 --- a/plugins/identity/app/controllers/identity/projects_controller.rb +++ b/plugins/identity/app/controllers/identity/projects_controller.rb @@ -288,6 +288,14 @@ def delete_with_prodel private + def project_id_required + + return unless params[:project_id].blank? + + raise Core::Error::ProjectNotFound, + 'The project you have requested was not found.' + end + def get_project get_project_id @project = diff --git a/spec/controllers/dashboard_controller_spec.rb b/spec/controllers/dashboard_controller_spec.rb index 034a501f55..5a133fe9ca 100644 --- a/spec/controllers/dashboard_controller_spec.rb +++ b/spec/controllers/dashboard_controller_spec.rb @@ -1,17 +1,28 @@ require "spec_helper" describe DashboardController, type: :controller do - controller do - def index - head :ok - end - end - default_params = { domain_id: AuthenticationStub.domain_id, project_id: AuthenticationStub.project_id, } - + + let(:current_user) do + double("User", + id: "user123", + name: "testuser", + email: "test@example.com", + full_name: "Test User", + user_domain_id: default_params[:domain_id], + user_domain_name: "default", + domain_id: default_params[:domain_id], + domain_name: "default", + project_id: default_params[:project_id], + project_name: "test-project", + project_domain_id: default_params[:domain_id], + project_domain_name: "default" + ) + end + before(:all) do FriendlyIdEntry.find_or_create_entry( "Domain", @@ -26,136 +37,179 @@ def index default_params[:project_id], ) end - - before :each do + + before(:each) do stub_authentication + allow(Settings).to receive_message_chain(:terms_of_use, :version).and_return("1.0") + allow_any_instance_of(DomainConfig).to receive(:terms_of_use_name).and_return(:terms_of_use) + allow(controller).to receive(:current_user).and_return(current_user) end - - describe "GET 'index'" do - it "returns http success" do - get :index, params: { domain_id: AuthenticationStub.domain_id } - expect(response).to be_successful + + describe "exception handling" do + before do + allow(UserProfile).to receive(:tou_accepted?).and_return(true) end - end - - describe "rescope_token" do - context "project id is provided" do - context "and user has access to project" do - before :each do - allow(controller.services.identity).to receive( - :has_project_access, - ).with(default_params[:project_id]).and_return(true) - end - - it "should rescope" do - expect(controller).to receive(:authentication_rescope_token) - get :index, params: default_params + + context "when NotAuthorized exception is raised" do + it "renders unauthorized template" do + p "==========================" + pp controller.class + pp controller.class.methods.sort + allow(controller.class).to receive(:authentication_required) do + p ":::::::::::::::::::::::::" + raise MonsoonOpenstackAuth::Authentication::NotAuthorized.new("insufficient permissions") end + + # Use an actual action that exists in DashboardController + get :terms_of_use, params: { domain_id: default_params[:domain_id] } + expect(response).to have_http_status(:unauthorized) + expect(response).to render_template('application/exceptions/unauthorized') end - - # NOTE: that is not working because we do not check auth/project explicitly anymore. - #context 'and user has no access to project' do - # before :each do - # allow(controller.services.identity) - # .to receive(:has_project_access) - # .with(default_params[:project_id]) - # .and_return(false) - # end - - # it 'should render unauthorized page' do - # get :index, params: default_params - # expect(response).to render_template('application/exceptions/unauthorized') - # end - #end - - context "and project does not exists" do - before :each do - allow_any_instance_of(Dashboard::RescopingService).to( - receive(:project_friendly_id).with( - default_params[:domain_id], - "BAD_PROJECT", - ).and_return(nil), - ) - end - it "should render project not found page" do - get :index, - params: { - domain_id: default_params[:domain_id], - project_id: "BAD_PROJECT", - } - expect(response).to render_template( - "application/exceptions/project_not_found", - ) - end + end + end + + describe "terms of use handling" do + context "when terms not accepted" do + before do + allow(UserProfile).to receive(:tou_accepted?).and_return(false) + allow_any_instance_of(DomainConfig).to receive(:feature_hidden?) + .with('terms_of_use').and_return(false) end - - context "and user has no access to the requested domain" do - before :each do - allow(Rails.cache).to receive(:fetch).with( - "user_domain_role_assignments/#{AuthenticationStub.test_token["user"]["id"]}/#{default_params[:domain_id]}", - anything, - ).and_return false - end - - it "should return with ok header" do - get :index, params: default_params - expect(response).to have_http_status(200) - end - - it "should not render unauthorized template" do - get :index, params: default_params - expect(response).not_to render_template( - "application/exceptions/unauthorized", - ) - end + + it "renders accept_terms_of_use template when accessing accept_terms_of_use" do + get :accept_terms_of_use, params: default_params + expect(response).to render_template(:accept_terms_of_use) + end + + it "stores the original URL" do + get :accept_terms_of_use, params: default_params + expect(assigns(:orginal_url)).to be_present end end - - context "project id is nil and domain id is provided" do - context "and user has access to domain" do - before :each do - allow(controller.services.identity).to receive( - :has_domain_access, - ).with(default_params[:domain_id]).and_return(true) - end - - it "should return with ok header" do - get :index, params: { domain_id: default_params[:domain_id] } - expect(response).to have_http_status(200) - end - - it "should render the domain page" do - expect(controller).to receive(:authentication_rescope_token) - get :index, params: { domain_id: default_params[:domain_id] } - end + + context "when terms of use feature is hidden for domain" do + before do + allow_any_instance_of(DomainConfig).to receive(:feature_hidden?) + .with('terms_of_use').and_return(true) + allow(UserProfile).to receive(:tou_accepted?).and_return(false) end - - context "and user has no access to the requested domain" do - before :each do - allow(controller.services.identity).to receive( - :has_domain_access, - ).with(default_params[:domain_id]).and_return(false) + + it "skips terms of use check" do + # terms_of_use action doesn't have check_terms_of_use before_action + get :terms_of_use, params: { domain_id: default_params[:domain_id] } + expect(response).to be_successful + expect(response).to render_template(:terms_of_use) + end + end + end + + describe "POST accept_terms_of_use" do + before do + allow(UserProfile).to receive(:tou_accepted?).and_return(false) + # Skip the check_terms_of_use for these tests so we can directly test the action + allow(controller).to receive(:check_terms_of_use) + end + + context "when user accepts terms" do + it "creates or updates user profile" do + expect { + post :accept_terms_of_use, params: default_params.merge(terms_of_use: "1") + }.to change(UserProfile, :count).by(1) + end + + it "creates domain profile with correct version" do + user_profile = create(:user_profile, uid: current_user.id) + + expect { + post :accept_terms_of_use, params: default_params.merge(terms_of_use: "1") + }.to change { user_profile.reload.domain_profiles.count }.by(1) + + domain_profile = user_profile.domain_profiles.last + expect(domain_profile.tou_version).to eq("1.0") + expect(domain_profile.domain_id).to eq(current_user.user_domain_id) + end + + context "redirects after acceptance" do + it "redirects to original URL if provided" do + original_url = "/#{default_params[:domain_id]}/instances" + post :accept_terms_of_use, params: default_params.merge( + terms_of_use: "1", + orginal_url: original_url + ) + expect(response).to redirect_to(original_url) end - - it "should return with ok header" do - get :index, params: { domain_id: default_params[:domain_id] } - expect(response).to have_http_status(200) + + it "redirects to domain home if identity plugin available" do + allow(controller).to receive(:plugin_available?).with('identity').and_return(true) + + post :accept_terms_of_use, params: default_params.merge(terms_of_use: "1") + # The route is defined as: get '/:domain_id/home' => 'domains#show', :as => :domain_home + expect(response).to redirect_to("/#{default_params[:domain_id]}/home") end - - it "should not render unauthorized template" do - get :index, params: { domain_id: default_params[:domain_id] } - expect(response).not_to render_template( - "application/exceptions/unauthorized", - ) + + it "redirects to root if identity plugin not available" do + allow(controller).to receive(:plugin_available?).with('identity').and_return(false) + + post :accept_terms_of_use, params: default_params.merge(terms_of_use: "1") + expect(response).to redirect_to("/") end - - # NOTE: this cannot longer be tested because the extra permissions check for auth/domains was removed - #it 'should rescope to unscoped token' do - # expect(controller).to receive(:authentication_rescope_token).with( - # domain: nil, project: nil - # ) - # get :index, params: { domain_id: default_params[:domain_id] } - #end + end + end + + context "when user does not accept terms" do + it "re-renders accept_terms_of_use page" do + allow(controller).to receive(:check_terms_of_use).and_call_original + allow(UserProfile).to receive(:tou_accepted?).and_return(false) + allow_any_instance_of(DomainConfig).to receive(:feature_hidden?) + .with('terms_of_use').and_return(false) + + post :accept_terms_of_use, params: default_params.merge(terms_of_use: nil) + expect(response).to render_template(:accept_terms_of_use) + end + end + end + + describe "GET terms_of_use" do + it "loads TOU information for current user" do + tou_data = { version: "1.0", accepted: true } + expect(UserProfile).to receive(:tou) + .with(current_user.id, current_user.user_domain_id, "1.0") + .and_return(tou_data) + + get :terms_of_use, params: { domain_id: default_params[:domain_id] } + expect(assigns(:tou)).to eq(tou_data) + end + + it "renders terms_of_use template" do + allow(UserProfile).to receive(:tou).and_return(nil) + get :terms_of_use, params: { domain_id: default_params[:domain_id] } + expect(response).to render_template(:terms_of_use) + end + end + + describe "mailer configuration" do + before do + allow(UserProfile).to receive(:tou_accepted?).and_return(true) + allow(controller).to receive(:set_mailer_host).and_call_original + end + + it "sets mailer host from request" do + get :terms_of_use, params: { domain_id: default_params[:domain_id] } + expect(ActionMailer::Base.default_url_options[:host]).to eq(request.host_with_port) + expect(ActionMailer::Base.default_url_options[:protocol]).to eq(request.protocol) + end + end + + describe "before_action callbacks" do + context "check_terms_of_use" do + it "is not called for terms_of_use action" do + expect(controller).not_to receive(:check_terms_of_use) + get :terms_of_use, params: { domain_id: default_params[:domain_id] } + end + + it "is not called for accept_terms_of_use action" do + expect(controller).not_to receive(:check_terms_of_use) + post :accept_terms_of_use, params: default_params.merge(terms_of_use: "1") end end end