Skip to content

Commit 1494fb7

Browse files
committed
Unify issuer block parameters across DiscoveryController and IdToken
The `issuer` configuration block received inconsistent parameters depending on the caller: - DiscoveryController passed `request` as the sole argument - IdToken passed `resource_owner, application` This made it impossible to write a single issuer block that works correctly in both contexts. Introduce `Doorkeeper::OpenidConnect.resolve_issuer` to centralize issuer resolution with a consistent interface: issuer do |resource_owner, application, request| request&.base_url || "https://default.example.com" end Backward compatibility is maintained via arity checks: - arity 0: called with no arguments - arity 1: called with `request` or `resource_owner` (legacy) - arity 2: called with `resource_owner, application` (generator default) - arity 3+: called with `resource_owner, application, request` (new) Closes #238
1 parent 225e319 commit 1494fb7

7 files changed

Lines changed: 181 additions & 11 deletions

File tree

app/controllers/doorkeeper/openid_connect/discovery_controller.rb

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,11 +130,7 @@ def discovery_url_default_options
130130
end
131131

132132
def issuer
133-
if Doorkeeper::OpenidConnect.configuration.issuer.respond_to?(:call)
134-
Doorkeeper::OpenidConnect.configuration.issuer.call(request).to_s
135-
else
136-
Doorkeeper::OpenidConnect.configuration.issuer
137-
end
133+
Doorkeeper::OpenidConnect.resolve_issuer(request: request)
138134
end
139135

140136
%i[authorization token revocation introspection userinfo jwks webfinger dynamic_client_registration].each do |endpoint|

lib/doorkeeper/openid_connect.rb

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,29 @@ def self.signing_key_normalized
6666
signing_key.export
6767
end
6868

69+
# Resolves the issuer value from the configuration, handling both
70+
# static values and callable blocks with backward-compatible arity checks.
71+
#
72+
# @param resource_owner [Object, nil] the authenticated user (nil in discovery context)
73+
# @param application [Object, nil] the OAuth application (nil in discovery context)
74+
# @param request [ActionDispatch::Request, nil] the current request (nil in token context)
75+
# @return [String] the issuer string
76+
def self.resolve_issuer(resource_owner: nil, application: nil, request: nil)
77+
issuer = configuration.issuer
78+
return issuer.to_s unless issuer.respond_to?(:call)
79+
80+
case issuer.arity
81+
when 0
82+
issuer.call
83+
when 1
84+
issuer.call(request || resource_owner)
85+
when 2
86+
issuer.call(resource_owner, application)
87+
else
88+
issuer.call(resource_owner, application, request)
89+
end.to_s
90+
end
91+
6992
Doorkeeper::GrantFlow.register(
7093
:id_token,
7194
response_type_matches: 'id_token',

lib/doorkeeper/openid_connect/id_token.rb

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,10 @@ def as_jws_token
4242
private
4343

4444
def issuer
45-
if Doorkeeper::OpenidConnect.configuration.issuer.respond_to?(:call)
46-
Doorkeeper::OpenidConnect.configuration.issuer.call(@resource_owner, @access_token.application).to_s
47-
else
48-
Doorkeeper::OpenidConnect.configuration.issuer
49-
end
45+
Doorkeeper::OpenidConnect.resolve_issuer(
46+
resource_owner: @resource_owner,
47+
application: @access_token.application
48+
)
5049
end
5150

5251
def subject

lib/generators/doorkeeper/openid_connect/templates/initializer.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
# frozen_string_literal: true
22

33
Doorkeeper::OpenidConnect.configure do
4-
issuer do |resource_owner, application|
4+
issuer do |resource_owner, application, request|
5+
# Example implementation:
6+
# request&.base_url || 'https://example.com'
57
'issuer string'
68
end
79

spec/controllers/discovery_controller_spec.rb

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,23 @@
136136
end
137137
end
138138

139+
context 'when issuer block has arity 3 with request' do
140+
before do
141+
Doorkeeper::OpenidConnect.configure do
142+
issuer do |_resource_owner, _application, request|
143+
request.base_url
144+
end
145+
end
146+
end
147+
148+
it 'passes the request object to the issuer block' do
149+
get :provider
150+
data = JSON.parse(response.body)
151+
152+
expect(data['issuer']).to eq 'http://test.host'
153+
end
154+
end
155+
139156
context 'when grant_flows is configured with only client_credentials' do
140157
before { Doorkeeper.configure { grant_flows %w[client_credentials] } }
141158

spec/lib/id_token_spec.rb

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,33 @@
7373
)
7474
end
7575
end
76+
77+
context 'when issuer block has arity 3' do
78+
before do
79+
Doorkeeper::OpenidConnect.configure do
80+
issuer do |resource_owner, application, _request|
81+
"#{resource_owner.id}-#{application&.uid}"
82+
end
83+
84+
resource_owner_from_access_token do |access_token|
85+
User.find_by(id: access_token.resource_owner_id)
86+
end
87+
88+
auth_time_from_resource_owner do |resource_owner|
89+
resource_owner.current_sign_in_at
90+
end
91+
92+
subject do |resource_owner|
93+
resource_owner.id
94+
end
95+
end
96+
end
97+
98+
it 'passes resource_owner and application to the issuer block' do
99+
claims = subject.claims
100+
expect(claims[:iss]).to eq "#{user.id}-#{access_token.application.uid}"
101+
end
102+
end
76103
end
77104

78105
describe '#as_json' do

spec/lib/openid_connect_spec.rb

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,4 +176,110 @@
176176
end
177177
end
178178
end
179+
180+
describe '.resolve_issuer' do
181+
let(:resource_owner) { double('ResourceOwner') }
182+
let(:application) { double('Application') }
183+
let(:request) { double('Request', base_url: 'https://example.com') }
184+
185+
context 'when issuer is a static string' do
186+
before do
187+
Doorkeeper::OpenidConnect.configure do
188+
issuer 'https://static-issuer.example.com'
189+
end
190+
end
191+
192+
it 'returns the static string' do
193+
expect(subject.resolve_issuer).to eq 'https://static-issuer.example.com'
194+
end
195+
end
196+
197+
context 'when issuer block has arity 0' do
198+
before do
199+
Doorkeeper::OpenidConnect.configure do
200+
issuer do
201+
'https://zero-arity.example.com'
202+
end
203+
end
204+
end
205+
206+
it 'calls the block without arguments' do
207+
expect(subject.resolve_issuer).to eq 'https://zero-arity.example.com'
208+
end
209+
end
210+
211+
context 'when issuer block has arity 1' do
212+
before do
213+
Doorkeeper::OpenidConnect.configure do
214+
issuer do |request_or_owner|
215+
"issuer-#{request_or_owner.class.name}"
216+
end
217+
end
218+
end
219+
220+
it 'passes request when called from discovery context' do
221+
result = subject.resolve_issuer(request: request)
222+
expect(result).to include 'RSpec::Mocks::Double'
223+
end
224+
225+
it 'passes resource_owner when called from token context' do
226+
result = subject.resolve_issuer(resource_owner: resource_owner)
227+
expect(result).to include 'RSpec::Mocks::Double'
228+
end
229+
230+
it 'prefers request over resource_owner when both are present' do
231+
result = subject.resolve_issuer(resource_owner: resource_owner, request: request)
232+
expect(result).to eq subject.resolve_issuer(request: request)
233+
end
234+
end
235+
236+
context 'when issuer block has arity 2' do
237+
before do
238+
Doorkeeper::OpenidConnect.configure do
239+
issuer do |resource_owner, application|
240+
"owner-#{resource_owner&.class&.name}-app-#{application&.class&.name}"
241+
end
242+
end
243+
end
244+
245+
it 'passes resource_owner and application' do
246+
result = subject.resolve_issuer(resource_owner: resource_owner, application: application)
247+
expect(result).to include('RSpec::Mocks::Double').twice
248+
end
249+
250+
it 'passes nils from discovery context' do
251+
result = subject.resolve_issuer(request: request)
252+
expect(result).to eq 'owner--app-'
253+
end
254+
end
255+
256+
context 'when issuer block has arity 3' do
257+
before do
258+
Doorkeeper::OpenidConnect.configure do
259+
issuer do |resource_owner, application, request|
260+
if request
261+
request.base_url
262+
else
263+
"owner-#{resource_owner&.class&.name}"
264+
end
265+
end
266+
end
267+
end
268+
269+
it 'passes all three arguments from discovery context' do
270+
result = subject.resolve_issuer(request: request)
271+
expect(result).to eq 'https://example.com'
272+
end
273+
274+
it 'passes all three arguments from token context' do
275+
result = subject.resolve_issuer(resource_owner: resource_owner, application: application)
276+
expect(result).to include 'RSpec::Mocks::Double'
277+
end
278+
279+
it 'passes all three arguments when all are present' do
280+
result = subject.resolve_issuer(resource_owner: resource_owner, application: application, request: request)
281+
expect(result).to eq 'https://example.com'
282+
end
283+
end
284+
end
179285
end

0 commit comments

Comments
 (0)