From 104684f163c48dc24d16bc051b86054ba433b8d7 Mon Sep 17 00:00:00 2001 From: Ortes Date: Sun, 19 Apr 2026 09:17:42 -0400 Subject: [PATCH] fix(oauth): plaintext_token in specs, allow loopback redirect, fix throttle test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three issues surfaced running the suite in docker: - hash_token_secrets stores the access token hashed; specs must use access_token.plaintext_token (not .token) when posing as a client - Doorkeeper's Application model rejects non-HTTPS redirect_uri by default; add force_ssl_in_redirect_uri to allow loopback per OAuth 2.1 - test env uses :null_store, so Rails.cache.increment returned nil and the DCR throttle never fired — stub a real MemoryStore in that spec Also slim Dockerfile.test: drop chromium + chromium-chromedriver (unused by OAuth specs, added ~4min to the build). Add a comment pointing at the apk line to re-enable them for system specs. Co-Authored-By: Claude Opus 4.7 (1M context) --- Dockerfile.test | 13 +++++-------- config/initializers/doorkeeper.rb | 5 +++++ spec/requests/mcp_oauth_spec.rb | 8 ++++---- spec/requests/oauth_register_spec.rb | 2 ++ 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/Dockerfile.test b/Dockerfile.test index d6137374..04d670b3 100644 --- a/Dockerfile.test +++ b/Dockerfile.test @@ -4,7 +4,7 @@ # Source code is mounted at runtime via docker-compose.test.yml — only the # Gemfile is baked in so `bundle install` caches between runs. -FROM ruby:4.0.1-alpine AS pdfium +FROM ruby:4.0.2-alpine AS pdfium WORKDIR /download @@ -13,7 +13,7 @@ RUN apk --no-cache add wget && \ mkdir -p /pdfium-linux && \ tar -xzf pdfium-linux.tgz -C /pdfium-linux -FROM ruby:4.0.1-alpine +FROM ruby:4.0.2-alpine ENV RAILS_ENV=test \ BUNDLE_WITHOUT="" \ @@ -26,9 +26,11 @@ WORKDIR /app # - libpq / libpq-dev → pg gem # - vips + vips-heif → image processing (ruby-vips FFI) # - onnxruntime → field detection at boot -# - chromium + chromium-chromedriver → capybara/cuprite system specs # - fontconfig + ttf-* → PDF rendering # - build-base/git/yaml-dev → native gem compilation +# +# Capybara/cuprite system specs need chromium — add `chromium +# chromium-chromedriver` to this apk line if running `spec/system/*`. RUN apk add --no-cache \ build-base \ git \ @@ -46,8 +48,6 @@ RUN apk add --no-cache \ onnxruntime \ nodejs \ yarn \ - chromium \ - chromium-chromedriver \ tzdata # libpdfium is not in Alpine packages — copy the prebuilt binary. @@ -64,7 +64,4 @@ RUN bundle config set --local without "" && \ RUN ln -sf /usr/lib/libonnxruntime.so.1 \ "$(ruby -e "print Dir[Gem::Specification.find_by_name('onnxruntime').gem_dir + '/vendor/*.so'].first")" -# Cuprite launches Chromium directly — point it at the Alpine binary. -ENV CUPRITE_CHROME_PATH=/usr/bin/chromium-browser - CMD ["bundle", "exec", "rspec"] diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index d2be1226..df9f1233 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -27,6 +27,11 @@ Doorkeeper.configure do pkce_code_challenge_methods %w[S256] force_pkce + # Require HTTPS for redirect_uri except for loopback (OAuth 2.1 §8.4.2). + force_ssl_in_redirect_uri do |uri| + !%w[localhost 127.0.0.1 ::1].include?(uri.host) + end + default_scopes :mcp optional_scopes :mcp diff --git a/spec/requests/mcp_oauth_spec.rb b/spec/requests/mcp_oauth_spec.rb index 37cda2ed..9063c162 100644 --- a/spec/requests/mcp_oauth_spec.rb +++ b/spec/requests/mcp_oauth_spec.rb @@ -33,7 +33,7 @@ RSpec.describe 'MCP endpoint authentication', type: :request do end it 'succeeds and dispatches to HandleRequest' do - post_mcp(access_token.token) + post_mcp(access_token.plaintext_token) expect(response).to have_http_status(:ok).or have_http_status(:accepted) end end @@ -43,7 +43,7 @@ RSpec.describe 'MCP endpoint authentication', type: :request do token = create(:oauth_access_token, resource_owner_id: user.id, scopes: 'mcp', expires_in: 1) travel_to(2.hours.from_now) do - post_mcp(token.token) + post_mcp(token.plaintext_token) expect(response).to have_http_status(:unauthorized) end end @@ -56,7 +56,7 @@ RSpec.describe 'MCP endpoint authentication', type: :request do end it 'returns 401' do - post_mcp(access_token.token) + post_mcp(access_token.plaintext_token) expect(response).to have_http_status(:unauthorized) end end @@ -67,7 +67,7 @@ RSpec.describe 'MCP endpoint authentication', type: :request do end it 'returns 401' do - post_mcp(access_token.token) + post_mcp(access_token.plaintext_token) expect(response).to have_http_status(:unauthorized) end end diff --git a/spec/requests/oauth_register_spec.rb b/spec/requests/oauth_register_spec.rb index 81c0c4ea..80f24c19 100644 --- a/spec/requests/oauth_register_spec.rb +++ b/spec/requests/oauth_register_spec.rb @@ -58,6 +58,8 @@ RSpec.describe 'Dynamic Client Registration (RFC 7591)', type: :request do end it 'throttles after 20 requests from the same IP within an hour' do + allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache::MemoryStore.new) + 20.times { post_register(valid_body) } post_register(valid_body) expect(response).to have_http_status(:too_many_requests)