From 5e1f1a57db6656c20aa6f9dee1e53225b882376c Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Fri, 17 Sep 2021 17:00:29 +0100
Subject: [PATCH] FIX: Correctly handle end_session_endpoint with query
 parameters (#18)

---
 plugin.rb                                 | 16 ++++++++++++----
 spec/requests/rp_initiated_logout_spec.rb | 14 +++++++++++---
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/plugin.rb b/plugin.rb
index 91c47ed..44e84cb 100644
--- a/plugin.rb
+++ b/plugin.rb
@@ -37,15 +37,23 @@ on(:before_session_destroy) do |data|
     next
   end
 
+  begin
+    uri = URI.parse(end_session_endpoint)
+  rescue URI::Error
+    authenticator.oidc_log "Logout: unable to parse end_session_endpoint #{end_session_endpoint}", error: true
+  end
+
   authenticator.oidc_log "Logout: Redirecting user_id=#{data[:user].id} to end_session_endpoint"
 
-  redirect_uri = end_session_endpoint
-  redirect_uri += "?id_token_hint=#{token}"
+  params = URI.decode_www_form(String(uri.query))
+
+  params << ["id_token_hint", token]
 
   post_logout_redirect = SiteSetting.openid_connect_rp_initiated_logout_redirect.presence
-  redirect_uri += "&post_logout_redirect_uri=#{post_logout_redirect}" if post_logout_redirect
+  params << ["post_logout_redirect_uri", post_logout_redirect] if post_logout_redirect
 
-  data[:redirect_url] = redirect_uri
+  uri.query = URI.encode_www_form(params)
+  data[:redirect_url] = uri.to_s
 end
 
 auth_provider authenticator: OpenIDConnectAuthenticator.new
diff --git a/spec/requests/rp_initiated_logout_spec.rb b/spec/requests/rp_initiated_logout_spec.rb
index b970788..c9d8fc4 100644
--- a/spec/requests/rp_initiated_logout_spec.rb
+++ b/spec/requests/rp_initiated_logout_spec.rb
@@ -11,14 +11,14 @@ describe "OIDC RP-Initiated Logout" do
       "token_endpoint": "https://id.example.com/token",
       "userinfo_endpoint": "https://id.example.com/userinfo",
       "end_session_endpoint": "https://id.example.com/endsession",
-    }.to_json
+    }
   end
   let(:user) { Fabricate(:user) }
 
   before do
     SiteSetting.openid_connect_enabled = true
     SiteSetting.openid_connect_rp_initiated_logout = true
-    stub_request(:get, document_url).to_return(body: document)
+    stub_request(:get, document_url).to_return(body: lambda { |r| document.to_json })
   end
 
   after do
@@ -52,11 +52,19 @@ describe "OIDC RP-Initiated Logout" do
       expect(response.parsed_body["redirect_url"]).to eq("https://id.example.com/endsession?id_token_hint=myoidctoken")
     end
 
+    it "correctly handles logout urls with existing query params" do
+      document[:end_session_endpoint] += "?param=true"
+
+      delete "/session/#{user.username}", xhr: true
+      expect(response.status).to eq(200)
+      expect(response.parsed_body["redirect_url"]).to eq("https://id.example.com/endsession?param=true&id_token_hint=myoidctoken")
+    end
+
     it "includes the redirect URI if set" do
       SiteSetting.openid_connect_rp_initiated_logout_redirect = "https://example.com"
       delete "/session/#{user.username}", xhr: true
       expect(response.status).to eq(200)
-      expect(response.parsed_body["redirect_url"]).to eq("https://id.example.com/endsession?id_token_hint=myoidctoken&post_logout_redirect_uri=https://example.com")
+      expect(response.parsed_body["redirect_url"]).to eq("https://id.example.com/endsession?id_token_hint=myoidctoken&post_logout_redirect_uri=https%3A%2F%2Fexample.com")
     end
 
     it "does not redirect if plugin disabled" do