From 2ce8e65a52b66a7e5cc86b05c41afad164988ccb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Mon, 4 Mar 2019 10:47:44 +0100 Subject: [PATCH 01/12] Add additional link to 2fa page --- app/controllers/groups/group_members_controller.rb | 2 +- app/controllers/profiles/two_factor_auths_controller.rb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 79eb2c3d14d85e..58b3dfbe99d5b7 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -11,7 +11,7 @@ def self.admin_not_required_endpoints # Authorize before_action :authorize_admin_group_member!, except: admin_not_required_endpoints - + skip_before_action :check_two_factor_requirement, only: :leave skip_cross_project_access_check :index, :create, :update, :destroy, :request_access, :approve_access_request, :leave, :resend_invite, :override diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb index ba94196b2f9243..2eae7b6a92cd18 100644 --- a/app/controllers/profiles/two_factor_auths_controller.rb +++ b/app/controllers/profiles/two_factor_auths_controller.rb @@ -22,10 +22,11 @@ def show end, group: lambda do |groups| group_links = groups.map { |group| view_context.link_to group.full_name, group_path(group) }.to_sentence + leave_group_links = groups.map{ |group| view_context.link_to "leave #{group.full_name}", leave_group_members_path(group), remote: false, method: :delete}.join(' or ') flash.now[:alert] = %{ The group settings for #{group_links} require you to enable - Two-Factor Authentication for your account. + Two-Factor Authentication for your account. You can #{leave_group_links}. }.html_safe end ) -- GitLab From 2b348a32b83a41d654857135ecb91a3d6a62fc8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Mon, 4 Mar 2019 11:25:46 +0100 Subject: [PATCH 02/12] Add missing space --- app/controllers/profiles/two_factor_auths_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb index 2eae7b6a92cd18..ae1f9764fd84f5 100644 --- a/app/controllers/profiles/two_factor_auths_controller.rb +++ b/app/controllers/profiles/two_factor_auths_controller.rb @@ -22,7 +22,7 @@ def show end, group: lambda do |groups| group_links = groups.map { |group| view_context.link_to group.full_name, group_path(group) }.to_sentence - leave_group_links = groups.map{ |group| view_context.link_to "leave #{group.full_name}", leave_group_members_path(group), remote: false, method: :delete}.join(' or ') + leave_group_links = groups.map { |group| view_context.link_to "leave #{group.full_name}", leave_group_members_path(group), remote: false, method: :delete}.join(' or ') flash.now[:alert] = %{ The group settings for #{group_links} require you to enable -- GitLab From b154d804f3b2d005e4f9cba7dedb1f84605a23b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Mon, 4 Mar 2019 13:38:30 +0100 Subject: [PATCH 03/12] Update specs to reflect new message --- spec/features/users/login_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index ad856bd062e4e7..f6c599dc504bbd 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -442,7 +442,9 @@ def sign_in_using_saml! expect(current_path).to eq profile_two_factor_auth_path expect(page).to have_content( 'The group settings for Group 1 and Group 2 require you to enable ' \ - 'Two-Factor Authentication for your account. You need to do this ' \ + 'Two-Factor Authentication for your account. '\ + 'You can leave Group 1 or leave Group 2. '\ + 'You need to do this ' \ 'before ') end -- GitLab From d8bfdc39382e930c6d6b67080c5b8b569820b3e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Tue, 5 Mar 2019 10:50:02 +0100 Subject: [PATCH 04/12] Add changelog to reflect current changes --- changelogs/unreleased/do-not-force-2fa.yml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelogs/unreleased/do-not-force-2fa.yml diff --git a/changelogs/unreleased/do-not-force-2fa.yml b/changelogs/unreleased/do-not-force-2fa.yml new file mode 100644 index 00000000000000..6f671673b2adef --- /dev/null +++ b/changelogs/unreleased/do-not-force-2fa.yml @@ -0,0 +1,6 @@ +--- +title: Add link on two-factor authorization settings page to leave group that enforces + two-factor authorization +merge_request: +author: +type: changed -- GitLab From 01b1de4205219e359e640817c47b081bcfef1ff4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Tue, 5 Mar 2019 23:12:26 +0100 Subject: [PATCH 05/12] Add string internalization --- .../profiles/two_factor_auths_controller.rb | 20 +++++++-------- locale/gitlab.pot | 25 +++++++++++++++++++ 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb index ae1f9764fd84f5..700f62ea1c321f 100644 --- a/app/controllers/profiles/two_factor_auths_controller.rb +++ b/app/controllers/profiles/two_factor_auths_controller.rb @@ -18,22 +18,22 @@ def show two_factor_authentication_reason( global: lambda do flash.now[:alert] = - 'The global settings require you to enable Two-Factor Authentication for your account.' + s_('The global settings require you to enable Two-Factor Authentication for your account.') end, group: lambda do |groups| group_links = groups.map { |group| view_context.link_to group.full_name, group_path(group) }.to_sentence - leave_group_links = groups.map { |group| view_context.link_to "leave #{group.full_name}", leave_group_members_path(group), remote: false, method: :delete}.join(' or ') + leave_group_links = groups.map { |group| view_context.link_to (s_("leave %{group_name}") % { group_name: group.full_name }), leave_group_members_path(group), remote: false, method: :delete}.join(s_(' or ')) - flash.now[:alert] = %{ - The group settings for #{group_links} require you to enable - Two-Factor Authentication for your account. You can #{leave_group_links}. - }.html_safe + flash.now[:alert] = s_(' + The group settings for %{group_links} require you to enable + Two-Factor Authentication for your account. You can %{leave_group_links}. + ').html_safe % { group_links: group_links.html_safe, leave_group_links: leave_group_links.html_safe } end ) unless two_factor_grace_period_expired? grace_period_deadline = current_user.otp_grace_period_started_at + two_factor_grace_period.hours - flash.now[:alert] = flash.now[:alert] + " You need to do this before #{l(grace_period_deadline)}." + flash.now[:alert] = flash.now[:alert] + s_(" You need to do this before %{grace_period_deadline}.") % { grace_period_deadline: l(grace_period_deadline)} end end @@ -50,7 +50,7 @@ def create render 'create' else - @error = 'Invalid pin code' + @error = s_('Invalid pin code') @qr_code = build_qr_code setup_u2f_registration render 'show' @@ -64,7 +64,7 @@ def create_u2f if @u2f_registration.persisted? session.delete(:challenges) - redirect_to profile_two_factor_auth_path, notice: "Your U2F device was registered!" + redirect_to profile_two_factor_auth_path, notice: s_("Your U2F device was registered!") else @qr_code = build_qr_code setup_u2f_registration @@ -86,7 +86,7 @@ def destroy def skip if two_factor_grace_period_expired? - redirect_to new_profile_two_factor_auth_path, alert: 'Cannot skip two factor authentication setup' + redirect_to new_profile_two_factor_auth_path, alert: s_('Cannot skip two factor authentication setup') else session[:skip_two_factor] = current_user.otp_grace_period_started_at + two_factor_grace_period.hours redirect_to root_path diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d68c680d5f9906..d313f2431c2924 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -16,9 +16,19 @@ msgstr "" "Content-Transfer-Encoding: 8bit\n" "Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\n" +msgid "" +"\n" +" The group settings for %{group_links} require you to enable\n" +" Two-Factor Authentication for your account. You can %{leave_group_links}.\n" +" " +msgstr "" + msgid " Status" msgstr "" +msgid " You need to do this before %{grace_period_deadline}." +msgstr "" + msgid " and" msgstr "" @@ -1830,6 +1840,9 @@ msgstr "" msgid "Cannot render the image. Maximum character count (%{charLimit}) has been exceeded." msgstr "" +msgid "Cannot skip two factor authentication setup" +msgstr "" + msgid "Capacity threshold" msgstr "" @@ -5677,6 +5690,9 @@ msgstr "" msgid "Invalid input, please avoid emojis" msgstr "" +msgid "Invalid pin code" +msgstr "" + msgid "Invitation" msgstr "" @@ -10051,6 +10067,9 @@ msgstr "" msgid "The fork relationship has been removed." msgstr "" +msgid "The global settings require you to enable Two-Factor Authentication for your account." +msgstr "" + msgid "The import will time out after %{timeout}. For repositories that take longer, use a clone/push combination." msgstr "" @@ -11714,6 +11733,9 @@ msgstr "" msgid "Your U2F device needs to be set up. Plug it in (if not already) and click the button on the left." msgstr "" +msgid "Your U2F device was registered!" +msgstr "" + msgid "Your applications (%{size})" msgstr "" @@ -12196,6 +12218,9 @@ msgstr "" msgid "latest version" msgstr "" +msgid "leave %{group_name}" +msgstr "" + msgid "license management" msgstr "" -- GitLab From e73da9f6589cc995c85c76f9b2d86fb985eff1bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Wed, 6 Mar 2019 10:03:22 +0100 Subject: [PATCH 06/12] Extract part of code to separate method --- .../profiles/two_factor_auths_controller.rb | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb index 700f62ea1c321f..d49a7ab766eead 100644 --- a/app/controllers/profiles/two_factor_auths_controller.rb +++ b/app/controllers/profiles/two_factor_auths_controller.rb @@ -21,19 +21,13 @@ def show s_('The global settings require you to enable Two-Factor Authentication for your account.') end, group: lambda do |groups| - group_links = groups.map { |group| view_context.link_to group.full_name, group_path(group) }.to_sentence - leave_group_links = groups.map { |group| view_context.link_to (s_("leave %{group_name}") % { group_name: group.full_name }), leave_group_members_path(group), remote: false, method: :delete}.join(s_(' or ')) - - flash.now[:alert] = s_(' - The group settings for %{group_links} require you to enable - Two-Factor Authentication for your account. You can %{leave_group_links}. - ').html_safe % { group_links: group_links.html_safe, leave_group_links: leave_group_links.html_safe } + flash.now[:alert] = groups_notification(groups) end ) unless two_factor_grace_period_expired? grace_period_deadline = current_user.otp_grace_period_started_at + two_factor_grace_period.hours - flash.now[:alert] = flash.now[:alert] + s_(" You need to do this before %{grace_period_deadline}.") % { grace_period_deadline: l(grace_period_deadline)} + flash.now[:alert] = flash.now[:alert] + s_(" You need to do this before %{grace_period_deadline}.") % { grace_period_deadline: l(grace_period_deadline) } end end @@ -127,4 +121,14 @@ def setup_u2f_registration def u2f_registration_params params.require(:u2f_registration).permit(:device_response, :name) end + + def groups_notification(groups) + group_links = groups.map { |group| view_context.link_to group.full_name, group_path(group) }.to_sentence + leave_group_links = groups.map { |group| view_context.link_to (s_("leave %{group_name}") % { group_name: group.full_name }), leave_group_members_path(group), remote: false, method: :delete}.join(s_(' or ')) + + s_(' + The group settings for %{group_links} require you to enable + Two-Factor Authentication for your account. You can %{leave_group_links}. + ').html_safe % { group_links: group_links.html_safe, leave_group_links: leave_group_links.html_safe } + end end -- GitLab From 6e422da93f04a47cea63d4f6c611ad9cbc7ad8ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Wed, 6 Mar 2019 11:11:39 +0100 Subject: [PATCH 07/12] Fix linter issues --- .../profiles/two_factor_auths_controller.rb | 6 ++---- locale/gitlab.pot | 10 +++------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb index d49a7ab766eead..2df7d9339de714 100644 --- a/app/controllers/profiles/two_factor_auths_controller.rb +++ b/app/controllers/profiles/two_factor_auths_controller.rb @@ -126,9 +126,7 @@ def groups_notification(groups) group_links = groups.map { |group| view_context.link_to group.full_name, group_path(group) }.to_sentence leave_group_links = groups.map { |group| view_context.link_to (s_("leave %{group_name}") % { group_name: group.full_name }), leave_group_members_path(group), remote: false, method: :delete}.join(s_(' or ')) - s_(' - The group settings for %{group_links} require you to enable - Two-Factor Authentication for your account. You can %{leave_group_links}. - ').html_safe % { group_links: group_links.html_safe, leave_group_links: leave_group_links.html_safe } + s_(%{The group settings for %{group_links} require you to enable Two-Factor Authentication for your account. You can %{leave_group_links}.}) + .html_safe % { group_links: group_links.html_safe, leave_group_links: leave_group_links.html_safe } end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d313f2431c2924..28ae007f4efb8b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -16,13 +16,6 @@ msgstr "" "Content-Transfer-Encoding: 8bit\n" "Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\n" -msgid "" -"\n" -" The group settings for %{group_links} require you to enable\n" -" Two-Factor Authentication for your account. You can %{leave_group_links}.\n" -" " -msgstr "" - msgid " Status" msgstr "" @@ -10070,6 +10063,9 @@ msgstr "" msgid "The global settings require you to enable Two-Factor Authentication for your account." msgstr "" +msgid "The group settings for %{group_links} require you to enable Two-Factor Authentication for your account. You can %{leave_group_links}." +msgstr "" + msgid "The import will time out after %{timeout}. For repositories that take longer, use a clone/push combination." msgstr "" -- GitLab From bfbfb426c1eef7604806d7750d738e1e8e17e539 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Thu, 7 Mar 2019 12:22:40 +0100 Subject: [PATCH 08/12] Add cr remarks --- app/controllers/profiles/two_factor_auths_controller.rb | 2 +- spec/features/users/login_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb index 2df7d9339de714..83e14275a8b020 100644 --- a/app/controllers/profiles/two_factor_auths_controller.rb +++ b/app/controllers/profiles/two_factor_auths_controller.rb @@ -124,7 +124,7 @@ def u2f_registration_params def groups_notification(groups) group_links = groups.map { |group| view_context.link_to group.full_name, group_path(group) }.to_sentence - leave_group_links = groups.map { |group| view_context.link_to (s_("leave %{group_name}") % { group_name: group.full_name }), leave_group_members_path(group), remote: false, method: :delete}.join(s_(' or ')) + leave_group_links = groups.map { |group| view_context.link_to (s_("leave %{group_name}") % { group_name: group.full_name }), leave_group_members_path(group), remote: false, method: :delete}.to_sentence s_(%{The group settings for %{group_links} require you to enable Two-Factor Authentication for your account. You can %{leave_group_links}.}) .html_safe % { group_links: group_links.html_safe, leave_group_links: leave_group_links.html_safe } diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index f6c599dc504bbd..3a82420b7bd5cd 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -443,7 +443,7 @@ def sign_in_using_saml! expect(page).to have_content( 'The group settings for Group 1 and Group 2 require you to enable ' \ 'Two-Factor Authentication for your account. '\ - 'You can leave Group 1 or leave Group 2. '\ + 'You can leave Group 1 and leave Group 2. '\ 'You need to do this ' \ 'before ') end -- GitLab From a7a84fe55dcf1d087b2583f6f7dc480862b9bb26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Fri, 8 Mar 2019 10:43:48 +0100 Subject: [PATCH 09/12] Add missing line --- app/controllers/groups/group_members_controller.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 58b3dfbe99d5b7..b0b5cfec803e9a 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -11,6 +11,7 @@ def self.admin_not_required_endpoints # Authorize before_action :authorize_admin_group_member!, except: admin_not_required_endpoints + skip_before_action :check_two_factor_requirement, only: :leave skip_cross_project_access_check :index, :create, :update, :destroy, :request_access, :approve_access_request, :leave, :resend_invite, -- GitLab From b8647fbc87c8c4e3631fe9f8a943154a31bc616e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Mon, 18 Mar 2019 09:30:44 +0100 Subject: [PATCH 10/12] Add merge request number to changelog --- changelogs/unreleased/do-not-force-2fa.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/unreleased/do-not-force-2fa.yml b/changelogs/unreleased/do-not-force-2fa.yml index 6f671673b2adef..f9be40e8f37697 100644 --- a/changelogs/unreleased/do-not-force-2fa.yml +++ b/changelogs/unreleased/do-not-force-2fa.yml @@ -1,6 +1,6 @@ --- title: Add link on two-factor authorization settings page to leave group that enforces two-factor authorization -merge_request: +merge_request: 25731 author: type: changed -- GitLab From f8aee37c13c8d76032c186f0093e91ab1707344b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Mon, 18 Mar 2019 09:40:50 +0100 Subject: [PATCH 11/12] Update specs --- spec/features/users/login_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index 3a82420b7bd5cd..b8e52f94588d70 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -502,7 +502,8 @@ def sign_in_using_saml! expect(current_path).to eq profile_two_factor_auth_path expect(page).to have_content( 'The group settings for Group 1 and Group 2 require you to enable ' \ - 'Two-Factor Authentication for your account.' + 'Two-Factor Authentication for your account. '\ + 'You can leave Group 1 and leave Group 2.' ) end end -- GitLab From 2a83e6653385f7e95dc12941527ca484c0e8ddce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Mon, 18 Mar 2019 10:11:19 +0100 Subject: [PATCH 12/12] Add spec to check time --- spec/features/users/login_spec.rb | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index b8e52f94588d70..368a814874f71d 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -434,18 +434,22 @@ def sign_in_using_saml! context 'within the grace period' do it 'redirects to two-factor configuration page' do - expect(authentication_metrics) - .to increment(:user_authenticated_counter) - - gitlab_sign_in(user) - - expect(current_path).to eq profile_two_factor_auth_path - expect(page).to have_content( - 'The group settings for Group 1 and Group 2 require you to enable ' \ - 'Two-Factor Authentication for your account. '\ - 'You can leave Group 1 and leave Group 2. '\ - 'You need to do this ' \ - 'before ') + Timecop.freeze do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + + expect(current_path).to eq profile_two_factor_auth_path + expect(page).to have_content( + 'The group settings for Group 1 and Group 2 require you to enable '\ + 'Two-Factor Authentication for your account. '\ + 'You can leave Group 1 and leave Group 2. '\ + 'You need to do this '\ + 'before '\ + "#{(Time.zone.now + 2.days).strftime("%a, %-d %b %Y %H:%M:%S %z")}" + ) + end end it 'allows skipping two-factor configuration', :js do -- GitLab