diff --git a/app/helpers/dashboard_helper.rb b/app/helpers/dashboard_helper.rb index 3756584e3b3af17e70c6de48c491333b59f3e83a..89f6d61ef4452f066d6458a4b11177c4f4450e36 100644 --- a/app/helpers/dashboard_helper.rb +++ b/app/helpers/dashboard_helper.rb @@ -33,6 +33,10 @@ def feature_entry(title, href: nil, enabled: true, doc_href: nil) end end end + + def user_groups_requiring_reauth + [] + end end DashboardHelper.prepend_mod_with('DashboardHelper') diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb index fc4d69dcdbcaa41472998dfaab40edce6481c3a4..7d29cd7a87788cb6c816691aed87d594bd59d10b 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -254,6 +254,10 @@ def todo_author_display?(todo) !todo.build_failed? && !todo.unmergeable? end + def todo_groups_requiring_saml_reauth(_todos) + [] + end + private def todos_design_path(todo, path_options) diff --git a/app/views/dashboard/issues.html.haml b/app/views/dashboard/issues.html.haml index 78c3270114e21a80c4510cf29c27df2d68d8b241..f7b2ba59549c767b3f609154f106c277a08fdbb4 100644 --- a/app/views/dashboard/issues.html.haml +++ b/app/views/dashboard/issues.html.haml @@ -6,6 +6,8 @@ = auto_discovery_link_tag(:atom, safe_params.merge(rss_url_options).to_h, title: "#{current_user.name} issues") = render_dashboard_ultimate_trial(current_user) += render_if_exists 'shared/dashboard/saml_reauth_notice', + groups_requiring_saml_reauth: user_groups_requiring_reauth .page-title-holder.gl-display-flex.gl-align-items-center %h1.page-title.gl-font-size-h-display= _('Issues') diff --git a/app/views/dashboard/merge_requests.html.haml b/app/views/dashboard/merge_requests.html.haml index 91cec50226b478ab71258c3cf6446ae3155a1d1e..d29cb56db0718f51750195902a7f9ad28520aa19 100644 --- a/app/views/dashboard/merge_requests.html.haml +++ b/app/views/dashboard/merge_requests.html.haml @@ -11,6 +11,8 @@ add_page_specific_style 'page_bundles/issuable_list' = render_dashboard_ultimate_trial(current_user) += render_if_exists 'shared/dashboard/saml_reauth_notice', + groups_requiring_saml_reauth: user_groups_requiring_reauth .page-title-holder.d-flex.align-items-start.flex-column.flex-sm-row.align-items-sm-center %h1.page-title.gl-font-size-h-display= title diff --git a/app/views/dashboard/todos/index.html.haml b/app/views/dashboard/todos/index.html.haml index 1b0bd10db77a292290e9266a83e6cdc9859abda8..0587ba61db493c754b7191fe3f5cd599a7472e94 100644 --- a/app/views/dashboard/todos/index.html.haml +++ b/app/views/dashboard/todos/index.html.haml @@ -2,7 +2,10 @@ = render_two_factor_auth_recovery_settings_check = render_dashboard_ultimate_trial(current_user) -= render_if_exists 'dashboard/todos/saml_reauth_notice' + += render_if_exists 'shared/dashboard/saml_reauth_notice', + groups_requiring_saml_reauth: todo_groups_requiring_saml_reauth(@todos) + - add_page_specific_style 'page_bundles/todos' - add_page_specific_style 'page_bundles/issuable' - filter_by_done = params[:state] == 'done' diff --git a/ee/app/helpers/ee/dashboard_helper.rb b/ee/app/helpers/ee/dashboard_helper.rb index a30cbe1c20a03dd278c084ce9882ea64fa89008a..37bfbe791912c1c2a495a1f584e9ab42fed6141f 100644 --- a/ee/app/helpers/ee/dashboard_helper.rb +++ b/ee/app/helpers/ee/dashboard_helper.rb @@ -31,6 +31,18 @@ def has_start_trial? !current_user.has_current_license? && current_user.can_admin_all_resources? end + override :user_groups_requiring_reauth + def user_groups_requiring_reauth + saml_providers = current_user.group_saml_identities.map(&:saml_provider) + return super unless saml_providers.any? + + saml_providers.select! do |saml_provider| + ::Gitlab::Auth::GroupSaml::SsoEnforcer.new(saml_provider, user: current_user).access_restricted? + end + + saml_providers.map(&:group) + end + private def security_dashboard_available? diff --git a/ee/app/helpers/ee/todos_helper.rb b/ee/app/helpers/ee/todos_helper.rb index 0fb5d8b3f45e532a28f207e7e4efd81569dd51d3..ef1dec50176b56d4e99fc4391a3a926f706d88f3 100644 --- a/ee/app/helpers/ee/todos_helper.rb +++ b/ee/app/helpers/ee/todos_helper.rb @@ -36,9 +36,11 @@ def todo_action_name(todo) super end + override :todo_groups_requiring_saml_reauth def todo_groups_requiring_saml_reauth(todos) - groups = todos.filter_map { |todo| todo.group || todo.project.group }.uniq + return super unless todos&.any? + groups = todos.filter_map { |todo| todo.group || todo.project.group }.uniq ::Gitlab::Auth::GroupSaml::SsoEnforcer.access_restricted_groups(groups, user: current_user) end end diff --git a/ee/app/views/dashboard/todos/_saml_reauth_notice.html.haml b/ee/app/views/shared/dashboard/_saml_reauth_notice.html.haml similarity index 66% rename from ee/app/views/dashboard/todos/_saml_reauth_notice.html.haml rename to ee/app/views/shared/dashboard/_saml_reauth_notice.html.haml index 2fd8a8ce550b91aed6f86ddcdeef89d0bfa73800..47bc4bcecc739d40ca85c8711ef6284a30bfb82f 100644 --- a/ee/app/views/dashboard/todos/_saml_reauth_notice.html.haml +++ b/ee/app/views/shared/dashboard/_saml_reauth_notice.html.haml @@ -1,9 +1,8 @@ -- groups_requiring_saml_reauth = todo_groups_requiring_saml_reauth(@todos) - return unless groups_requiring_saml_reauth.any? = render Pajamas::AlertComponent.new(variant: :warning, dismissible: false) do |c| - c.with_body do - = s_('GroupSAML|Some to-do items may be hidden because your SAML session has expired. Select the group’s path to reauthenticate and view the hidden to-do items.') + = s_('GroupSAML|Some items may be hidden because your SAML session has expired. Select the group’s path to reauthenticate and view any hidden items.') - c.with_actions do .gl-display-flex.gl-flex-wrap - groups_requiring_saml_reauth.each do |group| diff --git a/ee/spec/features/dashboards/issues_spec.rb b/ee/spec/features/dashboards/issues_spec.rb index 0bd0101e6a2d0daacefb8302edae927aac13b75f..7013757b1bc6a61c8621f549c0ec95b21520fa9c 100644 --- a/ee/spec/features/dashboards/issues_spec.rb +++ b/ee/spec/features/dashboards/issues_spec.rb @@ -3,8 +3,10 @@ require 'spec_helper' RSpec.describe 'Dashboard issues', feature_category: :team_planning do - let(:user) { create(:user) } - let(:page_path) { issues_dashboard_path } + let_it_be(:user) { create(:user) } + let_it_be(:page_path) { issues_dashboard_path } it_behaves_like 'dashboard ultimate trial callout' + + it_behaves_like 'dashboard SAML reauthentication banner' end diff --git a/ee/spec/features/dashboards/merge_requests_spec.rb b/ee/spec/features/dashboards/merge_requests_spec.rb index e7d0c3ef47523082b46db08ca6f9ff1c5581ecb0..61f13d756430c87dd73f5b4aebb3ca49348b8c7f 100644 --- a/ee/spec/features/dashboards/merge_requests_spec.rb +++ b/ee/spec/features/dashboards/merge_requests_spec.rb @@ -3,8 +3,10 @@ require 'spec_helper' RSpec.describe 'Dashboard merge requests', feature_category: :code_review_workflow do - let(:user) { create(:user) } - let(:page_path) { merge_requests_dashboard_path } + let_it_be(:user) { create(:user) } + let_it_be(:page_path) { merge_requests_dashboard_path } it_behaves_like 'dashboard ultimate trial callout' + + it_behaves_like 'dashboard SAML reauthentication banner' end diff --git a/ee/spec/features/dashboards/todos_spec.rb b/ee/spec/features/dashboards/todos_spec.rb index 9ad2fa51a151d3d7055ee0b9406f8ae3aad4c9e9..0637f443eafdc48c8667d18f5982de73e58ffbb9 100644 --- a/ee/spec/features/dashboards/todos_spec.rb +++ b/ee/spec/features/dashboards/todos_spec.rb @@ -40,43 +40,10 @@ end end - context 'when the user has todos in an SSO enforced group' do - let_it_be(:saml_provider) { create(:saml_provider, enabled: true, enforced_sso: true) } - let_it_be(:restricted_group) { create(:group, saml_provider: saml_provider) } + it_behaves_like 'dashboard SAML reauthentication banner' do let_it_be(:epic_todo) do create(:todo, group: restricted_group, user: user, target: create(:epic, group: restricted_group)) end - - before do - stub_licensed_features(group_saml: true) - create(:group_saml_identity, user: user, saml_provider: saml_provider) - - restricted_group.add_owner(user) - - sign_in(user) - end - - context 'and the session is not active' do - it 'shows the user an alert', :aggregate_failures do - visit page_path - - expect(page).to have_content(s_('GroupSAML|Some to-do items may be hidden because your SAML session has expired. Select the group’s path to reauthenticate and view the hidden to-do items.')) # rubocop:disable Layout/LineLength - expect(page).to have_link(restricted_group.path, href: /#{sso_group_saml_providers_path(restricted_group)}/) - end - end - - context 'and the session is active' do - before do - dummy_session = { active_group_sso_sign_ins: { saml_provider.id => DateTime.now } } - allow(Gitlab::Session).to receive(:current).and_return(dummy_session) - end - - it 'does not show the user an alert', :aggregate_failures do - visit page_path - - expect(page).not_to have_content(s_('GroupSAML|Some to-do items may be hidden because your SAML session has expired. Select the group’s path to reauthenticate and view the hidden to-do items.')) # rubocop:disable Layout/LineLength - end - end end context 'when user has review request todo', :saas do diff --git a/ee/spec/helpers/ee/dashboard_helper_spec.rb b/ee/spec/helpers/ee/dashboard_helper_spec.rb index 2352a79887114b8b5662df1df839cb6f05a57547..b175ebc81cdbf3182e5e70e510e050dae71329eb 100644 --- a/ee/spec/helpers/ee/dashboard_helper_spec.rb +++ b/ee/spec/helpers/ee/dashboard_helper_spec.rb @@ -28,4 +28,47 @@ it { is_expected.to eq(output) } end end + + describe '.user_groups_requiring_reauth', feature_category: :system_access do + subject(:user_groups_requiring_reauth) { helper.user_groups_requiring_reauth } + + let!(:current_user) { create(:user) } + + before do + allow(helper).to receive(:current_user).and_return(current_user) + end + + context 'when the user has no Group SAML identities' do + it 'returns an empty array' do + expect(user_groups_requiring_reauth).to match_array([]) + end + end + + context 'when the user has Group SAML identities' do + let_it_be(:saml_provider) { create(:saml_provider, group: create(:group), enforced_sso: true) } + + before do + stub_licensed_features(group_saml: true) + create(:group_saml_identity, user: current_user, saml_provider: saml_provider) + end + + context 'when access is not restricted' do + it 'returns an empty array' do + expect(user_groups_requiring_reauth).to match_array([]) + end + end + + context 'when access is restricted' do + before do + allow_next_instance_of(::Gitlab::Auth::GroupSaml::SsoEnforcer) do |instance| + allow(instance).to receive(:access_restricted?).and_return(true) + end + end + + it 'returns the group that the SAML provider belongs to' do + expect(user_groups_requiring_reauth).to match_array(saml_provider.group) + end + end + end + end end diff --git a/ee/spec/helpers/ee/todos_helper_spec.rb b/ee/spec/helpers/ee/todos_helper_spec.rb index f682665944f1d123bd4f6a480b2146cf143e359f..7fed2b8c09fabc2bde8e4963e1031b07cf8d6175 100644 --- a/ee/spec/helpers/ee/todos_helper_spec.rb +++ b/ee/spec/helpers/ee/todos_helper_spec.rb @@ -224,4 +224,42 @@ it { expect(helper.todo_action_name(todo)).to eq(expected_action_name) } end end + + describe '.todo_groups_requiring_saml_reauth', feature_category: :system_access do + subject(:todo_groups_requiring_saml_reauth) { helper.todo_groups_requiring_saml_reauth([issue_todo, group_todo]) } + + let_it_be(:current_user) { create(:user) } + + let_it_be(:group1) { create(:group) } + let_it_be(:project) { create(:project, group: group1) } + let_it_be(:issue) { create(:issue, title: 'Issue 1') } + let_it_be(:issue_todo) do + create(:todo, target: issue, project: project) + end + + let_it_be(:group2) { create(:group) } + let_it_be(:group_todo) do + create(:todo, target: group2, group: group2, project: nil, user: current_user) + end + + before do + allow(helper).to receive(:current_user).and_return(current_user) + end + + context 'when access is not restricted' do + it 'returns an empty array' do + expect(todo_groups_requiring_saml_reauth).to match_array([]) + end + end + + context 'when access is restricted' do + before do + allow(::Gitlab::Auth::GroupSaml::SsoEnforcer).to receive(:access_restricted_groups).and_return([group1, group2]) + end + + it 'returns the todo groups' do + expect(todo_groups_requiring_saml_reauth).to match_array([group1, group2]) + end + end + end end diff --git a/ee/spec/support/shared_examples/features/dashboard_saml_reauth_banner_shared_examples.rb b/ee/spec/support/shared_examples/features/dashboard_saml_reauth_banner_shared_examples.rb new file mode 100644 index 0000000000000000000000000000000000000000..8ef0f3fa83206361e94e091c4281317a278f172c --- /dev/null +++ b/ee/spec/support/shared_examples/features/dashboard_saml_reauth_banner_shared_examples.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.shared_examples_for 'dashboard SAML reauthentication banner' do + let_it_be(:restricted_group) { create(:group, :private) } + let_it_be(:saml_provider) { create(:saml_provider, group: restricted_group, enabled: true, enforced_sso: true) } + + before do + stub_licensed_features(group_saml: true) + end + + before_all do + create(:group_saml_identity, user: user, saml_provider: saml_provider) + + restricted_group.add_developer(user) + + sign_in(user) + end + + context 'and the session is not active' do + it 'shows the user an alert', :aggregate_failures do + visit page_path + + expect(page).to have_content( + s_('GroupSAML|Some items may be hidden because your SAML session has expired. Select the group’s path to reauthenticate and view any hidden items.') # rubocop:disable Layout/LineLength -- Single string + ) + + expect(page).to have_link(restricted_group.path, href: /#{sso_group_saml_providers_path(restricted_group)}/) + end + end + + context 'and the session is active' do + before do + dummy_session = { active_group_sso_sign_ins: { saml_provider.id => DateTime.now } } + allow(Gitlab::Session).to receive(:current).and_return(dummy_session) + end + + it 'does not show the user an alert', :aggregate_failures do + visit page_path + + expect(page).not_to have_content( + s_('GroupSAML|Some items may be hidden because your SAML session has expired. Select the group’s path to reauthenticate and view any hidden items.') # rubocop:disable Layout/LineLength -- Single string + ) + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 4becc3dc9f4802ded503aeade6a116328442c516..49edcd613d915504e0cd301ad209dce7e908df7b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -23595,7 +23595,7 @@ msgstr "" msgid "GroupSAML|Some branches are inaccessible because your SAML session has expired. To access the branches, select the group’s path to reauthenticate." msgstr "" -msgid "GroupSAML|Some to-do items may be hidden because your SAML session has expired. Select the group’s path to reauthenticate and view the hidden to-do items." +msgid "GroupSAML|Some items may be hidden because your SAML session has expired. Select the group’s path to reauthenticate and view any hidden items." msgstr "" msgid "GroupSAML|The SCIM token is now hidden. To see the value of the token again, you need to %{linkStart}reset it%{linkEnd}." diff --git a/spec/helpers/dashboard_helper_spec.rb b/spec/helpers/dashboard_helper_spec.rb index d52b3c9abb32caebe4f9caffa60ab25bba3e679c..95e69ebc0702a8fa99cdae5a18b3cbc02a6e207e 100644 --- a/spec/helpers/dashboard_helper_spec.rb +++ b/spec/helpers/dashboard_helper_spec.rb @@ -75,4 +75,10 @@ it { is_expected.to eq(false) } end + + describe '.user_groups_requiring_reauth', feature_category: :system_access do + it 'returns an empty array' do + expect(helper.user_groups_requiring_reauth).to match_array([]) + end + end end diff --git a/spec/helpers/todos_helper_spec.rb b/spec/helpers/todos_helper_spec.rb index bffb240dae433581917e4a2af9fa734fbb15d7c8..92df9e1581c874df531a676ea4b6a222e959b620 100644 --- a/spec/helpers/todos_helper_spec.rb +++ b/spec/helpers/todos_helper_spec.rb @@ -443,4 +443,10 @@ end end end + + describe '.todo_groups_requiring_saml_reauth', feature_category: :system_access do + it 'returns an empty array' do + expect(helper.todo_groups_requiring_saml_reauth([])).to match_array([]) + end + end end