Allow customising of the logout returnTo URL (#56)

Closes #46 

## What's changed

* A returnTo parameter can be added to the logout path to redirect users
to a different place having logged out.
* Altered how we handle returnTo parameters in general, as Rails 6
doesn't check for open redirects

Author: patch0

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+-  Allow for customisation of returnTo param on log out (#56)
+
 ## [v3.1.0]
 
 ### Changed

README.md

@@ -139,6 +139,16 @@ meaning (most) users will end up back on the page where they started the auth fl
 
 Finally, if none of these things are set, we end up back at the application root.
 
+#### Redirecting when logging out
+
+It is also possible to send users to pages within your app when logging out.  Just set the `returnTo` parameter again.
+
+```ruby
+link_to 'Log out', rpi_auth_logout_path, params: { returnTo: '/thanks-dude' }
+```
+
+This has to be a relative URL, i.e. it has to start with a slash.  This is to ensure there's no open redirect.
+
 ### Globbed/catch-all routes
 
 If your app has a catch-all route at the end of the routing table, you must

app/controllers/rpi_auth/auth_controller.rb

@@ -17,20 +17,18 @@ def callback
       self.current_user = RpiAuth.user_model.from_omniauth(auth)
 
       redirect_to RpiAuth.configuration.success_redirect.presence ||
-                  request.env.fetch('omniauth.origin', nil).presence ||
-                  '/'
+                  ensure_relative_url(request.env['omniauth.origin'])
     end
 
     def destroy
       reset_session
 
-      # Prevent redirect loops etc.
-      if RpiAuth.configuration.bypass_auth == true
-        redirect_to '/'
-        return
-      end
+      # Any redirect must be within our app, so it should start with a slash.
+      return_to = ensure_relative_url(params[:returnTo])
 
-      redirect_to "#{RpiAuth.configuration.identity_url}/logout?returnTo=#{RpiAuth.configuration.host_url}",
+      return redirect_to return_to if RpiAuth.configuration.bypass_auth == true
+
+      redirect_to "#{RpiAuth.configuration.identity_url}/logout?returnTo=#{RpiAuth.configuration.host_url}#{return_to}",
                   allow_other_host: true
     end
 
@@ -42,5 +40,22 @@ def failure
                       end
       redirect_to '/'
     end
+
+    private
+
+    def ensure_relative_url(url)
+      url = URI.parse(url)
+
+      # Bail out early if the URL doesn't look local. This condition is taken
+      # from ActionController::Redirecting#_url_host_allowed? in Rails 7.0
+      raise ArgumentError unless url.host == request.host || (url.host.nil? && url.to_s.start_with?('/'))
+
+      # This is a bit of an odd way to do things, but it means that this method
+      # works with both URI::Generic and URI::HTTP
+      relative_url = [url.path, url.query].compact.join('?')
+      [relative_url, url.fragment].compact.join('#')
+    rescue ArgumentError, URI::Error
+      '/'
+    end
   end
 end

spec/dummy/spec/requests/auth_request_spec.rb

@@ -17,7 +17,9 @@
 
   let(:bypass_auth) { false }
   let(:identity_url) { 'https://my.example.com' }
-  let(:host_url) { 'https://example.com' }
+  # We need to make sure we match the hostname Rails uses in test requests
+  # here, otherwise `returnTo` redirects will fail after login/logout.
+  let(:host_url) { 'https://www.example.com' }
 
   before do
     RpiAuth.configuration.user_model = 'User'
@@ -38,11 +40,35 @@
 
       get '/rpi_auth/logout'
 
-      expect(response).to redirect_to("#{identity_url}/logout?returnTo=#{host_url}")
+      expect(response).to redirect_to("#{identity_url}/logout?returnTo=#{host_url}/")
       expect(session.id).not_to eq previous_id
       expect(session['current_user']).to be_nil
     end
 
+    context 'when a returnTo param is given' do
+      let(:return_to) { '/next/page' }
+
+      it 'redirects to the correct URL' do
+        sign_in(user)
+
+        get '/rpi_auth/logout', params: { returnTo: return_to }
+
+        expect(response).to redirect_to("#{identity_url}/logout?returnTo=#{URI.join(host_url, return_to)}")
+      end
+
+      context 'when the returnTo param is on another host' do
+        let(:return_to) { 'https://a.bad.actor.com/bad/page' }
+
+        it 'redirects to the correct URL' do
+          sign_in(user)
+
+          get '/rpi_auth/logout', params: { returnTo: return_to }
+
+          expect(response).to redirect_to("#{identity_url}/logout?returnTo=#{host_url}/")
+        end
+      end
+    end
+
     context 'when bypass_auth is set' do
       let(:bypass_auth) { true }
 
@@ -59,6 +85,18 @@
         expect(session.id).not_to eq previous_id
         expect(session['current_user']).to be_nil
       end
+
+      context 'when a returnTo param is given' do
+        let(:return_to) { '/next/page' }
+
+        it 'redirects to the correct URL' do
+          sign_in(user)
+
+          get '/rpi_auth/logout', params: { returnTo: return_to }
+
+          expect(response).to redirect_to(return_to)
+        end
+      end
     end
   end
 
@@ -144,21 +182,31 @@
 
       context 'when having visited a page first' do
         it 'redirects back to the original page' do
-          post '/auth/rpi', headers: { Referer: 'http://www.example.com/foo' }
+          post '/auth/rpi', headers: { Referer: 'http://www.example.com/foo#foo' }
           expect(response).to redirect_to('/rpi_auth/auth/callback')
           follow_redirect!
 
-          expect(response).to redirect_to('/foo')
+          expect(response).to redirect_to('/foo#foo')
         end
       end
 
       context 'when returnTo param is set' do
         it 'redirects back to the original page' do
-          post '/auth/rpi', params: { returnTo: 'http://www.example.com/bar' }
+          post '/auth/rpi', params: { returnTo: 'http://www.example.com/bar?q=p#r' }
+          expect(response).to redirect_to('/rpi_auth/auth/callback')
+          follow_redirect!
+
+          expect(response).to redirect_to('/bar?q=p#r')
+        end
+      end
+
+      context 'when the returnTo param is on another host' do
+        it 'redirects to the root URL' do
+          post '/auth/rpi', params: { returnTo: 'https://a.bad.actor.com/bad/page' }
           expect(response).to redirect_to('/rpi_auth/auth/callback')
           follow_redirect!
 
-          expect(response).to redirect_to('/bar')
+          expect(response).to redirect_to('/')
         end
       end