From 7eb26c7ff7a78df9cb8fe5b30d48c80ce4eb8a99 Mon Sep 17 00:00:00 2001
From: Robin Bobbitt <ryehle@us.ibm.com>
Date: Mon, 12 Jun 2017 12:13:22 -0400
Subject: [PATCH] Provide hint to create a personal access token for Git over
HTTP
If internal auth is disabled and user is not an LDAP user, present
the user with an alert to create a personal access token if he does
not have one already.
---
app/helpers/button_helper.rb | 11 ++-
app/helpers/projects_helper.rb | 17 +++++
app/models/user.rb | 8 +-
app/views/shared/_no_password.html.haml | 7 +-
app/views/shared/_no_ssh.html.haml | 4 +-
.../pat-alert-when-signin-disabled.yml | 4 +
spec/features/login_spec.rb | 23 +-----
spec/features/projects/no_password_spec.rb | 69 +++++++++++++++++
spec/helpers/button_helper_spec.rb | 65 ++++++++++++++++
spec/helpers/projects_helper_spec.rb | 76 +++++++++++++++++++
spec/support/login_helpers.rb | 21 +++++
11 files changed, 274 insertions(+), 31 deletions(-)
create mode 100644 changelogs/unreleased/pat-alert-when-signin-disabled.yml
create mode 100644 spec/features/projects/no_password_spec.rb
create mode 100644 spec/helpers/button_helper_spec.rb
diff --git a/app/helpers/button_helper.rb b/app/helpers/button_helper.rb
index 00464810054..ba84dbe4a7a 100644
--- a/app/helpers/button_helper.rb
+++ b/app/helpers/button_helper.rb
@@ -50,10 +50,17 @@ module ButtonHelper
def http_clone_button(project, placement = 'right', append_link: true)
klass = 'http-selector'
- klass << ' has-tooltip' if current_user.try(:require_password?)
+ klass << ' has-tooltip' if current_user.try(:require_password?) || current_user.try(:require_personal_access_token?)
protocol = gitlab_config.protocol.upcase
+ tooltip_title =
+ if current_user.try(:require_password?)
+ _("Set a password on your account to pull or push via %{protocol}.") % { protocol: protocol }
+ else
+ _("Create a personal access token on your account to pull or push via %{protocol}.") % { protocol: protocol }
+ end
+
content_tag (append_link ? :a : :span), protocol,
class: klass,
href: (project.http_url_to_repo if append_link),
@@ -61,7 +68,7 @@ module ButtonHelper
html: true,
placement: placement,
container: 'body',
- title: _("Set a password on your account to pull or push via %{protocol}") % { protocol: protocol }
+ title: tooltip_title
}
end
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index d10e0bd45b0..c04b1419a19 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -198,6 +198,23 @@ module ProjectsHelper
.load_in_batch_for_projects(projects)
end
+ def show_no_ssh_key_message?
+ cookies[:hide_no_ssh_message].blank? && !current_user.hide_no_ssh_key && current_user.require_ssh_key?
+ end
+
+ def show_no_password_message?
+ cookies[:hide_no_password_message].blank? && !current_user.hide_no_password &&
+ ( current_user.require_password? || current_user.require_personal_access_token? )
+ end
+
+ def link_to_set_password
+ if current_user.require_password?
+ link_to s_('SetPasswordToCloneLink|set a password'), edit_profile_password_path
+ else
+ link_to s_('CreateTokenToCloneLink|create a personal access token'), profile_personal_access_tokens_path
+ end
+ end
+
private
def repo_children_classes(field)
diff --git a/app/models/user.rb b/app/models/user.rb
index 650b64e7551..6dd1b1415d6 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -570,7 +570,13 @@ class User < ActiveRecord::Base
end
def require_password?
- password_automatically_set? && !ldap_user?
+ password_automatically_set? && !ldap_user? && current_application_settings.signin_enabled?
+ end
+
+ def require_personal_access_token?
+ return false if current_application_settings.signin_enabled? || ldap_user?
+
+ PersonalAccessTokensFinder.new(user: self, impersonation: false, state: 'active').execute.none?
end
def can_change_username?
diff --git a/app/views/shared/_no_password.html.haml b/app/views/shared/_no_password.html.haml
index b561e6dc248..9b1a467df6b 100644
--- a/app/views/shared/_no_password.html.haml
+++ b/app/views/shared/_no_password.html.haml
@@ -1,9 +1,8 @@
-- if cookies[:hide_no_password_message].blank? && !current_user.hide_no_password && current_user.require_password?
+- if show_no_password_message?
.no-password-message.alert.alert-warning
- - set_password_link = link_to s_('SetPasswordToCloneLink|set a password'), edit_profile_password_path
- - translation_params = { protocol: gitlab_config.protocol.upcase, set_password_link: set_password_link }
+ - translation_params = { protocol: gitlab_config.protocol.upcase, set_password_link: link_to_set_password }
- set_password_message = _("You won't be able to pull or push project code via %{protocol} until you %{set_password_link} on your account") % translation_params
-
+ = set_password_message.html_safe
.alert-link-group
= link_to _("Don't show again"), profile_path(user: {hide_no_password: true}), method: :put
|
diff --git a/app/views/shared/_no_ssh.html.haml b/app/views/shared/_no_ssh.html.haml
index e7815e28017..17ef5327341 100644
--- a/app/views/shared/_no_ssh.html.haml
+++ b/app/views/shared/_no_ssh.html.haml
@@ -1,8 +1,8 @@
-- if cookies[:hide_no_ssh_message].blank? && !current_user.hide_no_ssh_key && current_user.require_ssh_key?
+- if show_no_ssh_key_message?
.no-ssh-key-message.alert.alert-warning
- add_ssh_key_link = link_to s_('MissingSSHKeyWarningLink|add an SSH key'), profile_keys_path, class: 'alert-link'
- ssh_message = _("You won't be able to pull or push project code via SSH until you %{add_ssh_key_link} to your profile") % { add_ssh_key_link: add_ssh_key_link }
- #{ ssh_message.html_safe }
+ = ssh_message.html_safe
.alert-link-group
= link_to _("Don't show again"), profile_path(user: {hide_no_ssh_key: true}), method: :put, class: 'alert-link'
|
diff --git a/changelogs/unreleased/pat-alert-when-signin-disabled.yml b/changelogs/unreleased/pat-alert-when-signin-disabled.yml
new file mode 100644
index 00000000000..dca3670aeb7
--- /dev/null
+++ b/changelogs/unreleased/pat-alert-when-signin-disabled.yml
@@ -0,0 +1,4 @@
+---
+title: Provide hint to create a personal access token for Git over HTTP
+merge_request: 12105
+author: Robin Bobbitt
diff --git a/spec/features/login_spec.rb b/spec/features/login_spec.rb
index 53b8ba5b0f7..a8055b21cee 100644
--- a/spec/features/login_spec.rb
+++ b/spec/features/login_spec.rb
@@ -143,29 +143,8 @@ feature 'Login', feature: true do
end
context 'logging in via OAuth' do
- def saml_config
- OpenStruct.new(name: 'saml', label: 'saml', args: {
- assertion_consumer_service_url: 'https://localhost:3443/users/auth/saml/callback',
- idp_cert_fingerprint: '26:43:2C:47:AF:F0:6B:D0:07:9C:AD:A3:74:FE:5D:94:5F:4E:9E:52',
- idp_sso_target_url: 'https://idp.example.com/sso/saml',
- issuer: 'https://localhost:3443/',
- name_identifier_format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient'
- })
- end
-
- def stub_omniauth_config(messages)
- Rails.application.env_config['devise.mapping'] = Devise.mappings[:user]
- Rails.application.routes.disable_clear_and_finalize = true
- Rails.application.routes.draw do
- post '/users/auth/saml' => 'omniauth_callbacks#saml'
- end
- allow(Gitlab::OAuth::Provider).to receive_messages(providers: [:saml], config_for: saml_config)
- allow(Gitlab.config.omniauth).to receive_messages(messages)
- expect_any_instance_of(Object).to receive(:omniauth_authorize_path).with(:user, "saml").and_return('/users/auth/saml')
- end
-
it 'shows 2FA prompt after OAuth login' do
- stub_omniauth_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [saml_config])
+ stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [mock_saml_config])
user = create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: 'saml')
gitlab_sign_in_via('saml', user, 'my-uid')
diff --git a/spec/features/projects/no_password_spec.rb b/spec/features/projects/no_password_spec.rb
new file mode 100644
index 00000000000..30a16e38e3c
--- /dev/null
+++ b/spec/features/projects/no_password_spec.rb
@@ -0,0 +1,69 @@
+require 'spec_helper'
+
+feature 'No Password Alert' do
+ let(:project) { create(:project, namespace: user.namespace) }
+
+ context 'with internal auth enabled' do
+ before do
+ sign_in(user)
+ visit namespace_project_path(project.namespace, project)
+ end
+
+ context 'when user has a password' do
+ let(:user) { create(:user) }
+
+ it 'shows no alert' do
+ expect(page).not_to have_content "You won't be able to pull or push project code via HTTP until you set a password on your account"
+ end
+ end
+
+ context 'when user has password automatically set' do
+ let(:user) { create(:user, password_automatically_set: true) }
+
+ it 'shows a password alert' do
+ expect(page).to have_content "You won't be able to pull or push project code via HTTP until you set a password on your account"
+ end
+ end
+ end
+
+ context 'with internal auth disabled' do
+ let(:user) { create(:omniauth_user, extern_uid: 'my-uid', provider: 'saml') }
+
+ before do
+ stub_application_setting(signin_enabled?: false)
+ stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [mock_saml_config])
+ end
+
+ context 'when user has no personal access tokens' do
+ it 'has a personal access token alert' do
+ gitlab_sign_in_via('saml', user, 'my-uid')
+ visit namespace_project_path(project.namespace, project)
+
+ expect(page).to have_content "You won't be able to pull or push project code via HTTP until you create a personal access token on your account"
+ end
+ end
+
+ context 'when user has a personal access token' do
+ it 'shows no alert' do
+ create(:personal_access_token, user: user)
+ gitlab_sign_in_via('saml', user, 'my-uid')
+ visit namespace_project_path(project.namespace, project)
+
+ expect(page).not_to have_content "You won't be able to pull or push project code via HTTP until you create a personal access token on your account"
+ end
+ end
+ end
+
+ context 'when user is ldap user' do
+ let(:user) { create(:omniauth_user, password_automatically_set: true) }
+
+ before do
+ sign_in(user)
+ visit namespace_project_path(project.namespace, project)
+ end
+
+ it 'shows no alert' do
+ expect(page).not_to have_content "You won't be able to pull or push project code via HTTP until you"
+ end
+ end
+end
diff --git a/spec/helpers/button_helper_spec.rb b/spec/helpers/button_helper_spec.rb
new file mode 100644
index 00000000000..661327d4432
--- /dev/null
+++ b/spec/helpers/button_helper_spec.rb
@@ -0,0 +1,65 @@
+require 'spec_helper'
+
+describe ButtonHelper do
+ describe 'http_clone_button' do
+ let(:user) { create(:user) }
+ let(:project) { create(:project) }
+ let(:has_tooltip_class) { 'has-tooltip' }
+
+ def element
+ element = helper.http_clone_button(project)
+
+ Nokogiri::HTML::DocumentFragment.parse(element).first_element_child
+ end
+
+ before do
+ allow(helper).to receive(:current_user).and_return(user)
+ end
+
+ context 'with internal auth enabled' do
+ context 'when user has a password' do
+ it 'shows no tooltip' do
+ expect(element.attr('class')).not_to include(has_tooltip_class)
+ end
+ end
+
+ context 'when user has password automatically set' do
+ let(:user) { create(:user, password_automatically_set: true) }
+
+ it 'shows a password tooltip' do
+ expect(element.attr('class')).to include(has_tooltip_class)
+ expect(element.attr('data-title')).to eq('Set a password on your account to pull or push via HTTP.')
+ end
+ end
+ end
+
+ context 'with internal auth disabled' do
+ before do
+ stub_application_setting(signin_enabled?: false)
+ end
+
+ context 'when user has no personal access tokens' do
+ it 'has a personal access token tooltip ' do
+ expect(element.attr('class')).to include(has_tooltip_class)
+ expect(element.attr('data-title')).to eq('Create a personal access token on your account to pull or push via HTTP.')
+ end
+ end
+
+ context 'when user has a personal access token' do
+ it 'shows no tooltip' do
+ create(:personal_access_token, user: user)
+
+ expect(element.attr('class')).not_to include(has_tooltip_class)
+ end
+ end
+ end
+
+ context 'when user is ldap user' do
+ let(:user) { create(:omniauth_user, password_automatically_set: true) }
+
+ it 'shows no tooltip' do
+ expect(element.attr('class')).not_to include(has_tooltip_class)
+ end
+ end
+ end
+end
diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb
index 9a4086725d2..487d9800707 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/spec/helpers/projects_helper_spec.rb
@@ -115,6 +115,82 @@ describe ProjectsHelper do
end
end
+ describe '#show_no_ssh_key_message?' do
+ let(:user) { create(:user) }
+
+ before do
+ allow(helper).to receive(:current_user).and_return(user)
+ end
+
+ context 'user has no keys' do
+ it 'returns true' do
+ expect(helper.show_no_ssh_key_message?).to be_truthy
+ end
+ end
+
+ context 'user has an ssh key' do
+ it 'returns false' do
+ create(:personal_key, user: user)
+
+ expect(helper.show_no_ssh_key_message?).to be_falsey
+ end
+ end
+ end
+
+ describe '#show_no_password_message?' do
+ let(:user) { create(:user) }
+
+ before do
+ allow(helper).to receive(:current_user).and_return(user)
+ end
+
+ context 'user has password set' do
+ it 'returns false' do
+ expect(helper.show_no_password_message?).to be_falsey
+ end
+ end
+
+ context 'user requires a password' do
+ let(:user) { create(:user, password_automatically_set: true) }
+
+ it 'returns true' do
+ expect(helper.show_no_password_message?).to be_truthy
+ end
+ end
+
+ context 'user requires a personal access token' do
+ it 'returns true' do
+ stub_application_setting(signin_enabled?: false)
+
+ expect(helper.show_no_password_message?).to be_truthy
+ end
+ end
+ end
+
+ describe '#link_to_set_password' do
+ before do
+ allow(helper).to receive(:current_user).and_return(user)
+ end
+
+ context 'user requires a password' do
+ let(:user) { create(:user, password_automatically_set: true) }
+
+ it 'returns link to set a password' do
+ expect(helper.link_to_set_password).to match %r{<a href="#{edit_profile_password_path}">set a password</a>}
+ end
+ end
+
+ context 'user requires a personal access token' do
+ let(:user) { create(:user) }
+
+ it 'returns link to create a personal access token' do
+ stub_application_setting(signin_enabled?: false)
+
+ expect(helper.link_to_set_password).to match %r{<a href="#{profile_personal_access_tokens_path}">create a personal access token</a>}
+ end
+ end
+ end
+
describe 'link_to_member' do
let(:group) { create(:group) }
let(:project) { create(:empty_project, group: group) }
diff --git a/spec/support/login_helpers.rb b/spec/support/login_helpers.rb
index 879386b5437..4c88958264b 100644
--- a/spec/support/login_helpers.rb
+++ b/spec/support/login_helpers.rb
@@ -89,4 +89,25 @@ module LoginHelpers
})
Rails.application.env_config['omniauth.auth'] = OmniAuth.config.mock_auth[:saml]
end
+
+ def mock_saml_config
+ OpenStruct.new(name: 'saml', label: 'saml', args: {
+ assertion_consumer_service_url: 'https://localhost:3443/users/auth/saml/callback',
+ idp_cert_fingerprint: '26:43:2C:47:AF:F0:6B:D0:07:9C:AD:A3:74:FE:5D:94:5F:4E:9E:52',
+ idp_sso_target_url: 'https://idp.example.com/sso/saml',
+ issuer: 'https://localhost:3443/',
+ name_identifier_format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient'
+ })
+ end
+
+ def stub_omniauth_saml_config(messages)
+ Rails.application.env_config['devise.mapping'] = Devise.mappings[:user]
+ Rails.application.routes.disable_clear_and_finalize = true
+ Rails.application.routes.draw do
+ post '/users/auth/saml' => 'omniauth_callbacks#saml'
+ end
+ allow(Gitlab::OAuth::Provider).to receive_messages(providers: [:saml], config_for: mock_saml_config)
+ stub_omniauth_setting(messages)
+ expect_any_instance_of(Object).to receive(:omniauth_authorize_path).with(:user, "saml").and_return('/users/auth/saml')
+ end
end
--
GitLab