From 16cd4752cb063eb9c957108a471a65deabae949d Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Thu, 27 Oct 2016 02:43:01 +0200 Subject: [PATCH 1/3] Move auth_token to users controller This token is only used for the chat, it isn't an official API. --- app/assets/javascripts/jsxc.js | 2 +- app/controllers/api/v1/tokens_controller.rb | 16 ---------------- app/controllers/users_controller.rb | 5 +++++ config/routes.rb | 7 +------ 4 files changed, 7 insertions(+), 23 deletions(-) delete mode 100644 app/controllers/api/v1/tokens_controller.rb diff --git a/app/assets/javascripts/jsxc.js b/app/assets/javascripts/jsxc.js index 9209fd7bf..9e90634ca 100644 --- a/app/assets/javascripts/jsxc.js +++ b/app/assets/javascripts/jsxc.js @@ -12,7 +12,7 @@ // initialize jsxc xmpp client $(document).ready(function() { if (app.currentUser.authenticated()) { - $.post('api/v1/tokens', null, function(data) { + $.post("/user/auth_token", null, function(data) { if (jsxc && data['token']) { var jid = app.currentUser.get('diaspora_id'); jsxc.init({ diff --git a/app/controllers/api/v1/tokens_controller.rb b/app/controllers/api/v1/tokens_controller.rb deleted file mode 100644 index f59c2eac5..000000000 --- a/app/controllers/api/v1/tokens_controller.rb +++ /dev/null @@ -1,16 +0,0 @@ -class Api::V1::TokensController < ApplicationController - skip_before_filter :verify_authenticity_token - before_filter :authenticate_user! - - respond_to :json - - def create - current_user.ensure_authentication_token! - render :status => 200, :json => { :token => current_user.authentication_token } - end - - def destroy - current_user.reset_authentication_token! - render :json => true, :status => 200 - end -end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 8e01dff62..958353e28 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -128,6 +128,11 @@ class UsersController < ApplicationController redirect_to edit_user_path end + def auth_token + current_user.ensure_authentication_token! + render status: 200, json: {token: current_user.authentication_token} + end + private # rubocop:disable Metrics/MethodLength diff --git a/config/routes.rb b/config/routes.rb index 4e7b9d498..02b15089c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -106,6 +106,7 @@ Diaspora::Application.routes.draw do get :download_profile post :export_photos get :download_photos + post :auth_token end controller :users do @@ -184,12 +185,6 @@ Diaspora::Application.routes.draw do end end - namespace :api do - namespace :v1 do - resources :tokens, :only => [:create, :destroy] - end - end - get 'community_spotlight' => "contacts#spotlight", :as => 'community_spotlight' # Mobile site From d421e42ddbd691b873f503d5b967f241ab6ac0e7 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Thu, 27 Oct 2016 04:07:25 +0200 Subject: [PATCH 2/3] Remove ability to authenticate with auth_token on the frontend Remove devise-token_authenticatable gem and only generate a token to be used by the chat. --- Gemfile | 1 - Gemfile.lock | 3 -- app/models/user.rb | 3 +- app/models/user/authentication_token.rb | 26 ++++++++++++ spec/models/user/authentication_token_spec.rb | 42 +++++++++++++++++++ 5 files changed, 70 insertions(+), 5 deletions(-) create mode 100644 app/models/user/authentication_token.rb create mode 100644 spec/models/user/authentication_token_spec.rb diff --git a/Gemfile b/Gemfile index 5e99577d4..e4614cfd7 100644 --- a/Gemfile +++ b/Gemfile @@ -25,7 +25,6 @@ gem "json-schema", "2.7.0" gem "devise", "4.2.0" gem "devise_lastseenable", "0.0.6" -gem "devise-token_authenticatable", "0.5.2" # Captcha diff --git a/Gemfile.lock b/Gemfile.lock index 0196e5d41..73f20f63e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -171,8 +171,6 @@ GEM railties (>= 4.1.0, < 5.1) responders warden (~> 1.2.3) - devise-token_authenticatable (0.5.2) - devise (>= 4.0.0, < 4.3.0) devise_lastseenable (0.0.6) devise rails (>= 3.0.4) @@ -931,7 +929,6 @@ DEPENDENCIES cucumber-rails (= 1.4.5) database_cleaner (= 1.5.3) devise (= 4.2.0) - devise-token_authenticatable (= 0.5.2) devise_lastseenable (= 0.0.6) diaspora-prosody-config (= 0.0.7) diaspora_federation-rails (= 0.1.5) diff --git a/app/models/user.rb b/app/models/user.rb index 77cdd8d21..e8a09a322 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -3,6 +3,7 @@ # the COPYRIGHT file. class User < ActiveRecord::Base + include AuthenticationToken include Connecting include Querying include SocialActions @@ -16,7 +17,7 @@ class User < ActiveRecord::Base scope :halfyear_actives, ->(time = Time.now) { logged_in_since(time - 6.month) } scope :active, -> { joins(:person).where(people: {closed_account: false}) } - devise :token_authenticatable, :database_authenticatable, :registerable, + devise :database_authenticatable, :registerable, :recoverable, :rememberable, :trackable, :validatable, :lockable, :lastseenable, :lock_strategy => :none, :unlock_strategy => :none diff --git a/app/models/user/authentication_token.rb b/app/models/user/authentication_token.rb new file mode 100644 index 000000000..ca72950b4 --- /dev/null +++ b/app/models/user/authentication_token.rb @@ -0,0 +1,26 @@ +class User + module AuthenticationToken + extend ActiveSupport::Concern + + # Generate new authentication token and save the record. + def reset_authentication_token! + self.authentication_token = self.class.authentication_token + save(validate: false) + end + + # Generate authentication token unless already exists and save the record. + def ensure_authentication_token! + reset_authentication_token! if authentication_token.blank? + end + + module ClassMethods + # Generate a token checking if one does not already exist in the database. + def authentication_token + loop do + token = Devise.friendly_token(30) + break token unless User.exists?(authentication_token: token) + end + end + end + end +end diff --git a/spec/models/user/authentication_token_spec.rb b/spec/models/user/authentication_token_spec.rb new file mode 100644 index 000000000..b4b4c2304 --- /dev/null +++ b/spec/models/user/authentication_token_spec.rb @@ -0,0 +1,42 @@ +require "spec_helper" + +describe User::AuthenticationToken, type: :model do + describe "#reset_authentication_token!" do + it "sets the authentication token" do + expect(alice.authentication_token).to be_nil + alice.reset_authentication_token! + expect(alice.authentication_token).not_to be_nil + end + + it "resets the authentication token" do + alice.reset_authentication_token! + expect { alice.reset_authentication_token! }.to change { alice.authentication_token } + end + end + + describe "#ensure_authentication_token!" do + it "doesn't change the authentication token" do + alice.reset_authentication_token! + expect { alice.ensure_authentication_token! }.to_not change { alice.authentication_token } + end + + it "sets the authentication token if not yet set" do + expect(alice.authentication_token).to be_nil + alice.ensure_authentication_token! + expect(alice.authentication_token).not_to be_nil + end + end + + describe ".authentication_token" do + it "generates an authentication token" do + expect(User.authentication_token.length).to eq(30) + end + + it "checks that the authentication token is not yet in use by another user" do + alice.reset_authentication_token! + expect(Devise).to receive(:friendly_token).with(30).and_return(alice.authentication_token, "some_unused_token") + + expect(User.authentication_token).to eq("some_unused_token") + end + end +end From 9f38a424e7a71c3321dd24e4f1f1e5cf9a3bc63a Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Thu, 27 Oct 2016 04:13:59 +0200 Subject: [PATCH 3/3] Revert "Test token authentication; should allow it" It shouldn't be allowed! This reverts commit 46097ba8c883ef51ed4159a9271536c720664dab. closes #7160 --- spec/controllers/users_controller_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index c73872d10..0d4a31b16 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -242,11 +242,11 @@ describe UsersController, :type => :controller do expect(assigns[:email_prefs]['mentioned']).to be false end - it 'does allow token auth' do + it "does not allow token auth" do sign_out :user bob.reset_authentication_token! get :edit, :auth_token => bob.authentication_token - expect(response.status).to eq(200) + expect(response).to redirect_to new_user_session_path end end