From 1ae26b6f28a603d0b1ebbc8dc36ccae6717bf977 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= <mksionek@gitlab.com>
Date: Wed, 10 Mar 2021 19:22:05 +0100
Subject: [PATCH] Create session enforcer class for group saml to add to git
check
Refactor naming into more readable
Add new database column
Add specs and model method
Add specs for group saml session enforcer class
WIP on git access specs
Refactor git access specs
Add specs to git access
Add new setting to view and form
Add new setting to controller
Add changelog entry
Add translations file
Refactor git access class
Add method to find group
Fix container method
Add cr remarks
Simplify spec example
Change copy in saml sso form
Add documentation regarding checking sso session on git activity
Add cr remarks
Reorganize code to make sso check more effective
Add remarks to git check migration
Add remarks about enforcing sso on Git activity docs
Move check_additional_conditions method
---
...on-git-activity-and-direct-user-to-sso.yml | 5 +
...add_enforced_git_check_to_saml_provider.rb | 13 ++
db/schema_migrations/20210312174321 | 1 +
db/structure.sql | 3 +-
doc/user/group/saml_sso/index.md | 11 +-
.../saml_providers/saml_settings_form.js | 5 +
.../groups/saml_providers_controller.rb | 2 +-
ee/app/models/saml_provider.rb | 12 ++
.../groups/saml_providers/_form.html.haml | 12 +-
ee/lib/ee/gitlab/git_access.rb | 23 ++-
.../auth/group_saml/session_enforcer.rb | 69 ++++++++
.../auth/group_saml/session_enforcer_spec.rb | 164 ++++++++++++++++++
ee/spec/lib/gitlab/git_access_spec.rb | 49 ++++++
ee/spec/models/saml_provider_spec.rb | 49 ++++++
lib/gitlab/git_access.rb | 5 +
locale/gitlab.pot | 14 +-
16 files changed, 426 insertions(+), 11 deletions(-)
create mode 100644 changelogs/unreleased/9152-check-sso-status-on-git-activity-and-direct-user-to-sso.yml
create mode 100644 db/migrate/20210312174321_add_enforced_git_check_to_saml_provider.rb
create mode 100644 db/schema_migrations/20210312174321
create mode 100644 ee/lib/gitlab/auth/group_saml/session_enforcer.rb
create mode 100644 ee/spec/lib/gitlab/auth/group_saml/session_enforcer_spec.rb
diff --git a/changelogs/unreleased/9152-check-sso-status-on-git-activity-and-direct-user-to-sso.yml b/changelogs/unreleased/9152-check-sso-status-on-git-activity-and-direct-user-to-sso.yml
new file mode 100644
index 0000000000000..2b681363bfe17
--- /dev/null
+++ b/changelogs/unreleased/9152-check-sso-status-on-git-activity-and-direct-user-to-sso.yml
@@ -0,0 +1,5 @@
+---
+title: Group SAML - Check SSO status on Git activity
+merge_request: 56867
+author:
+type: added
diff --git a/db/migrate/20210312174321_add_enforced_git_check_to_saml_provider.rb b/db/migrate/20210312174321_add_enforced_git_check_to_saml_provider.rb
new file mode 100644
index 0000000000000..89553a53084a4
--- /dev/null
+++ b/db/migrate/20210312174321_add_enforced_git_check_to_saml_provider.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class AddEnforcedGitCheckToSamlProvider < ActiveRecord::Migration[6.0]
+ DOWNTIME = false
+
+ def up
+ add_column :saml_providers, :git_check_enforced, :boolean, default: false, null: false
+ end
+
+ def down
+ remove_column :saml_providers, :git_check_enforced
+ end
+end
diff --git a/db/schema_migrations/20210312174321 b/db/schema_migrations/20210312174321
new file mode 100644
index 0000000000000..5126ab2675d1f
--- /dev/null
+++ b/db/schema_migrations/20210312174321
@@ -0,0 +1 @@
+4fa88193ae328f04465980210d9a43ce8cad978c157bda5e8ae9951538209268
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 78f959a71f10f..0ad3a3aaeadb1 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -17270,7 +17270,8 @@ CREATE TABLE saml_providers (
enforced_sso boolean DEFAULT false NOT NULL,
enforced_group_managed_accounts boolean DEFAULT false NOT NULL,
prohibited_outer_forks boolean DEFAULT true NOT NULL,
- default_membership_role smallint DEFAULT 10 NOT NULL
+ default_membership_role smallint DEFAULT 10 NOT NULL,
+ git_check_enforced boolean DEFAULT false NOT NULL
);
CREATE SEQUENCE saml_providers_id_seq
diff --git a/doc/user/group/saml_sso/index.md b/doc/user/group/saml_sso/index.md
index 6046258ee2970..6c3ffc4f6464d 100644
--- a/doc/user/group/saml_sso/index.md
+++ b/doc/user/group/saml_sso/index.md
@@ -95,6 +95,7 @@ Please note that the certificate [fingerprint algorithm](../../../integration/sa
- [Improved](https://gitlab.com/gitlab-org/gitlab/-/issues/9255) in GitLab 11.11 with ongoing enforcement in the GitLab UI.
- [Improved](https://gitlab.com/gitlab-org/gitlab/-/issues/292811) in GitLab 13.8, with an updated timeout experience.
- [Improved](https://gitlab.com/gitlab-org/gitlab/-/issues/211962) in GitLab 13.8 with allowing group owners to not go through SSO.
+- [Improved](https://gitlab.com/gitlab-org/gitlab/-/issues/9152) in GitLab 13.11 with enforcing open SSO session to use Git if this setting is switched on.
With this option enabled, users must go through your group's GitLab single sign-on URL. They may also be added via SCIM, if configured. Users can't be added manually, and may only access project/group resources via the UI by signing in through the SSO URL.
@@ -102,9 +103,15 @@ However, users are not prompted to sign in through SSO on each visit. GitLab che
has authenticated through SSO. If it's been more than 1 day since the last sign-in, GitLab
prompts the user to sign in again through SSO.
-We intend to add a similar SSO requirement for [Git and API activity](https://gitlab.com/gitlab-org/gitlab/-/issues/9152).
+We intend to add a similar SSO requirement for [API activity](https://gitlab.com/gitlab-org/gitlab/-/issues/9152).
-When SSO enforcement is enabled for a group, users can't share a project in the group outside the top-level group, even if the project is forked.
+SSO has the following effects when enabled:
+
+- For groups, users can't share a project in the group outside the top-level group,
+ even if the project is forked.
+- For a Git activity, users must be signed-in through SSO before they can push to or
+ pull from a GitLab repository.
+<!-- Add bullet for API activity when https://gitlab.com/gitlab-org/gitlab/-/issues/9152 is complete -->
## Providers
diff --git a/ee/app/assets/javascripts/saml_providers/saml_settings_form.js b/ee/app/assets/javascripts/saml_providers/saml_settings_form.js
index 9bce0f7b674b7..f732a34f521cf 100644
--- a/ee/app/assets/javascripts/saml_providers/saml_settings_form.js
+++ b/ee/app/assets/javascripts/saml_providers/saml_settings_form.js
@@ -39,6 +39,11 @@ export default class SamlSettingsForm {
el: this.form.querySelector('.js-group-saml-enforced-group-managed-accounts-toggle-area'),
dependsOn: 'enforced-sso',
},
+ {
+ name: 'enforced-git-activity-check',
+ el: this.form.querySelector('.js-group-saml-enforced-git-check-toggle-area'),
+ dependsOn: 'enforced-sso',
+ },
{
name: 'prohibited-outer-forks',
el: this.form.querySelector('.js-group-saml-prohibited-outer-forks-toggle-area'),
diff --git a/ee/app/controllers/groups/saml_providers_controller.rb b/ee/app/controllers/groups/saml_providers_controller.rb
index 497cec1320966..63e9786c28ea7 100644
--- a/ee/app/controllers/groups/saml_providers_controller.rb
+++ b/ee/app/controllers/groups/saml_providers_controller.rb
@@ -47,7 +47,7 @@ def load_test_response
end
def saml_provider_params
- allowed_params = %i[sso_url certificate_fingerprint enabled enforced_sso default_membership_role]
+ allowed_params = %i[sso_url certificate_fingerprint enabled enforced_sso default_membership_role git_check_enforced]
if Feature.enabled?(:group_managed_accounts, group)
allowed_params += [:enforced_group_managed_accounts, :prohibited_outer_forks]
diff --git a/ee/app/models/saml_provider.rb b/ee/app/models/saml_provider.rb
index 26fccaf61a997..86edd5c12f62f 100644
--- a/ee/app/models/saml_provider.rb
+++ b/ee/app/models/saml_provider.rb
@@ -10,6 +10,7 @@ class SamlProvider < ApplicationRecord
validates :sso_url, presence: true, addressable_url: { schemes: %w(https), ascii_only: true }
validates :certificate_fingerprint, presence: true, certificate_fingerprint: true
validates :default_membership_role, presence: true
+ validate :git_check_enforced_allowed
validate :access_level_inclusion
after_initialize :set_defaults, if: :new_record?
@@ -35,6 +36,10 @@ def enforced_sso?
enabled? && super && group.feature_available?(:group_saml)
end
+ def git_check_enforced?
+ super && enforced_sso?
+ end
+
def enforced_group_managed_accounts?
super && enforced_sso? && Feature.enabled?(:group_managed_accounts, group)
end
@@ -92,6 +97,13 @@ def access_level_inclusion
errors.add(:default_membership_role, "is not included in the list")
end
+ def git_check_enforced_allowed
+ return unless git_check_enforced
+ return if enforced_sso?
+
+ errors.add(:git_check_enforced, "is not allowed when SSO is not enforced.")
+ end
+
def set_defaults
self.enabled = true
end
diff --git a/ee/app/views/groups/saml_providers/_form.html.haml b/ee/app/views/groups/saml_providers/_form.html.haml
index 263956fbce63b..3a7eb6e2cbed0 100644
--- a/ee/app/views/groups/saml_providers/_form.html.haml
+++ b/ee/app/views/groups/saml_providers/_form.html.haml
@@ -10,10 +10,18 @@
%label.gl-mt-2.mb-0.js-group-saml-enforced-sso-toggle-area
= render "shared/buttons/project_feature_toggle", is_checked: saml_provider.enforced_sso, disabled: !saml_provider.enabled?, label: s_("GroupSAML|Enforced SSO"), class_list: "js-project-feature-toggle js-group-saml-enforced-sso-toggle project-feature-toggle d-inline", data: { qa_selector: 'enforced_sso_toggle_button' } do
= f.hidden_field :enforced_sso, { class: 'js-group-saml-enforced-sso-input js-project-feature-toggle-input'}
- %span.form-text.d-inline.font-weight-normal.align-text-bottom.ml-3= s_('GroupSAML|Enforce SSO-only authentication for this group.')
+ %span.form-text.d-inline.font-weight-normal.align-text-bottom.ml-3= s_('GroupSAML|Enforce SSO-only authentication for web activity for this group.')
.form-text.text-muted.js-helper-text{ style: "display: #{'none' if saml_provider.enabled?} #{'block' unless saml_provider.enabled?}" }
%span
- = s_('GroupSAML|To be able to enable enforced SSO, you first need to enable SAML authentication.')
+ = s_('GroupSAML|Before enforcing SSO, enable SAML authentication.')
+ .form-group
+ %label.gl-mt-2.mb-0.js-group-saml-enforced-git-check-toggle-area
+ = render "shared/buttons/project_feature_toggle", is_checked: saml_provider.git_check_enforced, disabled: !saml_provider.enabled?, label: s_("GroupSAML|Check SSO on git activity"), class_list: "js-project-feature-toggle js-group-saml-enforced-git-check-toggle-area project-feature-toggle d-inline", data: { qa_selector: 'git_activity_check_toggle_button' } do
+ = f.hidden_field :git_check_enforced, { class: 'js-group-enforced-git-check-input js-project-feature-toggle-input'}
+ %span.form-text.d-inline.font-weight-normal.align-text-bottom.ml-3= s_('GroupSAML|Enforce SSO-access for Git in this group.')
+ .form-text.text-muted.js-helper-text{ style: "display: #{saml_provider.enabled? ? 'none' : 'block'}" }
+ %span
+ = s_('GroupSAML|Before enforcing SSO, enable SAML authentication.')
- if Feature.enabled?(:group_managed_accounts, group)
.form-group
%label.gl-mt-2.mb-0.js-group-saml-enforced-group-managed-accounts-toggle-area
diff --git a/ee/lib/ee/gitlab/git_access.rb b/ee/lib/ee/gitlab/git_access.rb
index 25c9fc47b6c07..708661ccfaa0d 100644
--- a/ee/lib/ee/gitlab/git_access.rb
+++ b/ee/lib/ee/gitlab/git_access.rb
@@ -83,6 +83,13 @@ def check_active_user!
super
end
+ override :check_additional_conditions!
+ def check_additional_conditions!
+ check_sso_session!
+
+ super
+ end
+
def check_geo_license!
if ::Gitlab::Geo.secondary? && !::Gitlab::Geo.license_allows?
raise ::Gitlab::GitAccess::ForbiddenError, 'Your current license does not have GitLab Geo add-on enabled.'
@@ -104,12 +111,21 @@ def check_otp_session!
if ::Gitlab::Auth::Otp::SessionEnforcer.new(actor).access_restricted?
message = "OTP verification is required to access the repository.\n\n"\
- " Use: #{build_ssh_otp_verify_command}"
+ " Use: #{build_ssh_otp_verify_command}"
raise ::Gitlab::GitAccess::ForbiddenError, message
end
end
+ def check_sso_session!
+ return true unless user && container
+
+ return unless ::Gitlab::Auth::GroupSaml::SessionEnforcer.new(user, containing_group).access_restricted?
+
+ group_saml_url = Rails.application.routes.url_helpers.sso_group_saml_providers_url(containing_group, token: containing_group.saml_discovery_token)
+ raise ::Gitlab::GitAccess::ForbiddenError, "Cannot find valid SSO session. Please login via your group's SSO at #{group_saml_url}"
+ end
+
def build_ssh_otp_verify_command
user = "#{::Gitlab.config.gitlab_shell.ssh_user}@" unless ::Gitlab.config.gitlab_shell.ssh_user.empty?
user_host = "#{user}#{::Gitlab.config.gitlab_shell.ssh_host}"
@@ -155,6 +171,11 @@ def check_size_limit?
size_checker.enabled? && super
end
end
+
+ def containing_group
+ return group if group?
+ return project.group if project?
+ end
end
end
end
diff --git a/ee/lib/gitlab/auth/group_saml/session_enforcer.rb b/ee/lib/gitlab/auth/group_saml/session_enforcer.rb
new file mode 100644
index 0000000000000..e3f44e3c4671a
--- /dev/null
+++ b/ee/lib/gitlab/auth/group_saml/session_enforcer.rb
@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Auth
+ module GroupSaml
+ class SessionEnforcer
+ SESSION_STORE_KEY = :active_group_sso_sign_ins
+
+ def initialize(user, group)
+ @user = user
+ @group = group
+ end
+
+ def access_restricted?
+ return false if skip_check?
+
+ latest_sign_in = find_session
+
+ return true unless latest_sign_in
+ return SsoEnforcer::DEFAULT_SESSION_TIMEOUT.ago > latest_sign_in if ::Feature.enabled?(:enforced_sso_expiry, group)
+
+ false
+ end
+
+ private
+
+ attr_reader :user, :group
+
+ def skip_check?
+ return true if no_group_or_provider?
+ return true if user_allowed?
+ return true unless git_check_enforced?
+
+ false
+ end
+
+ def no_group_or_provider?
+ return true unless group
+ return true unless group.root_ancestor
+ return true unless saml_provider
+
+ false
+ end
+
+ def saml_provider
+ @saml_provider ||= group.root_ancestor.saml_provider
+ end
+
+ def git_check_enforced?
+ saml_provider.git_check_enforced?
+ end
+
+ def user_allowed?
+ return true if user.auditor? || user.can_read_all_resources?
+ return true if group.owned_by?(user)
+
+ false
+ end
+
+ def find_session
+ sessions = ActiveSession.list_sessions(user)
+ sessions.filter_map do |session|
+ Gitlab::NamespacedSessionStore.new(SESSION_STORE_KEY, session.with_indifferent_access)[saml_provider.id]
+ end.last
+ end
+ end
+ end
+ end
+end
diff --git a/ee/spec/lib/gitlab/auth/group_saml/session_enforcer_spec.rb b/ee/spec/lib/gitlab/auth/group_saml/session_enforcer_spec.rb
new file mode 100644
index 0000000000000..af7fa279e4bb6
--- /dev/null
+++ b/ee/spec/lib/gitlab/auth/group_saml/session_enforcer_spec.rb
@@ -0,0 +1,164 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Auth::GroupSaml::SessionEnforcer do
+ describe '#access_restricted' do
+ let_it_be(:saml_provider) { create(:saml_provider, enforced_sso: true) }
+ let_it_be(:user) { create(:user) }
+ let_it_be(:identity) { create(:group_saml_identity, saml_provider: saml_provider, user: user) }
+
+ let(:root_group) { saml_provider.group }
+
+ subject { described_class.new(user, root_group).access_restricted? }
+
+ before do
+ stub_licensed_features(group_saml: true)
+ end
+
+ context 'when git check is enforced' do
+ before do
+ allow(saml_provider).to receive(:git_check_enforced?).and_return(true)
+ end
+
+ context 'with an active session', :clean_gitlab_redis_shared_state do
+ let(:session_id) { '42' }
+ let(:session_time) { 5.minutes.ago }
+ let(:stored_session) do
+ { 'active_group_sso_sign_ins' => { saml_provider.id => session_time } }
+ end
+
+ before do
+ Gitlab::Redis::SharedState.with do |redis|
+ redis.set("session:gitlab:#{session_id}", Marshal.dump(stored_session))
+ redis.sadd("session:lookup:user:gitlab:#{user.id}", [session_id])
+ end
+ end
+
+ it { is_expected.to be_falsey }
+
+ context 'with sub-group' do
+ let(:group) { create(:group, parent: root_group) }
+
+ subject { described_class.new(user, group).access_restricted? }
+
+ it { is_expected.to be_falsey }
+ end
+
+ context 'with expired session' do
+ let(:session_time) { 2.days.ago }
+
+ it { is_expected.to be_truthy }
+ end
+
+ context 'with two active sessions', :clean_gitlab_redis_shared_state do
+ let(:second_session_id) { '52' }
+ let(:second_stored_session) do
+ { 'active_group_sso_sign_ins' => { create(:saml_provider, enforced_sso: true).id => session_time } }
+ end
+
+ before do
+ Gitlab::Redis::SharedState.with do |redis|
+ redis.set("session:gitlab:#{second_session_id}", Marshal.dump(second_stored_session))
+ redis.sadd("session:lookup:user:gitlab:#{user.id}", [session_id, second_session_id])
+ end
+ end
+
+ it { is_expected.to be_falsey }
+ end
+
+ context 'without enforced_sso_expiry feature flag' do
+ let(:session_time) { 2.days.ago }
+
+ before do
+ stub_feature_flags(enforced_sso_expiry: false)
+ end
+
+ it { is_expected.to be_falsey }
+ end
+
+ context 'without group' do
+ let(:root_group) { nil }
+
+ it { is_expected.to be_falsey }
+ end
+
+ context 'without saml_provider' do
+ let(:root_group) { create(:group) }
+
+ it { is_expected.to be_falsey }
+ end
+
+ context 'with admin', :enable_admin_mode do
+ let(:user) { create(:user, :admin) }
+
+ it { is_expected.to be_falsey }
+ end
+
+ context 'with auditor' do
+ let(:user) { create(:user, :auditor) }
+
+ it { is_expected.to be_falsey }
+ end
+
+ context 'with group owner' do
+ before do
+ root_group.add_owner(user)
+ end
+
+ it { is_expected.to be_falsey }
+ end
+ end
+
+ context 'without any session' do
+ it { is_expected.to be_truthy }
+
+ context 'with admin', :enable_admin_mode do
+ let(:user) { create(:user, :admin) }
+
+ it { is_expected.to be_falsey }
+ end
+
+ context 'with auditor' do
+ let(:user) { create(:user, :auditor) }
+
+ it { is_expected.to be_falsey }
+ end
+
+ context 'with group owner' do
+ before do
+ root_group.add_owner(user)
+ end
+
+ it { is_expected.to be_falsey }
+ end
+ end
+ end
+
+ context 'when git check is not enforced' do
+ before do
+ allow(saml_provider).to receive(:git_check_enforced?).and_return(false)
+ end
+
+ context 'with an active session', :clean_gitlab_redis_shared_state do
+ let(:session_id) { '42' }
+ let(:stored_session) do
+ { 'active_group_sso_sign_ins' => { saml_provider.id => 5.minutes.ago } }
+ end
+
+ before do
+ Gitlab::Redis::SharedState.with do |redis|
+ redis.set("session:gitlab:#{session_id}", Marshal.dump(stored_session))
+ redis.sadd("session:lookup:user:gitlab:#{user.id}", [session_id])
+ end
+ end
+
+ it { is_expected.to be_falsey }
+ end
+
+ context 'without any session' do
+ it { is_expected.to be_falsey }
+ end
+ end
+ end
+end
diff --git a/ee/spec/lib/gitlab/git_access_spec.rb b/ee/spec/lib/gitlab/git_access_spec.rb
index 0ee484696fc08..bb22c56d6dc4f 100644
--- a/ee/spec/lib/gitlab/git_access_spec.rb
+++ b/ee/spec/lib/gitlab/git_access_spec.rb
@@ -915,6 +915,55 @@ def stub_redis
end
end
+ describe '#check_sso_session!' do
+ before do
+ project.add_developer(user)
+ end
+
+ context 'with project without group' do
+ it 'allows pull and push changes' do
+ expect(Gitlab::Auth::GroupSaml::SessionEnforcer).to receive(:new).with(user, nil).and_return(double(access_restricted?: false))
+ pull_changes
+ end
+ end
+
+ context 'with project with group' do
+ let_it_be(:group) { create(:group) }
+
+ before do
+ project.update!(namespace: group)
+ end
+
+ context 'user with a sso session' do
+ let(:access_restricted?) { false }
+
+ it 'allows pull and push changes' do
+ expect(Gitlab::Auth::GroupSaml::SessionEnforcer).to receive(:new).with(user, group).twice.and_return(double(access_restricted?: access_restricted?))
+
+ expect { pull_changes }.not_to raise_error
+ expect { push_changes }.not_to raise_error
+ end
+ end
+
+ context 'user without a sso session' do
+ let(:access_restricted?) { true }
+
+ before do
+ expect(Gitlab::Auth::GroupSaml::SessionEnforcer).to receive(:new).with(user, group).twice.and_return(double(access_restricted?: access_restricted?))
+ end
+
+ it 'does not allow pull or push changes with proper url in the message' do
+ aggregate_failures do
+ address = "http://localhost/groups/#{group.name}/-/saml/sso"
+
+ expect { pull_changes }.to raise_error(Gitlab::GitAccess::ForbiddenError, /#{Regexp.quote(address)}/)
+ expect { push_changes }.to raise_error(Gitlab::GitAccess::ForbiddenError, /#{Regexp.quote(address)}/)
+ end
+ end
+ end
+ end
+ end
+
describe '#check_maintenance_mode!' do
let(:changes) { Gitlab::GitAccess::ANY }
diff --git a/ee/spec/models/saml_provider_spec.rb b/ee/spec/models/saml_provider_spec.rb
index a52adc30bfa9a..ea8224ac3a6ba 100644
--- a/ee/spec/models/saml_provider_spec.rb
+++ b/ee/spec/models/saml_provider_spec.rb
@@ -98,6 +98,27 @@
end
end
end
+
+ describe 'git_check_enforced' do
+ let_it_be(:group) { create(:group) }
+
+ context 'sso is enforced' do
+ it 'git_check_enforced is valid' do
+ expect(build(:saml_provider, group: group, enabled: true, enforced_sso: true, git_check_enforced: true)).to be_valid
+ expect(build(:saml_provider, group: group, enabled: true, enforced_sso: true, git_check_enforced: false)).to be_valid
+ end
+ end
+
+ context 'sso is not enforced' do
+ it 'git_check_enforced is invalid when set to true' do
+ expect(build(:saml_provider, group: group, enabled: true, enforced_sso: false, git_check_enforced: true)).to be_invalid
+ end
+
+ it 'git_check_enforced is valid when set to false' do
+ expect(build(:saml_provider, group: group, enabled: true, enforced_sso: false, git_check_enforced: false)).to be_valid
+ end
+ end
+ end
end
describe 'Default values' do
@@ -216,6 +237,34 @@
end
end
+ describe '#git_check_enforced?' do
+ context 'without enforced sso' do
+ before do
+ allow(subject).to receive(:enforced_sso?).and_return(false)
+ end
+
+ it 'does not enforce git activity check' do
+ subject.git_check_enforced = true
+ expect(subject).not_to be_git_check_enforced
+ subject.git_check_enforced = false
+ expect(subject).not_to be_git_check_enforced
+ end
+ end
+
+ context 'with enforced sso' do
+ before do
+ allow(subject).to receive(:enforced_sso?).and_return(true)
+ end
+
+ it 'enforces git activity check when attribute is set to true' do
+ subject.git_check_enforced = true
+ expect(subject).to be_git_check_enforced
+ subject.git_check_enforced = false
+ expect(subject).not_to be_git_check_enforced
+ end
+ end
+ end
+
describe '#prohibited_outer_forks?' do
context 'without enforced GMA' do
it 'is false when prohibited_outer_forks flag value is true' do
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index c5ca46827cbd9..31e4755192ee3 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -91,6 +91,7 @@ def check(cmd, changes)
when *PUSH_COMMANDS
check_push_access!
end
+ check_additional_conditions!
success_result
end
@@ -530,6 +531,10 @@ def check_size_limit?
def size_checker
container.repository_size_checker
end
+
+ # overriden in EE
+ def check_additional_conditions!
+ end
end
end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index ce9dadc16e8e6..16eb1e80911e0 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -14851,9 +14851,15 @@ msgstr ""
msgid "GroupSAML|Are you sure you want to remove the SAML group link?"
msgstr ""
+msgid "GroupSAML|Before enforcing SSO, enable SAML authentication."
+msgstr ""
+
msgid "GroupSAML|Certificate fingerprint"
msgstr ""
+msgid "GroupSAML|Check SSO on git activity"
+msgstr ""
+
msgid "GroupSAML|Configuration"
msgstr ""
@@ -14869,7 +14875,10 @@ msgstr ""
msgid "GroupSAML|Enable SAML authentication for this group."
msgstr ""
-msgid "GroupSAML|Enforce SSO-only authentication for this group."
+msgid "GroupSAML|Enforce SSO-access for Git in this group."
+msgstr ""
+
+msgid "GroupSAML|Enforce SSO-only authentication for web activity for this group."
msgstr ""
msgid "GroupSAML|Enforce users to have dedicated group managed accounts for this group."
@@ -14965,9 +14974,6 @@ msgstr ""
msgid "GroupSAML|This will be set as the access level of users added to the group."
msgstr ""
-msgid "GroupSAML|To be able to enable enforced SSO, you first need to enable SAML authentication."
-msgstr ""
-
msgid "GroupSAML|To be able to enable group managed accounts, you first need to enable enforced SSO."
msgstr ""
--
GitLab