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
3 changes: 3 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,6 @@ gem 'rspec-core'

# Common code needed by the other RSpec gems. Not intended for direct use [https://github.com/rspec/rspec-support]
gem 'rspec-support'

# Code coverage analysis tool for Ruby [https://github.com/simplecov-ruby/simplecov]
gem 'simplecov', '~> 0.22', require: false
11 changes: 10 additions & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: .
specs:
devise-api (0.1.3)
devise-api (0.2.0)
devise (>= 4.7.2)
dry-configurable (~> 1.0, >= 1.0.1)
dry-initializer (>= 3.1.1)
Expand Down Expand Up @@ -98,6 +98,7 @@ GEM
responders
warden (~> 1.2.3)
diff-lcs (1.5.0)
docile (1.4.1)
dry-configurable (1.0.1)
dry-core (~> 1.0, < 2)
zeitwerk (~> 2.6)
Expand Down Expand Up @@ -236,6 +237,12 @@ GEM
rubocop-ast (1.24.1)
parser (>= 3.1.1.0)
ruby-progressbar (1.11.0)
simplecov (0.22.0)
docile (~> 1.1)
simplecov-html (~> 0.11)
simplecov_json_formatter (~> 0.1)
simplecov-html (0.13.2)
simplecov_json_formatter (0.1.4)
sprockets (4.2.0)
concurrent-ruby (~> 1.0)
rack (>= 2.2.4, < 4)
Expand All @@ -260,6 +267,7 @@ PLATFORMS
arm64-darwin-21
arm64-darwin-22
arm64-darwin-23
arm64-darwin-25

DEPENDENCIES
awesome_print
Expand All @@ -275,6 +283,7 @@ DEPENDENCIES
rspec-rails (~> 6.0, >= 6.0.1)
rspec-support
rubocop (~> 1.21)
simplecov (~> 0.22)
sprockets-rails
sqlite3 (~> 1.4)

Expand Down
6 changes: 6 additions & 0 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ require 'rspec/core/rake_task'

RSpec::Core::RakeTask.new(:rspec)

# Full-suite runs enforce the SimpleCov minimum; single-file `rspec` runs do not.
task :enforce_coverage do
ENV['ENFORCE_COVERAGE'] = '1'
end
task rspec: :enforce_coverage

require 'rubocop/rake_task'

RuboCop::RakeTask.new
Expand Down
5 changes: 0 additions & 5 deletions app/services/devise/api/tokens_service/create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@ def call

private

def authenticate_service
Devise::Api::ResourceOwnerService::Authenticate.new(params: params,
resource_class: resource_class).call
end

def create_devise_api_token
devise_api_token = resource_owner.access_tokens.new(params)

Expand Down
16 changes: 8 additions & 8 deletions docs/analysis/known-issues.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ Defined twice: memoized in `TokensController` (`app/controllers/devise/api/token

## Dead / vestigial code

### KI-5 · Dead method in `TokensService::Create`
`#authenticate_service` (`app/services/devise/api/tokens_service/create.rb:20-23`) is never called and references `params` / `resource_class`, which don't exist on this service — it would `NameError` if invoked. Copy-paste leftover; delete.
### KI-5 · ~~Dead method in `TokensService::Create`~~ (resolved)
`#authenticate_service` was never called and referenced `params` / `resource_class`, which didn't exist on this service — it would have `NameError`d if invoked. Copy-paste leftover; deleted as part of the coverage push.

### KI-6 · RBS stub
`sig/devise/api.rbs` declares only the `VERSION` constant. Either flesh out signatures or drop the `sig/` directory to avoid signaling type support that doesn't exist.
Expand All @@ -43,14 +43,14 @@ Only records `0.0.0` while the gem is at `0.2.0` with substantive releases in be

## Test-coverage gaps (feeds the "add more tests" milestone)

### KI-12 · Service specs are placeholders
All six `spec/services/**` files only assert inheritance from `BaseService`. Real branch coverage (monad contracts per [services.md](../services.md)) is missing at the unit level.
### KI-12 · ~~Service specs are placeholders~~ (resolved)
All six `spec/services/**` files now assert the monad contracts (`Success`/`Failure` per branch, per [services.md](../services.md)), including the failure paths unreachable through the HTTP API (`:invalid_resource_owner`, `:devise_api_token_create_error`, `:devise_api_token_revoke_error`, sign-up transaction rollback).

### KI-13 · No generator specs
`rails g devise_api:install` (migration template rendering incl. UUID primary-key handling, locale copy) is untested despite `spec_helper` requiring the generator test harness.
### KI-13 · ~~No generator specs~~ (resolved)
`rails g devise_api:install` is covered by `spec/devise/api/generators/install_generator_spec.rb` (migration template rendering with the current Active Record version, locale copy, migration numbering).

### KI-14 · Non-default configuration is untested
No specs exercise: `authorization.location = :header`/`:params` exclusively, custom `authorization.key`/`scheme`/`params_key`, `sign_up.enabled = false`, `refresh_token.enabled = false`, `expires_in_infinite` procs, custom generators, `sign_up.extra_fields`, `base_token_model`/`base_controller` overrides, or any before/after callback invocation.
### KI-14 · Non-default configuration is untested (mostly resolved)
`spec/requests/configuration_overrides_spec.rb` covers `authorization.location = :header`/`:params` exclusively, `sign_up.enabled = false`, `refresh_token.enabled = false` and `sign_up.extra_fields` end-to-end; `spec/devise/api/token_spec.rb` covers `expires_in_infinite` procs and custom generators; `spec/devise/api/configuration_spec.rb` covers overrides on fresh instances; callback invocation was already asserted in `spec/requests/tokens_spec.rb`. Still untested: custom `authorization.key`/`scheme`/`params_key` and `base_token_model`/`base_controller` overrides.

## Cross-references into security review

Expand Down
2 changes: 1 addition & 1 deletion docs/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ Two workflows on every push, matrix over Ruby 2.7.7 / 3.0.5 / 3.1.3 / 3.2.0:
- `test.yml` — `bundle install` + `bundle exec rake rspec`
- `rubocop.yml` — `bundle exec rubocop --config .rubocop.yml --parallel`

No coverage reporting, no scheduled builds, no release automation.
SimpleCov coverage runs with the suite (95% line minimum enforced on full-suite runs — see [testing.md](testing.md)). No scheduled builds, no release automation.

## Pointers for common change types

Expand Down
1 change: 0 additions & 1 deletion docs/services.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ graph LR
- **Inputs:** `resource_owner` (untyped), `previous_refresh_token: String | Nil = nil`
- **Logic:** guards `resource_owner.respond_to?(:access_tokens)` → builds row with generated unique access/refresh tokens, `expires_in` snapshot from config, `previous_refresh_token` passthrough.
- **Success:** the token. **Failures:** `:invalid_resource_owner`, `:devise_api_token_create_error`.
- ⚠ Contains a dead private method `authenticate_service` referencing undefined `params`/`resource_class` (never called; tracked in known-issues).

### `Refresh`
- **Inputs:** `devise_api_token` (typed `Types.Instance(<base_token_model>)` — resolved at class load), `resource_owner` (defaults to the token's owner)
Expand Down
27 changes: 20 additions & 7 deletions docs/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,18 @@ bundle exec rspec --only-failures # uses .rspec_status

CI (`.github/workflows/test.yml`, `rubocop.yml`) runs on push across Ruby 2.7 / 3.0 / 3.1 / 3.2.

## Coverage

SimpleCov runs automatically with every spec run (started at the top of `spec/spec_helper.rb`, before the gem
is required); the HTML report lands in `coverage/index.html` and the summary in `coverage/.last_run.json`.
A **95% line-coverage minimum** is enforced whenever `CI` or `ENFORCE_COVERAGE` is set — `bundle exec rake`
sets `ENFORCE_COVERAGE` via the `enforce_coverage` prerequisite task, so full-suite runs (local and CI) fail
below the bar while single-file `bundle exec rspec` runs stay unaffected. Branch coverage is reported but not
enforced. `lib/devise/api/version.rb` is filtered because the gemspec loads it before SimpleCov can start.

## How the suite is wired

- `spec/spec_helper.rb` sets `RAILS_ENV=test`, requires the gem, then boots the **dummy Rails app** at `spec/dummy` (`require 'dummy/config/environment'`) — a real Rails 7 app with sqlite3 whose `User` model enables `database_authenticatable, registerable, recoverable, rememberable, validatable, confirmable, lockable, trackable, :api` (schema: `spec/dummy/db/schema.rb`).
- `spec/spec_helper.rb` sets `RAILS_ENV=test`, starts SimpleCov, requires the gem, then boots the **dummy Rails app** at `spec/dummy` (`require 'dummy/config/environment'`) — a real Rails 7 app with sqlite3 whose `User` model enables `database_authenticatable, registerable, recoverable, rememberable, validatable, confirmable, lockable, trackable, :api` (schema: `spec/dummy/db/schema.rb`). A second bare model, `AdminUser` (`database_authenticatable, registerable, validatable, :api` only), exists to exercise the "optional Devise module not enabled" branches (non-trackable sign-in, error/token responses without `lockable`/`confirmable` info); it has its own `devise_for :admin_users` routes.
- `DatabaseCleaner` wraps every example; spec types are inferred from file location; monkey-patching is disabled (`RSpec.describe` only).
- `spec/supports/` is auto-required: FactoryBot setup, ActiveRecord config, and two request-spec helpers:
- `authentication_headers_for(owner, token = nil, token_type = :access_token)` → `{ Authorization: "Bearer …" }` (creates a token via FactoryBot when none given; pass `:refresh_token` to authenticate refresh calls)
Expand All @@ -23,6 +32,7 @@ CI (`.github/workflows/test.yml`, `rubocop.yml`) runs on push across Ruby 2.7 /
## Factories (`spec/factories/`)

- `:user` — Faker email/password.
- `:admin_user` — Faker email/password (bare model without optional Devise modules).
- `:devise_api_token` — random hex tokens, `expires_in: 1.hour`, associated `:user`. Traits: `:access_token_expired` (backdates `created_at` 2h), `:refresh_token_expired` (2 months), `:revoked`.

Note the traits work by **backdating `created_at`** because all expiry math derives from it — keep that in mind when adding time-sensitive specs (or use `travel_to`).
Expand All @@ -32,13 +42,16 @@ Note the traits work by **backdating `created_at`** because all expiry math deri
| Area | Spec | State |
|---|---|---|
| All 5 endpoints × valid/invalid/expired/revoked × header/param | `spec/requests/tokens_spec.rb` (~700 lines) | ✅ primary coverage |
| Endpoints for a bare model (no trackable/lockable/confirmable) | `spec/requests/admin_user_tokens_spec.rb` | ✅ |
| `authenticate_devise_api_token!` on a host controller | `spec/requests/authentication_spec.rb` (via dummy `HomeController`) | ✅ |
| Default + customized routes (`controllers:`, `path:` overrides) | `spec/routing/*.rb` | ✅ |
| Config defaults | `spec/devise/api/configuration_spec.rb` | ✅ defaults only |
| Response classes | `spec/devise/api/responses/*_spec.rb` | partial |
| **Service objects** | `spec/services/**` | ⚠ **placeholders** — each spec only asserts inheritance from `BaseService` |
| Generator | required in spec_helper, no assertions | ⚠ gap |
| Non-default config (custom generators, `:header`-only location, disabled sign_up/refresh, `expires_in_infinite`, extra_fields) | — | ⚠ gap |
| Non-default config (disabled sign_up/refresh, extra_fields, `:header`/`:params`-only location, revoke failure) | `spec/requests/configuration_overrides_spec.rb` | ✅ |
| Default + customized routes (`controllers:`, `path:`, `path_names:` overrides) | `spec/routing/*.rb` | ✅ |
| Config defaults + overrides on fresh instances | `spec/devise/api/configuration_spec.rb` | ✅ |
| Response classes (incl. locked/unconfirmed/bare-model variants, disabled refresh, extra_fields) | `spec/devise/api/responses/*_spec.rb` | ✅ |
| **Service objects** (monad contract: `Success`/`Failure` per branch) | `spec/services/**` | ✅ |
| Token model (`active?`, expiry incl. `expires_in_infinite`, generator collision retry, conditional validations) | `spec/devise/api/token_spec.rb` | ✅ |
| Controller helpers (extraction rescue, invalid location `ArgumentError`, unmemoized refresh-token lookup) | `spec/devise/api/controllers/helpers_spec.rb` | ✅ |
| Generator (migration template, locale copy) | `spec/devise/api/generators/install_generator_spec.rb` | ✅ |

## Conventions for new specs

Expand Down
6 changes: 5 additions & 1 deletion lib/devise/api/responses/token_response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,11 @@ def status
def signed_up_body
return default_body unless resource_owner.class.supported_devise_modules.confirmable?

message = resource_owner.confirmed? ? nil : I18n.t('devise.api.error_response.registerable.signed_up_but_unconfirmed')
message = if resource_owner.confirmed?
nil
else
I18n.t('devise.api.error_response.registerable.signed_up_but_unconfirmed')
end

default_body.merge(confirmable: { confirmed: resource_owner.confirmed?, message: message }.compact)
end
Expand Down
29 changes: 29 additions & 0 deletions spec/devise/api/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,33 @@
end
end
end

context 'overridden settings' do
before do
config.config.access_token.expires_in = 2.hours
config.config.access_token.generator = proc { |_resource_owner| 'custom token' }
config.config.refresh_token.enabled = false
config.config.sign_up.enabled = false
config.config.sign_up.extra_fields = [:name]
config.config.authorization.location = :header
end

it 'reflects the overridden values' do
expect(config.access_token.expires_in).to eq 2.hours
expect(config.access_token.generator.call).to eq 'custom token'
expect(config.refresh_token.enabled).to eq false
expect(config.sign_up.enabled).to eq false
expect(config.sign_up.extra_fields).to eq [:name]
expect(config.authorization.location).to eq :header
end

it 'does not affect other instances' do
other_config = described_class.new

expect(other_config.access_token.expires_in).to eq 1.hour
expect(other_config.refresh_token.enabled).to eq true
expect(other_config.sign_up.enabled).to eq true
expect(other_config.authorization.location).to eq :both
end
end
end
91 changes: 91 additions & 0 deletions spec/devise/api/controllers/helpers_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
# frozen_string_literal: true

require 'spec_helper'

RSpec.describe Devise::Api::Controllers::Helpers do
let(:host_class) do
Class.new do
include Devise::Api::Controllers::Helpers

attr_accessor :request, :params
end
end
let(:host) { host_class.new }

describe '#current_devise_api_refresh_token' do
context 'when the extracted token matches a refresh token' do
let(:devise_api_token) { create(:devise_api_token) }

before do
allow(host).to receive(:find_devise_api_token).and_return(devise_api_token.refresh_token)
end

it 'returns the token record' do
expect(host.current_devise_api_refresh_token).to eq(devise_api_token)
end
end

context 'when no token can be extracted' do
before do
allow(host).to receive(:find_devise_api_token).and_return(nil)
end

it 'returns nil' do
expect(host.current_devise_api_refresh_token).to be_nil
end
end
end

describe '#current_devise_api_user' do
context 'when the extracted token matches an access token' do
let(:devise_api_token) { create(:devise_api_token) }

before do
allow(host).to receive(:find_devise_api_token).and_return(devise_api_token.access_token)
end

it 'returns the resource owner' do
expect(host.current_devise_api_user).to eq(devise_api_token.resource_owner)
end
end

context 'when no token can be extracted' do
before do
allow(host).to receive(:find_devise_api_token).and_return(nil)
end

it 'returns nil' do
expect(host.current_devise_api_user).to be_nil
end
end
end

describe '#extract_devise_api_token_from_headers' do
context 'when stripping the authorization scheme raises an error' do
let(:token) { double('token', blank?: false) }

before do
host.request = double('request', headers: { 'Authorization' => token })

allow(token).to receive(:gsub).and_raise(StandardError)
end

it 'returns the raw token' do
expect(host.send(:extract_devise_api_token_from_headers)).to eq(token)
end
end
end

describe '#find_devise_api_token' do
context 'when the authorization location is invalid' do
before do
allow(Devise.api.config.authorization).to receive(:location).and_return(:invalid)
end

it 'raises an ArgumentError' do
expect { host.send(:find_devise_api_token) }
.to raise_error(ArgumentError, 'Invalid authorization location, must be :header, :params or :both')
end
end
end
end
54 changes: 54 additions & 0 deletions spec/devise/api/generators/install_generator_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# frozen_string_literal: true

require 'spec_helper'
require 'tmpdir'

RSpec.describe Devise::Api::Generators::InstallGenerator do
describe '.next_migration_number' do
it 'returns a migration number in the timestamp format' do
expect(described_class.next_migration_number('db/migrate')).to match(/\A\d{14}\z/)
end
end

describe '#install' do
let(:destination) { Dir.mktmpdir }
let(:migration_paths) { Dir.glob(File.join(destination, 'db/migrate/*_create_devise_api_tables.rb')) }
let(:locale_path) { File.join(destination, 'config/locales/devise_api.en.yml') }

before do
described_class.start(['--quiet'], destination_root: destination)
end

after do
FileUtils.remove_entry(destination)
end

context 'migration template' do
it 'creates the migration' do
expect(migration_paths.size).to eq 1
end

it 'renders the migration for the current Active Record version' do
migration_version = "[#{ActiveRecord::VERSION::MAJOR}.#{ActiveRecord::VERSION::MINOR}]"

expect(File.read(migration_paths.first)).to include("ActiveRecord::Migration#{migration_version}")
end

it 'creates the devise api tokens table' do
expect(File.read(migration_paths.first)).to include('create_table :devise_api_tokens')
end
end

context 'locale file' do
it 'copies the locale file' do
expect(File.exist?(locale_path)).to eq true
end

it 'copies the gem locale content' do
gem_locale_path = File.expand_path('../../../../config/locales/en.yml', __dir__)

expect(File.read(locale_path)).to eq File.read(gem_locale_path)
end
end
end
end
Loading
Loading