Commit c8c4822c authored by Stan Hu's avatar Stan Hu

Merge branch 'top_level_group_recaptcha' into 'master'

Add recaptcha to group creation [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!56707
parents de555479 bf22bb71
......@@ -8,6 +8,7 @@ class GroupsController < Groups::ApplicationController
include RecordUserLastActivity
include SendFileUpload
include FiltersEvents
include Recaptcha::Verify
extend ::Gitlab::Utils::Override
respond_to :html
......@@ -15,6 +16,7 @@ class GroupsController < Groups::ApplicationController
prepend_before_action(only: [:show, :issues]) { authenticate_sessionless_user!(:rss) }
prepend_before_action(only: [:issues_calendar]) { authenticate_sessionless_user!(:ics) }
prepend_before_action :ensure_export_enabled, only: [:export, :download_export]
prepend_before_action :check_captcha, only: :create, if: -> { captcha_enabled? }
before_action :authenticate_user!, only: [:new, :create]
before_action :group, except: [:index, :new, :create]
......@@ -22,6 +24,7 @@ class GroupsController < Groups::ApplicationController
# Authorize
before_action :authorize_admin_group!, only: [:edit, :update, :destroy, :projects, :transfer, :export, :download_export]
before_action :authorize_create_group!, only: [:new]
before_action :load_recaptcha, only: [:new], if: -> { captcha_required? }
before_action :group_projects, only: [:projects, :activity, :issues, :merge_requests]
before_action :event_filter, only: [:activity]
......@@ -38,6 +41,8 @@ class GroupsController < Groups::ApplicationController
before_action :export_rate_limit, only: [:export, :download_export]
helper_method :captcha_required?
skip_cross_project_access_check :index, :new, :create, :edit, :update,
:destroy, :projects
# When loading show as an atom feed, we render events that could leak cross
......@@ -319,6 +324,23 @@ class GroupsController < Groups::ApplicationController
private
def load_recaptcha
Gitlab::Recaptcha.load_configurations!
end
def check_captcha
return if group_params[:parent_id].present? # Only require for top-level groups
load_recaptcha
return if verify_recaptcha
flash[:alert] = _('There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.')
flash.delete :recaptcha_error
@group = Group.new(group_params)
render action: 'new'
end
def successful_creation_hooks; end
def groups
......@@ -336,6 +358,14 @@ class GroupsController < Groups::ApplicationController
def has_project_list?
%w(details show index).include?(action_name)
end
def captcha_enabled?
Gitlab::Recaptcha.enabled? && Feature.enabled?(:recaptcha_on_top_level_group_creation, type: :ops)
end
def captcha_required?
captcha_enabled? && !params[:parent_id]
end
end
GroupsController.prepend_if_ee('EE::GroupsController')
......@@ -16,6 +16,11 @@
.row
.col-sm-4
= render_if_exists 'shared/groups/invite_members'
- if captcha_required?
.row.recaptcha
.col-sm-4
= recaptcha_tags
.row
.form-actions.col-sm-12
= f.submit _('Create group'), class: "btn gl-button btn-confirm"
......
---
title: Add recaptcha to top-level group creation behind feature flag
merge_request: 56707
author:
type: added
---
name: recaptcha_on_top_level_group_creation
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56707
rollout_issue_url:
milestone: '13.11'
type: ops
group: group::access
default_enabled: false
......@@ -312,6 +312,64 @@ RSpec.describe GroupsController, factory_default: :keep do
end
end
end
context 'when creating a group with captcha protection' do
before do
sign_in(user)
stub_application_setting(recaptcha_enabled: true)
end
after do
# Avoid test ordering issue and ensure `verify_recaptcha` returns true
unless Recaptcha.configuration.skip_verify_env.include?('test')
Recaptcha.configuration.skip_verify_env << 'test'
end
end
it 'displays an error when the reCAPTCHA is not solved' do
allow(controller).to receive(:verify_recaptcha).and_return(false)
post :create, params: { group: { name: 'new_group', path: "new_group" } }
expect(response).to render_template(:new)
expect(flash[:alert]).to eq(_('There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.'))
end
it 'allows creating a group when the reCAPTCHA is solved' do
expect do
post :create, params: { group: { name: 'new_group', path: "new_group" } }
end.to change { Group.count }.by(1)
expect(response).to have_gitlab_http_status(:found)
end
it 'allows creating a sub-group without checking the captcha' do
expect(controller).not_to receive(:verify_recaptcha)
expect do
post :create, params: { group: { name: 'new_group', path: "new_group", parent_id: group.id } }
end.to change { Group.count }.by(1)
expect(response).to have_gitlab_http_status(:found)
end
context 'with feature flag switched off' do
before do
stub_feature_flags(recaptcha_on_top_level_group_creation: false)
end
it 'allows creating a group without the reCAPTCHA' do
expect(controller).not_to receive(:verify_recaptcha)
expect do
post :create, params: { group: { name: 'new_group', path: "new_group" } }
end.to change { Group.count }.by(1)
expect(response).to have_gitlab_http_status(:found)
end
end
end
end
describe 'GET #index' do
......
......@@ -141,6 +141,30 @@ RSpec.describe 'Group' do
end
end
end
describe 'showing recaptcha on group creation when it is enabled' do
before do
stub_application_setting(recaptcha_enabled: true)
allow(Gitlab::Recaptcha).to receive(:load_configurations!)
visit new_group_path
end
it 'renders recaptcha' do
expect(page).to have_css('.recaptcha')
end
end
describe 'not showing recaptcha on group creation when it is disabled' do
before do
stub_feature_flags(recaptcha_on_top_level_group_creation: false)
stub_application_setting(recaptcha_enabled: true)
visit new_group_path
end
it 'does not render recaptcha' do
expect(page).not_to have_css('.recaptcha')
end
end
end
describe 'create a nested group' do
......@@ -189,6 +213,23 @@ RSpec.describe 'Group' do
expect(page).to have_content("Group 'bar' was successfully created.")
end
end
context 'when recaptcha is enabled' do
before do
stub_application_setting(recaptcha_enabled: true)
allow(Gitlab::Recaptcha).to receive(:load_configurations!)
end
context 'when creating subgroup' do
let(:path) { new_group_path(group, parent_id: group.id) }
it 'does not render recaptcha' do
visit path
expect(page).not_to have_css('.recaptcha')
end
end
end
end
it 'checks permissions to avoid exposing groups by parent_id' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment