Skip to content

Commit 751c39e

Browse files
committed
fix(oauth2): emit legacy redirect_uri (?id=) + b2clogin path rewrite
External IdP app registrations pin the exact redirect_uri the auth service sends. The legacy Ruby service (generic_oauth#callback_url) sends .../auth/oauth2/callback?id=<strat>, and its RewriteRedirectResponse middleware rewrites that to the path form .../auth/oauth2/callback/<strat> for *.b2clogin.com hosts (Azure B2C won't round-trip a query string on redirect_uri). auth.cr was sending a bare .../callback with no id, which every external IdP would reject as an unregistered redirect_uri, and the strat id never round-tripped. - callback_uri now carries ?id=<strat>, byte-identical to the legacy URL, used for both the authorize redirect and the token exchange. - Add rewrite_b2clogin_redirect to convert the authorize redirect's encoded redirect_uri to the path form for *.b2clogin.com hosts, mirroring RewriteRedirectResponse. The inbound path form was already handled by the callback_alias route (RewriteCallbackRequest equivalent). This makes external OAuth2 SSO a true drop-in: no IdP re-registration.
1 parent 1985565 commit 751c39e

2 files changed

Lines changed: 90 additions & 5 deletions

File tree

spec/controllers/provider_callbacks_spec.cr

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,70 @@ module PlaceOS::Auth
105105
end
106106
end
107107

108+
# ---- redirect_uri parity (drop-in) ------------------------------
109+
110+
# The legacy Ruby service (generic_oauth#callback_url) sends
111+
# `redirect_uri = <host>/auth/oauth2/callback?id=<strat>` to the IdP,
112+
# and its RewriteRedirectResponse middleware rewrites that to the
113+
# path form `<host>/auth/oauth2/callback/<strat>` for `*.b2clogin.com`
114+
# hosts (B2C won't round-trip a query string on redirect_uri). These
115+
# URLs are registered in external IdPs and cannot change, so auth.cr
116+
# must emit them byte-for-byte.
117+
describe "redirect_uri parity", tags: "oauth-redirect-uri" do
118+
it "sends redirect_uri carrying ?id=<strat> (matches legacy)" do
119+
strat = create_strat.call("https://idp.example.test", "openid")
120+
result = client.get(
121+
"/auth/oauth2?id=#{URI.encode_www_form(strat.id.as(String))}",
122+
headers: HTTP::Headers{"Host" => "localhost"},
123+
)
124+
result.status_code.should eq 303
125+
location = result.headers["Location"]
126+
params = URI::Params.parse(location.split('?', 2).last)
127+
params["redirect_uri"].should eq "http://localhost/auth/oauth2/callback?id=#{strat.id}"
128+
ensure
129+
strat.try &.destroy
130+
end
131+
132+
it "rewrites redirect_uri to the path form for *.b2clogin.com hosts" do
133+
strat = create_strat.call("https://org.b2clogin.com", "openid")
134+
result = client.get(
135+
"/auth/oauth2?id=#{URI.encode_www_form(strat.id.as(String))}",
136+
headers: HTTP::Headers{"Host" => "localhost"},
137+
)
138+
result.status_code.should eq 303
139+
location = result.headers["Location"]
140+
location.should start_with "https://org.b2clogin.com/authorize?"
141+
params = URI::Params.parse(location.split('?', 2).last)
142+
params["redirect_uri"].should eq "http://localhost/auth/oauth2/callback/#{strat.id}"
143+
ensure
144+
strat.try &.destroy
145+
end
146+
147+
it "accepts the path-form callback and completes the round-trip" do
148+
strat = create_strat.call("https://idp.example.test", "openid email")
149+
stub_token_endpoint.call("https://idp.example.test", "idp-token-xyz")
150+
stub_userinfo.call("https://idp.example.test", {
151+
"id" => "idp-uid-#{Random.rand(99999)}",
152+
"email" => "bob-#{Random.rand(99999)}@localhost",
153+
"name" => "Bob Barker",
154+
})
155+
156+
kickoff_data = kickoff.call(strat.id.as(String))
157+
158+
# IdP returns via the path-form callback (B2C style).
159+
result = client.get(
160+
"/auth/oauth2/callback/#{URI.encode_www_form(strat.id.as(String))}?code=test-code&state=#{kickoff_data[:state]}",
161+
headers: HTTP::Headers{
162+
"Host" => "localhost",
163+
"Cookie" => kickoff_data[:cookie],
164+
},
165+
)
166+
result.status_code.should eq 303
167+
ensure
168+
strat.try &.destroy
169+
end
170+
end
171+
108172
# ---- GET /auth/:provider/callback -------------------------------
109173

110174
describe "GET /auth/:provider/callback" do

src/placeos-auth/controllers/provider_callbacks.cr

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ module PlaceOS::Auth
4040
raise Error::NotFound.new("authority not found") if current_authority.nil?
4141

4242
provider_id = id
43-
redirect_uri = callback_uri(provider)
43+
redirect_uri = callback_uri(provider, provider_id)
4444

4545
# Validate provider + strat exist before redirecting out. If the
4646
# caller invented a strat id we'd rather 404 here than after the
@@ -56,7 +56,7 @@ module PlaceOS::Auth
5656
state = Random::Secure.hex(16)
5757
session[SESSION_OAUTH_STATE] = "#{provider}|#{provider_id || ""}|#{state}"
5858

59-
redirect_to engine.authorize_uri(state: state), :see_other
59+
redirect_to rewrite_b2clogin_redirect(engine.authorize_uri(state: state)), :see_other
6060
end
6161

6262
# ---- GET/POST /auth/:provider/callback ----------------------------
@@ -78,7 +78,7 @@ module PlaceOS::Auth
7878
raise Error::Unauthorized.new("oauth state mismatch")
7979
end
8080

81-
redirect_uri = callback_uri(provider)
81+
redirect_uri = callback_uri(provider, id)
8282
engine = ::MultiAuth.make(provider, redirect_uri, id)
8383

8484
# `request.query_params` is `URI::Params` (Enumerable of
@@ -122,10 +122,31 @@ module PlaceOS::Auth
122122

123123
# ---- private helpers -----------------------------------------------
124124

125-
private def callback_uri(provider : String) : String
125+
# Builds the OAuth/SAML callback `redirect_uri`. The strategy id is
126+
# carried as an `?id=<id>` query param — byte-for-byte what the legacy
127+
# Ruby service sent (`generic_oauth#callback_url`) so the value stays
128+
# identical to what external IdPs already have registered, and so the
129+
# id round-trips back to `#callback`.
130+
private def callback_uri(provider : String, id : String? = nil) : String
126131
scheme = request.headers["X-Forwarded-Proto"]? || (PlaceOS::Auth.production? ? "https" : "http")
127132
host = request.hostname || "localhost"
128-
"#{scheme}://#{host}/auth/#{provider}/callback"
133+
uri = "#{scheme}://#{host}/auth/#{provider}/callback"
134+
uri += "?id=#{id}" if id && !id.empty?
135+
uri
136+
end
137+
138+
# Mirror the legacy `RewriteRedirectResponse` middleware. Azure AD B2C
139+
# won't round-trip a query string on `redirect_uri`, so the strategy
140+
# id is carried as a path segment instead. When the outbound authorize
141+
# redirect targets a `*.b2clogin.com` host, rewrite the encoded
142+
# `.../callback?id=<id>` to `.../callback/<id>` (the inbound path form
143+
# is accepted by `callback_alias`). Only the authorize redirect is
144+
# rewritten; the token-exchange `redirect_uri` stays in `?id=` form,
145+
# exactly as the Ruby service behaved.
146+
private def rewrite_b2clogin_redirect(authorize_uri : String) : String
147+
host = URI.parse(authorize_uri).host
148+
return authorize_uri unless host && host.ends_with?(".b2clogin.com")
149+
authorize_uri.gsub("%3Fid%3D", "%2F").gsub("?id=", "/")
129150
end
130151

131152
private def consume_stored_state : Tuple(String?, String?, String?)

0 commit comments

Comments
 (0)