[master] Persist only SHA digest of PersonalAccessToken#token

3 files changed, 297 insertions, 11 deletions

diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb
index 9b804429138..782687516ae 100644
--- a/spec/models/concerns/token_authenticatable_spec.rb
+++ b/spec/models/concerns/token_authenticatable_spec.rb
@@ -2,8 +2,6 @@ require 'spec_helper'
 
 shared_examples 'TokenAuthenticatable' do
   describe 'dynamically defined methods' do
-    it { expect(described_class).to be_private_method_defined(:generate_token) }
-    it { expect(described_class).to be_private_method_defined(:write_new_token) }
     it { expect(described_class).to respond_to("find_by_#{token_field}") }
     it { is_expected.to respond_to("ensure_#{token_field}") }
     it { is_expected.to respond_to("set_#{token_field}") }
@@ -66,13 +64,275 @@ describe ApplicationSetting, 'TokenAuthenticatable' do
   end
 
   describe 'multiple token fields' do
-    before do
+    before(:all) do
       described_class.send(:add_authentication_token_field, :yet_another_token)
     end
 
-    describe '.token_fields' do
-      subject { described_class.authentication_token_fields }
-      it { is_expected.to include(:runners_registration_token, :yet_another_token) }
+    it { is_expected.to respond_to(:ensure_runners_registration_token) }
+    it { is_expected.to respond_to(:ensure_yet_another_token) }
+  end
+
+  describe 'setting same token field multiple times' do
+    subject { described_class.send(:add_authentication_token_field, :runners_registration_token) }
+
+    it 'raises error' do
+      expect {subject}.to raise_error(ArgumentError)
+    end
+  end
+end
+
+describe PersonalAccessToken, 'TokenAuthenticatable' do
+  let(:personal_access_token_name) { 'test-pat-01' }
+  let(:token_value) { 'token' }
+  let(:user) { create(:user) }
+  let(:personal_access_token) do
+    described_class.new(name: personal_access_token_name,
+                        user_id: user.id,
+                        scopes: [:api],
+                        token: token,
+                        token_digest: token_digest)
+  end
+
+  before do
+    allow(Devise).to receive(:friendly_token).and_return(token_value)
+  end
+
+  describe '.find_by_token' do
+    subject { PersonalAccessToken.find_by_token(token_value) }
+
+    before do
+      personal_access_token.save
+    end
+
+    context 'token_digest already exists' do
+      let(:token) { nil }
+      let(:token_digest) { Gitlab::CryptoHelper.sha256(token_value) }
+
+      it 'finds the token' do
+        expect(subject).not_to be_nil
+        expect(subject.name).to eql(personal_access_token_name)
+      end
+    end
+
+    context 'token_digest does not exist' do
+      let(:token) { token_value }
+      let(:token_digest) { nil }
+
+      it 'finds the token' do
+        expect(subject).not_to be_nil
+        expect(subject.name).to eql(personal_access_token_name)
+      end
+    end
+  end
+
+  describe '#set_token'   do
+    let(:new_token_value) { 'new-token' }
+    subject { personal_access_token.set_token(new_token_value) }
+
+    context 'token_digest already exists' do
+      let(:token) { nil }
+      let(:token_digest) { Gitlab::CryptoHelper.sha256(token_value) }
+
+      it 'overwrites token_digest' do
+        subject
+
+        expect(personal_access_token.read_attribute('token')).to be_nil
+        expect(personal_access_token.token).to eql(new_token_value)
+        expect(personal_access_token.token_digest).to eql( Gitlab::CryptoHelper.sha256(new_token_value))
+      end
+    end
+
+    context 'token_digest does not exist but token does' do
+      let(:token) { token_value }
+      let(:token_digest) { nil }
+
+      it 'creates new token_digest and clears token' do
+        subject
+
+        expect(personal_access_token.read_attribute('token')).to be_nil
+        expect(personal_access_token.token).to eql(new_token_value)
+        expect(personal_access_token.token_digest).to eql(Gitlab::CryptoHelper.sha256(new_token_value))
+      end
+    end
+
+    context 'token_digest does not exist, nor token' do
+      let(:token) { nil }
+      let(:token_digest) { nil }
+
+      it 'creates new token_digest' do
+        subject
+
+        expect(personal_access_token.read_attribute('token')).to be_nil
+        expect(personal_access_token.token).to eql(new_token_value)
+        expect(personal_access_token.token_digest).to eql( Gitlab::CryptoHelper.sha256(new_token_value))
+      end
+    end
+  end
+
+  describe '#ensure_token' do
+    subject { personal_access_token.ensure_token }
+
+    context 'token_digest already exists' do
+      let(:token) { nil }
+      let(:token_digest) { Gitlab::CryptoHelper.sha256(token_value) }
+
+      it 'does not change token fields' do
+        subject
+
+        expect(personal_access_token.read_attribute('token')).to be_nil
+        expect(personal_access_token.token).to be_nil
+        expect(personal_access_token.token_digest).to eql( Gitlab::CryptoHelper.sha256(token_value))
+      end
+    end
+
+    context 'token_digest does not exist but token does' do
+      let(:token) { token_value }
+      let(:token_digest) { nil }
+
+      it 'does not change token fields' do
+        subject
+
+        expect(personal_access_token.read_attribute('token')).to eql(token_value)
+        expect(personal_access_token.token).to eql(token_value)
+        expect(personal_access_token.token_digest).to be_nil
+      end
+    end
+
+    context 'token_digest does not exist, nor token' do
+      let(:token) { nil }
+      let(:token_digest) { nil }
+
+      it 'creates token_digest' do
+        subject
+
+        expect(personal_access_token.read_attribute('token')).to be_nil
+        expect(personal_access_token.token).to eql(token_value)
+        expect(personal_access_token.token_digest).to eql( Gitlab::CryptoHelper.sha256(token_value))
+      end
+    end
+  end
+
+  describe '#ensure_token!' do
+    subject { personal_access_token.ensure_token! }
+
+    context 'token_digest already exists' do
+      let(:token) { nil }
+      let(:token_digest) { Gitlab::CryptoHelper.sha256(token_value) }
+
+      it 'does not change token fields' do
+        subject
+
+        expect(personal_access_token.read_attribute('token')).to be_nil
+        expect(personal_access_token.token).to be_nil
+        expect(personal_access_token.token_digest).to eql( Gitlab::CryptoHelper.sha256(token_value))
+      end
+    end
+
+    context 'token_digest does not exist but token does' do
+      let(:token) { token_value }
+      let(:token_digest) { nil }
+
+      it 'does not change token fields' do
+        subject
+
+        expect(personal_access_token.read_attribute('token')).to eql(token_value)
+        expect(personal_access_token.token).to eql(token_value)
+        expect(personal_access_token.token_digest).to be_nil
+      end
+    end
+
+    context 'token_digest does not exist, nor token' do
+      let(:token) { nil }
+      let(:token_digest) { nil }
+
+      it 'creates token_digest' do
+        subject
+
+        expect(personal_access_token.read_attribute('token')).to be_nil
+        expect(personal_access_token.token).to eql(token_value)
+        expect(personal_access_token.token_digest).to eql( Gitlab::CryptoHelper.sha256(token_value))
+      end
+    end
+  end
+
+  describe '#reset_token!' do
+    subject { personal_access_token.reset_token! }
+
+    context 'token_digest already exists' do
+      let(:token) { nil }
+      let(:token_digest) { Gitlab::CryptoHelper.sha256('old-token') }
+
+      it 'creates new token_digest' do
+        subject
+
+        expect(personal_access_token.read_attribute('token')).to be_nil
+        expect(personal_access_token.token).to eql(token_value)
+        expect(personal_access_token.token_digest).to eql( Gitlab::CryptoHelper.sha256(token_value))
+      end
+    end
+
+    context 'token_digest does not exist but token does' do
+      let(:token) { 'old-token' }
+      let(:token_digest) { nil }
+
+      it 'creates new token_digest and clears token' do
+        subject
+
+        expect(personal_access_token.read_attribute('token')).to be_nil
+        expect(personal_access_token.token).to eql(token_value)
+        expect(personal_access_token.token_digest).to eql(Gitlab::CryptoHelper.sha256(token_value))
+      end
+    end
+
+    context 'token_digest does not exist, nor token' do
+      let(:token) { nil }
+      let(:token_digest) { nil }
+
+      it 'creates new token_digest' do
+        subject
+
+        expect(personal_access_token.read_attribute('token')).to be_nil
+        expect(personal_access_token.token).to eql(token_value)
+        expect(personal_access_token.token_digest).to eql( Gitlab::CryptoHelper.sha256(token_value))
+      end
+    end
+
+    context 'token_digest exists and newly generated token would be the same' do
+      let(:token) { nil }
+      let(:token_digest) { Gitlab::CryptoHelper.sha256('old-token') }
+
+      before do
+        personal_access_token.save
+        allow(Devise).to receive(:friendly_token).and_return(
+          'old-token', token_value, 'boom!')
+      end
+
+      it 'regenerates a new token_digest' do
+        subject
+
+        expect(personal_access_token.read_attribute('token')).to be_nil
+        expect(personal_access_token.token).to eql(token_value)
+        expect(personal_access_token.token_digest).to eql( Gitlab::CryptoHelper.sha256(token_value))
+      end
+    end
+
+    context 'token exists and newly generated token would be the same' do
+      let(:token) { 'old-token' }
+      let(:token_digest) { nil }
+
+      before do
+        personal_access_token.save
+        allow(Devise).to receive(:friendly_token).and_return(
+          'old-token', token_value, 'boom!')
+      end
+
+      it 'regenerates a new token_digest' do
+        subject
+
+        expect(personal_access_token.read_attribute('token')).to be_nil
+        expect(personal_access_token.token).to eql(token_value)
+        expect(personal_access_token.token_digest).to eql( Gitlab::CryptoHelper.sha256(token_value))
+      end
     end
   end
 end
diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb
index 2bb1c49b740..c82ab9c9e62 100644
--- a/spec/models/personal_access_token_spec.rb
+++ b/spec/models/personal_access_token_spec.rb
@@ -49,18 +49,36 @@ describe PersonalAccessToken do
 
   describe 'Redis storage' do
     let(:user_id) { 123 }
-    let(:token) { 'abc000foo' }
+    let(:token) { 'KS3wegQYXBLYhQsciwsj' }
 
-    before do
-      subject.redis_store!(user_id, token)
+    context 'reading encrypted data' do
+      before do
+        subject.redis_store!(user_id, token)
+      end
+
+      it 'returns stored data' do
+        expect(subject.redis_getdel(user_id)).to eq(token)
+      end
     end
 
-    it 'returns stored data' do
-      expect(subject.redis_getdel(user_id)).to eq(token)
+    context 'reading unencrypted data' do
+      before do
+        Gitlab::Redis::SharedState.with do |redis|
+          redis.set(described_class.redis_shared_state_key(user_id),
+                    token,
+                    ex: PersonalAccessToken::REDIS_EXPIRY_TIME)
+        end
+      end
+
+      it 'returns stored data unmodified' do
+        expect(subject.redis_getdel(user_id)).to eq(token)
+      end
     end
 
     context 'after deletion' do
       before do
+        subject.redis_store!(user_id, token)
+
         expect(subject.redis_getdel(user_id)).to eq(token)
       end
 
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index b3474e74aa4..4e7c8523e65 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -730,6 +730,14 @@ describe User do
 
       expect(user.incoming_email_token).not_to be_blank
     end
+
+    it 'uses SecureRandom to generate the incoming email token' do
+      expect(SecureRandom).to receive(:hex).and_return('3b8ca303')
+
+      user = create(:user)
+
+      expect(user.incoming_email_token).to eql('gitlab')
+    end
   end
 
   describe '#ensure_user_rights_and_limits' do