Fix API token usage and specs

app/controllers/account_controller.rb

@@ -154,12 +154,21 @@ def allow_lost_password_recovery?
 
   # Token based account activation
   def activate
-    token = :Token::Invitation.find_by(value: params[:token])
+    token = ::Token::Invitation.find_by_plaintext_value(params[:token])
 
-    if token && Setting.self_registration?
+    if token.nil? || token.expired?
+      flash[:error] = I18n.t(:notice_account_invalid_token)
+      redirect_to home_url
+      return
+    end
+
+    if token.user.invited?
+      activate_by_invite_token token
+    elsif Setting.self_registration?
       activate_self_registered token
     else
-      activate_by_invite_token token
+      flash[:error] = I18n.t(:notice_account_invalid_token)
+      redirect_to home_url
     end
   end
 
@@ -441,7 +450,7 @@ def login_user!(user)
   def set_autologin_cookie(user)
     token = Token::AutoLogin.create(user: user)
     cookie_options = {
-      value: token.value,
+      value: token.plain_value,
       expires: 1.year.from_now,
       path: OpenProject::Configuration['autologin_cookie_path'],
       secure: OpenProject::Configuration['autologin_cookie_secure'],
@@ -654,7 +663,7 @@ def account_pending
 
   def invited_user
     if session.include? :invitation_token
-      token = Token::Invitation.find_by value: session[:invitation_token]
+      token = Token::Invitation.find_by_plaintext_value session[:invitation_token]
 
       token.user
     end

app/controllers/concerns/user_invitation.rb

@@ -89,13 +89,13 @@ def assign_user_attributes(user)
   def reinvite_user(user_id)
     clear_tokens user_id
 
-    Token.create(user_id: user_id, action: token_action).tap do |token|
+    Token::Invitation.create!(user_id: user_id).tap do |token|
       OpenProject::Notifications.send Events.user_reinvited, token
     end
   end
 
   def clear_tokens(user_id)
-    Token.where(user_id: user_id, action: token_action).destroy_all
+    Token::Invitation.where(user_id: user_id).delete_all
   end
 
   ##
@@ -155,12 +155,4 @@ def user_invitation(user)
 
     [user, nil]
   end
-
-  def token_action
-    'invite'
-  end
-
-  def invitation_token(user)
-    Token::Invitation.new user: user
-  end
 end

app/models/token/hashed_token.rb

@@ -33,16 +33,6 @@ def create_and_return_value(user)
       def find_by_plaintext_value(input)
         find_by(value: hash_function(input))
       end
-
-      private
-
-      ##
-      # Use a fixed salt for hashing token values.
-      # We still want to be able to index the hash value for fast lookups,
-      # so we need to determine the hash without knowing the associated user (and thus its salt) first.
-      def token_salt
-        '28f939460f6f852268a534f449928af54026af6c16aebf04f5975307b9d72de389f0'.freeze
-      end
     end
 
     ##
@@ -55,7 +45,10 @@ def valid_plaintext?(input)
     end
 
     def self.hash_function(input)
-      Digest::SHA256.hexdigest(input + token_salt)
+      # Use a fixed salt for hashing token values.
+      # We still want to be able to index the hash value for fast lookups,
+      # so we need to determine the hash without knowing the associated user (and thus its salt) first.
+      Digest::SHA256.hexdigest(input + Rails.application.secrets.fetch(:secret_key_base))
     end
     delegate :hash_function, to: :class
 

app/models/user.rb

@@ -230,7 +230,7 @@ def self.try_authentication_for_existing_user(user, password, session = nil)
 
   def self.activate_user!(user, session)
     if session[:invitation_token]
-      token = Token::Invitation.find_active_uuid session[:invitation_token]
+      token = Token::Invitation.find_by_plaintext_value session[:invitation_token]
       invited_id = token && token.user.id
 
       if user.id == invited_id
@@ -490,7 +490,7 @@ def self.find_by_rss_key(key)
 
   def self.find_by_api_key(key)
     return nil unless Setting.rest_api_enabled?
-    token = Token::Api.find_by(value: key)
+    token = Token::Api.find_by_plaintext_value(key)
 
     if token && token.user.active?
       token.user

spec/controllers/account_controller_spec.rb

@@ -325,7 +325,7 @@
     end
 
     context 'with self registration off but an ongoing invitation activation' do
-      let(:token) { FactoryGirl.create :token }
+      let(:token) { FactoryGirl.create :invitation_token }
 
       before do
         allow(Setting).to receive(:self_registration).and_return('0')
@@ -421,7 +421,7 @@
 
         it "doesn't activate the user but sends out a token instead" do
           expect(User.find_by_login('register')).not_to be_active
-          token = Token::Invitation.delete_all
+          token = Token::Invitation.last
           expect(token.user.mail).to eq('register@example.com')
           expect(token).not_to be_expired
         end

spec/controllers/api/v2/authentication_spec.rb

@@ -78,7 +78,7 @@ def fetch
   end
 
   describe 'session' do
-    let(:api_key) { user.api_key }
+    let(:api_key) { ::Token::Api.create!(user: user).plain_value }
     let(:user) { FactoryGirl.create(:admin) }
     let(:ttl) { 42 }
 
@@ -111,7 +111,7 @@ def fetch
   end
 
   describe 'WWW-Authenticate response header upon failure' do
-    let(:api_key) { user.api_key }
+    let(:api_key) { ::Token::Api.create(user: user).plain_value }
     let(:user) { FactoryGirl.create(:admin) }
     let(:ttl) { 42 }
 

spec/controllers/concerns/user_invitation_spec.rb

@@ -49,7 +49,7 @@
 
   describe '.reinvite_user' do
     let(:user) { FactoryGirl.create :invited_user }
-    let!(:token) { FactoryGirl.create :token, user: user, action: UserInvitation.token_action }
+    let!(:token) { FactoryGirl.create :invitation_token, user: user }
 
     it 'notifies listeners of the re-invite' do
       expect(OpenProject::Notifications).to receive(:send) do |event, new_token|
@@ -63,7 +63,7 @@
       new_token = UserInvitation.reinvite_user user.id
 
       expect(new_token.value).not_to eq token.value
-      expect(Token.exists?(token.id)).to eq false
+      expect(Token::Invitation.exists?(token.id)).to eq false
     end
   end
 end

spec/factories/token_factory.rb

@@ -29,17 +29,19 @@
 require 'securerandom'
 
 FactoryGirl.define do
-  factory :token do
+  factory :invitation_token, class: ::Token::Invitation do
     user
-    action 'invite'
-    value do SecureRandom.hex(16) end
+  end
 
-    factory :api_key do
-      action 'api'
-    end
+  factory :api_token, class: ::Token::Api do
+    user
+  end
 
-    factory :rss_key do
-      action 'rss'
-    end
+  factory :rss_token, class: ::Token::Rss do
+    user
+  end
+
+  factory :recovery_token, class: ::Token::Recovery do
+    user
   end
 end

spec/features/users/my_spec.rb

@@ -102,32 +102,30 @@ def expect_changed!
       end
     end
 
-    it 'in Access Tokens they can generate and view their API key' do
+    it 'in Access Tokens they can generate their API key' do
       visit my_access_token_path
       expect(page).to have_content 'Missing API access key'
       find(:xpath, "//tr[contains(.,'API')]/td/a", text: 'Generate').click
 
-      expect(page).to have_content 'Your API access key was generated.'
-      expect(page).not_to have_content 'Missing API access key'
+      expect(page).to have_content 'A new API token has been generated. Your access token is'
+
+      User.current.reload
+      visit my_access_token_path
 
-      find(:xpath, "//tr[contains(.,'API')]/td/a", text: 'Show').click
-      expect(page).to have_content "Your API access key is: #{user.api_token.value}"
+      expect(page).not_to have_content 'Missing API access key'
     end
 
-    it 'in Access Tokens they can generate and view their RSS key' do
+    it 'in Access Tokens they can generate their RSS key' do
       visit my_access_token_path
       expect(page).to have_content 'Missing RSS access key'
       find(:xpath, "//tr[contains(.,'RSS')]/td/a", text: 'Generate').click
 
-      expect(page).to have_content 'Your RSS access key was generated.'
+      expect(page).to have_content 'A new RSS token has been generated. Your access token is'
 
       User.current.reload
       visit my_access_token_path
 
       expect(page).not_to have_content 'Missing RSS access key'
-
-      find(:xpath, "//tr[contains(.,'RSS')]/td/a", text: 'Show').click
-      expect(page).to have_content "Your RSS access key is: #{user.rss_token.value}"
     end
   end
 end

spec/features/users/resend_invitation_spec.rb

@@ -43,17 +43,4 @@
 
     expect(page).to have_text 'Another invitation has been sent to holly@openproject.com.'
   end
-
-  context 'with some error occuring' do
-    before do
-      allow(UserInvitation).to receive(:token_action).and_return(nil)
-    end
-
-    scenario 'resending fails' do
-      click_on 'Resend invitation'
-
-      expect(page).to have_text 'An error occurred'
-      expect(page).to have_text 'You are here: HomeAdministrationUsers'
-    end
-  end
 end

spec/requests/api/v3/authentication_spec.rb

@@ -166,21 +166,21 @@ def set_basic_auth_header(user, password)
         it_behaves_like 'it is basic auth protected'
 
         describe 'user basic auth' do
-          let(:api_key) { FactoryGirl.create :api_key }
+          let(:api_key) { FactoryGirl.create :api_token }
 
           let(:username) { 'apikey' }
-          let(:password) { api_key.value }
+          let(:password) { api_key.plain_value }
 
           # check that user basic auth is tried when global basic auth fails
           it_behaves_like 'it is basic auth protected'
         end
       end
 
       describe 'user basic auth' do
-        let(:api_key) { FactoryGirl.create :api_key }
+        let(:api_key) { FactoryGirl.create :api_token }
 
         let(:username) { 'apikey' }
-        let(:password) { api_key.value }
+        let(:password) { api_key.plain_value }
 
         # check that user basic auth works on its own too
         it_behaves_like 'it is basic auth protected'
@@ -199,7 +199,7 @@ def set_basic_auth_header(user, password)
           let(:password) { 'olooleol' }
 
           let(:api_user) { FactoryGirl.create :user, login: 'user_account' }
-          let(:api_key)  { FactoryGirl.create :api_key, user: api_user }
+          let(:api_key)  { FactoryGirl.create :api_token, user: api_user }
 
           before do
             config = { user: 'global_account', password: 'global_password' }
@@ -249,7 +249,7 @@ def set_basic_auth_header(user, password)
 
           context 'with valid user credentials' do
             before do
-              set_basic_auth_header('apikey', api_key.value)
+              set_basic_auth_header('apikey', api_key.plain_value)
               get resource
             end
 

spec/requests/auth/api_v2_spec.rb

@@ -49,7 +49,7 @@
   end
 
   describe 'API authentication' do
-    let(:api_key) { admin.api_key }
+    let(:api_key) { ::Token::Api.create(user: admin).plain_value }
 
     shared_examples_for 'API key access' do
       context 'invalid' do

spec_legacy/fixtures/tokens.yml

@@ -29,13 +29,13 @@
 ---
 tokens_001:
   created_on: 2007-01-21 00:39:12 +01:00
-  action: register
+  type: Token::Invitation
   id: 1
   value: DwMJ2yIxBNeAk26znMYzYmz5dAiIina0GFrPnGTM
   user_id: 1
 tokens_002:
   created_on: 2007-01-21 00:39:52 +01:00
-  action: recovery
   id: 2
   value: sahYSIaoYrsZUef86sTHrLISdznW6ApF36h5WSnm
+  type: Token::Recovery
   user_id: 2

spec_legacy/functional/user_mailer_spec.rb

@@ -371,13 +371,13 @@
 
   it 'should lost password' do
     user  = FactoryGirl.create(:user)
-    token = FactoryGirl.create(:token, user: user)
+    token = FactoryGirl.create(:recovery_token, user: user)
     assert UserMailer.password_lost(token).deliver_now
   end
 
   it 'should register' do
     user  = FactoryGirl.create(:user)
-    token = FactoryGirl.create(:token, user: user)
+    token = FactoryGirl.create(:invitation_token, user: user)
     Setting.host_name = 'redmine.foo'
     Setting.protocol = 'https'
 

spec_legacy/integration/api_spec/disabled_rest_api_spec.rb

@@ -47,7 +47,7 @@
       context 'with a valid api token' do
         before do
           @user = FactoryGirl.create(:user)
-          @token = FactoryGirl.create(:token, user: @user, action: 'api')
+          @token = FactoryGirl.create(:api_token, user: @user)
           get "/api/v2/projects.xml?key=#{@token.value}"
         end
 
@@ -75,7 +75,7 @@
       context 'with a valid HTTP authentication using the API token' do
         before do
           @user = FactoryGirl.create(:user)
-          @token = FactoryGirl.create(:token, user: @user, action: 'api')
+          @token = FactoryGirl.create(:api_token, user: @user)
           @authorization = ActionController::HttpAuthentication::Basic.encode_credentials(@token.value, 'X')
           get '/api/v2/projects.xml', params: { authorization: @authorization }
         end
@@ -92,7 +92,7 @@
       context 'with a valid api token' do
         before do
           @user = FactoryGirl.create(:user)
-          @token = FactoryGirl.create(:token, user: @user, action: 'api')
+          @token = FactoryGirl.create(:api_token, user: @user)
           get "/api/v2/projects.json?key=#{@token.value}"
         end
 
@@ -120,7 +120,7 @@
       context 'with a valid HTTP authentication using the API token' do
         before do
           @user = FactoryGirl.create(:user)
-          @token = FactoryGirl.create(:token, user: @user, action: 'api')
+          @token = FactoryGirl.create(:api_token, user: @user)
           @authorization = ActionController::HttpAuthentication::Basic.encode_credentials(@token.value, 'DoesNotMatter')
           get '/api/v2/projects.json', params: { authorization: @authorization }
         end