diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 1fd3999f1c1e447f950895bc58344e6ad270b732..b79af562f46f8d146c5b5f4740bb35bbd6eaddb8 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -112,8 +112,26 @@ def destroy end end + def sign_in_path + return render_404 unless Feature.enabled?(:two_step_sign_in, Feature.current_request) + + respond_to do |format| + format.json do + render json: { sign_in_path: determine_sign_in_path } + end + format.html do + render_404 + end + end + end + private + # Overridden in EE + def determine_sign_in_path + nil + end + override :after_pending_invitations_hook def after_pending_invitations_hook member = resource.members.last diff --git a/config/feature_flags/beta/two_step_sign_in.yml b/config/feature_flags/beta/two_step_sign_in.yml new file mode 100644 index 0000000000000000000000000000000000000000..b7a868bfe172b8a724176db84dd56db9494fabaa --- /dev/null +++ b/config/feature_flags/beta/two_step_sign_in.yml @@ -0,0 +1,10 @@ +--- +name: two_step_sign_in +description: GitLab.com 2-step sign-in +feature_issue_url: https://gitlab.com/groups/gitlab-org/-/epics/19425 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/216203 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/583981 +milestone: '18.8' +group: group::organizations +type: beta +default_enabled: false diff --git a/config/routes/user.rb b/config/routes/user.rb index 57eda7cb80ef770ebca5c820f86889140fb6771b..65f7f994ee6f66726d6b96d453cb77f8acaa5c8e 100644 --- a/config/routes/user.rb +++ b/config/routes/user.rb @@ -59,6 +59,7 @@ def override_omniauth(provider, controller, path_prefix = '/users/auth') get '/users/skip_verification_confirmation', to: 'sessions#skip_verification_confirmation' post '/users/fallback_to_email_otp', to: 'sessions#fallback_to_email_otp' post '/users/passkeys/sign_in', to: 'sessions#new_passkey', as: :users_passkeys_sign_in + get '/users/sign_in_path', to: 'sessions#sign_in_path', as: :users_sign_in_path # Redirect on GitHub authorization request errors. E.g. it could happen when user: # 1. cancel authorization the GitLab OAuth app via GitHub to import GitHub repos diff --git a/ee/app/controllers/ee/sessions_controller.rb b/ee/app/controllers/ee/sessions_controller.rb index 6ef9115e495b0af3bc4d24c94be2ad3410f34ef2..2267f1bbb0e767b98d7e0c68a126ccb0ab8ccdae 100644 --- a/ee/app/controllers/ee/sessions_controller.rb +++ b/ee/app/controllers/ee/sessions_controller.rb @@ -30,6 +30,24 @@ def new private + def safe_login_param + login = params.permit(:login)[:login].to_s + login.truncate(::User::MAX_USERNAME_LENGTH, omission: '') + end + + override :determine_sign_in_path + def determine_sign_in_path + return super unless ::Gitlab::Saas.feature_available?(:redirect_sign_in_when_login_not_found) + + login = safe_login_param + return if login.blank? + + user = ::User.find_by_login(login) + return if user.present? + + new_user_session_path(login: login) + end + def gitlab_geo_logout return unless ::Gitlab::Geo.secondary? diff --git a/ee/config/saas_features/redirect_sign_in_when_login_not_found.yml b/ee/config/saas_features/redirect_sign_in_when_login_not_found.yml new file mode 100644 index 0000000000000000000000000000000000000000..0211acad06aa61e6cad3ee1ec42bae6f13eb9743 --- /dev/null +++ b/ee/config/saas_features/redirect_sign_in_when_login_not_found.yml @@ -0,0 +1,5 @@ +--- +name: redirect_sign_in_when_login_not_found +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/216203 +milestone: '18.8' +group: group::organizations diff --git a/ee/lib/gitlab/saas.rb b/ee/lib/gitlab/saas.rb index 7f0d6c01159669271938740ded7be08c7c20200a..d0c86cf69503e1eaea5fe03fa979ca0fdfd4f643 100644 --- a/ee/lib/gitlab/saas.rb +++ b/ee/lib/gitlab/saas.rb @@ -35,6 +35,7 @@ module Saas secret_detection_service ci_component_usages_in_projects disable_ropc_for_all_applications + redirect_sign_in_when_login_not_found disable_ropc_for_new_applications deduplicate_ci_tags targeted_messages diff --git a/ee/spec/requests/ee/sessions_spec.rb b/ee/spec/requests/ee/sessions_spec.rb index e6fc2eeaa296963f05679be3e1630b2b0af4f630..2c235d48e544a20fde485b90393eebaa99643eb9 100644 --- a/ee/spec/requests/ee/sessions_spec.rb +++ b/ee/spec/requests/ee/sessions_spec.rb @@ -93,4 +93,103 @@ end end end + + describe 'GET /users/sign_in_path', :saas_redirect_sign_in_when_login_not_found do + shared_examples 'returns nil sign_in_path' do |login_value| + it 'returns nil' do + params = login_value.nil? ? {} : { login: login_value } + get users_sign_in_path_path, params: params, as: :json + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ 'sign_in_path' => nil }) + end + end + + context 'when redirect_sign_in_when_login_not_found saas feature is available' do + context 'when requesting JSON format' do + it 'renders 404 when the feature flag is disabled' do + stub_feature_flags(two_step_sign_in: false) + + get users_sign_in_path_path, params: { login: 'nonexistant' }, as: :json + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'when login parameter is not provided' do + it_behaves_like 'returns nil sign_in_path', nil + end + + context 'when login parameter is blank' do + it_behaves_like 'returns nil sign_in_path', '' + end + + context 'when user is found by username' do + it_behaves_like 'returns nil sign_in_path', -> { user.username } + end + + context 'when login is not a string' do + it 'ensures User.find_by_login does not receive an array' do + array_param = [user.username, 'attacker@example.com'] + + expect(User).not_to receive(:find_by_login) + + get users_sign_in_path_path, params: { login: array_param }, as: :json + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ 'sign_in_path' => nil }) + end + end + + context 'when login parameter exceeds maximum length' do + it 'truncates login to MAX_USERNAME_LENGTH characters' do + long_login = 'a' * 300 + truncated_login = 'a' * ::User::MAX_USERNAME_LENGTH + + expect(User).to receive(:find_by_login).with(truncated_login).and_call_original + + get users_sign_in_path_path, params: { login: long_login }, as: :json + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ 'sign_in_path' => "/users/sign_in?login=#{truncated_login}" }) + end + + it 'handles login at exactly MAX_USERNAME_LENGTH characters' do + exact_length_login = 'b' * ::User::MAX_USERNAME_LENGTH + + expect(User).to receive(:find_by_login).with(exact_length_login).and_call_original + + get users_sign_in_path_path, params: { login: exact_length_login }, as: :json + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ 'sign_in_path' => "/users/sign_in?login=#{exact_length_login}" }) + end + end + + context 'when user is found by email' do + it_behaves_like 'returns nil sign_in_path', -> { user.email } + end + + context 'when user is not found' do + it 'returns sign in path with login parameter' do + get users_sign_in_path_path, params: { login: 'nonexistent' }, as: :json + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ 'sign_in_path' => '/users/sign_in?login=nonexistent' }) + end + end + end + + context 'when requesting HTML format' do + it 'returns 404' do + get users_sign_in_path_path, as: :html + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'when redirect_sign_in_when_login_not_found feature is disabled' do + it_behaves_like 'returns nil sign_in_path', -> { 'nonexistent' } + end + end end diff --git a/spec/requests/sessions_spec.rb b/spec/requests/sessions_spec.rb index 2bb44ec457dc76a49227670a98bf248c78dc6f84..790c44f3a51e4d62036674b1ff7ec0b1a0e2aa7a 100644 --- a/spec/requests/sessions_spec.rb +++ b/spec/requests/sessions_spec.rb @@ -60,4 +60,42 @@ def authenticate_2fa(otp_attempt:) post(user_session_path(params: { user: user_params.merge(otp_attempt: otp_attempt) })) end end + + describe 'GET /users/sign_in_path' do + shared_examples 'returns nil sign_in_path' do |login_value| + it 'returns nil' do + params = login_value.nil? ? {} : { login: login_value } + get users_sign_in_path_path, params: params, as: :json + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ 'sign_in_path' => nil }) + end + end + + context 'when requesting JSON format' do + context 'when login parameter is not provided' do + it_behaves_like 'returns nil sign_in_path', nil + end + + context 'when login parameter is blank' do + it_behaves_like 'returns nil sign_in_path', '' + end + + context 'when user is found by username' do + it_behaves_like 'returns nil sign_in_path', -> { user.username } + end + + context 'when user is not found found' do + it_behaves_like 'returns nil sign_in_path', -> { 'nonexistent' } + end + end + + context 'when requesting HTML format' do + it 'returns 404' do + get users_sign_in_path_path, as: :html + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end end