diff --git a/app/models/user.rb b/app/models/user.rb
index 7ae7996aad2e0573ba9f734802cb6111cd6423d9..866fc089b297c58fff6325f9072b721816d2b26b 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1464,11 +1464,6 @@ def allow_password_authentication_for_git?
Gitlab::CurrentSettings.password_authentication_enabled_for_git?
end
- # method overriden in EE
- def password_based_login_forbidden?
- false
- end
-
def can_change_username?
gitlab_config.username_changing_enabled
end
diff --git a/config/feature_flags/ops/block_password_auth_for_saml_users.yml b/config/feature_flags/ops/block_password_auth_for_saml_users.yml
deleted file mode 100644
index ffc04d14a19fbab5e71d3166b341a32e354e0d72..0000000000000000000000000000000000000000
--- a/config/feature_flags/ops/block_password_auth_for_saml_users.yml
+++ /dev/null
@@ -1,8 +0,0 @@
----
-name: block_password_auth_for_saml_users
-introduced_by_url:
-rollout_issue_url:
-milestone: '13.11'
-type: ops
-group: group::authentication
-default_enabled: false
diff --git a/ee/app/controllers/concerns/ee/membership_actions.rb b/ee/app/controllers/concerns/ee/membership_actions.rb
index a0b889dbe5ee7ff0d1ded47cde81dfb8ddc47eea..1ceea4182b9a160d5b8149f3698deee54bff9ebb 100644
--- a/ee/app/controllers/concerns/ee/membership_actions.rb
+++ b/ee/app/controllers/concerns/ee/membership_actions.rb
@@ -4,15 +4,6 @@ module EE
module MembershipActions
extend ::Gitlab::Utils::Override
- override :leave
- def leave
- super
-
- if current_user.authorized_by_provisioning_group?(membershipable)
- sign_out current_user
- end
- end
-
private
def update_params
diff --git a/ee/app/controllers/groups/sso_controller.rb b/ee/app/controllers/groups/sso_controller.rb
index f2b2ddeeb4922124ec1e715e3663a27e5339cf9f..3d6195dd64797448c7e92ddde7dac9256bcbdf6a 100644
--- a/ee/app/controllers/groups/sso_controller.rb
+++ b/ee/app/controllers/groups/sso_controller.rb
@@ -36,11 +36,7 @@ def unlink
GroupSaml::Identity::DestroyService.new(linked_identity).execute
- if current_user.authorized_by_provisioning_group?(unauthenticated_group)
- sign_out current_user
- else
- redirect_to profile_account_path
- end
+ redirect_to profile_account_path
end
private
diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb
index 930bcf6e3f8d790789deb00b2f1ec54642ee4e1f..d68b94f118fcfbc0a860690682d7cefee8fbb57f 100644
--- a/ee/app/models/ee/user.rb
+++ b/ee/app/models/ee/user.rb
@@ -521,7 +521,6 @@ def ldap_sync_time
override :allow_password_authentication_for_web?
def allow_password_authentication_for_web?(*)
return false if group_managed_account?
- return false if user_authorized_by_provisioning_group?
return false if password_authentication_disabled_by_enterprise_group?
super
@@ -530,7 +529,6 @@ def allow_password_authentication_for_web?(*)
override :allow_password_authentication_for_git?
def allow_password_authentication_for_git?(*)
return false if group_managed_account?
- return false if user_authorized_by_provisioning_group?
return false if password_authentication_disabled_by_enterprise_group?
super
@@ -543,19 +541,6 @@ def password_authentication_disabled_by_enterprise_group?
enterprise_group.saml_provider.enabled? && enterprise_group.saml_provider.disable_password_authentication_for_enterprise_users?
end
- override :password_based_login_forbidden?
- def password_based_login_forbidden?
- user_authorized_by_provisioning_group? || super
- end
-
- def user_authorized_by_provisioning_group?
- user_detail.provisioned_by_group? && ::Feature.enabled?(:block_password_auth_for_saml_users, user_detail.provisioned_by_group, type: :ops)
- end
-
- def authorized_by_provisioning_group?(group)
- user_authorized_by_provisioning_group? && provisioned_by_group == group
- end
-
def enterprise_user_of_group?(group)
enterprise_group_id == group.id
end
diff --git a/ee/app/models/ee/user_detail.rb b/ee/app/models/ee/user_detail.rb
index 9c9fcafd80af06c9cbfe0e007e8cba9fa0cae21a..7778ce08031be2f31382ba51d29ae9ab4a0e5cc1 100644
--- a/ee/app/models/ee/user_detail.rb
+++ b/ee/app/models/ee/user_detail.rb
@@ -16,9 +16,5 @@ module UserDetail
prefix: true
)
end
-
- def provisioned_by_group?
- !!provisioned_by_group
- end
end
end
diff --git a/ee/spec/controllers/ee/sessions_controller_spec.rb b/ee/spec/controllers/ee/sessions_controller_spec.rb
index a445eb899f0562f9def46f14e82c8d87094e17e2..f2c9b2660ab6bc88999f4b7617177d99d1bace75 100644
--- a/ee/spec/controllers/ee/sessions_controller_spec.rb
+++ b/ee/spec/controllers/ee/sessions_controller_spec.rb
@@ -137,18 +137,6 @@ def authenticate_2fa(otp_user_id: user.id, **user_params)
end
end
- context 'when user is not allowed to log in using password' do
- let_it_be(:user) { create(:user, provisioned_by_group: build(:group)) }
-
- it 'does not authenticate the user' do
- post(:create, params: { user: { login: user.username, password: user.password } })
-
- expect(response).to have_gitlab_http_status(:ok)
- expect(@request.env['warden']).not_to be_authenticated
- expect(flash[:alert]).to include(I18n.t('devise.failure.invalid'))
- end
- end
-
context 'when password authentication disabled by enterprise group' do
let_it_be(:enterprise_group) { create(:group) }
let_it_be(:saml_provider) do
diff --git a/ee/spec/controllers/groups/sso_controller_spec.rb b/ee/spec/controllers/groups/sso_controller_spec.rb
index 278a7c8eeb688104a0905824e06b7fd2b8151076..65647c76ad944035f756bd8746bc4b0dcccfd500 100644
--- a/ee/spec/controllers/groups/sso_controller_spec.rb
+++ b/ee/spec/controllers/groups/sso_controller_spec.rb
@@ -34,41 +34,12 @@
expect(assigns[:group_name]).to eq 'our-group'
end
- context 'unlinking user' do
- before do
- create(:group_saml_identity, saml_provider: saml_provider, user: user)
- user.update!(provisioned_by_group: saml_provider.group)
- end
-
- it 'allows account unlinking' do
- expect do
- delete :unlink, params: { group_id: group }
- end.to change { Identity.count }.by(-1)
- end
-
- context 'with block_password_auth_for_saml_users feature flag switched off' do
- before do
- stub_feature_flags(block_password_auth_for_saml_users: false)
- end
+ it 'allows account unlinking' do
+ create(:group_saml_identity, saml_provider: saml_provider, user: user)
- it 'does not sign out user provisioned by this group' do
- delete :unlink, params: { group_id: group }
-
- expect(response).to redirect_to(profile_account_path)
- end
- end
-
- context 'with block_password_auth_for_saml_users feature flag switched on' do
- before do
- stub_feature_flags(block_password_auth_for_saml_users: true)
- end
-
- it 'signs out user provisioned by this group' do
- delete :unlink, params: { group_id: group }
-
- expect(subject.current_user).to eq(nil)
- end
- end
+ expect do
+ delete :unlink, params: { group_id: group }
+ end.to change { Identity.count }.by(-1)
end
context 'when SAML is disabled for the group' do
diff --git a/ee/spec/features/groups/members/leave_group_spec.rb b/ee/spec/features/groups/members/leave_group_spec.rb
deleted file mode 100644
index 7dc39da3ae4f17d7806a93d971b763e701e21fb6..0000000000000000000000000000000000000000
--- a/ee/spec/features/groups/members/leave_group_spec.rb
+++ /dev/null
@@ -1,78 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe 'Groups > Members > Leave group', feature_category: :groups_and_projects do
- include Features::MembersHelpers
- include Spec::Support::Helpers::ModalHelpers
-
- let_it_be(:other_user) { create(:user) }
- let_it_be(:group) { create(:group) }
-
- let(:user) { create(:user) }
- let(:more_actions_dropdown) do
- find('[data-testid="groups-projects-more-actions-dropdown"] .gl-new-dropdown-custom-toggle')
- end
-
- before do
- user.update!(provisioned_by_group: group)
- sign_in(user)
- end
-
- context 'with block_password_auth_for_saml_users feature flag switched on' do
- it 'guest provisioned by this group leaves the group and is signed off', :js do
- group.add_guest(user)
- group.add_owner(other_user)
-
- visit group_path(group)
- more_actions_dropdown.click
- click_link 'Leave group'
- accept_gl_confirm(button_text: 'Leave group')
-
- expect(page).to have_current_path(new_user_session_path, ignore_query: true)
- expect(group).not_to have_user(user)
- end
-
- it 'guest leaves the group by url param and is signed off', :js do
- group.add_guest(user)
- group.add_owner(other_user)
-
- visit group_path(group, leave: 1)
- accept_gl_confirm(button_text: 'Leave group')
- wait_for_all_requests
-
- expect(page).to have_current_path(new_user_session_path, ignore_query: true)
- expect(group).not_to have_user(user)
- end
- end
-
- context 'with block_password_auth_for_saml_users feature flag switched off' do
- before do
- stub_feature_flags(block_password_auth_for_saml_users: false)
- end
-
- it 'guest leaves the group by url param', :js do
- group.add_guest(user)
- group.add_owner(other_user)
-
- visit group_path(group, leave: 1)
- accept_gl_confirm(button_text: 'Leave group')
- wait_for_all_requests
-
- expect(page).to have_current_path(dashboard_groups_path, ignore_query: true)
- expect(group).not_to have_user(user)
- end
-
- it 'guest leaves the group as last member', :js do
- group.add_guest(user)
-
- visit group_path(group)
- more_actions_dropdown.click
- click_link 'Leave group'
- accept_gl_confirm(button_text: 'Leave group')
-
- expect(page).to have_current_path(dashboard_groups_path, ignore_query: true)
- expect(group).not_to have_user(user)
- end
- end
-end
diff --git a/ee/spec/models/ee/user_detail_spec.rb b/ee/spec/models/ee/user_detail_spec.rb
index 618068c807492999743e1d74f314990986fa9f23..9f3a203120cc16a24bcdece48b0a325cd95a516a 100644
--- a/ee/spec/models/ee/user_detail_spec.rb
+++ b/ee/spec/models/ee/user_detail_spec.rb
@@ -32,22 +32,6 @@
end
end
- describe '#provisioned_by_group?' do
- let(:user) { create(:user, provisioned_by_group: build(:group)) }
-
- subject { user.user_detail.provisioned_by_group? }
-
- it 'returns true when user is provisioned by group' do
- expect(subject).to eq(true)
- end
-
- it 'returns true when user is provisioned by group' do
- user.user_detail.update!(provisioned_by_group: nil)
-
- expect(subject).to eq(false)
- end
- end
-
context 'with loose foreign key on user_details.provisioned_by_group_id' do
it_behaves_like 'cleanup by a loose foreign key' do
let_it_be(:parent) { create(:group) }
diff --git a/ee/spec/models/ee/user_spec.rb b/ee/spec/models/ee/user_spec.rb
index f56992bcbb0f4915d0cbe8facc1bc3653266fa26..4de5a0568c2c880c025143dff4042aa809fc15ab 100644
--- a/ee/spec/models/ee/user_spec.rb
+++ b/ee/spec/models/ee/user_spec.rb
@@ -1574,26 +1574,6 @@
end
end
- context 'when user is provisioned by group' do
- before do
- user.user_detail.provisioned_by_group = build(:group)
- end
-
- it 'is false' do
- expect(user.allow_password_authentication_for_web?).to eq false
- end
-
- context 'with feature flag switched off' do
- before do
- stub_feature_flags(block_password_auth_for_saml_users: false)
- end
-
- it 'is true' do
- expect(user.allow_password_authentication_for_web?).to eq true
- end
- end
- end
-
context 'when password authentication disabled by enterprise group' do
let_it_be(:enterprise_group) { create(:group) }
let_it_be(:saml_provider) { create(:saml_provider, group: enterprise_group, enabled: true, disable_password_authentication_for_enterprise_users: true) }
@@ -1617,26 +1597,6 @@
end
end
- context 'when user is provisioned by group' do
- before do
- user.user_detail.provisioned_by_group = build(:group)
- end
-
- it 'is false' do
- expect(user.allow_password_authentication_for_git?).to eq false
- end
-
- context 'with feature flag switched off' do
- before do
- stub_feature_flags(block_password_auth_for_saml_users: false)
- end
-
- it 'is true' do
- expect(user.allow_password_authentication_for_git?).to eq true
- end
- end
- end
-
context 'when password authentication disabled by enterprise group' do
let_it_be(:enterprise_group) { create(:group) }
let_it_be(:saml_provider) { create(:saml_provider, group: enterprise_group, enabled: true, disable_password_authentication_for_enterprise_users: true) }
@@ -1704,110 +1664,6 @@
end
end
- describe '#user_authorized_by_provisioning_group?' do
- context 'when user is provisioned by group' do
- let(:group) { build(:group) }
-
- before do
- user.user_detail.provisioned_by_group = group
- end
-
- it 'is true' do
- expect(user.user_authorized_by_provisioning_group?).to eq true
- end
-
- context 'with feature flag switched off' do
- before do
- stub_feature_flags(block_password_auth_for_saml_users: false)
- end
-
- it 'is false' do
- expect(user.user_authorized_by_provisioning_group?).to eq false
- end
- end
-
- context 'with feature flag switched on for particular groups' do
- before do
- stub_feature_flags(block_password_auth_for_saml_users: false)
- end
-
- it 'is false when provisioned by group without feature flag' do
- stub_feature_flags(block_password_auth_for_saml_users: create(:group))
-
- expect(user.user_authorized_by_provisioning_group?).to eq false
- end
-
- it 'is true when provisioned by group with feature flag' do
- stub_feature_flags(block_password_auth_for_saml_users: group)
-
- expect(user.user_authorized_by_provisioning_group?).to eq true
- end
- end
- end
-
- context 'when user is not provisioned by group' do
- it 'is false' do
- expect(user.user_authorized_by_provisioning_group?).to eq false
- end
-
- context 'with feature flag switched off' do
- before do
- stub_feature_flags(block_password_auth_for_saml_users: false)
- end
-
- it 'is false' do
- expect(user.user_authorized_by_provisioning_group?).to eq false
- end
- end
- end
- end
-
- describe '#authorized_by_provisioning_group?' do
- let_it_be(:group) { create(:group) }
-
- context 'when user is provisioned by group' do
- before do
- user.user_detail.provisioned_by_group = group
- end
-
- it 'is true' do
- expect(user.authorized_by_provisioning_group?(group)).to eq true
- end
-
- context 'when other group is provided' do
- it 'is false' do
- expect(user.authorized_by_provisioning_group?(create(:group))).to eq false
- end
- end
-
- context 'with feature flag switched off' do
- before do
- stub_feature_flags(block_password_auth_for_saml_users: false)
- end
-
- it 'is false' do
- expect(user.authorized_by_provisioning_group?(group)).to eq false
- end
- end
- end
-
- context 'when user is not provisioned by group' do
- it 'is false' do
- expect(user.authorized_by_provisioning_group?(group)).to eq false
- end
-
- context 'with feature flag switched off' do
- before do
- stub_feature_flags(block_password_auth_for_saml_users: false)
- end
-
- it 'is false' do
- expect(user.authorized_by_provisioning_group?(group)).to eq false
- end
- end
- end
- end
-
describe '#password_authentication_disabled_by_enterprise_group?' do
subject(:password_authentication_disabled_by_enterprise_group?) { user.password_authentication_disabled_by_enterprise_group? }
@@ -1915,44 +1771,6 @@
end
end
- describe '#password_based_login_forbidden?' do
- context 'when user is provisioned by group' do
- before do
- user.user_detail.provisioned_by_group = build(:group)
- end
-
- it 'is true' do
- expect(user.password_based_login_forbidden?).to eq true
- end
-
- context 'with feature flag switched off' do
- before do
- stub_feature_flags(block_password_auth_for_saml_users: false)
- end
-
- it 'is false' do
- expect(user.password_based_login_forbidden?).to eq false
- end
- end
- end
-
- context 'when user is not provisioned by group' do
- it 'is false' do
- expect(user.password_based_login_forbidden?).to eq false
- end
-
- context 'with feature flag switched off' do
- before do
- stub_feature_flags(block_password_auth_for_saml_users: false)
- end
-
- it 'is false' do
- expect(user.password_based_login_forbidden?).to eq false
- end
- end
- end
- end
-
describe '#using_license_seat?' do
let(:user) { create(:user) }
diff --git a/ee/spec/requests/git_http_spec.rb b/ee/spec/requests/git_http_spec.rb
index c17973bba6e6b9aa4e4a8c57ce5fc25dea883ace..907b7127d14b21ddad6922c00c2677cae3fcad30 100644
--- a/ee/spec/requests/git_http_spec.rb
+++ b/ee/spec/requests/git_http_spec.rb
@@ -155,104 +155,6 @@
it_behaves_like 'pulls are allowed'
end
- describe 'when user cannot use password-based login' do
- let_it_be(:group) { create(:group) }
- let_it_be(:project) { create(:project, :repository, :private, group: group) }
- let_it_be(:user) { create(:user, provisioned_by_group: group) }
-
- let(:env) { { user: user.username, password: user.password } }
- let(:path) { "#{project.full_path}.git" }
-
- before do
- project.add_developer(user)
- end
-
- context 'with feature flag switched off' do
- before do
- stub_feature_flags(block_password_auth_for_saml_users: false)
- end
-
- it_behaves_like 'pulls are allowed'
- it_behaves_like 'pushes are allowed'
- end
-
- context 'with feature flag switched on' do
- it 'responds with status 401 Unauthorized for pull action' do
- download(path, **env) do |response|
- expect(response).to have_gitlab_http_status(:unauthorized)
- end
- end
-
- it 'responds with status 401 Unauthorized for push action' do
- upload(path, **env) do |response|
- expect(response).to have_gitlab_http_status(:unauthorized)
- end
- end
-
- context 'when username and personal access token are provided' do
- let(:user) { create(:user, provisioned_by_group: group) }
- let(:access_token) { create(:personal_access_token, user: user) }
- let(:env) { { user: user.username, password: access_token.token } }
-
- it_behaves_like 'pulls are allowed'
- it_behaves_like 'pushes are allowed'
- end
-
- context 'when user has 2FA enabled' do
- let(:user) { create(:user, :two_factor, provisioned_by_group: group) }
- let(:access_token) { create(:personal_access_token, user: user) }
-
- context 'when username and personal access token are provided' do
- let(:env) { { user: user.username, password: access_token.token } }
-
- it_behaves_like 'pulls are allowed'
- it_behaves_like 'pushes are allowed'
-
- it 'rejects the push attempt for read_repository scope' do
- read_access_token = create(:personal_access_token, user: user, scopes: [:read_repository])
-
- upload(path, user: user.username, password: read_access_token.token) do |response|
- expect(response).to have_gitlab_http_status(:forbidden)
- expect(response.body).to include('You are not allowed to upload code')
- end
- end
-
- it 'accepts the push attempt for write_repository scope' do
- write_access_token = create(:personal_access_token, user: user, scopes: [:write_repository])
-
- upload(path, user: user.username, password: write_access_token.token) do |response|
- expect(response).to have_gitlab_http_status(:ok)
- end
- end
-
- it 'accepts the pull attempt for read_repository scope' do
- read_access_token = create(:personal_access_token, user: user, scopes: [:read_repository])
-
- download(path, user: user.username, password: read_access_token.token) do |response|
- expect(response).to have_gitlab_http_status(:ok)
- end
- end
-
- it 'accepts the pull attempt for api scope' do
- read_access_token = create(:personal_access_token, user: user, scopes: [:api])
-
- download(path, user: user.username, password: read_access_token.token) do |response|
- expect(response).to have_gitlab_http_status(:ok)
- end
- end
-
- it 'accepts the push attempt for api scope' do
- write_access_token = create(:personal_access_token, user: user, scopes: [:api])
-
- upload(path, user: user.username, password: write_access_token.token) do |response|
- expect(response).to have_gitlab_http_status(:ok)
- end
- end
- end
- end
- end
- end
-
context 'when password authentication disabled by enterprise group' do
let_it_be(:enterprise_group) { create(:group) }
let_it_be(:saml_provider) { create(:saml_provider, group: enterprise_group, enabled: true, disable_password_authentication_for_enterprise_users: true) }
diff --git a/lib/gitlab/auth/database/authentication.rb b/lib/gitlab/auth/database/authentication.rb
index bf35a9abe41dd198335b695871f7cb96db3d2ea4..c0dc2b0875fd2712bfb951eb2b9e1a0727973ddc 100644
--- a/lib/gitlab/auth/database/authentication.rb
+++ b/lib/gitlab/auth/database/authentication.rb
@@ -9,7 +9,6 @@ module Database
class Authentication < Gitlab::Auth::OAuth::Authentication
def login(login, password)
return false unless Gitlab::CurrentSettings.password_authentication_enabled_for_git?
- return false if user.password_based_login_forbidden?
return user if user&.valid_password?(password)
end
diff --git a/lib/gitlab/auth/o_auth/user.rb b/lib/gitlab/auth/o_auth/user.rb
index 8a376d1b0a413bf64c9e1834d1d058ae8ce434bc..283ba4fa8a200edf91f49b08efacadc167e6fbc5 100644
--- a/lib/gitlab/auth/o_auth/user.rb
+++ b/lib/gitlab/auth/o_auth/user.rb
@@ -134,8 +134,6 @@ def add_or_update_user_identities
log.info "Correct LDAP account has been found. identity to user: #{gl_user.username}."
gl_user.identities.build(provider: ldap_person.provider, extern_uid: ldap_person.dn)
end
-
- identity
end
def find_or_build_ldap_user
diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml
index 71cd9f5605411ea44c294cd26568129ff4da1b47..7aa52f7914eb855de047092ce05acc6db9f1984d 100644
--- a/spec/support/rspec_order_todo.yml
+++ b/spec/support/rspec_order_todo.yml
@@ -91,7 +91,6 @@
- './ee/spec/features/groups/iterations/user_views_iteration_cadence_spec.rb'
- './ee/spec/features/groups/iterations/user_views_iteration_spec.rb'
- './ee/spec/features/groups/ldap_group_links_spec.rb'
-- './ee/spec/features/groups/members/leave_group_spec.rb'
- './ee/spec/features/groups/members/list_members_spec.rb'
- './ee/spec/features/groups/members/manage_groups_spec.rb'
- './ee/spec/features/groups/members/manage_members_spec.rb'