Remove plain text token from everywhere
Some checks failed
CI / scan_ruby (push) Has been cancelled
CI / scan_js (push) Has been cancelled
CI / lint (push) Has been cancelled
CI / test (push) Has been cancelled
CI / system-test (push) Has been cancelled

This commit is contained in:
Dan Milne
2025-12-30 11:58:11 +11:00
parent 99c3ac905f
commit 71d59e7367
5 changed files with 21 additions and 28 deletions

View File

@@ -1,5 +1,5 @@
# frozen_string_literal: true # frozen_string_literal: true
module Clinch module Clinch
VERSION = "0.6.4" VERSION = "0.7.0"
end end

View File

@@ -0,0 +1,10 @@
class RemovePlaintextTokenFromOidcAccessTokens < ActiveRecord::Migration[8.1]
def change
# Remove the unique index first
remove_index :oidc_access_tokens, :token, if_exists: true
# Remove the plaintext token column - no longer needed
# Tokens are now stored as BCrypt-hashed token_digest with HMAC token_prefix
remove_column :oidc_access_tokens, :token, :string
end
end

4
db/schema.rb generated
View File

@@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema[8.1].define(version: 2025_12_29_220739) do ActiveRecord::Schema[8.1].define(version: 2025_12_30_005248) do
create_table "active_storage_attachments", force: :cascade do |t| create_table "active_storage_attachments", force: :cascade do |t|
t.bigint "blob_id", null: false t.bigint "blob_id", null: false
t.datetime "created_at", null: false t.datetime "created_at", null: false
@@ -100,7 +100,6 @@ ActiveRecord::Schema[8.1].define(version: 2025_12_29_220739) do
t.datetime "expires_at", null: false t.datetime "expires_at", null: false
t.datetime "revoked_at" t.datetime "revoked_at"
t.string "scope" t.string "scope"
t.string "token"
t.string "token_digest" t.string "token_digest"
t.string "token_prefix", limit: 8 t.string "token_prefix", limit: 8
t.datetime "updated_at", null: false t.datetime "updated_at", null: false
@@ -109,7 +108,6 @@ ActiveRecord::Schema[8.1].define(version: 2025_12_29_220739) do
t.index ["application_id"], name: "index_oidc_access_tokens_on_application_id" t.index ["application_id"], name: "index_oidc_access_tokens_on_application_id"
t.index ["expires_at"], name: "index_oidc_access_tokens_on_expires_at" t.index ["expires_at"], name: "index_oidc_access_tokens_on_expires_at"
t.index ["revoked_at"], name: "index_oidc_access_tokens_on_revoked_at" t.index ["revoked_at"], name: "index_oidc_access_tokens_on_revoked_at"
t.index ["token"], name: "index_oidc_access_tokens_on_token", unique: true
t.index ["token_digest"], name: "index_oidc_access_tokens_on_token_digest", unique: true t.index ["token_digest"], name: "index_oidc_access_tokens_on_token_digest", unique: true
t.index ["token_prefix"], name: "index_oidc_access_tokens_on_token_prefix" t.index ["token_prefix"], name: "index_oidc_access_tokens_on_token_prefix"
t.index ["user_id"], name: "index_oidc_access_tokens_on_user_id" t.index ["user_id"], name: "index_oidc_access_tokens_on_user_id"

View File

@@ -1,14 +1,16 @@
# Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html # Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html
one: one:
token: <%= SecureRandom.urlsafe_base64(32) %> token_digest: <%= BCrypt::Password.create(SecureRandom.urlsafe_base64(48)) %>
token_prefix: <%= SecureRandom.urlsafe_base64(8)[0..7] %>
application: kavita_app application: kavita_app
user: alice user: alice
scope: "openid profile email" scope: "openid profile email"
expires_at: 2025-12-31 23:59:59 expires_at: 2025-12-31 23:59:59
two: two:
token: <%= SecureRandom.urlsafe_base64(32) %> token_digest: <%= BCrypt::Password.create(SecureRandom.urlsafe_base64(48)) %>
token_prefix: <%= SecureRandom.urlsafe_base64(8)[0..7] %>
application: another_app application: another_app
user: bob user: bob
scope: "openid profile email" scope: "openid profile email"

View File

@@ -24,10 +24,10 @@ class OidcAccessTokenTest < ActiveSupport::TestCase
application: applications(:kavita_app), application: applications(:kavita_app),
user: users(:alice) user: users(:alice)
) )
assert_nil new_token.token assert_nil new_token.plaintext_token
assert new_token.save assert new_token.save
assert_not_nil new_token.token assert_not_nil new_token.plaintext_token
assert_match /^[A-Za-z0-9_-]+$/, new_token.token assert_match /^[A-Za-z0-9_-]+$/, new_token.plaintext_token
end end
test "should set expiry before validation on create" do test "should set expiry before validation on create" do
@@ -42,23 +42,6 @@ class OidcAccessTokenTest < ActiveSupport::TestCase
assert new_token.expires_at <= 61.minutes.from_now # Allow some variance assert new_token.expires_at <= 61.minutes.from_now # Allow some variance
end end
test "should validate presence of token" do
@access_token.token = nil
assert_not @access_token.valid?
assert_includes @access_token.errors[:token], "can't be blank"
end
test "should validate uniqueness of token" do
@access_token.save! if @access_token.changed?
duplicate = OidcAccessToken.new(
token: @access_token.token,
application: applications(:another_app),
user: users(:bob)
)
assert_not duplicate.valid?
assert_includes duplicate.errors[:token], "has already been taken"
end
test "should identify expired tokens correctly" do test "should identify expired tokens correctly" do
@access_token.expires_at = 5.minutes.ago @access_token.expires_at = 5.minutes.ago
assert @access_token.expired?, "Should identify past expiry as expired" assert @access_token.expired?, "Should identify past expiry as expired"
@@ -153,7 +136,7 @@ class OidcAccessTokenTest < ActiveSupport::TestCase
application: applications(:kavita_app), application: applications(:kavita_app),
user: users(:alice) user: users(:alice)
) )
tokens << token.token tokens << token.plaintext_token
end end
# All tokens should be unique # All tokens should be unique
@@ -180,7 +163,7 @@ class OidcAccessTokenTest < ActiveSupport::TestCase
user: users(:alice) user: users(:alice)
) )
assert access_token.token.length > auth_code.code.length, assert access_token.plaintext_token.length > auth_code.code.length,
"Access tokens should be longer than authorization codes" "Access tokens should be longer than authorization codes"
end end