From bafbf22c6ab613d25287d7119d7e30770c531fdb Mon Sep 17 00:00:00 2001
From: Timothy Andrew <mail@timothyandrew.net>
Date: Mon, 25 Apr 2016 14:30:59 +0530
Subject: [PATCH] Address @DouweM's feedback on !3749.
- Use `TokenAuthenticatable` to generate the personal access token
- Remove a check for `authenticity_token` in application controller;
this should've been `authentication_token`, maybe, and doesn't make
any sense now.
- Have the datepicker appear inline
---
app/controllers/application_controller.rb | 2 +-
app/models/personal_access_token.rb | 5 ++++-
.../personal_access_tokens/index.html.haml | 18 ++++--------------
.../controllers/application_controller_spec.rb | 6 ------
.../profiles/personal_access_tokens_spec.rb | 18 +++++++++++-------
spec/models/personal_access_token_spec.rb | 15 +++++++++++++++
6 files changed, 35 insertions(+), 29 deletions(-)
create mode 100644 spec/models/personal_access_token_spec.rb
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 2b2726c048c..eb5ffc44c3b 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -67,7 +67,7 @@ class ApplicationController < ActionController::Base
# From https://github.com/plataformatec/devise/wiki/How-To:-Simple-Token-Authentication-Example
# https://gist.github.com/josevalim/fb706b1e933ef01e4fb6
def authenticate_user_from_private_token!
- user_token = params[:authenticity_token].presence || params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence
+ user_token = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence
user = user_token && User.find_by_authentication_token(user_token.to_s)
if user
diff --git a/app/models/personal_access_token.rb b/app/models/personal_access_token.rb
index c7c3932ba40..c4b095e0c04 100644
--- a/app/models/personal_access_token.rb
+++ b/app/models/personal_access_token.rb
@@ -1,4 +1,7 @@
class PersonalAccessToken < ActiveRecord::Base
+ include TokenAuthenticatable
+ add_authentication_token_field :token
+
belongs_to :user
scope :active, -> { where(revoked: false).where("expires_at >= NOW() OR expires_at IS NULL") }
@@ -6,7 +9,7 @@ class PersonalAccessToken < ActiveRecord::Base
def self.generate(params)
personal_access_token = self.new(params)
- personal_access_token.token = Devise.friendly_token(50)
+ personal_access_token.ensure_token
personal_access_token
end
diff --git a/app/views/profiles/personal_access_tokens/index.html.haml b/app/views/profiles/personal_access_tokens/index.html.haml
index 02800c37917..f7482d2c87d 100644
--- a/app/views/profiles/personal_access_tokens/index.html.haml
+++ b/app/views/profiles/personal_access_tokens/index.html.haml
@@ -21,7 +21,8 @@
.form-group
= f.label :expires_at, class: 'label-light'
- = f.text_field :expires_at, class: "form-control datepicker", required: false
+ = f.hidden_field :expires_at, class: "form-control", required: false
+ .datepicker
.prepend-top-default
= f.submit 'Add Personal Access Token', class: "btn btn-create"
@@ -90,16 +91,5 @@
:javascript
$(".datepicker").datepicker({
dateFormat: "yy-mm-dd",
- beforeShow: function() {
- ////////////////////////////////////////////////////////////////
- // 1. Need the setTimeout because the datepicker doesn't have //
- // an `afterShow` callback. //
- // 2. Need to set the z-index like this because we don't want //
- // to target datepickers outside the current page, which //
- // will happen if we set this in CSS directly. //
- ////////////////////////////////////////////////////////////////
- setTimeout(function(){
- $('.ui-datepicker').css('z-index', 3);
- }, 0);
- }
- });
+ onSelect: function(dateText, inst) { $("#personal_access_token_params_expires_at").val(dateText) }
+ }).datepicker("setDate", $.datepicker.parseDate('yy-mm-dd', $('#personal_access_token_params_expires_at').val()));
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb
index 1ec51465cd3..e8bdbf1afb7 100644
--- a/spec/controllers/application_controller_spec.rb
+++ b/spec/controllers/application_controller_spec.rb
@@ -40,12 +40,6 @@ describe ApplicationController do
let(:user) { create(:user) }
- it "logs the user in when the 'authenticity_token' param is populated with the private token" do
- get :index, authenticity_token: user.private_token
- expect(response.status).to eq(200)
- expect(response.body).to eq("authenticated")
- end
-
it "logs the user in when the 'private_token' param is populated with the private token" do
get :index, private_token: user.private_token
expect(response.status).to eq(200)
diff --git a/spec/features/profiles/personal_access_tokens_spec.rb b/spec/features/profiles/personal_access_tokens_spec.rb
index ce972776ffe..e9fbeefae75 100644
--- a/spec/features/profiles/personal_access_tokens_spec.rb
+++ b/spec/features/profiles/personal_access_tokens_spec.rb
@@ -1,6 +1,6 @@
require 'spec_helper'
-describe 'Profile > Personal Access Tokens', feature: true do
+describe 'Profile > Personal Access Tokens', feature: true, js: true do
let(:user) { create(:user) }
before do
@@ -13,18 +13,22 @@ describe 'Profile > Personal Access Tokens', feature: true do
fill_in "Name", with: FFaker::Product.brand
expect {click_on "Add Personal Access Token"}.to change { PersonalAccessToken.count }.by(1)
- active_personal_access_tokens = find(".table.active-personal-access-tokens").native.inner_html
+ active_personal_access_tokens = find(".table.active-personal-access-tokens").native['innerHTML']
expect(active_personal_access_tokens).to match(PersonalAccessToken.last.name)
expect(active_personal_access_tokens).to match("Never")
expect(active_personal_access_tokens).to match(PersonalAccessToken.last.token)
fill_in "Name", with: FFaker::Product.brand
- fill_in "Expires at", with: 5.days.from_now
+
+ # Set date to 1st of next month
+ find("a[title='Next']").click
+ click_on "1"
+
expect {click_on "Add Personal Access Token"}.to change { PersonalAccessToken.count }.by(1)
- active_personal_access_tokens = find(".table.active-personal-access-tokens").native.inner_html
+ active_personal_access_tokens = find(".table.active-personal-access-tokens").native['innerHTML']
expect(active_personal_access_tokens).to match(PersonalAccessToken.last.name)
- expect(active_personal_access_tokens).to match(5.days.from_now.to_date.to_s)
+ expect(active_personal_access_tokens).to match(Date.today.next_month.at_beginning_of_month.to_s)
expect(active_personal_access_tokens).to match(PersonalAccessToken.last.token)
end
end
@@ -35,7 +39,7 @@ describe 'Profile > Personal Access Tokens', feature: true do
visit profile_personal_access_tokens_path
click_on "Revoke"
- inactive_personal_access_tokens = find(".table.inactive-personal-access-tokens").native.inner_html
+ inactive_personal_access_tokens = find(".table.inactive-personal-access-tokens").native['innerHTML']
expect(inactive_personal_access_tokens).to match(personal_access_token.name)
expect(inactive_personal_access_tokens).to match(personal_access_token.token)
end
@@ -44,7 +48,7 @@ describe 'Profile > Personal Access Tokens', feature: true do
personal_access_token = create(:personal_access_token, expires_at: 5.days.ago, user: user)
visit profile_personal_access_tokens_path
- inactive_personal_access_tokens = find(".table.inactive-personal-access-tokens").native.inner_html
+ inactive_personal_access_tokens = find(".table.inactive-personal-access-tokens").native['innerHTML']
expect(inactive_personal_access_tokens).to match(personal_access_token.name)
expect(inactive_personal_access_tokens).to match(personal_access_token.token)
end
diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb
new file mode 100644
index 00000000000..3e80a48175a
--- /dev/null
+++ b/spec/models/personal_access_token_spec.rb
@@ -0,0 +1,15 @@
+require 'spec_helper'
+
+describe PersonalAccessToken, models: true do
+ describe ".generate" do
+ it "generates a random token" do
+ personal_access_token = PersonalAccessToken.generate({})
+ expect(personal_access_token.token).to be_present
+ end
+
+ it "doesn't save the record" do
+ personal_access_token = PersonalAccessToken.generate({})
+ expect(personal_access_token).to_not be_persisted
+ end
+ end
+end
--
GitLab