Harden OIDC token endpoints from security review

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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F7cwhwDJp3MJJDoNPVE6zq

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

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

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

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

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?

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