From ebf16041de7a32ce4b85de29e662bbfa876f2967 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 5 Oct 2026 21:27:03 -0700 Subject: [PATCH 1/4] Answer API authentication failures with a Bearer challenge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The resource side answered every bearer failure with Rails' default `WWW-Authenticate: Token realm="Application"`, with no error code, and sent JSON requests that carried no credentials to the sign-in page. RFC 6750 §3 requires a challenge using the Bearer scheme, including when credentials are missing. With OAuth access tokens now expiring hourly, clients rely on that challenge to know when to refresh. - An unknown, expired, revoked or unhonored token: 401 with error="invalid_token". - A valid read-only token on a write: 403 with error="insufficient_scope", scope="write" (§3.1). It used to be the same 401 as a dead token, so an OAuth client would refresh into another read token and loop. - A JSON API request with no credentials: 401 with a Bearer challenge and no error code, instead of a redirect. Our own pages keep the redirect. They send a session cookie (stale or not) or ask over XHR, and @rails/request.js would navigate to a 401's WWW-Authenticate value as if it were a URL. - A Bearer header on a non-JSON request is still refused with a 401, now as a Bearer challenge with error="invalid_request". - The scheme is matched case-insensitively, as a parsed token (RFC 9110 §11.1), rather than by finding "Bearer" anywhere in the header. - Challenges carry resource_metadata (RFC 9728 §5.1) only while OAuth acceptance is on, so a dark server never advertises a URL that 404s. Identity.find_by_permissable_access_token becomes Identity::AccessToken.find_honored, which leaves the method check to the caller so it can tell a dead token from one without enough scope. --- app/controllers/concerns/authentication.rb | 71 ++++++++-- app/models/identity/access_token.rb | 6 +- docs/api/README.md | 12 ++ .../direct_uploads_controller_test.rb | 14 +- test/controllers/api_test.rb | 125 +++++++++++++++++- .../my/access_tokens_controller_test.rb | 2 +- test/models/identity/access_token_test.rb | 17 ++- 7 files changed, 228 insertions(+), 19 deletions(-) diff --git a/app/controllers/concerns/authentication.rb b/app/controllers/concerns/authentication.rb index dd937387c6..ba29e8427b 100644 --- a/app/controllers/concerns/authentication.rb +++ b/app/controllers/concerns/authentication.rb @@ -77,30 +77,79 @@ def find_session_by_cookie end def authenticate_by_bearer_token - if request.authorization.to_s.include?("Bearer") + if bearer_authorization? if bearer_token_authenticatable_request? - authenticate_or_request_with_http_token do |token| - if access_token = Identity::AccessToken.find_permissable(token, method: request.method) - Current.access_token = access_token - Current.identity = access_token.identity - end - end + authenticate_with_bearer_token else - request_http_token_authentication + request_bearer_authentication error: "invalid_request", error_description: "Bearer tokens authenticate JSON requests only" end end end + # The auth-scheme is a case-insensitive token (RFC 9110 §11.1), read with the + # same pattern Rails parses the header with. + def bearer_authorization? + request.authorization.to_s[ActionController::HttpAuthentication::Token::AUTH_SCHEME_REGEX, 1].to_s.casecmp?("Bearer") + end + + # A token that is unknown, expired, revoked or not honored is invalid_token, + # which tells an OAuth client to refresh. A good token without the permission + # the method needs is insufficient_scope: refreshing can't widen a grant, so + # the client must ask for write instead (RFC 6750 §3.1). + def authenticate_with_bearer_token + access_token = authenticate_with_http_token(scheme: "Bearer") { |token| Identity::AccessToken.find_honored(token) } + + if access_token&.allows?(request.method) + Current.access_token = access_token + Current.identity = access_token.identity + elsif access_token + request_bearer_authentication status: :forbidden, error: "insufficient_scope", + error_description: "The access token is read-only", scope: "write" + else + request_bearer_authentication error: "invalid_token", error_description: "The access token is expired, revoked, or invalid" + end + end + def bearer_token_authenticatable_request? request.format.json? end def request_authentication - if Current.account.present? - session[:return_to_after_authenticating] = request.url + if bearer_challengeable_request? + request_bearer_authentication + else + if Current.account.present? + session[:return_to_after_authenticating] = request.url + end + + redirect_to_login_url end + end + + # API clients that sent no credentials get a challenge rather than a sign-in + # page (RFC 6750 §3). Our own pages keep the redirect: they send a session + # cookie, stale or not, or ask over XHR, and @rails/request.js navigates to a + # 401's WWW-Authenticate value as if it were a URL. + def bearer_challengeable_request? + request.format.json? && !request.xhr? && cookies[:session_token].blank? + end + + def request_bearer_authentication(status: :unauthorized, **params) + headers["WWW-Authenticate"] = bearer_challenge(**params) + head status + end + + def bearer_challenge(**params) + params = { realm: "Application", resource_metadata: protected_resource_metadata_url, **params }.compact + "Bearer " + params.map { |name, value| %(#{name}="#{value}") }.join(", ") + end - redirect_to_login_url + # RFC 9728 §5.1. Discovery 404s while OAuth is dark, so the challenge names it + # only while it answers. + def protected_resource_metadata_url + if Oauth::Availability.acceptance_enabled? + "#{main_app.root_url(script_name: nil)}.well-known/oauth-protected-resource" + end end def after_authentication_url diff --git a/app/models/identity/access_token.rb b/app/models/identity/access_token.rb index 71152d088d..bd7b003ff1 100644 --- a/app/models/identity/access_token.rb +++ b/app/models/identity/access_token.rb @@ -14,8 +14,10 @@ class Identity::AccessToken < ApplicationRecord before_create :set_expiry_and_refresh_token, if: :oauth_client_id? class << self - def find_permissable(token, method:) - if (access_token = active.find_by(token: token)) && access_token.honored? && access_token.allows?(method) + # The token a bearer request may authenticate with: active and honored. + # Whether it permits the request's method is the caller's question. + def find_honored(token) + if (access_token = active.find_by(token: token)) && access_token.honored? access_token end end diff --git a/docs/api/README.md b/docs/api/README.md index 12dd705444..e8edb1282f 100644 --- a/docs/api/README.md +++ b/docs/api/README.md @@ -92,6 +92,18 @@ When a request fails, the API response will communicate the source of the proble | `422 Unprocessable Entity` | Validation failed (see error response format above) | | `500 Internal Server Error` | An unexpected error occurred on the server | +Authentication failures carry a `WWW-Authenticate: Bearer` challenge (RFC 6750) that says what went wrong: + +- A request with no credentials gets `401` and a challenge with no `error`. +- An access token that is expired, revoked or unknown gets `401` with `error="invalid_token"`. For an OAuth token, refresh it and retry. +- A read-only access token used to change data gets `403` with `error="insufficient_scope", scope="write"`. Refreshing won't help: get a token with write access. + +``` +WWW-Authenticate: Bearer realm="Application", resource_metadata="https://app.fizzy.do/.well-known/oauth-protected-resource", error="invalid_token", error_description="The access token is expired, revoked, or invalid" +``` + +When the server offers OAuth, `resource_metadata` points to its protected resource metadata (RFC 9728), from which a client can find the authorization server. + If a request contains invalid data for fields, such as entering a string into a number field, in most cases the API will respond with a `500 Internal Server Error`. Clients are expected to perform some validation on their end before making a request. A validation error will produce a `422 Unprocessable Entity` response, which will sometimes be accompanied by details about the error: diff --git a/test/controllers/active_storage/direct_uploads_controller_test.rb b/test/controllers/active_storage/direct_uploads_controller_test.rb index 9ee0cb8b9c..b2a47d00bf 100644 --- a/test/controllers/active_storage/direct_uploads_controller_test.rb +++ b/test/controllers/active_storage/direct_uploads_controller_test.rb @@ -77,7 +77,8 @@ class ActiveStorage::DirectUploadsControllerTest < ActionDispatch::IntegrationTe headers: bearer_token_header(identity_access_tokens(:jasons_api_token).token), as: :json - assert_response :unauthorized + assert_response :forbidden + assert_match 'error="insufficient_scope"', response.headers["WWW-Authenticate"] end test "create with invalid access token" do @@ -87,6 +88,7 @@ class ActiveStorage::DirectUploadsControllerTest < ActionDispatch::IntegrationTe as: :json assert_response :unauthorized + assert_match 'error="invalid_token"', response.headers["WWW-Authenticate"] end test "create unauthenticated" do @@ -94,6 +96,16 @@ class ActiveStorage::DirectUploadsControllerTest < ActionDispatch::IntegrationTe params: @blob_params, as: :json + assert_response :unauthorized + assert_match(/\ABearer /, response.headers["WWW-Authenticate"]) + end + + test "create unauthenticated from the uploader in our own pages still goes to sign in" do + post rails_direct_uploads_path, + params: @blob_params, + xhr: true, + as: :json + assert_response :redirect end diff --git a/test/controllers/api_test.rb b/test/controllers/api_test.rb index f52bc17425..42bfe96887 100644 --- a/test/controllers/api_test.rb +++ b/test/controllers/api_test.rb @@ -1,6 +1,8 @@ require "test_helper" class ApiTest < ActionDispatch::IntegrationTest + include OauthAvailabilityTestHelper + setup do @davids_bearer_token = bearer_token_env(identity_access_tokens(:davids_api_token).token) @jasons_bearer_token = bearer_token_env(identity_access_tokens(:jasons_api_token).token) @@ -49,19 +51,140 @@ class ApiTest < ActionDispatch::IntegrationTest test "fail to authenticate with invalid access token" do get boards_path(format: :json), env: bearer_token_env("nonsense") + + assert_response :unauthorized + assert_bearer_challenge error: "invalid_token" + end + + test "an expired access token is challenged as invalid_token so the client refreshes" do + token = identities(:david).access_tokens.create!(oauth_client: oauth_clients(:mcp_client), permission: :write) + + travel Identity::AccessToken::EXPIRES_IN + 1.second do + get boards_path(format: :json), env: bearer_token_env(token.token) + end + assert_response :unauthorized + assert_bearer_challenge error: "invalid_token" + end + + test "an OAuth token is challenged as invalid_token while OAuth is dark" do + token = identities(:david).access_tokens.create!(oauth_client: oauth_clients(:mcp_client), permission: :write) + + with_oauth_availability acceptance: false do + get boards_path(format: :json), env: bearer_token_env(token.token) + end + + assert_response :unauthorized + assert_match 'error="invalid_token"', response.headers["WWW-Authenticate"] + assert_no_match "resource_metadata", response.headers["WWW-Authenticate"] end test "changing data requires a write-endowed access token" do post boards_path(format: :json), params: { board: { name: "My new board" } }, env: @jasons_bearer_token - assert_response :unauthorized + assert_response :forbidden + assert_bearer_challenge error: "insufficient_scope" + assert_match 'scope="write"', response.headers["WWW-Authenticate"] post boards_path(format: :json), params: { board: { name: "My new board" } }, env: @davids_bearer_token assert_response :success end + test "a read-only token still reads" do + get boards_path(format: :json), env: @jasons_bearer_token + assert_response :success + end + + test "an expired read-only token on a write is invalid_token, not insufficient_scope" do + token = identities(:jason).access_tokens.create!(oauth_client: oauth_clients(:mcp_client), permission: :read) + + travel Identity::AccessToken::EXPIRES_IN + 1.second do + post boards_path(format: :json), params: { board: { name: "My new board" } }, env: bearer_token_env(token.token) + end + + assert_response :unauthorized + assert_bearer_challenge error: "invalid_token" + end + + test "a JSON request without credentials is challenged instead of redirected to sign in" do + get boards_path(format: :json) + + assert_response :unauthorized + assert_bearer_challenge + assert_no_match "error=", response.headers["WWW-Authenticate"] + end + + test "the challenge names protected resource metadata that resolves" do + get boards_path(format: :json) + metadata_url = response.headers["WWW-Authenticate"][/resource_metadata="([^"]+)"/, 1] + + get URI(metadata_url).request_uri + assert_response :success + assert_equal "http://www.example.com/", response.parsed_body["resource"] + end + + test "the challenge leaves out resource metadata while OAuth is dark" do + with_oauth_availability acceptance: false do + get boards_path(format: :json) + end + + assert_response :unauthorized + assert_match(/\ABearer realm="Application"\z/, response.headers["WWW-Authenticate"]) + end + + test "a JSON request from our own pages without a session still goes to sign in" do + get boards_path(format: :json), xhr: true + assert_response :redirect + + cookies[:session_token] = "stale" + get boards_path(format: :json) + assert_response :redirect + end + + test "an HTML request without credentials still goes to sign in" do + get boards_path + assert_response :redirect + end + + test "the bearer scheme is matched case-insensitively" do + get boards_path(format: :json), env: { "HTTP_AUTHORIZATION" => "bearer #{identity_access_tokens(:davids_api_token).token}" } + assert_response :success + + get boards_path(format: :json), env: { "HTTP_AUTHORIZATION" => "BEARER nonsense" } + assert_response :unauthorized + assert_bearer_challenge error: "invalid_token" + end + + test "Bearer elsewhere in another scheme's credentials is not bearer authentication" do + get boards_path(format: :json), env: { "HTTP_AUTHORIZATION" => "Basic Bearer #{identity_access_tokens(:davids_api_token).token}" } + + assert_response :unauthorized + assert_no_match "error=", response.headers["WWW-Authenticate"] + end + + test "a bearer token on an HTML request is refused with a Bearer challenge" do + get boards_path, env: @davids_bearer_token + + assert_response :unauthorized + assert_bearer_challenge error: "invalid_request" + end + + test "a session cookie still authenticates JSON" do + sign_in_as :david + + get boards_path(format: :json) + assert_response :success + end + private def bearer_token_env(token) { "HTTP_AUTHORIZATION" => "Bearer #{token}" } end + + def assert_bearer_challenge(error: nil) + challenge = response.headers["WWW-Authenticate"] + + assert_match(/\ABearer realm="Application"/, challenge) + assert_match 'resource_metadata="http://www.example.com/.well-known/oauth-protected-resource"', challenge + assert_match %(error="#{error}"), challenge if error + end end diff --git a/test/controllers/my/access_tokens_controller_test.rb b/test/controllers/my/access_tokens_controller_test.rb index 9bec86fad0..f4b5771c37 100644 --- a/test/controllers/my/access_tokens_controller_test.rb +++ b/test/controllers/my/access_tokens_controller_test.rb @@ -88,7 +88,7 @@ class My::AccessTokensControllerTest < ActionDispatch::IntegrationTest assert_no_difference -> { identities(:jason).access_tokens.count } do post my_access_tokens_path, params: { access_token: { description: "Fizzy CLI", permission: "read" } }, env: bearer_token, as: :json end - assert_response :unauthorized + assert_response :forbidden end test "index as JSON" do diff --git a/test/models/identity/access_token_test.rb b/test/models/identity/access_token_test.rb index 722fc665fc..c8daf759c5 100644 --- a/test/models/identity/access_token_test.rb +++ b/test/models/identity/access_token_test.rb @@ -59,12 +59,23 @@ class Identity::AccessTokenTest < ActiveSupport::TestCase end end - test "find_permissable rejects expired tokens" do + test "find_honored rejects expired tokens" do token = identities(:david).access_tokens.create!(oauth_client: oauth_clients(:mcp_client), permission: :write) - assert_equal token, Identity::AccessToken.find_permissable(token.token, method: "GET") + assert_equal token, Identity::AccessToken.find_honored(token.token) travel Identity::AccessToken::EXPIRES_IN + 1.second do - assert_nil Identity::AccessToken.find_permissable(token.token, method: "GET") + assert_nil Identity::AccessToken.find_honored(token.token) end end + + test "find_honored returns a read-only token and leaves the method check to the caller" do + token = identity_access_tokens(:jasons_api_token) + + assert_equal token, Identity::AccessToken.find_honored(token.token) + assert_not token.allows?("POST") + end + + test "find_honored rejects unknown tokens" do + assert_nil Identity::AccessToken.find_honored("nonsense") + end end From dd2b951f18af15b7b5443e08376dc079285adc5d Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 5 Oct 2026 21:32:03 -0700 Subject: [PATCH 2/4] Expect the Bearer challenge for unauthenticated device registration in SaaS My::DevicesController's JSON endpoints now answer a request without a session cookie with a 401 Bearer challenge, not a sign-in redirect. The native apps register devices after signing in, carrying the session cookie; a stale cookie still gets the redirect. --- saas/test/controllers/my/devices_controller_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/saas/test/controllers/my/devices_controller_test.rb b/saas/test/controllers/my/devices_controller_test.rb index 5ba2172f16..530d1ef1f4 100644 --- a/saas/test/controllers/my/devices_controller_test.rb +++ b/saas/test/controllers/my/devices_controller_test.rb @@ -144,7 +144,7 @@ class My::DevicesControllerTest < ActionDispatch::IntegrationTest }, as: :json end - assert_response :redirect + assert_response :unauthorized end test "destroys device by id" do @@ -251,7 +251,7 @@ class My::DevicesControllerTest < ActionDispatch::IntegrationTest untenanted { delete saas.my_device_path("my_token"), as: :json } - assert_response :redirect + assert_response :unauthorized assert ApplicationPushDevice.exists?(device.id) end end From 600d347762495665b35528d449e913c94c59b920 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sat, 17 Oct 2026 02:17:30 -0700 Subject: [PATCH 3/4] Keep shipped clients' answers: 401 for a read-only PAT, sign-in for no credentials MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The native apps and fizzy-cli rely on both (#3191 pins them). A read-only token writing now gets 403 insufficient_scope only when it's an OAuth app's grant; a personal access token keeps its 401, with an insufficient_scope challenge. A JSON request with no credentials goes to sign in again, as it always has: nothing in it tells a shipped client from an OAuth one. That's a recorded deviation from RFC 6750 §3 (DS-3), until the oldest supported client handles a challenge. OAuth clients discover through the MCP server's challenge and /.well-known instead. --- app/controllers/concerns/authentication.rb | 32 ++++++++----------- docs/api/README.md | 4 +-- .../controllers/my/devices_controller_test.rb | 4 +-- .../direct_uploads_controller_test.rb | 7 ++-- test/controllers/api_test.rb | 30 ++++++++++------- .../my/access_tokens_controller_test.rb | 2 +- 6 files changed, 41 insertions(+), 38 deletions(-) diff --git a/app/controllers/concerns/authentication.rb b/app/controllers/concerns/authentication.rb index ba29e8427b..a60773c598 100644 --- a/app/controllers/concerns/authentication.rb +++ b/app/controllers/concerns/authentication.rb @@ -95,7 +95,9 @@ def bearer_authorization? # A token that is unknown, expired, revoked or not honored is invalid_token, # which tells an OAuth client to refresh. A good token without the permission # the method needs is insufficient_scope: refreshing can't widen a grant, so - # the client must ask for write instead (RFC 6750 §3.1). + # the client must ask for write instead (RFC 6750 §3.1). That's a 403 for an + # app's grant, but a personal access token keeps the 401 shipped clients + # (fizzy-cli, the native apps) have always had for it. def authenticate_with_bearer_token access_token = authenticate_with_http_token(scheme: "Bearer") { |token| Identity::AccessToken.find_honored(token) } @@ -103,8 +105,8 @@ def authenticate_with_bearer_token Current.access_token = access_token Current.identity = access_token.identity elsif access_token - request_bearer_authentication status: :forbidden, error: "insufficient_scope", - error_description: "The access token is read-only", scope: "write" + request_bearer_authentication status: access_token.oauth_client_id? ? :forbidden : :unauthorized, + error: "insufficient_scope", error_description: "The access token is read-only", scope: "write" else request_bearer_authentication error: "invalid_token", error_description: "The access token is expired, revoked, or invalid" end @@ -114,24 +116,18 @@ def bearer_token_authenticatable_request? request.format.json? end + # A request with no credentials goes to sign in, JSON included. RFC 6750 §3 + # would have an API request challenged instead, but shipped clients (the + # native apps, fizzy-cli) rely on the redirect, so it stays until the oldest + # supported one handles a challenge (docs/oauth.md, DS-3). OAuth clients + # don't need it to find us: they discover through the MCP server's challenge + # and our /.well-known metadata. def request_authentication - if bearer_challengeable_request? - request_bearer_authentication - else - if Current.account.present? - session[:return_to_after_authenticating] = request.url - end - - redirect_to_login_url + if Current.account.present? + session[:return_to_after_authenticating] = request.url end - end - # API clients that sent no credentials get a challenge rather than a sign-in - # page (RFC 6750 §3). Our own pages keep the redirect: they send a session - # cookie, stale or not, or ask over XHR, and @rails/request.js navigates to a - # 401's WWW-Authenticate value as if it were a URL. - def bearer_challengeable_request? - request.format.json? && !request.xhr? && cookies[:session_token].blank? + redirect_to_login_url end def request_bearer_authentication(status: :unauthorized, **params) diff --git a/docs/api/README.md b/docs/api/README.md index e8edb1282f..ec1a37b5ec 100644 --- a/docs/api/README.md +++ b/docs/api/README.md @@ -94,9 +94,9 @@ When a request fails, the API response will communicate the source of the proble Authentication failures carry a `WWW-Authenticate: Bearer` challenge (RFC 6750) that says what went wrong: -- A request with no credentials gets `401` and a challenge with no `error`. +- A request with no credentials is redirected to sign in (`302` to `/session/new`), JSON included. - An access token that is expired, revoked or unknown gets `401` with `error="invalid_token"`. For an OAuth token, refresh it and retry. -- A read-only access token used to change data gets `403` with `error="insufficient_scope", scope="write"`. Refreshing won't help: get a token with write access. +- A read-only token used to change data gets `error="insufficient_scope", scope="write"`: `403` for an OAuth app's token, `401` for a personal access token. Refreshing won't help: get a token with write access. ``` WWW-Authenticate: Bearer realm="Application", resource_metadata="https://app.fizzy.do/.well-known/oauth-protected-resource", error="invalid_token", error_description="The access token is expired, revoked, or invalid" diff --git a/saas/test/controllers/my/devices_controller_test.rb b/saas/test/controllers/my/devices_controller_test.rb index 530d1ef1f4..5ba2172f16 100644 --- a/saas/test/controllers/my/devices_controller_test.rb +++ b/saas/test/controllers/my/devices_controller_test.rb @@ -144,7 +144,7 @@ class My::DevicesControllerTest < ActionDispatch::IntegrationTest }, as: :json end - assert_response :unauthorized + assert_response :redirect end test "destroys device by id" do @@ -251,7 +251,7 @@ class My::DevicesControllerTest < ActionDispatch::IntegrationTest untenanted { delete saas.my_device_path("my_token"), as: :json } - assert_response :unauthorized + assert_response :redirect assert ApplicationPushDevice.exists?(device.id) end end diff --git a/test/controllers/active_storage/direct_uploads_controller_test.rb b/test/controllers/active_storage/direct_uploads_controller_test.rb index b2a47d00bf..f5662b9e5f 100644 --- a/test/controllers/active_storage/direct_uploads_controller_test.rb +++ b/test/controllers/active_storage/direct_uploads_controller_test.rb @@ -77,7 +77,7 @@ class ActiveStorage::DirectUploadsControllerTest < ActionDispatch::IntegrationTe headers: bearer_token_header(identity_access_tokens(:jasons_api_token).token), as: :json - assert_response :forbidden + assert_response :unauthorized assert_match 'error="insufficient_scope"', response.headers["WWW-Authenticate"] end @@ -96,11 +96,10 @@ class ActiveStorage::DirectUploadsControllerTest < ActionDispatch::IntegrationTe params: @blob_params, as: :json - assert_response :unauthorized - assert_match(/\ABearer /, response.headers["WWW-Authenticate"]) + assert_response :redirect end - test "create unauthenticated from the uploader in our own pages still goes to sign in" do + test "create unauthenticated from the uploader in our own pages goes to sign in" do post rails_direct_uploads_path, params: @blob_params, xhr: true, diff --git a/test/controllers/api_test.rb b/test/controllers/api_test.rb index 42bfe96887..a0e1d943af 100644 --- a/test/controllers/api_test.rb +++ b/test/controllers/api_test.rb @@ -81,7 +81,7 @@ class ApiTest < ActionDispatch::IntegrationTest test "changing data requires a write-endowed access token" do post boards_path(format: :json), params: { board: { name: "My new board" } }, env: @jasons_bearer_token - assert_response :forbidden + assert_response :unauthorized assert_bearer_challenge error: "insufficient_scope" assert_match 'scope="write"', response.headers["WWW-Authenticate"] @@ -89,6 +89,16 @@ class ApiTest < ActionDispatch::IntegrationTest assert_response :success end + test "a read-only grant changing data is forbidden, so the app asks for write" do + token = identities(:jason).access_tokens.create!(oauth_client: oauth_clients(:mcp_client), permission: :read) + + post boards_path(format: :json), params: { board: { name: "My new board" } }, env: bearer_token_env(token.token) + + assert_response :forbidden + assert_bearer_challenge error: "insufficient_scope" + assert_match 'scope="write"', response.headers["WWW-Authenticate"] + end + test "a read-only token still reads" do get boards_path(format: :json), env: @jasons_bearer_token assert_response :success @@ -105,16 +115,15 @@ class ApiTest < ActionDispatch::IntegrationTest assert_bearer_challenge error: "invalid_token" end - test "a JSON request without credentials is challenged instead of redirected to sign in" do + test "a JSON request without credentials goes to sign in, as shipped clients expect" do get boards_path(format: :json) - assert_response :unauthorized - assert_bearer_challenge - assert_no_match "error=", response.headers["WWW-Authenticate"] + assert_response :redirect + assert_nil response.headers["WWW-Authenticate"] end test "the challenge names protected resource metadata that resolves" do - get boards_path(format: :json) + get boards_path(format: :json), env: bearer_token_env("nonsense") metadata_url = response.headers["WWW-Authenticate"][/resource_metadata="([^"]+)"/, 1] get URI(metadata_url).request_uri @@ -124,14 +133,14 @@ class ApiTest < ActionDispatch::IntegrationTest test "the challenge leaves out resource metadata while OAuth is dark" do with_oauth_availability acceptance: false do - get boards_path(format: :json) + get boards_path(format: :json), env: bearer_token_env("nonsense") end assert_response :unauthorized - assert_match(/\ABearer realm="Application"\z/, response.headers["WWW-Authenticate"]) + assert_no_match "resource_metadata", response.headers["WWW-Authenticate"] end - test "a JSON request from our own pages without a session still goes to sign in" do + test "a JSON request from our own pages without a session goes to sign in" do get boards_path(format: :json), xhr: true assert_response :redirect @@ -157,8 +166,7 @@ class ApiTest < ActionDispatch::IntegrationTest test "Bearer elsewhere in another scheme's credentials is not bearer authentication" do get boards_path(format: :json), env: { "HTTP_AUTHORIZATION" => "Basic Bearer #{identity_access_tokens(:davids_api_token).token}" } - assert_response :unauthorized - assert_no_match "error=", response.headers["WWW-Authenticate"] + assert_response :redirect end test "a bearer token on an HTML request is refused with a Bearer challenge" do diff --git a/test/controllers/my/access_tokens_controller_test.rb b/test/controllers/my/access_tokens_controller_test.rb index f4b5771c37..9bec86fad0 100644 --- a/test/controllers/my/access_tokens_controller_test.rb +++ b/test/controllers/my/access_tokens_controller_test.rb @@ -88,7 +88,7 @@ class My::AccessTokensControllerTest < ActionDispatch::IntegrationTest assert_no_difference -> { identities(:jason).access_tokens.count } do post my_access_tokens_path, params: { access_token: { description: "Fizzy CLI", permission: "read" } }, env: bearer_token, as: :json end - assert_response :forbidden + assert_response :unauthorized end test "index as JSON" do From aa77436f8154751b6c4956b6e168234124dee371 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sun, 25 Oct 2026 22:27:07 -0500 Subject: [PATCH 4/4] Bring main's SQL redaction tests up to bearer lookup by find_honored --- test/lib/rails_ext/active_record_sql_redaction_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/lib/rails_ext/active_record_sql_redaction_test.rb b/test/lib/rails_ext/active_record_sql_redaction_test.rb index 0ef079a37a..ed914a5029 100644 --- a/test/lib/rails_ext/active_record_sql_redaction_test.rb +++ b/test/lib/rails_ext/active_record_sql_redaction_test.rb @@ -49,7 +49,7 @@ class ActiveRecordSqlRedactionTest < ActiveSupport::TestCase test "an access token is absent from its logged insert and lookup" do log, executed = capture do access_token = identities(:kevin).access_tokens.create!(token: TOKEN, permission: "write") - assert_equal identities(:kevin), Identity::AccessToken.find_permissable(TOKEN, method: "POST")&.identity + assert_equal identities(:kevin), Identity::AccessToken.find_honored(TOKEN)&.identity access_token.update!(description: "Made-up script") end @@ -139,7 +139,7 @@ class ActiveRecordSqlRedactionTest < ActiveSupport::TestCase ActiveSupport::Notifications.subscribe(/active_record/) { |_name, _start, _finish, _id, payload| seen << payload.dup } ] - assert_equal identities(:kevin), Identity::AccessToken.find_permissable(identities(:kevin).access_tokens.create!(token: TOKEN).token, method: "GET")&.identity + assert_equal identities(:kevin), Identity::AccessToken.find_honored(identities(:kevin).access_tokens.create!(token: TOKEN).token)&.identity redacted = seen.select { |payload| payload[:sql].to_s.include?("identity_access_tokens") } assert redacted.many?