Commit 117e2842 authored by Robert Speicher's avatar Robert Speicher Committed by Ruben Davila

Merge branch '17334-u2f-device-identifiers' into 'master'

Allow naming (and deleting) U2F devices.

## What does this MR do?

- Allow giving each U2F device a name (at the time of registration).
- Allow deleting individual U2F devices.
- Display a list of registered U2F devices.

## What are the relevant issue numbers?

- Closes #17334 
- Closes #17335 

See merge request !5833
parent b07cd8ec
......@@ -20,6 +20,7 @@ v 8.11.0 (unreleased)
- Remove magic comments (`# encoding: UTF-8`) from Ruby files. !5456 (winniehell)
- GitLab Performance Monitoring can now track custom events such as the number of tags pushed to a repository
- Add support for relative links starting with ./ or / to RelativeLinkFilter (winniehell)
- Allow naming U2F devices !5833
- Ignore URLs starting with // in Markdown links !5677 (winniehell)
- Fix CI status icon link underline (ClemMakesApps)
- The Repository class is now instrumented
......
......@@ -228,3 +228,9 @@
}
}
}
table.u2f-registrations {
th:not(:last-child), td:not(:last-child) {
border-right: solid 1px transparent;
}
}
\ No newline at end of file
......@@ -43,11 +43,11 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
# A U2F (universal 2nd factor) device's information is stored after successful
# registration, which is then used while 2FA authentication is taking place.
def create_u2f
@u2f_registration = U2fRegistration.register(current_user, u2f_app_id, params[:device_response], session[:challenges])
@u2f_registration = U2fRegistration.register(current_user, u2f_app_id, u2f_registration_params, session[:challenges])
if @u2f_registration.persisted?
session.delete(:challenges)
redirect_to profile_account_path, notice: "Your U2F device was registered!"
redirect_to profile_two_factor_auth_path, notice: "Your U2F device was registered!"
else
@qr_code = build_qr_code
setup_u2f_registration
......@@ -91,15 +91,19 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
# Actual communication is performed using a Javascript API
def setup_u2f_registration
@u2f_registration ||= U2fRegistration.new
@registration_key_handles = current_user.u2f_registrations.pluck(:key_handle)
@u2f_registrations = current_user.u2f_registrations
u2f = U2F::U2F.new(u2f_app_id)
registration_requests = u2f.registration_requests
sign_requests = u2f.authentication_requests(@registration_key_handles)
sign_requests = u2f.authentication_requests(@u2f_registrations.map(&:key_handle))
session[:challenges] = registration_requests.map(&:challenge)
gon.push(u2f: { challenges: session[:challenges], app_id: u2f_app_id,
register_requests: registration_requests,
sign_requests: sign_requests })
end
def u2f_registration_params
params.require(:u2f_registration).permit(:device_response, :name)
end
end
class Profiles::U2fRegistrationsController < Profiles::ApplicationController
def destroy
u2f_registration = current_user.u2f_registrations.find(params[:id])
u2f_registration.destroy
redirect_to profile_two_factor_auth_path, notice: "Successfully deleted U2F device."
end
end
......@@ -3,18 +3,19 @@
class U2fRegistration < ActiveRecord::Base
belongs_to :user
def self.register(user, app_id, json_response, challenges)
def self.register(user, app_id, params, challenges)
u2f = U2F::U2F.new(app_id)
registration = self.new
begin
response = U2F::RegisterResponse.load_from_json(json_response)
response = U2F::RegisterResponse.load_from_json(params[:device_response])
registration_data = u2f.register!(challenges, response)
registration.update(certificate: registration_data.certificate,
key_handle: registration_data.key_handle,
public_key: registration_data.public_key,
counter: registration_data.counter,
user: user)
user: user,
name: params[:name])
rescue JSON::ParserError, NoMethodError, ArgumentError
registration.errors.add(:base, 'Your U2F device did not send a valid JSON response.')
rescue U2F::Error => e
......
......@@ -60,13 +60,38 @@
two-factor authentication app before a U2F device. That way you'll always be able to
log in - even when you're using an unsupported browser.
.col-lg-9
%p
- if @registration_key_handles.present?
= icon "check inverse", base: "circle", class: "text-success", text: "You have #{pluralize(@registration_key_handles.size, 'U2F device')} registered with GitLab."
- if @u2f_registration.errors.present?
= form_errors(@u2f_registration)
= render "u2f/register"
%hr
%h5 U2F Devices (#{@u2f_registrations.length})
- if @u2f_registrations.present?
.table-responsive
%table.table.table-bordered.u2f-registrations
%colgroup
%col{ width: "50%" }
%col{ width: "30%" }
%col{ width: "20%" }
%thead
%tr
%th Name
%th Registered On
%th
%tbody
- @u2f_registrations.each do |registration|
%tr
%td= registration.name.presence || "<no name set>"
%td= registration.created_at.to_date.to_s(:medium)
%td= link_to "Delete", profile_u2f_registration_path(registration), method: :delete, class: "btn btn-danger pull-right", data: { confirm: "Are you sure you want to delete this device? This action cannot be undone." }
- else
.settings-message.text-center
You don't have any U2F devices registered yet.
- if two_factor_skippable?
:javascript
var button = "<a class='btn btn-xs btn-warning pull-right' data-method='patch' href='#{skip_profile_two_factor_auth_path}'>Configure it later</a>";
......
......@@ -28,9 +28,14 @@
%script#js-register-u2f-registered{ type: "text/template" }
%div.row.append-bottom-10
%p Your device was successfully set up! Click this button to register with the GitLab server.
.col-md-12
%p Your device was successfully set up! Give it a name and register it with the GitLab server.
= form_tag(create_u2f_profile_two_factor_auth_path, method: :post) do
= hidden_field_tag :device_response, nil, class: 'form-control', required: true, id: "js-device-response"
.row.append-bottom-10
.col-md-3
= text_field_tag 'u2f_registration[name]', nil, class: 'form-control', placeholder: "Pick a name"
.col-md-3
= hidden_field_tag 'u2f_registration[device_response]', nil, class: 'form-control', required: true, id: "js-device-response"
= submit_tag "Register U2F Device", class: "btn btn-success"
:javascript
......
......@@ -375,6 +375,8 @@ Rails.application.routes.draw do
patch :skip
end
end
resources :u2f_registrations, only: [:destroy]
end
end
......
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddColumnNameToU2fRegistrations < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
# When a migration requires downtime you **must** uncomment the following
# constant and define a short and easy to understand explanation as to why the
# migration requires downtime.
# DOWNTIME_REASON = ''
# When using the methods "add_concurrent_index" or "add_column_with_default"
# you must disable the use of transactions as these methods can not run in an
# existing transaction. When using "add_concurrent_index" make sure that this
# method is the _only_ method called in the migration, any other changes
# should go in a separate migration. This ensures that upon failure _only_ the
# index creation fails and can be retried or reverted easily.
#
# To disable transactions uncomment the following line and remove these
# comments:
# disable_ddl_transaction!
def change
add_column :u2f_registrations, :name, :string
end
end
......@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20160810142633) do
ActiveRecord::Schema.define(version: 20160816161312) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
......@@ -1015,6 +1015,7 @@ ActiveRecord::Schema.define(version: 20160810142633) do
t.integer "user_id"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.string "name"
end
add_index "u2f_registrations", ["key_handle"], name: "index_u2f_registrations_on_key_handle", using: :btree
......
......@@ -12,10 +12,12 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
end
def register_u2f_device(u2f_device = nil)
u2f_device ||= FakeU2fDevice.new(page)
name = FFaker::Name.first_name
u2f_device ||= FakeU2fDevice.new(page, name)
u2f_device.respond_to_u2f_registration
click_on 'Setup New U2F Device'
expect(page).to have_content('Your device was successfully set up')
fill_in "Pick a name", with: name
click_on 'Register U2F Device'
u2f_device
end
......@@ -40,13 +42,14 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
end
describe 'when 2FA via OTP is enabled' do
it 'allows registering a new device' do
it 'allows registering a new device with a name' do
visit profile_account_path
manage_two_factor_authentication
expect(page.body).to match("You've already enabled two-factor authentication using mobile")
register_u2f_device
u2f_device = register_u2f_device
expect(page.body).to match(u2f_device.name)
expect(page.body).to match('Your U2F device was registered')
end
......@@ -55,15 +58,31 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
# First device
manage_two_factor_authentication
register_u2f_device
first_device = register_u2f_device
expect(page.body).to match('Your U2F device was registered')
# Second device
manage_two_factor_authentication
register_u2f_device
second_device = register_u2f_device
expect(page.body).to match('Your U2F device was registered')
expect(page.body).to match(first_device.name)
expect(page.body).to match(second_device.name)
expect(U2fRegistration.count).to eq(2)
end
it 'allows deleting a device' do
visit profile_account_path
manage_two_factor_authentication
expect(page.body).to match('You have 2 U2F devices registered')
expect(page.body).to match("You've already enabled two-factor authentication using mobile")
first_u2f_device = register_u2f_device
second_u2f_device = register_u2f_device
click_on "Delete", match: :first
expect(page.body).to match('Successfully deleted')
expect(page.body).not_to match(first_u2f_device.name)
expect(page.body).to match(second_u2f_device.name)
end
end
......@@ -208,7 +227,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
describe "when a given U2F device has not been registered" do
it "does not allow logging in with that particular device" do
unregistered_device = FakeU2fDevice.new(page)
unregistered_device = FakeU2fDevice.new(page, FFaker::Name.first_name)
login_as(user)
unregistered_device.respond_to_u2f_authentication
click_on "Login Via U2F Device"
......@@ -262,6 +281,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
end
it "deletes u2f registrations" do
visit profile_account_path
expect { click_on "Disable" }.to change { U2fRegistration.count }.by(-1)
end
end
......
class FakeU2fDevice
def initialize(page)
attr_reader :name
def initialize(page, name)
@page = page
@name = name
end
def respond_to_u2f_registration
......
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