Skip to content

Commit

Permalink
Fix OIDC redirect_uri for CDDO SSO
Browse files Browse the repository at this point in the history
For some reason the omniauth_openid_connect gem wants us to supply the
`redirect_uri` (the URL to redirect to after a successful OAuth2
negotiation, which in our case is the OmniAuth callback route) when the
provider is configured. This is really annoying to do, and I can't see
why it wouldn't be able to use the OmniAuth `callback_url` in the same
way the omniauth-oauth2 gem does [[1]]. There is an open feature request
for this [[2]], but it hasn't been acknowledged by the maintainers as
yet. So instead we'll just monkeypatch the strategy.

[1]: https://github.com/omniauth/omniauth-oauth2/blob/3a43234ab5dd36a75f9c125c58fcfe1a37b26805/lib/omniauth/strategies/oauth2.rb#L59
[2]: omniauth/omniauth_openid_connect#136 (comment)
  • Loading branch information
lfdebrux committed Jun 30, 2023
1 parent a818fdd commit 6344591
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 0 deletions.
7 changes: 7 additions & 0 deletions config/initializers/authentication.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,10 @@
warden.failure_app = AuthenticationController
end
end

# Monkeypatch omniauth_openid_connect
class OmniAuth::Strategies::OpenIDConnect
def redirect_uri
callback_url
end
end
12 changes: 12 additions & 0 deletions spec/integration/cddo_sso_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,18 @@

expect(request.env["warden"].authenticated?).to be true
end

it "redirects to the OmniAuth callback URL" do
OmniAuth.config.test_mode = false

allow(Settings.cddo_sso).to receive(:identifier).and_return("foo")
allow(Settings.cddo_sso).to receive(:secret).and_return("bar")

get "/auth/cddo_sso"

expect(response).to redirect_to %r{^https://sso\.service\.security\.gov\.uk}
expect(response).to redirect_to %r{redirect_uri=http%3A%2F%2Fwww\.example\.com%2Fauth%2Fcddo_sso%2Fcallback}
end
end

describe "signing out" do
Expand Down

0 comments on commit 6344591

Please sign in to comment.