Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions spec/controllers/authorize_flow_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,31 @@ module PlaceOS::Auth
user.try &.destroy
end

it "bounces an unauthenticated caller to login rather than denying (AU-07)" do
# The deny path is gated on the session exactly like the grant path.
# It matters that it BOUNCES rather than emitting `access_denied`: an
# unauthenticated deny that redirected to the client would let anyone
# who can reach the endpoint fabricate a user's refusal, and the
# client would take it as a decision the user made.
user, password, app = make_app.call
query = "redirect_uri=#{URI.encode_www_form(app.redirect_uri.as(String))}" \
"&client_id=#{URI.encode_www_form(app.uid.as(String))}&response_type=code&state=abc"

result = client.delete("/auth/oauth/authorize?#{query}",
headers: HTTP::Headers{"Host" => "localhost"})

result.status_code.should eq 303
location = result.headers["Location"]
location.should eq "/auth/login"
# Nothing reached the client — no decision was invented on the
# user's behalf.
location.should_not contain "access_denied"
location.should_not start_with app.redirect_uri.as(String)
ensure
app.try &.destroy
user.try &.destroy
end

it "refuses to redirect to a URI not registered for the client (no open redirect)" do
user, password, app = make_app.call
cookie = Spec.signin!(client, user, password)
Expand Down
38 changes: 38 additions & 0 deletions spec/controllers/authorize_validation_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -808,5 +808,43 @@ module PlaceOS::Auth
user.try &.destroy
end
end

# ---- AU-12: the authorization code's lifetime ----------------------

describe "code expiry (AU-12)" do
it "mints codes with the 10-minute Doorkeeper lifetime" do
# Doorkeeper's `authorization_code_expires_in` default is 10 minutes
# and the legacy service never overrode it, so a cutover must not
# silently shorten or lengthen the window a half-finished login has
# to complete in. `authly_adapter.cr` sets `config.code_ttl =
# 10.minutes`; this asserts the value REACHES the code rather than
# asserting the constant back to itself.
#
# The rejection side — an expired code refused with 400
# `invalid_grant` rather than a 500 — lives in
# `token_disclosure_spec.cr`.
::Authly.config.code_ttl.should eq 10.minutes

redirect = "https://au12.example/cb-#{Random.rand(999_999)}"
user, password = make_user.call
app = make_app.call(redirect)
cookie = Spec.signin!(client, user, password)

result = authorize.call(cookie, {
"response_type" => "code",
"client_id" => app.uid.as(String),
"redirect_uri" => redirect,
"scope" => "public",
})
result.status_code.should eq 302
code = URI::Params.parse(result.headers["Location"].split('?', 2).last)["code"]

payload, _ = JWT.decode(code, ::Authly.config.public_key.as(String), JWT::Algorithm::RS256)
(payload["exp"].as_i64 - payload["iat"].as_i64).should eq 600
ensure
app.try &.destroy
user.try &.destroy
end
end
end
end
59 changes: 59 additions & 0 deletions spec/controllers/jwks_spec.cr
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require "../helper"
require "base64"

module PlaceOS::Auth
# JWKS parity for the Doorkeeper-openid_connect mount (PPT-2536).
Expand Down Expand Up @@ -32,5 +33,63 @@ module PlaceOS::Auth
n.size.should eq 342
n.should_not start_with "A" # a leading zero byte would encode as "A..."
end

# ---- OI-11: does a token point at the key that signed it? ----------

it "issues tokens whose header carries no kid (OI-11 — DIVERGENCE)" do
# An RP validating our tokens fetches the JWKS and has to pick a key.
# The standard way is `kid`: OIDC Core §10.1 says the header SHOULD
# carry one when the JWKS publishes more than one key, and most
# libraries look for it unconditionally.
#
# `Authly.jwt_encode` is `JWT.encode(payload, key, alg)`, which emits
# `{"alg":"RS256","typ":"JWT"}` and nothing else, so our tokens name no
# key at all. It works today only because the JWKS publishes exactly
# ONE key and every sane library falls back to "try the only one".
#
# Two things make this worth pinning rather than shrugging at. It is a
# latent blocker on key rotation: the moment the JWKS holds two keys, a
# kid-less token is ambiguous and strict RPs reject it — so rotation
# needs this fixed FIRST, not during. And it is the same class as the
# missing `nonce` (OI-04): fine for PlaceOS's own clients, a surprise
# for anyone integrating a conformant RP.
app = ::PlaceOS::Model::DoorkeeperApplication.new
app.name = "jwks-kid-#{Random.rand(999_999)}"
app.redirect_uri = "https://jwks.example/cb-#{Random.rand(999_999)}"
app.scopes = "public"
app.confidential = true
app.owner_id = "authority-owner"
app.save!

issued = client.post("/auth/token",
headers: HTTP::Headers{
"Host" => "localhost", "Content-Type" => "application/x-www-form-urlencoded",
},
body: URI::Params.build { |fp|
fp.add("grant_type", "client_credentials")
fp.add("client_id", app.uid.as(String))
fp.add("client_secret", app.secret)
fp.add("scope", "public")
})
issued.status_code.should eq 200
access = JSON.parse(issued.body)["access_token"].as_s

# Decode the JOSE header without verifying — that is exactly what an
# RP does before it knows which key to verify with.
header = JSON.parse(String.new(Base64.decode(access.split('.').first + "==")))
header["alg"].as_s.should eq "RS256"
header["typ"].as_s.should eq "JWT"
header.as_h.has_key?("kid").should be_false

# The key it WOULD have named, and the reason a fallback works today:
# there is exactly one.
keys = JSON.parse(
client.get("/auth/oauth/discovery/keys", headers: HTTP::Headers{"Host" => "localhost"}).body
)["keys"].as_a
keys.size.should eq 1
keys.first["kid"].as_s.should_not be_empty
ensure
app.try &.destroy
end
end
end
Loading
Loading