Skip to content

Commit fc12cd4

Browse files
camreevesclaude
andcommitted
fix(oauth): allow public clients to exchange + refresh tokens without a secret
The token endpoint declared `client_secret` as a required parameter, so a public client (SPA / native app, `confidential: false`) that authenticates with PKCE — Backoffice, for example — was rejected with a 422 "missing required parameter 'client_secret'" before any OAuth logic ran. Doorkeeper made `client_secret` optional for public clients and skipped client-secret validation for them. This restores that: - `token` now takes `client_secret : String? = nil`. - `AuthlyAdapter::Client#authorized?` returns true for public clients (they authenticate via PKCE); confidential clients still require a constant-time secret match. - `allowed_grant_type?` denies `client_credentials` to public clients, so the secret bypass can't be used to mint owner-scoped tokens from a (public) client_id. - Discovery advertises `token_endpoint_auth_methods_supported: [..., "none"]`. PKCE is left optional (matching Doorkeeper); enforcement can be enabled later via `Authly.config.enforce_pkce`. Specs updated: fixtures that authenticate with a real secret now model a confidential client, and a new "public client" block covers the no-secret code exchange + refresh and the client_credentials denial. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 7883020 commit fc12cd4

6 files changed

Lines changed: 121 additions & 6 deletions

File tree

spec/controllers/oauth_alias_spec.cr

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ module PlaceOS::Auth
1919
app.redirect_uri = redirect
2020
app.scopes = scopes
2121
app.owner_id = user.id.as(String)
22+
app.confidential = true
2223
app.save!
2324
{user, app}
2425
}

spec/controllers/oauth_spec.cr

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ module PlaceOS::Auth
2020
app.redirect_uri = redirect
2121
app.scopes = scopes
2222
app.owner_id = user.id.as(String)
23+
# These fixtures authenticate with a real client_secret, so they
24+
# model a confidential client. Public clients are covered separately
25+
# (see the "public client" describe block below).
26+
app.confidential = true
2327
app.save!
2428
{user, app}
2529
}
@@ -189,6 +193,92 @@ module PlaceOS::Auth
189193
end
190194
end
191195

196+
# ---- POST /auth/token — public clients (no client_secret) ---------
197+
#
198+
# SPAs / native apps (`confidential: false`) can't hold a secret; they
199+
# authenticate via PKCE. Doorkeeper let them exchange a code and refresh
200+
# without a `client_secret`, and Backoffice relies on exactly this. See
201+
# `AuthlyAdapter::Client#authorized?`.
202+
describe "POST /auth/token (public client)" do
203+
# Public variant of `new_application`: confidential is left false and
204+
# no secret is ever presented at the token endpoint.
205+
new_public_application = ->(redirect : String, scopes : String) {
206+
authority = ::PlaceOS::Model::Authority.find_by_domain("localhost").not_nil!
207+
user = ::PlaceOS::Model::Generator.user(authority)
208+
password = "bcrypt-please-#{Random.rand(99999)}"
209+
user.password = password
210+
user.save!
211+
app = ::PlaceOS::Model::DoorkeeperApplication.new
212+
app.name = "public-test-#{Random.rand(99999)}"
213+
app.redirect_uri = redirect
214+
app.scopes = scopes
215+
app.owner_id = user.id.as(String)
216+
app.confidential = false
217+
app.save!
218+
{user, app, password}
219+
}
220+
221+
it "exchanges a code and refreshes with no client_secret" do
222+
user, app, password = new_public_application.call("https://spa.example/cb", "public")
223+
cookie = Spec.signin!(client, user, password)
224+
225+
authorize_result = client.get(
226+
"/auth/authorize?response_type=code" \
227+
"&client_id=#{URI.encode_www_form(app.uid.as(String))}" \
228+
"&redirect_uri=#{URI.encode_www_form("https://spa.example/cb")}" \
229+
"&scope=public",
230+
headers: HTTP::Headers{"Host" => "localhost", "Cookie" => cookie},
231+
)
232+
authorize_result.status_code.should eq 302
233+
location = authorize_result.headers["Location"]
234+
code = URI::Params.parse(location.split('?', 2).last)["code"]
235+
236+
# Token exchange WITHOUT client_secret — this is the drop-in fix.
237+
token_result = form_post.call("/auth/token", {
238+
"grant_type" => "authorization_code",
239+
"client_id" => app.uid.as(String),
240+
"code" => code,
241+
"redirect_uri" => "https://spa.example/cb",
242+
})
243+
token_result.status_code.should eq 200
244+
token_body = JSON.parse(token_result.body)
245+
token_body["access_token"].as_s.should_not be_empty
246+
refresh_token = token_body["refresh_token"].as_s
247+
refresh_token.should_not be_empty
248+
249+
# Refresh, again with no client_secret.
250+
refresh_result = form_post.call("/auth/token", {
251+
"grant_type" => "refresh_token",
252+
"client_id" => app.uid.as(String),
253+
"refresh_token" => refresh_token,
254+
})
255+
refresh_result.status_code.should eq 200
256+
JSON.parse(refresh_result.body)["access_token"].as_s.should_not be_empty
257+
ensure
258+
app.try &.destroy
259+
user.try &.destroy
260+
end
261+
262+
it "denies the client_credentials grant to a public client" do
263+
user, app, _password = new_public_application.call("https://spa.example/cb2", "public")
264+
265+
# Even presenting the app's real secret, a public client must not be
266+
# able to mint client-credentials tokens — the grant is confidential
267+
# only, so a leaked (public) client_id can't be used as a principal.
268+
result = form_post.call("/auth/token", {
269+
"grant_type" => "client_credentials",
270+
"client_id" => app.uid.as(String),
271+
"client_secret" => app.secret,
272+
"scope" => "public",
273+
})
274+
result.status_code.should eq 401
275+
JSON.parse(result.body)["error"].as_s.should eq "unauthorized_client"
276+
ensure
277+
app.try &.destroy
278+
user.try &.destroy
279+
end
280+
end
281+
192282
# ---- GET /auth/authorize -----------------------------------------
193283

194284
describe "GET /auth/authorize" do

spec/controllers/oauth_token_claims_spec.cr

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ module PlaceOS::Auth
3232
app.redirect_uri = "https://app.example/cb"
3333
app.scopes = scopes
3434
app.owner_id = user.id.as(String)
35+
app.confidential = true
3536
app.save!
3637

3738
cookie = Spec.signin!(client, user, password)
@@ -149,6 +150,7 @@ module PlaceOS::Auth
149150
app.redirect_uri = "https://app.example/cb"
150151
app.scopes = "public"
151152
app.owner_id = user.id.as(String)
153+
app.confidential = true
152154
app.save!
153155

154156
headers = HTTP::Headers{

spec/controllers/token_meta_spec.cr

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ module PlaceOS::Auth
1717
app.redirect_uri = "https://app.example/cb/#{UUID.random}"
1818
app.scopes = scopes
1919
app.owner_id = user.id.as(String)
20+
app.confidential = true
2021
app.save!
2122
{user, app}
2223
}

src/placeos-auth/authly_adapter/client.cr

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,12 @@ module PlaceOS::Auth::AuthlyAdapter
5252
def authorized?(client_id : String, client_secret : String) : Bool
5353
app = find_app(client_id)
5454
return false unless app
55+
# Public clients (SPAs / native apps, `confidential: false`) don't
56+
# authenticate with a secret — they use PKCE. Doorkeeper skipped
57+
# client-secret validation for them, so we do too; `client_credentials`
58+
# is still denied to public clients in `allowed_grant_type?` below, so
59+
# this bypass can't be used to mint tokens without proof of possession.
60+
return true unless app.confidential
5561
Crypto::Subtle.constant_time_compare(app.secret, client_secret)
5662
end
5763

@@ -72,7 +78,14 @@ module PlaceOS::Auth::AuthlyAdapter
7278
# `Authly.config.clients` must implement it.
7379
def allowed_grant_type?(client_id : String, grant_type : String) : Bool
7480
return false unless ALLOWED_GRANT_TYPES.includes?(grant_type)
75-
!find_app(client_id).nil?
81+
app = find_app(client_id)
82+
return false unless app
83+
# `client_credentials` authenticates purely by secret, so it is only
84+
# ever valid for a confidential client. A public client that reached
85+
# `authorized?` via the secret bypass above must not be able to fall
86+
# through to a client-credentials grant.
87+
return false if grant_type == "client_credentials" && !app.confidential
88+
true
7689
end
7790

7891
# Returns the owning user's ID for use as the `sub` claim of

src/placeos-auth/controllers/oauth.cr

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,13 @@ module PlaceOS::Auth
110110
def token(
111111
grant_type : String,
112112
client_id : String,
113-
client_secret : String,
113+
# Public clients (SPAs / native apps registered with
114+
# `confidential: false`) cannot hold a secret — they authenticate
115+
# via PKCE. Doorkeeper made `client_secret` optional for them, so a
116+
# required param here would 422 every Backoffice token exchange
117+
# before any OAuth logic ran. The client adapter enforces the secret
118+
# only for confidential clients (see `AuthlyAdapter::Client`).
119+
client_secret : String? = nil,
114120
code : String? = nil,
115121
redirect_uri : String? = nil,
116122
refresh_token : String? = nil,
@@ -129,7 +135,7 @@ module PlaceOS::Auth
129135
::Authly.access_token(
130136
grant_type: grant_type,
131137
client_id: client_id,
132-
client_secret: client_secret,
138+
client_secret: client_secret || "",
133139
scope: scope,
134140
)
135141
when "authorization_code"
@@ -138,7 +144,7 @@ module PlaceOS::Auth
138144
::Authly.access_token(
139145
grant_type: grant_type,
140146
client_id: client_id,
141-
client_secret: client_secret,
147+
client_secret: client_secret || "",
142148
code: code,
143149
redirect_uri: redirect_uri,
144150
verifier: code_verifier || "",
@@ -148,7 +154,7 @@ module PlaceOS::Auth
148154
::Authly.access_token(
149155
grant_type: grant_type,
150156
client_id: client_id,
151-
client_secret: client_secret,
157+
client_secret: client_secret || "",
152158
refresh_token: refresh_token,
153159
)
154160
else
@@ -574,7 +580,9 @@ module PlaceOS::Auth
574580
@grant_types_supported = ["authorization_code", "client_credentials", "refresh_token"]
575581
@subject_types_supported = ["public"]
576582
@id_token_signing_alg_values_supported = ["RS256"]
577-
@token_endpoint_auth_methods_supported = ["client_secret_post"]
583+
# `none` advertises that public clients may authenticate the token
584+
# endpoint with PKCE alone (no client_secret) — see the token action.
585+
@token_endpoint_auth_methods_supported = ["client_secret_post", "none"]
578586
@code_challenge_methods_supported = ["S256"]
579587
@claims_supported = ["sub", "iss", "aud", "exp", "iat", "email", "full_name", "given_name", "family_name", "nickname", "phone_number", "preferred_username"]
580588
end

0 commit comments

Comments
 (0)