diff --git a/app/controllers/concerns/authentication.rb b/app/controllers/concerns/authentication.rb index dd937387c6..a60773c598 100644 --- a/app/controllers/concerns/authentication.rb +++ b/app/controllers/concerns/authentication.rb @@ -77,24 +77,51 @@ 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). 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) } + + if access_token&.allows?(request.method) + Current.access_token = access_token + Current.identity = access_token.identity + elsif access_token + 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 + end + 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 Current.account.present? session[:return_to_after_authenticating] = request.url @@ -103,6 +130,24 @@ def request_authentication redirect_to_login_url 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 + + # 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 session.delete(:return_to_after_authenticating) || landing_url end 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..ec1a37b5ec 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 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 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" +``` + +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..f5662b9e5f 100644 --- a/test/controllers/active_storage/direct_uploads_controller_test.rb +++ b/test/controllers/active_storage/direct_uploads_controller_test.rb @@ -78,6 +78,7 @@ class ActiveStorage::DirectUploadsControllerTest < ActionDispatch::IntegrationTe as: :json assert_response :unauthorized + 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 @@ -97,6 +99,15 @@ class ActiveStorage::DirectUploadsControllerTest < ActionDispatch::IntegrationTe assert_response :redirect end + 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, + as: :json + + assert_response :redirect + end + test "create in another account is forbidden" do sign_in_as :david diff --git a/test/controllers/api_test.rb b/test/controllers/api_test.rb index f52bc17425..a0e1d943af 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,148 @@ 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_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 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 + 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 goes to sign in, as shipped clients expect" do + get boards_path(format: :json) + + 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), env: bearer_token_env("nonsense") + 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), env: bearer_token_env("nonsense") + end + + assert_response :unauthorized + assert_no_match "resource_metadata", response.headers["WWW-Authenticate"] + end + + 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 + + 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 :redirect + 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/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? 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