Validate rs/oauth redirect_uri before storing return URL

app/controllers/rs/oauth_controller.rb

@@ -1,4 +1,5 @@
 class Rs::OauthController < ApplicationController
+  prepend_before_action :assert_redirect_uri, only: [:new, :create]
   before_action :require_signed_in_with_username, only: :new
   before_action :authenticate_user!, only: :create
 
@@ -16,8 +17,6 @@ class Rs::OauthController < ApplicationController
                         ["In 1 month", 1.month.from_now],
                         ["In 1 day", 1.day.from_now]]
 
-    http_status :bad_request and return unless @redirect_uri.present?
-
     unless current_user == @user
       sign_out :user
 
@@ -64,8 +63,6 @@ class Rs::OauthController < ApplicationController
     state        = params[:state].presence
     expire_at    = params[:expire_at].presence
 
-    http_status :bad_request and return unless redirect_uri.present?
-
     if permissions.empty?
       redirect_to(url_with_state("#{redirect_uri}#error=invalid_scope", state),
                   allow_other_host: true) and return
@@ -97,6 +94,10 @@ class Rs::OauthController < ApplicationController
 
   private
 
+  def assert_redirect_uri
+    http_status :bad_request unless params[:redirect_uri].present?
+  end
+
   def require_signed_in_with_username
     unless user_signed_in?
       session[:user_return_to] = request.url

spec/controllers/rs/oauth_controller_spec.rb

@@ -222,6 +222,19 @@ RSpec.describe Rs::OauthController, type: :controller do
 
         expect(response).to redirect_to(new_user_session_path(cn: user.cn, ou: user.ou))
       end
+
+      context "without a redirect_uri" do
+        it "returns a 400 without storing the return location" do
+          get :new, params: {
+            username: user.cn,
+            scope: "documents,photos",
+            client_id: "https://example.com"
+          }
+
+          expect(response.response_code).to eq(400)
+          expect(session[:user_return_to]).to be_nil
+        end
+      end
     end
 
     describe "root access" do