Extract OidcUserConsent.record! upsert for shared consent
The consent record is unique on user+application and shared between the browser authorization flow and device-flow approval. Centralize the upsert in OidcUserConsent.record!(merge:) so a narrower device request unions its scopes into any existing grant (merge: true) rather than overwriting it and wiping stored claims, while the browser consent screen records exactly the approved scopes (merge: false). scopes is now nil-safe for not-yet-saved records. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7cwhwDJp3MJJDoNPVE6zq
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
7eddea8356
commit
e3f0bd4cab
@@ -87,14 +87,14 @@ class DeviceAuthorizationsController < ApplicationController
|
|||||||
end
|
end
|
||||||
|
|
||||||
def record_consent(device_code, user)
|
def record_consent(device_code, user)
|
||||||
consent = OidcUserConsent.find_or_initialize_by(user: user, application: device_code.application)
|
# merge: true — the consent record is shared with the browser flow, so a
|
||||||
# Merge into any existing consent instead of overwriting it. The consent
|
# narrower device request must not shrink previously granted scopes or wipe
|
||||||
# record is shared with the browser flow (unique on user+application) and
|
# stored claims.
|
||||||
# scopes_granted is treated as a granted superset, so a narrower device
|
OidcUserConsent.record!(
|
||||||
# request must not shrink previously granted scopes or wipe stored claims.
|
user: user,
|
||||||
consent.scopes = consent.scopes_granted.to_s.split | granted_scopes(device_code)
|
application: device_code.application,
|
||||||
consent.claims_requests = {} if consent.new_record?
|
scopes: granted_scopes(device_code),
|
||||||
consent.granted_at = Time.current
|
merge: true
|
||||||
consent.save!
|
)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -8,9 +8,29 @@ class OidcUserConsent < ApplicationRecord
|
|||||||
before_validation :set_granted_at, on: :create
|
before_validation :set_granted_at, on: :create
|
||||||
before_validation :set_sid, on: :create
|
before_validation :set_sid, on: :create
|
||||||
|
|
||||||
# Parse scopes_granted into an array
|
# Upsert a user's consent for an application. The record is unique on
|
||||||
|
# user+application and shared across the browser and device flows.
|
||||||
|
#
|
||||||
|
# merge: false (the browser consent screen) records exactly the scopes the user
|
||||||
|
# just approved. merge: true (device approval) unions the scopes into any
|
||||||
|
# existing grant and leaves stored claims untouched, so a narrower device
|
||||||
|
# request can never shrink a prior grant or wipe its claims. claims_requests is
|
||||||
|
# written only when supplied (nil = keep whatever is stored, defaulting to {}
|
||||||
|
# for a brand-new record).
|
||||||
|
def self.record!(user:, application:, scopes:, claims_requests: nil, merge: false)
|
||||||
|
consent = find_or_initialize_by(user: user, application: application)
|
||||||
|
incoming = Array(scopes)
|
||||||
|
consent.scopes = merge ? (consent.scopes | incoming) : incoming
|
||||||
|
consent.claims_requests = claims_requests unless claims_requests.nil?
|
||||||
|
consent.claims_requests ||= {}
|
||||||
|
consent.granted_at = Time.current
|
||||||
|
consent.save!
|
||||||
|
consent
|
||||||
|
end
|
||||||
|
|
||||||
|
# Parse scopes_granted into an array (nil-safe for not-yet-saved records).
|
||||||
def scopes
|
def scopes
|
||||||
scopes_granted.split(" ")
|
scopes_granted.to_s.split(" ")
|
||||||
end
|
end
|
||||||
|
|
||||||
# Set scopes from an array
|
# Set scopes from an array
|
||||||
|
|||||||
@@ -206,6 +206,45 @@ class OidcUserConsentTest < ActiveSupport::TestCase
|
|||||||
assert consent.covers_scopes?(["openid", "email", "profile"])
|
assert consent.covers_scopes?(["openid", "email", "profile"])
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# --- .record! upsert semantics --------------------------------------------
|
||||||
|
|
||||||
|
test "record! creates a new consent with the given scopes and claims" do
|
||||||
|
user = users(:alice)
|
||||||
|
app = applications(:another_app)
|
||||||
|
OidcUserConsent.where(user: user, application: app).delete_all
|
||||||
|
|
||||||
|
consent = OidcUserConsent.record!(user: user, application: app,
|
||||||
|
scopes: %w[openid email], claims_requests: {"userinfo" => {"email" => nil}})
|
||||||
|
|
||||||
|
assert_equal %w[openid email], consent.scopes
|
||||||
|
assert_equal({"userinfo" => {"email" => nil}}, consent.parsed_claims_requests)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "record! without merge overwrites scopes (browser flow)" do
|
||||||
|
user = users(:alice)
|
||||||
|
app = applications(:another_app)
|
||||||
|
OidcUserConsent.create!(user: user, application: app,
|
||||||
|
scopes_granted: "openid email profile", granted_at: 1.day.ago)
|
||||||
|
|
||||||
|
consent = OidcUserConsent.record!(user: user, application: app, scopes: %w[openid],
|
||||||
|
claims_requests: {})
|
||||||
|
|
||||||
|
assert_equal %w[openid], consent.scopes, "browser flow records exactly what was consented"
|
||||||
|
end
|
||||||
|
|
||||||
|
test "record! with merge unions scopes and preserves stored claims (device flow)" do
|
||||||
|
user = users(:alice)
|
||||||
|
app = applications(:another_app)
|
||||||
|
OidcUserConsent.create!(user: user, application: app,
|
||||||
|
scopes_granted: "openid email profile", claims_requests: {"userinfo" => {"email" => nil}},
|
||||||
|
granted_at: 1.day.ago)
|
||||||
|
|
||||||
|
consent = OidcUserConsent.record!(user: user, application: app, scopes: %w[openid], merge: true)
|
||||||
|
|
||||||
|
assert_equal %w[openid email profile].sort, consent.scopes.sort, "merge must not shrink a prior grant"
|
||||||
|
assert_equal({"userinfo" => {"email" => nil}}, consent.parsed_claims_requests, "merge must not wipe claims")
|
||||||
|
end
|
||||||
|
|
||||||
test "should validate scope coverage logic with real OIDC scenarios" do
|
test "should validate scope coverage logic with real OIDC scenarios" do
|
||||||
# Typical OIDC consent scenario
|
# Typical OIDC consent scenario
|
||||||
@consent.scopes_granted = "openid profile email"
|
@consent.scopes_granted = "openid profile email"
|
||||||
|
|||||||
Reference in New Issue
Block a user