From b62d605c724115ae6e34d042c37bb1c83fb3a211 Mon Sep 17 00:00:00 2001
From: Laurence de Bruxelles
 <laurence.debruxelles@digital.cabinet-office.gov.uk>
Date: Fri, 15 Dec 2023 14:14:38 +0200
Subject: [PATCH 1/2] Add uniqueness validation for access tokens

We could end up with weird behaviour if the same access token digest is
present twice in the database.
---
 app/models/access_token.rb       |  1 +
 spec/models/access_token_spec.rb | 16 ++++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/app/models/access_token.rb b/app/models/access_token.rb
index 7244d999..51d8bd43 100644
--- a/app/models/access_token.rb
+++ b/app/models/access_token.rb
@@ -1,5 +1,6 @@
 class AccessToken < ApplicationRecord
   validates :token_digest, :owner, presence: true
+  validates :token_digest, uniqueness: { conditions: -> { active } }
 
   scope :active, -> { where(deactivated_at: nil) }
 
diff --git a/spec/models/access_token_spec.rb b/spec/models/access_token_spec.rb
index ec25d21a..9f3b0f39 100644
--- a/spec/models/access_token_spec.rb
+++ b/spec/models/access_token_spec.rb
@@ -24,6 +24,22 @@
       expect(access_token).to be_invalid
       expect(access_token.errors[:token_digest]).to include("can't be blank")
     end
+
+    it "is invalid to have two active tokens with the same digest" do
+      access_token_1 = described_class.create!(owner: "test1", token_digest: "baabaa")
+      expect(access_token_1).to be_valid
+
+      access_token_2 = described_class.new(owner: "test2", token_digest: "baabaa")
+      expect(access_token_2).not_to be_valid
+    end
+
+    it "is valid to reuse the same token digest" do
+      access_token_1 = described_class.create!(owner: "test1", token_digest: "baabaa")
+      access_token_1.update!(deactivated_at: Time.zone.now)
+
+      access_token_2 = described_class.new(owner: "test2", token_digest: "baabaa")
+      expect(access_token_2).to be_valid
+    end
   end
 
   describe "scopes" do

From a7eb582699a6d4da65e6153d8b1fb9fc45112375 Mon Sep 17 00:00:00 2001
From: Laurence de Bruxelles
 <laurence.debruxelles@digital.cabinet-office.gov.uk>
Date: Fri, 15 Dec 2023 14:29:29 +0200
Subject: [PATCH 2/2] Add prefix to API access tokens

It is considered good practice to add a prefix to API keys so that they
are easier to identify [[1], [2]], both for humans and machines.

[1]: https://github.blog/2021-04-05-behind-githubs-new-authentication-token-formats/
[2]: https://tailscale.com/kb/1277/key-prefixes
---
 app/models/access_token.rb       | 2 +-
 spec/models/access_token_spec.rb | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/models/access_token.rb b/app/models/access_token.rb
index 51d8bd43..85dd832c 100644
--- a/app/models/access_token.rb
+++ b/app/models/access_token.rb
@@ -5,7 +5,7 @@ class AccessToken < ApplicationRecord
   scope :active, -> { where(deactivated_at: nil) }
 
   def generate_token
-    users_token = SecureRandom.uuid
+    users_token = "forms_#{SecureRandom.uuid}"
     self.token_digest = Digest::SHA256.hexdigest(users_token)
     users_token
   end
diff --git a/spec/models/access_token_spec.rb b/spec/models/access_token_spec.rb
index 9f3b0f39..ff797c6e 100644
--- a/spec/models/access_token_spec.rb
+++ b/spec/models/access_token_spec.rb
@@ -66,12 +66,12 @@
     let(:result) { access_token.generate_token }
 
     it "generates a user token before validation" do
-      expect(result).to eq("testing-123")
+      expect(result).to eq("forms_testing-123")
     end
 
     it "generates a sha-256 token before validation" do
       result
-      expect(access_token.token_digest).to eq("8d9754db9759ab1785644440dbf19f88ab45ae326e421da6c1cb6e45140d534f")
+      expect(access_token.token_digest).to eq("f3aed9ecfc2db207800ca641f45e24d2f6de030487b7270871c04046808c1b22")
     end
   end
 end