From 017dfdff0e5497dc6d2a692fca758e785b1976e9 Mon Sep 17 00:00:00 2001 From: Dan Milne Date: Sun, 19 Jul 2026 13:19:41 +1000 Subject: [PATCH] Harden OIDC token endpoints from security review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes from review of the device flow / introspection / DCR work: Introspection over-disclosure (RFC 7662): - Restrict introspection to authorized callers — the client a token was issued to, or a resource server registered (resource_identifiers) to serve the token's bound RFC 8707 audience. Unauthorized callers get {active:false}, disclosing nothing, instead of any confidential client reading any token. - Scope-gate disclosed claims: username only with the email scope, groups only with the groups scope (mirrors userinfo). ADR 0005. Tokens minted for revoked users: - Re-check application.user_allowed?(user) at mint time in the device, refresh, and authorization-code grants (covers app-active, user-active, group membership). A user deactivated or removed from the allowed group between approval and the token request is refused with access_denied. The refresh check runs before rotation so a denied refresh has no side effects. Device authorization hardening: - Require confidential clients to authenticate, and require PKCE (code_challenge) up front for clients that require it, so an intercepted device_code plus a known public client_id cannot redeem tokens. - Merge (not overwrite) the shared consent record on device approval. Tests: new oidc_introspection_test and oidc_mint_authorization_test; existing PKCE/claims tests updated to grant app access (they created ungrouped apps and relied on the token endpoint not checking authorization). Full suite green. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01F7cwhwDJp3MJJDoNPVE6zq --- .../device_authorizations_controller.rb | 8 +- app/controllers/oidc_controller.rb | 94 +++++++++++- app/models/application.rb | 13 ++ ...dd_resource_identifiers_to_applications.rb | 8 ++ db/schema.rb | 3 +- db/seeds.rb | 17 ++- .../0005-introspection-authorization.md | 46 ++++++ docs/decisions/README.md | 1 + test/controllers/oidc_claims_security_test.rb | 4 + .../oidc_device_flow_controller_test.rb | 131 +++++++++++------ test/controllers/oidc_introspection_test.rb | 136 ++++++++++++++++++ .../oidc_mint_authorization_test.rb | 114 +++++++++++++++ test/controllers/oidc_pkce_controller_test.rb | 3 + .../oidc_resource_indicators_test.rb | 22 ++- 14 files changed, 533 insertions(+), 67 deletions(-) create mode 100644 db/migrate/20260719000004_add_resource_identifiers_to_applications.rb create mode 100644 docs/decisions/0005-introspection-authorization.md create mode 100644 test/controllers/oidc_introspection_test.rb create mode 100644 test/controllers/oidc_mint_authorization_test.rb diff --git a/app/controllers/device_authorizations_controller.rb b/app/controllers/device_authorizations_controller.rb index e1958e0..ce93428 100644 --- a/app/controllers/device_authorizations_controller.rb +++ b/app/controllers/device_authorizations_controller.rb @@ -80,8 +80,12 @@ class DeviceAuthorizationsController < ApplicationController def record_consent(device_code, user) consent = OidcUserConsent.find_or_initialize_by(user: user, application: device_code.application) - consent.scopes_granted = granted_scopes(device_code).join(" ") - consent.claims_requests = {} + # Merge into any existing consent instead of overwriting it. The consent + # record is shared with the browser flow (unique on user+application) and + # scopes_granted is treated as a granted superset, so a narrower device + # request must not shrink previously granted scopes or wipe stored claims. + consent.scopes = consent.scopes_granted.to_s.split | granted_scopes(device_code) + consent.claims_requests = {} if consent.new_record? consent.granted_at = Time.current consent.save! end diff --git a/app/controllers/oidc_controller.rb b/app/controllers/oidc_controller.rb index 6c466d9..b837afb 100644 --- a/app/controllers/oidc_controller.rb +++ b/app/controllers/oidc_controller.rb @@ -80,7 +80,7 @@ class OidcController < ApplicationController # Public (PKCE) client presents its client_id and gets back a device_code the # client polls with, plus a short user_code the human types on the /device page. def device_authorization - client_id, _client_secret = extract_client_credentials + client_id, client_secret = extract_client_credentials application = Application.find_by(client_id: client_id, app_type: "oidc") unless application&.active? @@ -88,6 +88,17 @@ class OidcController < ApplicationController return end + # RFC 8628 §3.1: the device authorization request must authenticate the client + # per its type. Public (PKCE) clients present only their client_id; a + # confidential client must also prove possession of its secret, otherwise an + # attacker knowing the public client_id could initiate a request in its name. + if application.confidential_client? + unless client_secret.present? && application.authenticate_client_secret(client_secret) + render json: {error: "invalid_client", error_description: "Invalid client credentials"}, status: :unauthorized + return + end + end + # Only accept scopes we support (mirrors the authorize endpoint). requested_scope = (params[:scope].to_s.split & SUPPORTED_SCOPES).join(" ") requested_scope = "openid" if requested_scope.blank? @@ -97,6 +108,14 @@ class OidcController < ApplicationController code_challenge = params[:code_challenge].presence code_challenge_method = params[:code_challenge_method].presence + # Public clients have no secret, so PKCE is their only proof-of-possession. + # Require the code_challenge up front — otherwise an intercepted device_code + # plus the well-known public client_id would be enough to redeem tokens. + if application.requires_pkce? && code_challenge.blank? + render json: {error: "invalid_request", error_description: "code_challenge is required for this client"}, status: :bad_request + return + end + if code_challenge_method.present? && code_challenge_method != "S256" render json: {error: "invalid_request", error_description: "Only S256 code_challenge_method is supported"}, status: :bad_request return @@ -605,6 +624,16 @@ class OidcController < ApplicationController # Approved: mint tokens via the same path as the authorization code grant. user = device_code.user + + # Re-check authorization at mint time. Approval may have happened minutes ago; + # an admin could have deactivated the user or removed them from the allowed + # group in the meantime. user_allowed? covers app active, user active, and + # group membership, so a now-unauthorized user is refused their tokens. + unless user && application.user_allowed?(user) + render json: {error: "access_denied", error_description: "User is no longer permitted to access this application"}, status: :bad_request + return + end + consent = OidcUserConsent.find_by(user: user, application: application) unless consent Rails.logger.error "OIDC Security: Device token requested without consent record (user: #{user&.id}, app: #{application.id})" @@ -612,8 +641,15 @@ class OidcController < ApplicationController return end - # PKCE is optional for device flow: only enforced when the device - # authorization request supplied a code_challenge. + # PKCE is enforced whenever the device authorization request supplied a + # code_challenge. Clients that require PKCE (all public clients) are also + # guaranteed to have one by the device_authorization endpoint; re-check here + # so a device_code minted without a challenge can never redeem tokens. + if application.requires_pkce? && !device_code.uses_pkce? + render json: {error: "invalid_grant", error_description: "PKCE is required for this client"}, status: :bad_request + return + end + if device_code.uses_pkce? pkce_result = validate_pkce(application, device_code, params[:code_verifier]) unless pkce_result[:valid] @@ -768,6 +804,13 @@ class OidcController < ApplicationController # Get the user user = auth_code.user + # Re-check authorization at mint time: the user may have been deactivated or + # removed from the allowed group between /authorize and this token request. + unless user && application.user_allowed?(user) + render json: {error: "access_denied", error_description: "User is no longer permitted to access this application"}, status: :bad_request + return + end + # Generate access token record (opaque token with BCrypt hashing) access_token_record = OidcAccessToken.create!( application: application, @@ -904,6 +947,15 @@ class OidcController < ApplicationController # Get the user user = refresh_token_record.user + # Re-check authorization at mint time. Refresh tokens are long-lived (up to + # 30 days), so re-evaluate every refresh: a user deactivated or removed from + # the allowed group must not be able to keep minting access tokens. Checked + # before rotation so a denied refresh has no side effects. + unless user && application.user_allowed?(user) + render json: {error: "access_denied", error_description: "User is no longer permitted to access this application"}, status: :bad_request + return + end + # Revoke the old refresh token (token rotation) refresh_token_record.revoke! @@ -1119,11 +1171,24 @@ class OidcController < ApplicationController return end + # RFC 7662 §4: a token must only be disclosed to a resource server authorized + # to introspect it. Otherwise any confidential client could harvest every + # user's identity by introspecting tokens issued to other clients. A caller is + # authorized only for tokens issued to itself, or tokens whose bound audience + # (RFC 8707 resource) it is registered to serve. Unauthorized callers get the + # same inactive response as an unknown token, disclosing nothing. + unless caller_may_introspect?(caller, access_token) + Rails.logger.warn "OAuth: Client #{caller.client_id} not authorized to introspect token for resource #{access_token.resource.inspect}" + render json: {active: false} + return + end + user = access_token.user application = access_token.application consent = OidcUserConsent.find_by(user: user, application: application) + scopes = access_token.scope.to_s.split - render json: { + body = { active: true, scope: access_token.scope, client_id: application.client_id, @@ -1133,10 +1198,25 @@ class OidcController < ApplicationController sub: consent&.sid || user.id.to_s, # RFC 8707: the resource the token was bound to (falls back to the client # when no resource indicator was used at authorization time). - aud: access_token.resource.presence || application.client_id, - username: user.email_address, - groups: user.groups.pluck(:name) + aud: access_token.resource.presence || application.client_id } + + # Disclose identity claims only when the token actually carries the scope that + # grants them (mirrors the userinfo endpoint) — a token without `email`/`groups` + # scope must not leak the user's email or group memberships. + body[:username] = user.email_address if scopes.include?("email") + body[:groups] = user.groups.pluck(:name) if scopes.include?("groups") + + render json: body + end + + # A caller may introspect a token issued to itself, or a token bound (RFC 8707) + # to a resource the caller is registered to serve. + def caller_may_introspect?(caller, access_token) + return true if access_token.application_id == caller.id + + resource = access_token.resource.presence + resource.present? && caller.serves_resource?(resource) end # POST /oauth/revoke diff --git a/app/models/application.rb b/app/models/application.rb index ba96aa9..25d85e0 100644 --- a/app/models/application.rb +++ b/app/models/application.rb @@ -155,6 +155,19 @@ class Application < ApplicationRecord redirect_uris.split("\n").map(&:strip).reject(&:blank?) end + # RFC 8707 resource identifier(s) this application serves as a resource server. + # Used to authorize token introspection (see OidcController#caller_may_introspect?). + def parsed_resource_identifiers + return [] unless resource_identifiers.present? + JSON.parse(resource_identifiers) + rescue JSON::ParserError + resource_identifiers.split("\n").map(&:strip).reject(&:blank?) + end + + def serves_resource?(uri) + parsed_resource_identifiers.include?(uri) + end + def parsed_metadata return {} unless metadata.present? JSON.parse(metadata) diff --git a/db/migrate/20260719000004_add_resource_identifiers_to_applications.rb b/db/migrate/20260719000004_add_resource_identifiers_to_applications.rb new file mode 100644 index 0000000..9698996 --- /dev/null +++ b/db/migrate/20260719000004_add_resource_identifiers_to_applications.rb @@ -0,0 +1,8 @@ +class AddResourceIdentifiersToApplications < ActiveRecord::Migration[8.1] + # The RFC 8707 resource identifier(s) this application serves as a resource + # server. Used to authorize RFC 7662 introspection: a caller may only introspect + # tokens bound to a resource it serves (or tokens issued to itself). + def change + add_column :applications, :resource_identifiers, :text + end +end diff --git a/db/schema.rb b/db/schema.rb index 5c1fda7..e11c52f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_07_19_000003) do +ActiveRecord::Schema[8.1].define(version: 2026_07_19_000004) do create_table "active_storage_attachments", force: :cascade do |t| t.bigint "blob_id", null: false t.datetime "created_at", null: false @@ -96,6 +96,7 @@ ActiveRecord::Schema[8.1].define(version: 2026_07_19_000003) do t.text "redirect_uris" t.integer "refresh_token_ttl", default: 2592000 t.boolean "require_pkce", default: true, null: false + t.text "resource_identifiers" t.boolean "skip_consent", default: false, null: false t.string "slug", null: false t.datetime "updated_at", null: false diff --git a/db/seeds.rb b/db/seeds.rb index 3c308d4..aae2d4d 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -34,18 +34,27 @@ end # Confidential client that resource servers (e.g. c2a2) use to authenticate to # the introspection endpoint. The secret is only shown once, on creation. +# +# resource_identifiers declares the RFC 8707 resource URI(s) this server answers +# for. Introspection is authorized against it: c2a2 may only introspect tokens +# whose bound audience is one of these. The CLI/agent must therefore request its +# token with resource=. Set C2A2_RESOURCE to c2a2's real URL. unless Application.exists?(client_id: "c2a2-introspection") secret = SecureRandom.urlsafe_base64(48) + c2a2_resource = ENV["C2A2_RESOURCE"].presence || "https://c2a2.example.com" Application.create!( name: "c2a2 (introspection caller)", slug: "c2a2-introspection", client_id: "c2a2-introspection", client_secret: secret, app_type: "oidc", - active: true + active: true, + resource_identifiers: [c2a2_resource].to_json ) puts "Seeded 'c2a2-introspection' confidential client:" - puts " client_id: c2a2-introspection" - puts " client_secret: #{secret}" - puts " Store these in c2a2 now — the secret is hashed and cannot be recovered." + puts " client_id: c2a2-introspection" + puts " client_secret: #{secret}" + puts " resource_identifier: #{c2a2_resource}" + puts " Store the secret in c2a2 now — it is hashed and cannot be recovered." + puts " The CLI must request tokens with resource=#{c2a2_resource} for c2a2 to introspect them." end diff --git a/docs/decisions/0005-introspection-authorization.md b/docs/decisions/0005-introspection-authorization.md new file mode 100644 index 0000000..840bb20 --- /dev/null +++ b/docs/decisions/0005-introspection-authorization.md @@ -0,0 +1,46 @@ +# 0005 — Introspection authorization & claim scope-gating + +**Status:** Accepted · **Date:** 2026-07-19 + +## Decision + +The RFC 7662 introspection endpoint restricts *which* tokens a caller may see and +*which* claims it returns: + +1. **Authorization to introspect.** A caller may introspect a token only if it was + issued to that caller, or the token is bound (RFC 8707 `resource`) to a resource + the caller is registered to serve (`Application#serves_resource?`, backed by a new + `resource_identifiers` column). Unauthorized callers get the same `{active:false}` + as an unknown token — disclosing nothing. +2. **Claim scope-gating.** `username` (email) is returned only when the token carries + the `email` scope; `groups` only with the `groups` scope — mirroring the userinfo + endpoint. `sub` is a pairwise pseudonym and is always safe to return. + +## Context + +The first cut authenticated the caller (any confidential client) but then returned +`active:true` plus the user's email and **all** group names for **any** token — even +tokens issued to a different client and regardless of the token's scopes. A +low-privilege second client could therefore harvest every user's email and group +memberships by replaying tokens it observed. RFC 7662 §4 explicitly calls for the AS +to verify the resource server is authorized to introspect the particular token, +typically via audience restriction. + +## How it fits together + +This is the enforcement half of the RFC 8707 resource indicators +([0004](0004-resource-indicators.md)): the CLI/agent requests a token with +`resource=`, the resource server is registered with that same +identifier, and only it can introspect (and thereby read the user's groups to +authorize). A token minted for one resource server cannot be introspected by another. + +## Consequences + +- **c2a2 setup:** the `c2a2-introspection` client must declare its + `resource_identifiers` (seeded from `C2A2_RESOURCE`), and the CLI must request its + token with `resource=`. A token with no bound resource can only be + introspected by the client it was issued to. +- Resource servers see only the identity claims the token was actually granted, so an + over-broad token (or a misconfigured scope) can't leak email/groups. +- Unauthorized introspection is indistinguishable from an unknown token, preventing + token-scanning and cross-client identity harvesting. diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 860868b..29e8881 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -11,3 +11,4 @@ Each file is one decision. Newest decisions get the next number. | [0002](0002-device-authorization-grant.md) | CLI/agent auth uses the OAuth 2.0 Device Authorization Grant (RFC 8628) | | [0003](0003-dynamic-client-registration.md) | Dynamic Client Registration (RFC 7591), runtime-gated + default-deny | | [0004](0004-resource-indicators.md) | Resource Indicators (RFC 8707) bind token audience; pass-through validation | +| [0005](0005-introspection-authorization.md) | Introspection restricted to authorized callers + claim scope-gating | diff --git a/test/controllers/oidc_claims_security_test.rb b/test/controllers/oidc_claims_security_test.rb index 16129f5..b8e8295 100644 --- a/test/controllers/oidc_claims_security_test.rb +++ b/test/controllers/oidc_claims_security_test.rb @@ -16,6 +16,10 @@ class OidcClaimsSecurityTest < ActionDispatch::IntegrationTest @application.generate_new_client_secret! @plain_client_secret = @application.client_secret @application.save! + + # The user must be allowed on the app for tokens to be minted (mint-time + # authorization re-check); these tests create codes/tokens directly. + grant_everyone_access(@application) end def teardown diff --git a/test/controllers/oidc_device_flow_controller_test.rb b/test/controllers/oidc_device_flow_controller_test.rb index a79dfef..ad77be1 100644 --- a/test/controllers/oidc_device_flow_controller_test.rb +++ b/test/controllers/oidc_device_flow_controller_test.rb @@ -52,7 +52,10 @@ class OidcDeviceFlowControllerTest < ActionDispatch::IntegrationTest # --- Device authorization endpoint ----------------------------------------- test "device_authorization issues a device_code and user_code" do - post "/oauth/device_authorization", params: {client_id: @cli.client_id, scope: "openid groups"} + post "/oauth/device_authorization", params: { + client_id: @cli.client_id, scope: "openid groups", + code_challenge: code_challenge_for(CODE_VERIFIER) + } assert_response :success body = JSON.parse(@response.body) @@ -64,12 +67,41 @@ class OidcDeviceFlowControllerTest < ActionDispatch::IntegrationTest assert body["expires_in"].positive? end + test "device_authorization requires PKCE for a public client" do + post "/oauth/device_authorization", params: {client_id: @cli.client_id, scope: "openid"} + assert_response :bad_request + assert_equal "invalid_request", JSON.parse(@response.body)["error"] + assert_equal 0, OidcDeviceCode.where(application: @cli).count + end + test "device_authorization rejects an unknown client" do post "/oauth/device_authorization", params: {client_id: "does-not-exist"} assert_response :unauthorized assert_equal "invalid_client", JSON.parse(@response.body)["error"] end + test "device_authorization rejects a confidential client with no secret" do + post "/oauth/device_authorization", params: {client_id: @resource.client_id, scope: "openid"} + assert_response :unauthorized + assert_equal "invalid_client", JSON.parse(@response.body)["error"] + end + + test "device_authorization rejects a confidential client with a wrong secret" do + post "/oauth/device_authorization", + params: {client_id: @resource.client_id, client_secret: "wrong-secret", scope: "openid"} + assert_response :unauthorized + assert_equal "invalid_client", JSON.parse(@response.body)["error"] + end + + test "device_authorization accepts a confidential client with a valid secret" do + post "/oauth/device_authorization", params: { + client_id: @resource.client_id, client_secret: @resource_secret, scope: "openid", + code_challenge: code_challenge_for(CODE_VERIFIER), code_challenge_method: "S256" + } + assert_response :success + assert JSON.parse(@response.body)["device_code"].present? + end + # --- Token endpoint device_code grant -------------------------------------- test "token endpoint returns authorization_pending while pending" do @@ -104,10 +136,13 @@ class OidcDeviceFlowControllerTest < ActionDispatch::IntegrationTest test "token endpoint issues tokens once approved, then the code is single-use" do OidcUserConsent.create!(user: @user, application: @cli, scopes_granted: "openid groups", granted_at: Time.current) - dc = OidcDeviceCode.create!(application: @cli, scope: "openid groups") + dc = OidcDeviceCode.create!( + application: @cli, scope: "openid groups", + code_challenge: code_challenge_for(CODE_VERIFIER), code_challenge_method: "S256" + ) dc.approve!(user: @user, acr: "1", auth_time: Time.current.to_i) - poll(dc) + poll(dc, code_verifier: CODE_VERIFIER) assert_response :success body = JSON.parse(@response.body) assert body["access_token"].present? @@ -117,6 +152,18 @@ class OidcDeviceFlowControllerTest < ActionDispatch::IntegrationTest assert_equal "openid groups", body["scope"] # Replaying the (now consumed) device_code fails. + poll(dc, code_verifier: CODE_VERIFIER) + assert_response :bad_request + assert_equal "invalid_grant", JSON.parse(@response.body)["error"] + end + + test "token endpoint refuses a PKCE-required client whose device_code lacks a challenge" do + OidcUserConsent.create!(user: @user, application: @cli, scopes_granted: "openid", granted_at: Time.current) + # A device_code minted without PKCE (e.g. slipped past the front door) must + # never redeem tokens for a public client. + dc = OidcDeviceCode.create!(application: @cli, scope: "openid") + dc.approve!(user: @user, acr: "1", auth_time: Time.current.to_i) + poll(dc) assert_response :bad_request assert_equal "invalid_grant", JSON.parse(@response.body)["error"] @@ -148,6 +195,28 @@ class OidcDeviceFlowControllerTest < ActionDispatch::IntegrationTest assert OidcUserConsent.exists?(user: @user, application: @cli) end + test "approving a narrower device request merges into existing consent" do + # User already consented to a broader scope set (with stored claims) via the + # browser flow. + existing = OidcUserConsent.create!( + user: @user, application: @cli, + scopes_granted: "openid email profile groups", + claims_requests: {"userinfo" => {"email" => nil}}, + granted_at: 1.day.ago + ) + + sign_in_as(@user) + dc = OidcDeviceCode.create!(application: @cli, scope: "openid") + post "/device", params: {user_code: dc.user_code} + assert_response :success + + existing.reload + # Prior scopes are preserved (union), not shrunk to the device request's "openid". + assert_equal %w[openid email profile groups].sort, existing.scopes.sort + # Stored claims are not wiped. + assert_equal({"userinfo" => {"email" => nil}}, existing.parsed_claims_requests) + end + test "denying marks the device code denied" do sign_in_as(@user) dc = OidcDeviceCode.create!(application: @cli, scope: "openid") @@ -176,56 +245,24 @@ class OidcDeviceFlowControllerTest < ActionDispatch::IntegrationTest assert_redirected_to signin_path end - # --- Introspection --------------------------------------------------------- - - test "introspection reports an active token with groups" do - token = OidcAccessToken.create!(application: @cli, user: @user, scope: "openid groups") - - post "/oauth/introspect", params: { - token: token.plaintext_token, - client_id: @resource.client_id, - client_secret: @resource_secret - } - assert_response :success - body = JSON.parse(@response.body) - - assert_equal true, body["active"] - assert_equal @cli.client_id, body["client_id"] - assert_includes body["groups"], @group.name - assert body["sub"].present? - end - - test "introspection reports inactive for a revoked token" do - token = OidcAccessToken.create!(application: @cli, user: @user, scope: "openid") - token.revoke! - - post "/oauth/introspect", params: { - token: token.plaintext_token, - client_id: @resource.client_id, - client_secret: @resource_secret - } - assert_response :success - assert_equal false, JSON.parse(@response.body)["active"] - end - - test "introspection requires valid caller credentials" do - token = OidcAccessToken.create!(application: @cli, user: @user, scope: "openid") - - post "/oauth/introspect", params: { - token: token.plaintext_token, - client_id: @resource.client_id, - client_secret: "wrong-secret" - } - assert_response :unauthorized - end + # Introspection is covered in depth in oidc_introspection_test.rb. private - def poll(device_code) - post "/oauth/token", params: { + # A valid PKCE verifier (48 chars, RFC 7636 charset) and its S256 challenge. + CODE_VERIFIER = "device_flow_pkce_code_verifier_0123456789_abcdef".freeze + + def code_challenge_for(verifier) + Base64.urlsafe_encode64(Digest::SHA256.digest(verifier), padding: false) + end + + def poll(device_code, code_verifier: nil) + params = { grant_type: DEVICE_GRANT, device_code: device_code.plaintext_device_code, client_id: @cli.client_id } + params[:code_verifier] = code_verifier if code_verifier + post "/oauth/token", params: params end end diff --git a/test/controllers/oidc_introspection_test.rb b/test/controllers/oidc_introspection_test.rb new file mode 100644 index 0000000..cb9c2a9 --- /dev/null +++ b/test/controllers/oidc_introspection_test.rb @@ -0,0 +1,136 @@ +require "test_helper" + +# RFC 7662 token introspection: authorization (who may introspect which token) +# and claim scope-gating (only disclose identity claims the token was granted). +class OidcIntrospectionTest < ActionDispatch::IntegrationTest + RESOURCE = "https://api.example.com".freeze + + def setup + @group = Group.create!(name: "introspection-testers", description: "test") + @user = User.create!(email_address: "introspect@example.com", password: "password123") + @user.groups << @group + + # The OAuth client the tokens are issued to (a public CLI-style client). + @client = Application.create!(name: "Introspect Client", slug: "introspect-client", + app_type: "oidc", is_public_client: true, active: true) + + # The resource server that serves RESOURCE and is allowed to introspect + # tokens bound to it. + @rs_secret = "rs-secret-value-abcdefghijklmnop" + @rs = Application.create!(name: "Introspect RS", slug: "introspect-rs", app_type: "oidc", + client_secret: @rs_secret, active: true, resource_identifiers: [RESOURCE].to_json) + + # A confidential client that neither issued the token nor serves its resource. + @other_secret = "other-secret-value-abcdefghijklmn" + @other = Application.create!(name: "Introspect Other", slug: "introspect-other", + app_type: "oidc", client_secret: @other_secret, active: true) + end + + def teardown + [@client, @rs, @other].each do |app| + OidcAccessToken.where(application: app).delete_all + OidcUserConsent.where(application: app).delete_all + end + end + + # --- Authorization --------------------------------------------------------- + + test "a resource server may introspect a token bound to a resource it serves" do + token = issue(scope: "openid groups email", resource: RESOURCE) + body = introspect(token, @rs.client_id, @rs_secret) + + assert_equal true, body["active"] + assert_equal @client.client_id, body["client_id"] + assert_equal RESOURCE, body["aud"] + end + + test "a client may introspect its own token" do + token = OidcAccessToken.create!(application: @rs, user: @user, scope: "openid") + body = introspect(token, @rs.client_id, @rs_secret) + + assert_equal true, body["active"] + assert_equal @rs.client_id, body["aud"] + end + + test "a caller cannot introspect a token bound to a resource it does not serve" do + token = issue(scope: "openid groups email", resource: RESOURCE) + body = introspect(token, @other.client_id, @other_secret) + + assert_equal false, body["active"], "unauthorized caller must learn nothing" + assert_nil body["username"] + assert_nil body["groups"] + end + + test "a caller cannot introspect an unbound token it did not issue" do + token = issue(scope: "openid groups", resource: nil) + body = introspect(token, @rs.client_id, @rs_secret) + + assert_equal false, body["active"] + end + + # --- Claim scope-gating ---------------------------------------------------- + + test "omits email and groups when the token lacks those scopes" do + token = issue(scope: "openid", resource: RESOURCE) + body = introspect(token, @rs.client_id, @rs_secret) + + assert_equal true, body["active"] + assert_not body.key?("username"), "email must not leak without the email scope" + assert_not body.key?("groups"), "groups must not leak without the groups scope" + end + + test "includes email only with the email scope" do + token = issue(scope: "openid email", resource: RESOURCE) + body = introspect(token, @rs.client_id, @rs_secret) + + assert_equal @user.email_address, body["username"] + assert_not body.key?("groups") + end + + test "includes groups only with the groups scope" do + token = issue(scope: "openid groups", resource: RESOURCE) + body = introspect(token, @rs.client_id, @rs_secret) + + assert_includes body["groups"], @group.name + assert_not body.key?("username") + end + + # --- Token / caller validity ---------------------------------------------- + + test "reports inactive for a revoked token even to an authorized caller" do + token = issue(scope: "openid groups", resource: RESOURCE) + token.revoke! + assert_equal false, introspect(token, @rs.client_id, @rs_secret)["active"] + end + + test "requires valid caller credentials" do + token = issue(scope: "openid", resource: RESOURCE) + post "/oauth/introspect", params: {token: token.plaintext_token, client_id: @rs.client_id, client_secret: "wrong"} + assert_response :unauthorized + end + + test "rejects a public (non-confidential) caller" do + token = issue(scope: "openid", resource: RESOURCE) + post "/oauth/introspect", params: {token: token.plaintext_token, client_id: @client.client_id} + assert_response :unauthorized + end + + test "requires a token parameter" do + post "/oauth/introspect", params: {client_id: @rs.client_id, client_secret: @rs_secret} + assert_response :bad_request + assert_equal "invalid_request", JSON.parse(@response.body)["error"] + end + + private + + def issue(scope:, resource:) + OidcAccessToken.create!(application: @client, user: @user, scope: scope, resource: resource) + end + + def introspect(token, client_id, secret) + plaintext = token.respond_to?(:plaintext_token) ? token.plaintext_token : token + post "/oauth/introspect", params: {token: plaintext, client_id: client_id, client_secret: secret} + assert_response :success + JSON.parse(@response.body) + end +end diff --git a/test/controllers/oidc_mint_authorization_test.rb b/test/controllers/oidc_mint_authorization_test.rb new file mode 100644 index 0000000..92cef15 --- /dev/null +++ b/test/controllers/oidc_mint_authorization_test.rb @@ -0,0 +1,114 @@ +require "test_helper" + +# Tokens must not be minted for a user who has lost access (deactivated, or removed +# from the application's allowed group) between authorization and the token request. +# Every grant re-checks Application#user_allowed? at mint time. +class OidcMintAuthorizationTest < ActionDispatch::IntegrationTest + DEVICE_GRANT = "urn:ietf:params:oauth:grant-type:device_code".freeze + + def setup + @group = Group.create!(name: "mint-authz-testers", description: "test") + @user = User.create!(email_address: "mint_authz@example.com", password: "password123") + @user.groups << @group + + @secret = "mint-authz-secret-value-abcdefghij" + @application = Application.create!(name: "Mint Authz App", slug: "mint-authz-app", app_type: "oidc", + client_secret: @secret, active: true, require_pkce: false, + redirect_uris: ["https://app.example.com/cb"].to_json) + @application.allowed_groups << @group + + OidcUserConsent.create!(user: @user, application: @application, scopes_granted: "openid", granted_at: Time.current) + end + + def teardown + OidcRefreshToken.where(application: @application).delete_all + OidcAccessToken.where(application: @application).delete_all + OidcDeviceCode.where(application: @application).delete_all + OidcAuthorizationCode.where(application: @application).delete_all + OidcUserConsent.where(application: @application).delete_all + end + + # --- Device grant ---------------------------------------------------------- + + test "device grant issues tokens for a still-allowed user" do + poll(approved_device_code) + assert_response :success + assert JSON.parse(@response.body)["access_token"].present? + end + + test "device grant refuses a user removed from the allowed group after approval" do + dc = approved_device_code + revoke_group! + poll(dc) + assert_access_denied + end + + test "device grant refuses a deactivated user after approval" do + dc = approved_device_code + @user.disabled! + poll(dc) + assert_access_denied + end + + # --- Authorization code grant --------------------------------------------- + + test "authorization_code grant refuses a user removed from the allowed group" do + code = OidcAuthorizationCode.create!(application: @application, user: @user, + redirect_uri: "https://app.example.com/cb", scope: "openid", auth_time: Time.current.to_i, acr: "1") + revoke_group! + post "/oauth/token", params: {grant_type: "authorization_code", code: code.plaintext_code, + redirect_uri: "https://app.example.com/cb", client_id: @application.client_id, client_secret: @secret} + assert_access_denied + end + + # --- Refresh grant --------------------------------------------------------- + + test "refresh_token grant refuses a deactivated user" do + refresh = issue_refresh_token + @user.disabled! + refresh_with(refresh) + assert_access_denied + end + + test "refresh_token grant refuses a removed user and leaves the token intact" do + refresh = issue_refresh_token + revoke_group! + refresh_with(refresh) + assert_access_denied + assert_not refresh.reload.revoked?, "a denied refresh must not rotate/revoke the token" + end + + private + + def approved_device_code + dc = OidcDeviceCode.create!(application: @application, scope: "openid") + dc.approve!(user: @user, acr: "1", auth_time: Time.current.to_i) + dc + end + + def issue_refresh_token + access = OidcAccessToken.create!(application: @application, user: @user, scope: "openid") + OidcRefreshToken.create!(application: @application, user: @user, oidc_access_token: access, + scope: "openid", auth_time: Time.current.to_i, acr: "1") + end + + def revoke_group! + UserGroup.where(user: @user, group: @group).delete_all + @user.reload + end + + def poll(dc) + post "/oauth/token", params: {grant_type: DEVICE_GRANT, device_code: dc.plaintext_device_code, + client_id: @application.client_id, client_secret: @secret} + end + + def refresh_with(refresh) + post "/oauth/token", params: {grant_type: "refresh_token", refresh_token: refresh.token, + client_id: @application.client_id, client_secret: @secret} + end + + def assert_access_denied + assert_response :bad_request + assert_equal "access_denied", JSON.parse(@response.body)["error"] + end +end diff --git a/test/controllers/oidc_pkce_controller_test.rb b/test/controllers/oidc_pkce_controller_test.rb index a6f5161..fd61826 100644 --- a/test/controllers/oidc_pkce_controller_test.rb +++ b/test/controllers/oidc_pkce_controller_test.rb @@ -322,6 +322,7 @@ class OidcPkceControllerTest < ActionDispatch::IntegrationTest require_pkce: false ) legacy_app.generate_new_client_secret! + grant_everyone_access(legacy_app) # Create consent for token endpoint OidcUserConsent.create!( @@ -379,6 +380,7 @@ class OidcPkceControllerTest < ActionDispatch::IntegrationTest active: true, is_public_client: true ) + grant_everyone_access(public_app) assert public_app.public_client? assert public_app.requires_pkce? @@ -442,6 +444,7 @@ class OidcPkceControllerTest < ActionDispatch::IntegrationTest active: true, is_public_client: true ) + grant_everyone_access(public_app) assert public_app.public_client? assert public_app.requires_pkce? diff --git a/test/controllers/oidc_resource_indicators_test.rb b/test/controllers/oidc_resource_indicators_test.rb index bde496c..d09bf7e 100644 --- a/test/controllers/oidc_resource_indicators_test.rb +++ b/test/controllers/oidc_resource_indicators_test.rb @@ -24,8 +24,10 @@ class OidcResourceIndicatorsTest < ActionDispatch::IntegrationTest @web.allowed_groups << @group @resource_secret = "resource-server-secret-value-abcdefghij" + # The resource server is registered to serve RESOURCE, so it is authorized to + # introspect tokens bound to it (see OidcController#caller_may_introspect?). @resource = Application.create!(name: "Resource RS", slug: "resource-rs2", app_type: "oidc", - client_secret: @resource_secret, active: true) + client_secret: @resource_secret, active: true, resource_identifiers: [RESOURCE].to_json) end def teardown @@ -84,7 +86,10 @@ class OidcResourceIndicatorsTest < ActionDispatch::IntegrationTest # --- Device flow ----------------------------------------------------------- test "device flow binds the resource to the issued token" do - post "/oauth/device_authorization", params: {client_id: @cli.client_id, scope: "openid", resource: RESOURCE} + post "/oauth/device_authorization", params: { + client_id: @cli.client_id, scope: "openid", resource: RESOURCE, + code_challenge: PKCE_CHALLENGE, code_challenge_method: "S256" + } assert_response :success auth = JSON.parse(@response.body) @@ -94,7 +99,7 @@ class OidcResourceIndicatorsTest < ActionDispatch::IntegrationTest OidcUserConsent.create!(user: @user, application: @cli, scopes_granted: "openid", granted_at: Time.current) dc.approve!(user: @user, acr: "1", auth_time: Time.current.to_i) - post "/oauth/token", params: {grant_type: DEVICE_GRANT, device_code: auth["device_code"], client_id: @cli.client_id} + post "/oauth/token", params: {grant_type: DEVICE_GRANT, device_code: auth["device_code"], client_id: @cli.client_id, code_verifier: PKCE_VERIFIER} assert_response :success access = JSON.parse(@response.body)["access_token"] @@ -102,7 +107,10 @@ class OidcResourceIndicatorsTest < ActionDispatch::IntegrationTest end test "device_authorization rejects an invalid resource" do - post "/oauth/device_authorization", params: {client_id: @cli.client_id, resource: "not-an-absolute-uri"} + post "/oauth/device_authorization", params: { + client_id: @cli.client_id, resource: "not-an-absolute-uri", + code_challenge: PKCE_CHALLENGE, code_challenge_method: "S256" + } assert_response :bad_request assert_equal "invalid_target", JSON.parse(@response.body)["error"] end @@ -110,8 +118,10 @@ class OidcResourceIndicatorsTest < ActionDispatch::IntegrationTest # --- Fallback -------------------------------------------------------------- test "introspection aud falls back to the client when no resource was bound" do - token = OidcAccessToken.create!(application: @cli, user: @user, scope: "openid") - assert_equal @cli.client_id, introspect(token.plaintext_token)["aud"] + # A confidential client introspecting its own unbound token (no RFC 8707 + # resource) sees aud fall back to the client_id. + token = OidcAccessToken.create!(application: @resource, user: @user, scope: "openid") + assert_equal @resource.client_id, introspect(token.plaintext_token)["aud"] end private