fix: any remaining open redirects (#61)

the continue param redirects internally or to whitelisted URLs

app/controllers/auth/coauth_controller.rb

@@ -20,6 +20,23 @@ def login_path
 
     protected
 
+    def redirect_continue(path)
+      # checking for `.attacker.com` OR `//<external_domain>`
+      # no internal paths use `//`
+      if !path.start_with?("/") || path.include?("//")
+        authority = current_authority
+        uri = Addressable::URI.parse(path)
+
+        path = if uri.domain == authority.domain
+          "#{uri.request_uri}#{uri.fragment ? "##{uri.fragment}" : nil}"
+        else
+          yield
+        end
+      end
+
+      redirect_to path
+    end
+
     def new_session(user)
       @current_user = user
       value = {
@@ -50,7 +67,7 @@ def store_social(uid, provider)
     end
 
     def set_continue(path)
-      if path.include?("://")
+      if !path.start_with?("/") || path.include?("//")
         uri = Addressable::URI.parse(path)
         path = "#{uri.request_uri}#{uri.fragment ? "##{uri.fragment}" : nil}"
       end

app/controllers/auth/sessions_controller.rb

@@ -34,7 +34,7 @@ def signin
 
         # If there is a path we are using an inline login form
         if path
-          redirect_to path
+          redirect_continue(path) { "/" }
         else
           head :accepted
         end
@@ -67,7 +67,7 @@ def create
         user.save
 
         Authentication.create_with_omniauth(authority.id, auth, user.id)
-        redirect_to path
+        redirect_continue(path) { success_path }
         instance_exec user, auth["provider"], auth, &Authentication.after_login_block
 
       # new auth and new user
@@ -104,7 +104,7 @@ def create
 
           # redirect the user to the page they were trying to access and
           # run any custom post-login actions
-          redirect_to path
+          redirect_continue(path) { success_path }
           instance_exec user, auth["provider"], auth, &Authentication.after_login_block
         else
           info = "User creation failed with #{auth.inspect}"
@@ -138,7 +138,7 @@ def create
           user.assign_attributes(args)
           user.save
           new_session(user)
-          redirect_to path
+          redirect_continue(path) { success_path }
           instance_exec user, auth["provider"], auth, &Authentication.after_login_block
         rescue => e
           logger.error "Error with user account. Possibly due to a database failure:\nAuth model: #{auth_model.inspect}\n#{e.inspect}"
@@ -151,22 +151,12 @@ def create
     def destroy
       remove_session
 
-      # do we want to redirect externally?
-      path = params.permit(:continue)[:continue] || "/"
-
-      if !path.start_with?("/") || path.include?("//")
-        authority = current_authority
-        uri = Addressable::URI.parse(path)
-
-        if uri.domain == authority.domain
-          path = "#{uri.request_uri}#{uri.fragment ? "##{uri.fragment}" : nil}"
-        else
-          path = authority.logout_url
-          path = URI.decode_www_form_component(path.split("continue=", 2)[-1]) if path.include?("continue=")
-        end
+      # Only whitelisted URLs can redirect externally
+      redirect_continue(params.permit(:continue)[:continue] || "/") do
+        uri = authority.logout_url
+        uri = URI.decode_www_form_component(uri.split("continue=", 2)[-1]) if uri.include?("continue=")
+        uri
       end
-
-      redirect_to path
     end
 
     protected

app/controllers/auth/signups_controller.rb

@@ -43,7 +43,7 @@ def create
             new_session(user)
 
             # we're in a pop-up so redirect to a page that can communicate to the main page
-            redirect_to path
+            redirect_continue(path) { success_path }
           else
             # Email address taken (all other validation can be checked on the client)
             head :conflict