Skip to content
Snippets Groups Projects
Commit 71509035 authored by Eduardo Sanz García's avatar Eduardo Sanz García :zero:
Browse files

Remove `webauthn_without_totp` feature flag

We deemed the `webauthn_without_totp` feature flag safe after it was
rollout for Gitlab.com.  See:
#386270

Now we enable this new feature by default and delete the feature flag.

Closes #396931

Changelog: changed
parent 79c7e438
No related branches found
No related tags found
No related merge requests found
Showing
with 33 additions and 426 deletions
......@@ -19,7 +19,7 @@ def create
::Users::ValidateManualOtpService.new(current_user).execute(params[:pin_code])
validated = (otp_validation_result[:status] == :success)
if validated && current_user.otp_backup_codes? && Feature.enabled?(:webauthn_without_totp)
if validated && current_user.otp_backup_codes?
ActiveSession.destroy_all_but_current(current_user, session)
Users::UpdateService.new(current_user, user: current_user, otp_required_for_login: true).execute!
redirect_to profile_two_factor_auth_path, notice: _("Your Time-based OTP device was registered!")
......@@ -50,21 +50,16 @@ def create_webauthn
if @webauthn_registration.persisted?
session.delete(:challenge)
if Feature.enabled?(:webauthn_without_totp)
if current_user.otp_backup_codes?
redirect_to profile_two_factor_auth_path, notice: notice
else
if current_user.otp_backup_codes?
redirect_to profile_two_factor_auth_path, notice: notice
else
Users::UpdateService.new(current_user, user: current_user).execute! do |user|
@codes = current_user.generate_otp_backup_codes!
end
helpers.dismiss_two_factor_auth_recovery_settings_check
flash[:notice] = notice
render 'create'
Users::UpdateService.new(current_user, user: current_user).execute! do |user|
@codes = current_user.generate_otp_backup_codes!
end
else
redirect_to profile_two_factor_auth_path, notice: notice
helpers.dismiss_two_factor_auth_recovery_settings_check
flash[:notice] = notice
render 'create'
end
else
@qr_code = build_qr_code
......@@ -117,7 +112,6 @@ def update_current_user_otp!
end
def validate_current_password
return if Feature.disabled?(:webauthn_without_totp) && params[:action] == 'create_webauthn'
return if current_user.valid_password?(params[:current_password])
current_user.increment_failed_attempts!
......
- if Feature.enabled?(:webauthn_without_totp)
#js-device-registration{ data: device_registration_data(current_password_required: current_password_required?, target_path: target_path, webauthn_error: @webauthn_error) }
- else
#js-register-token-2fa
-# haml-lint:disable InlineJavaScript
%script#js-register-2fa-message{ type: "text/template" }
%p <%= message %>
-# haml-lint:disable InlineJavaScript
%script#js-register-token-2fa-setup{ type: "text/template" }
- if current_user.two_factor_otp_enabled?
.row.gl-mb-3
.col-md-5
= render Pajamas::ButtonComponent.new(variant: :confirm,
button_options: { id: 'js-setup-token-2fa-device' }) do
= _("Set up new device")
.col-md-7
%p= _("Your device needs to be set up. Plug it in (if needed) and click the button on the left.")
- else
.row.gl-mb-3
.col-md-4
= render Pajamas::ButtonComponent.new(variant: :confirm,
disabled: true,
button_options: { id: 'js-setup-token-2fa-device' }) do
= _("Set up new device")
.col-md-8
%p= _("You need to register a two-factor authentication app before you can set up a device.")
-# haml-lint:disable InlineJavaScript
%script#js-register-token-2fa-error{ type: "text/template" }
%div
%p
%span <%= error_message %> (<%= error_name %>)
= render Pajamas::ButtonComponent.new(button_options: { id: 'js-token-2fa-try-again' }) do
= _("Try again?")
-# haml-lint:disable InlineJavaScript
%script#js-register-token-2fa-registered{ type: "text/template" }
.row.gl-mb-3
.col-md-12
%p= _("Your device was successfully set up! Give it a name and register it with the GitLab server.")
= form_tag(target_path, method: :post) do
.row.gl-mb-3
.col-md-3
= text_field_tag 'device_registration[name]', nil, class: 'form-control', placeholder: _("Pick a name")
.col-md-3
= hidden_field_tag 'device_registration[device_response]', nil, class: 'form-control', required: true, id: "js-device-response"
= render Pajamas::ButtonComponent.new(type: :submit, variant: :confirm) do
= _("Register device")
#js-device-registration{ data: device_registration_data(current_password_required: current_password_required?, target_path: target_path, webauthn_error: @webauthn_error) }
......@@ -2,7 +2,7 @@
= render 'devise/shared/tab_single', tab_title: _('Two-Factor Authentication') if Feature.disabled?(:restyle_login_page, @project)
.login-box.gl-p-5
.login-body
- if @user.two_factor_otp_enabled? || (Feature.enabled?(:webauthn_without_totp) && @user.two_factor_enabled?)
- if @user.two_factor_otp_enabled? || @user.two_factor_enabled?
= gitlab_ui_form_for(resource, as: resource_name, url: session_path(resource_name), method: :post, html: { class: "edit_user gl-show-field-errors js-2fa-form #{'hidden' if @user.two_factor_webauthn_enabled?}" }) do |f|
- resource_params = params[resource_name].presence || params
= f.hidden_field :remember_me, value: resource_params.fetch(:remember_me, 0)
......
......@@ -66,10 +66,7 @@
%p
= _('Set up a hardware device to enable two-factor authentication (2FA).')
%p
- if Feature.enabled?(:webauthn_without_totp)
= _("Not all browsers support WebAuthn. You must save your recovery codes after you first register a two-factor authenticator to be able to sign in, even from an unsupported browser.")
- else
= _("Not all browsers support WebAuthn. Therefore, we require that you set up a two-factor authentication app first. That way you'll always be able to sign in, even from an unsupported browser.")
= _("Not all browsers support WebAuthn. You must save your recovery codes after you first register a two-factor authenticator to be able to sign in, even from an unsupported browser.")
.col-lg-8
- if @webauthn_registration.errors.present?
= form_errors(@webauthn_registration)
......
---
name: webauthn_without_totp
introduced_by_url: "https://gitlab.com/gitlab-org/gitlab/-/merge_requests/107438"
rollout_issue_url: "https://gitlab.com/gitlab-org/gitlab/-/issues/386270"
milestone: '15.9'
type: development
group: group::authentication and authorization
default_enabled: false
......@@ -264,7 +264,6 @@ Configure FortiToken Cloud in GitLab. On your GitLab server:
> - WebAuthn devices [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/22506) in GitLab 13.4 [with a flag](../../../administration/feature_flags.md) named `webauthn`. Disabled by default.
> - WebAuthn devices [enabled on GitLab.com and self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/232671) in GitLab 14.6.
> - Optional one-time password authentication for WebAuthn devices [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/378844) in GitLab 15.10 [with a flag](../../../administration/feature_flags.md) named `webauthn_without_topt`. [Enabled on GitLab.com and self-managed by default](https://gitlab.com/gitlab-org/gitlab/-/issues/232671).
FLAG:
On self-managed GitLab, by default, WebAuthn devices are available. To disable the feature, ask an administrator to
......@@ -272,10 +271,6 @@ On self-managed GitLab, by default, WebAuthn devices are available. To disable t
feature flag after WebAuthn devices have been registered, these devices are not usable until you re-enable this feature.
On GitLab.com, WebAuthn devices are available.
FLAG:
On self-managed GitLab, by default, optional one-time password authentication for WebAuthn devices is not available. To enable the feature, ask an administrator to [enable the feature flag](../../../administration/feature_flags.md) named `webauthn_without_totp`.
On GitLab.com, this feature is available.
WebAuthn is [supported by](https://caniuse.com/#search=webauthn) the following:
- Desktop browsers:
......
......@@ -201,51 +201,6 @@ def go
expect(ActiveSession).to receive(:destroy_all_but_current)
go
end
context 'when webauthn_without_totp flag is disabled' do
before do
stub_feature_flags(webauthn_without_totp: false)
expect(user).to receive(:validate_and_consume_otp!).with(pin).and_return(true)
end
it 'enables 2fa for the user' do
go
user.reload
expect(user).to be_two_factor_enabled
end
it 'presents plaintext codes for the user to save' do
expect(user).to receive(:generate_otp_backup_codes!).and_return(%w(a b c))
go
expect(assigns[:codes]).to match_array %w(a b c)
end
it 'calls to delete other sessions' do
expect(ActiveSession).to receive(:destroy_all_but_current)
go
end
it 'dismisses the `TWO_FACTOR_AUTH_RECOVERY_SETTINGS_CHECK` callout' do
expect(controller.helpers).to receive(:dismiss_two_factor_auth_recovery_settings_check)
go
end
it 'renders create' do
go
expect(response).to render_template(:create)
end
it 'renders create even if backup code already exists' do
expect(user).to receive(:otp_backup_codes?).and_return(true)
go
expect(response).to render_template(:create)
end
end
end
context 'with invalid pin' do
......@@ -363,22 +318,6 @@ def device_response
expect(flash[:notice]).to match(/Your WebAuthn device was registered!/)
end
end
context "when the feature flag 'webauthn_without_totp' is disabled" do
before do
stub_feature_flags(webauthn_without_totp: false)
session[:challenge] = challenge
end
let(:params) { { device_registration: { name: 'touch id', device_response: device_response } } } # rubocop:disable Rails/SaveBang
it "does not validate the current_password" do
go
expect(flash[:notice]).to match(/Your WebAuthn device was registered!/)
expect(response).to redirect_to(profile_two_factor_auth_path)
end
end
end
describe 'DELETE destroy' do
......
......@@ -11,8 +11,7 @@
end
context 'when the webauth_without_totp feature flag is enabled' do
# Some of the shared tests don't apply. After removing U2F support and the `webauthn_without_totp` feature flag, refactor the shared tests.
# TODO: it_behaves_like 'hardware device for 2fa', 'WebAuthn'
it_behaves_like 'hardware device for 2fa', 'WebAuthn'
describe 'registration' do
let(:user) { create(:user) }
......@@ -117,112 +116,6 @@
end
end
context 'when the webauth_without_totp feature flag is disabled' do
before do
stub_feature_flags(webauthn_without_totp: false)
end
it_behaves_like 'hardware device for 2fa', 'WebAuthn'
describe 'registration' do
let(:user) { create(:user) }
before do
gitlab_sign_in(user)
user.update_attribute(:otp_required_for_login, true)
end
describe 'when 2FA via OTP is enabled' do
it 'allows registering more than one device' do
visit profile_account_path
# First device
manage_two_factor_authentication
first_device = register_webauthn_device
expect(page).to have_content('Your WebAuthn device was registered')
# Second device
second_device = register_webauthn_device(name: 'My other device')
expect(page).to have_content('Your WebAuthn device was registered')
expect(page).to have_content(first_device.name)
expect(page).to have_content(second_device.name)
expect(WebauthnRegistration.count).to eq(2)
end
end
it 'allows the same device to be registered for multiple users' do
# First user
visit profile_account_path
manage_two_factor_authentication
webauthn_device = register_webauthn_device
expect(page).to have_content('Your WebAuthn device was registered')
gitlab_sign_out
# Second user
user = gitlab_sign_in(:user)
user.update_attribute(:otp_required_for_login, true)
visit profile_account_path
manage_two_factor_authentication
register_webauthn_device(webauthn_device, name: 'My other device')
expect(page).to have_content('Your WebAuthn device was registered')
expect(WebauthnRegistration.count).to eq(2)
end
context 'when there are form errors' do
let(:mock_register_js) do
<<~JS
const mockResponse = {
type: 'public-key',
id: '',
rawId: '',
response: {
clientDataJSON: '',
attestationObject: '',
},
getClientExtensionResults: () => {},
};
navigator.credentials.create = function(_) {return Promise.resolve(mockResponse);}
JS
end
it "doesn't register the device if there are errors" do
visit profile_account_path
manage_two_factor_authentication
# Have the "webauthn device" respond with bad data
page.execute_script(mock_register_js)
click_on 'Set up new device'
expect(page).to have_content('Your device was successfully set up')
click_on 'Register device'
expect(WebauthnRegistration.count).to eq(0)
expect(page).to have_content('The form contains the following error')
expect(page).to have_content('did not send a valid JSON response')
end
it 'allows retrying registration' do
visit profile_account_path
manage_two_factor_authentication
# Failed registration
page.execute_script(mock_register_js)
click_on 'Set up new device'
expect(page).to have_content('Your device was successfully set up')
click_on 'Register device'
expect(page).to have_content('The form contains the following error')
# Successful registration
register_webauthn_device
expect(page).to have_content('Your WebAuthn device was registered')
expect(WebauthnRegistration.count).to eq(1)
end
end
end
end
describe 'authentication' do
let(:otp_required_for_login) { true }
let(:user) { create(:user, webauthn_xid: WebAuthn.generate_user_id, otp_required_for_login: otp_required_for_login) }
......@@ -278,7 +171,7 @@
current_user = gitlab_sign_in(:user)
visit profile_account_path
manage_two_factor_authentication
register_webauthn_device(webauthn_device)
webauthn_device_registration(webauthn_device, password: current_user.password)
gitlab_sign_out
# Try authenticating user with the same WebAuthn device
......@@ -308,7 +201,7 @@
# Authenticate as both devices
[first_device, second_device].each do |device|
gitlab_sign_in(user)
# register_webauthn_device(device)
# webauthn_device_registration(password: user.password)
device.respond_to_webauthn_authentication
expect(page).to have_css('.sign-out-link', visible: false)
......
import $ from 'jquery';
import { loadHTMLFixture, resetHTMLFixture } from 'helpers/fixtures';
import { trimText } from 'helpers/text_helper';
import setWindowLocation from 'helpers/set_window_location_helper';
import waitForPromises from 'helpers/wait_for_promises';
import WebAuthnRegister from '~/authentication/webauthn/register';
import MockWebAuthnDevice from './mock_webauthn_device';
import { useMockNavigatorCredentials } from './util';
describe('WebAuthnRegister', () => {
useMockNavigatorCredentials();
const mockResponse = {
type: 'public-key',
id: '',
rawId: '',
response: {
clientDataJSON: '',
attestationObject: '',
},
getClientExtensionResults: () => {},
};
let webAuthnDevice;
let container;
let component;
beforeEach(() => {
loadHTMLFixture('webauthn/register.html');
webAuthnDevice = new MockWebAuthnDevice();
container = $('#js-register-token-2fa');
component = new WebAuthnRegister(container, {
options: {
rp: '',
user: {
id: '',
name: '',
displayName: '',
},
challenge: '',
pubKeyCredParams: '',
},
});
component.start();
});
afterEach(() => {
resetHTMLFixture();
});
const findSetupButton = () => container.find('#js-setup-token-2fa-device');
const findMessage = () => container.find('p');
const findDeviceResponse = () => container.find('#js-device-response');
const findRetryButton = () => container.find('#js-token-2fa-try-again');
it('shows setup button', () => {
expect(trimText(findSetupButton().text())).toBe('Set up new device');
});
describe('when unsupported', () => {
const { PublicKeyCredential } = window;
beforeEach(() => {
delete window.credentials;
window.PublicKeyCredential = undefined;
});
afterEach(() => {
window.PublicKeyCredential = PublicKeyCredential;
});
it.each`
httpsEnabled | expectedText
${false} | ${'WebAuthn only works with HTTPS-enabled websites'}
${true} | ${'Please use a supported browser, e.g. Chrome (67+) or Firefox'}
`('when https is $httpsEnabled', ({ httpsEnabled, expectedText }) => {
setWindowLocation(`${httpsEnabled ? 'https:' : 'http:'}//localhost`);
component.start();
expect(findMessage().text()).toContain(expectedText);
});
});
describe('when setup', () => {
beforeEach(() => {
findSetupButton().trigger('click');
});
it('shows in progress message', () => {
expect(findMessage().text()).toContain('Trying to communicate with your device');
});
it('registers device', () => {
webAuthnDevice.respondToRegisterRequest(mockResponse);
return waitForPromises().then(() => {
expect(findMessage().text()).toContain('Your device was successfully set up!');
expect(findDeviceResponse().val()).toBe(JSON.stringify(mockResponse));
});
});
it.each`
errorName | expectedText
${'NotSupportedError'} | ${'Your device is not compatible with GitLab'}
${'NotAllowedError'} | ${'There was a problem communicating with your device'}
`('when fails with $errorName', ({ errorName, expectedText }) => {
webAuthnDevice.rejectRegisterRequest(new DOMException('', errorName));
return waitForPromises().then(() => {
expect(findMessage().text()).toContain(expectedText);
expect(findRetryButton().length).toBe(1);
});
});
it('can retry', () => {
webAuthnDevice.respondToRegisterRequest({
errorCode: 'error!',
});
return waitForPromises()
.then(() => {
findRetryButton().click();
expect(findMessage().text()).toContain('Trying to communicate with your device');
webAuthnDevice.respondToRegisterRequest(mockResponse);
return waitForPromises();
})
.then(() => {
expect(findMessage().text()).toContain('Your device was successfully set up!');
expect(findDeviceResponse().val()).toBe(JSON.stringify(mockResponse));
});
});
});
});
......@@ -23,22 +23,4 @@
expect(response).to be_successful
end
end
describe Profiles::TwoFactorAuthsController, '(JavaScript fixtures)', type: :controller do
render_views
before do
sign_in(user)
allow_next_instance_of(Profiles::TwoFactorAuthsController) do |instance|
allow(instance).to receive(:build_qr_code).and_return('qrcode:blackandwhitesquares')
end
stub_feature_flags(webauthn_without_totp: false)
end
it 'webauthn/register.html' do
get :show
expect(response).to be_successful
end
end
end
......@@ -29,17 +29,6 @@ def manage_two_factor_authentication
end
# Registers webauthn device via UI
# Remove after `webauthn_without_totp` feature flag is deleted.
def register_webauthn_device(webauthn_device = nil, name: 'My device')
webauthn_device ||= FakeWebauthnDevice.new(page, name)
webauthn_device.respond_to_webauthn_registration
click_on 'Set up new device'
expect(page).to have_content('Your device was successfully set up')
fill_in 'Pick a name', with: name
click_on 'Register device'
webauthn_device
end
def webauthn_device_registration(webauthn_device: nil, name: 'My device', password: 'fake')
webauthn_device ||= FakeWebauthnDevice.new(page, name)
webauthn_device.respond_to_webauthn_registration
......
......@@ -7,7 +7,7 @@
def register_device(device_type, **kwargs)
case device_type.downcase
when "webauthn"
register_webauthn_device(**kwargs)
webauthn_device_registration(**kwargs)
else
raise "Unknown device type #{device_type}"
end
......@@ -26,11 +26,16 @@ def register_device(device_type, **kwargs)
user.update_attribute(:otp_required_for_login, false)
end
it 'does not allow registering a new device' do
it 'allows registering a new device' do
visit profile_account_path
click_on 'Enable two-factor authentication'
click_on _('Enable two-factor authentication')
expect(page).to have_button("Set up new device", disabled: true)
device = register_device(device_type, password: user.password)
expect(page).to have_content("Your #{device_type} device was registered")
copy_recovery_codes
manage_two_factor_authentication
expect(page).to have_content(device.name)
end
end
......@@ -40,10 +45,12 @@ def register_device(device_type, **kwargs)
manage_two_factor_authentication
expect(page).to have_content("You've already enabled two-factor authentication using one time password authenticators")
device = register_device(device_type)
device = register_device(device_type, password: user.password)
expect(page).to have_content("Your #{device_type} device was registered")
copy_recovery_codes
manage_two_factor_authentication
expect(page).to have_content(device.name)
expect(page).to have_content("Your #{device_type} device was registered")
end
it 'allows deleting a device' do
......@@ -51,8 +58,10 @@ def register_device(device_type, **kwargs)
manage_two_factor_authentication
expect(page).to have_content("You've already enabled two-factor authentication using one time password authenticators")
first_device = register_device(device_type)
second_device = register_device(device_type, name: 'My other device')
first_device = register_device(device_type, password: user.password)
copy_recovery_codes
manage_two_factor_authentication
second_device = register_device(device_type, name: 'My other device', password: user.password)
expect(page).to have_content(first_device.name)
expect(page).to have_content(second_device.name)
......@@ -90,7 +99,7 @@ def register_device(device_type, **kwargs)
describe 'when a device is registered' do
before do
manage_two_factor_authentication
register_device(device_type)
register_device(device_type, password: user.password)
gitlab_sign_out
gitlab_sign_in(user)
end
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment