Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,10 @@ jobs:
path: test/reports/
retention-days: 7

# The 0.7.0 migrations as the generators write them, and the row locks of
# Idempotency-Key and the device flow, on real databases with concurrent
# connections (test/databases/check.rb).
# The upgrade migrations as the generators write them and db:migrate runs
# them, with strong_migrations, and the row locks of Idempotency-Key and the
# device flow, on real databases with concurrent connections
# (test/databases/check.rb).
databases:
runs-on: ubuntu-latest
timeout-minutes: 15
Expand Down
4 changes: 4 additions & 0 deletions Appraisals
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ end

appraise "rails-8.0" do
gem "rails", ">= 8.0.5.1", "< 8.1.a"
# Active Support 8.0 passes quirks_mode to JSON.generate, which json 3
# removed. RuboCop, for the style test of the generated migrations, allows
# json 3, so keep json 2 here.
gem "json", "< 3"
end

# Test against Rails 8.1 (latest)
Expand Down
25 changes: 25 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,28 @@
## [0.8.1] - 2026-10-26

Two fixes found when an app on the fleet upgraded from 0.5 to 0.8.0. The second one now covers every upgrade generator.

### Fixed

- The one-time token is readable in transactional tests again, once, as in production. Since 0.6.0 a retrieval inside any open transaction was refused, and Rails' transactional tests run each test inside one, so a test that created a key and read its token got nil (`:inside_transaction`), and the dashboard's token page redirected. When `Rails.env` is test and the only open transaction is an outer, non-joinable one (what Rails' transactional tests, rspec-rails and DatabaseCleaner's `:transaction` strategy open, and what a system test's server thread shares), the claim of `token_shown_at` runs in it: the first retrieval returns the token, and a second one, or one from a copy of the session, does not. A transaction opened on top of it is still refused. Outside the test environment nothing changes. Unlike `config.idempotency_in_test_transactions`, this needs no switch: apps whose tests broke on 0.6.0 must get them back without new configuration, and the exemption covers only the test's own transaction, not every depth. The check reads the shape of the transaction, so in a test without a transaction of its own, an outer `transaction(joinable: false)` that the app opens is exempt too; the README lists the cases. Apps can remove their test-only workarounds.
- Every upgrade generator now writes a migration that strong_migrations accepts on PostgreSQL. Before, it refused three of them. Those three now run with `disable_ddl_transaction!`, and each step checks what exists, so a run that stopped half way can run again. An invalid index that a failed concurrent build left on PostgreSQL counts as absent: the next run removes it and builds it again (`add_created_by`, `add_key_types`, `add_authentication_index`). On MySQL and SQLite they run as before, except where this list says otherwise. A migration that an earlier version generated and that already ran needs nothing.
- `add_created_by` (0.7.0) built the polymorphic index inside the migration's transaction, which blocks writes to `api_keys`. It now adds the columns, then builds `index_api_keys_on_created_by` with `algorithm: :concurrently` on PostgreSQL (and PostGIS).
- `add_key_types` (0.3.0) built its three indexes inside the transaction. They are now built concurrently on PostgreSQL. strong_migrations calls the four-column index a best-practice problem on every database. The install migration has the same index, so the migration keeps it inside `safety_assured` when the app uses strong_migrations.
- `add_restrictions` (0.5.0) added its check constraint and checked every row under a lock that blocks reads and writes. On PostgreSQL it now adds the constraint without that check (`validate: false`), then validates it, which does not block reads and writes. On MySQL, which refuses a literal default on a JSON column, the column now defaults to the expression `JSON_OBJECT()` (MySQL 8.0.13 or later), so the migration runs and existing keys get an empty object. On MySQL strong_migrations still refuses this migration twice: at `add_column`, for the callable default, and at the check constraint, which MySQL adds by copying the table. There, check the size of `api_keys`, then wrap the whole body of `up` in `safety_assured`, and run it when the table can be locked for the copy.
- `add_authentication_index` (0.4.0) already built its index concurrently. It now also does so on PostGIS.
- `add_key_activity`, `add_device_authorizations` and `add_idempotency_keys` already passed strong_migrations.
- Every generated migration, the install included, passes rubocop-rails-omakase, the Rails default style. Array literals of symbols are `%i[]`, the key types are two methods instead of an array, and each `case` has its own line under `expression =`. Before, four files had 16 offenses.

### Changed

- `ApiKeys.test_environment?` says whether `Rails.env` is test. The token handoff and `config.idempotency_in_test_transactions` both use it.

### Tests

- The database check (`test/databases/check.rb`, in CI on PostgreSQL, MySQL and SQLite) now runs all seven upgrade migrations as `rails db:migrate` runs them, with strong_migrations, from an install made before 0.3.0 that has a key. It runs them down and up again, and again after a run that stopped half way. On PostgreSQL it also leaves an invalid index from a failed concurrent build and shows that the next run builds it again.
- A generator test generates all eight migrations and runs RuboCop with the omakase config on them.
- The token handoff is tested in transactional tests (the helper and the dashboard) and with real commits.

## [0.8.0] - 2026-10-25

### Added
Expand Down
3 changes: 3 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,7 @@ group :development, :test do

# Ruby 4.0+ compatibility: ostruct removed from stdlib
gem "ostruct"

# Lints the generated migrations as a host app on omakase does
gem "rubocop-rails-omakase", require: false
end
13 changes: 12 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ And you're done!

### Upgrading an existing installation

Every upgrade generator below writes a migration that strong_migrations accepts on PostgreSQL (0.8.1). There the indexes on `api_keys` are built concurrently, and the restrictions check constraint is validated apart, outside a transaction, so writes to `api_keys` go on. If a concurrent build fails (a lock or statement timeout, for example), PostgreSQL keeps an invalid index; the next run of the migration removes it and builds it again. On MySQL strong_migrations accepts all of them except `add_restrictions`, which it refuses twice: first at `add_column`, for the callable `JSON_OBJECT()` default (`add_column_default_callable`), then at the check constraint, which MySQL adds by copying the table. On MySQL with strong_migrations, check the size of `api_keys` first. Then wrap the whole body of `up` in the generated migration (the column and the constraint) in `safety_assured do ... end`, and run it when `api_keys` can be locked for the copy, for example in a maintenance window.

Install the bounded bcrypt authentication lookup index before deploying the current hardening changes:

```bash
Expand Down Expand Up @@ -66,6 +68,8 @@ rails generate api_keys:add_created_by
rails db:migrate
```

On PostgreSQL this migration builds its index concurrently, outside a transaction, so it does not block writes to `api_keys` and strong_migrations accepts it (0.8.1). Each step checks what exists, so a run that stopped half way can run again, and an invalid index from a failed concurrent build is built again.

The device flow and Idempotency-Key each need a table of their own. New and existing installations run these only if they use the feature:

```bash
Expand All @@ -91,7 +95,7 @@ On 0.6.x, nothing changes until you opt in to a 0.7.0 feature. A 0.6.0 job worke
On 0.4.x, the calls of a custom key controller keep working unchanged: `create_api_key!(name:, key_type:, expires_at:)`, `ApiKeys::TokenSession.store(session, @api_key)` and `ApiKeys::TokenSession.retrieve_once(session)`. A handoff stored by 0.4.x stays readable after the deploy. Read the [CHANGELOG](CHANGELOG.md) for 0.5.0 to 0.6.0 before you upgrade. These changes are the ones a 0.4.x app sees:

- `api_key_scopes_checkboxes` renders an empty `api_key[scopes][]` field before the boxes (0.5.1). A form with every box cleared now saves no scopes, where 0.4.x kept the old ones. If an edit form offers only some scopes, save `ApiKey#scopes_after_form_edit` (see [Form Builder Extensions](#form-builder-extensions-opt-in)).
- A token's one-time handoff is one-time across copies of the session (0.5.1), and once ever per key after `add_key_activity` (0.6.0). A retrieval inside an open database transaction is refused (0.6.0): exclude the token page from a host `around_action` that wraps requests in a transaction.
- A token's one-time handoff is one-time across copies of the session (0.5.1), and once ever per key after `add_key_activity` (0.6.0). A retrieval inside an open database transaction is refused (0.6.0): exclude the token page from a host `around_action` that wraps requests in a transaction. The transaction of a transactional test does not count (0.8.1).
- Deploy job workers with the web processes. A refusal job from 0.6.0 or later errors on a 0.4.x worker.

## Quick Start
Expand Down Expand Up @@ -554,6 +558,7 @@ The database applies it atomically, and the claim commits before the token is re

- **With the column, the token is released at most once per key.** A cache flush, a restart, or an eviction changes nothing, and two simultaneous first retrievals are serialized by the database: one wins. `ApiKey#token_shown_at` and `#token_shown?` report it. A key whose row no longer exists gets nothing: zero rows claimed fails closed.
- **A retrieval attempted inside a database transaction is refused, and can be retried outside it.** Inside a transaction the claim would commit only with it, after the token had been returned, and a rollback would undo it. No cache can stand in for that (a process-local store protects one process), so whatever the cache, the token is withheld with `:inside_transaction`, nothing is recorded, and the handoff stays in the session for a retrieval outside the transaction. The check is Active Record's transaction manager on the `ApiKey` connection: a transaction opened with raw SQL (`BEGIN`) is not detected, and one on another database's connection does not matter. The dashboard opens no transaction itself, but its controller inherits from your `ApplicationController` (or the configured parent): a host `around_action` that wraps requests in a transaction will see the refusal and must exclude the token page.
- **In a transactional test, the token is released once, as in production (0.8.1).** Rails' transactional tests (also rspec-rails and DatabaseCleaner's `:transaction` strategy) open one outer, non-joinable transaction around each test, and a system test's server thread shares it. When `Rails.env` is test and that is the only open transaction, the claim runs in it: the first retrieval in the test returns the token, and a second one, or one from a copy of the session, does not. The rollback at the end of the test removes the key with its claim. A transaction that the app or the test opens on top of it still refuses the retrieval, as in production, so a test sees a host `around_action` that would break the token page. From 0.6.0 to 0.8.0 every retrieval in a transactional test was refused. The check reads the shape of the transaction, not where it came from, so three cases follow. In a test without a transaction of its own (`use_transactional_tests = false`, or truncation), an outer `transaction(joinable: false)` that the app opens is exempt too, where production refuses it. An environment with another name (`ci`, `staging`) is never exempt: only `Rails.env` test is. A test transaction inside another one (fixtures and DatabaseCleaner's `:transaction` strategy together) is two deep, so it is refused, as in 0.8.0. No setting turns this on: apps whose tests broke on 0.6.0 need no new configuration.
- **The column is written only by that claim.** The model marks it read-only, so no form, mass assignment, or `update` can clear it and reopen a handoff.
- **The cache marker from 0.5.1 stays as a second layer.** Each handoff carries a random ID inside its encrypted payload, and retrieval records it in `Rails.cache` with `write(key, true, unless_exist: true, expires_in: 10.minutes)`. With the column it refuses a release the claim did not see (a 0.5.1 process that served the token during a rolling deploy), and it is the only cross-copy layer for a handoff that names no key (`TokenSession.store(session, "a token string")`).
- **Without the column** (an existing install that has not run `rails generate api_keys:add_key_activity`), the gem behaves as 0.5.1 and the marker is the only cross-copy layer. The token is then released at most once only when every application process shares a store that implements `unless_exist:` atomically (Redis, Memcached), and that store keeps the marker for the whole ten-minute handoff lifetime. If the cache is flushed, restarted without persistence, or evicts the marker within those ten minutes, a copy of the session taken before retrieval can recover the token again for the rest of that window. **`MemoryStore`** only covers the process that holds it. **`FileStore`** checks and then writes, so it stops a later replay on the same host but not two requests that arrive at the same instant. **Solid Cache** (Rails 8's default production store) implements `unless_exist:` as a `SELECT … FOR UPDATE` followed by an upsert; on PostgreSQL that locks nothing while the key does not exist yet, so two simultaneous first retrievals can both succeed. **`ActiveSupport::Cache::NullStore`** (Rails' generated development and test default), or no `Rails.cache` at all, records nothing, so the session deletion is the only protection.
Expand Down Expand Up @@ -1879,6 +1884,12 @@ bundle exec appraisal rails-8.0 rake test
bundle exec appraisal rails-8.1 rake test
```

`test/databases/check.rb` runs the generated upgrade migrations as `rails db:migrate` runs them, with strong_migrations, and the row locks of Idempotency-Key and the device flow, on a real database. CI runs it on PostgreSQL, MySQL and SQLite:

```bash
BUNDLE_GEMFILE=test/databases/Gemfile DB_ADAPTER=postgresql bundle exec ruby test/databases/check.rb
```

## Development

After checking out the repo, run `bin/setup` to install dependencies. Then run `bundle exec rake test`. You can also run `bin/console` for an interactive prompt.
Expand Down
1 change: 1 addition & 0 deletions gemfiles/rails_7.2.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ group :development, :test do
gem "bootsnap", require: false
gem "mocha", "~> 2.0"
gem "ostruct"
gem "rubocop-rails-omakase", require: false
end

gemspec path: "../"
2 changes: 2 additions & 0 deletions gemfiles/rails_8.0.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ source "https://rubygems.org"

gem "rake", "~> 13.0"
gem "rails", ">= 8.0.5.1", "< 8.1.a"
gem "json", "< 3"

group :development, :test do
gem "appraisal"
Expand All @@ -18,6 +19,7 @@ group :development, :test do
gem "bootsnap", require: false
gem "mocha", "~> 2.0"
gem "ostruct"
gem "rubocop-rails-omakase", require: false
end

gemspec path: "../"
1 change: 1 addition & 0 deletions gemfiles/rails_8.1.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ group :development, :test do
gem "bootsnap", require: false
gem "mocha", "~> 2.0"
gem "ostruct"
gem "rubocop-rails-omakase", require: false
end

gemspec path: "../"
8 changes: 8 additions & 0 deletions lib/api_keys.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ def reset_configuration!
@configuration = ::ApiKeys::Configuration.new
end

# Whether Rails.env is test, read at each call. The test-only exemptions
# use it: config.idempotency_in_test_transactions, and the one-time token
# handoff inside a transactional test's own transaction.
# @return [Boolean]
def test_environment?
defined?(Rails) && Rails.respond_to?(:env) && Rails.env.test? ? true : false
end

# Add-only backfill after widening a key type's `permissions:` ceiling, for
# types that keep stored scopes. See ApiKeys::ApiKey.reconcile_scopes!.
# @return [Integer] How many keys gained scopes.
Expand Down
8 changes: 2 additions & 6 deletions lib/api_keys/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ def tenant_resolver=(value)

def idempotency_in_test_transactions=(value)
raise ArgumentError, "idempotency_in_test_transactions must be true or false" unless [true, false].include?(value)
if value && !test_environment?
if value && !ApiKeys.test_environment?
raise ArgumentError, "idempotency_in_test_transactions is for transactional tests only: Rails.env must be test"
end

Expand All @@ -317,7 +317,7 @@ def idempotency_in_test_transactions=(value)
# Whether hold may run inside an open transaction: the switch is on, and
# Rails.env is still test when it is used.
def idempotency_in_test_transactions?
@idempotency_in_test_transactions == true && test_environment?
@idempotency_in_test_transactions == true && ApiKeys.test_environment?
end

def trusted_key_check=(value)
Expand Down Expand Up @@ -454,10 +454,6 @@ def environments=(value)

private

def test_environment?
defined?(Rails) && Rails.respond_to?(:env) && Rails.env.test? ? true : false
end

def validate_resolved_prefix!(prefix)
valid = prefix.is_a?(String) && prefix.present? && prefix.valid_encoding? && prefix.bytesize <= 64 &&
prefix.each_codepoint.none? { |codepoint| codepoint <= 0x20 || codepoint == 0x7f }
Expand Down
Loading
Loading