From a62d84e20a0bde8d1ff3b25ac53ab43fd6744010 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Mon, 10 Aug 2026 11:21:33 -0600 Subject: [PATCH 1/3] Validate rs/oauth redirect_uri before storing return URL --- app/controllers/rs/oauth_controller.rb | 9 +++++---- spec/controllers/rs/oauth_controller_spec.rb | 13 +++++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/app/controllers/rs/oauth_controller.rb b/app/controllers/rs/oauth_controller.rb index 2e2933f..360d50c 100644 --- a/app/controllers/rs/oauth_controller.rb +++ b/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 diff --git a/spec/controllers/rs/oauth_controller_spec.rb b/spec/controllers/rs/oauth_controller_spec.rb index 03f4750..78ea3d0 100644 --- a/spec/controllers/rs/oauth_controller_spec.rb +++ b/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 From 29a9dee1cbb766633b6dcda0e8d0337a60321ed8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Mon, 10 Aug 2026 11:21:59 -0600 Subject: [PATCH 2/3] Consume user_return_to on sign-in --- app/controllers/application_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 4eb4820..d2b0bc4 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -39,7 +39,7 @@ class ApplicationController < ActionController::Base end def after_sign_in_path_for(user) - session[:user_return_to] || root_path + session.delete(:user_return_to) || root_path end def lndhub_authenticate(options={}) From 89184cd85146168adbdb506b2cbb97ded19eda1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Mon, 10 Aug 2026 11:22:03 -0600 Subject: [PATCH 3/3] Redirect to root after password reset --- .../devise/passwords_controller.rb | 7 +++- spec/features/devise/password_reset.rb | 21 +++++++++++ spec/requests/devise/passwords_spec.rb | 36 +++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 spec/requests/devise/passwords_spec.rb diff --git a/app/controllers/devise/passwords_controller.rb b/app/controllers/devise/passwords_controller.rb index ab99fc8..370645e 100644 --- a/app/controllers/devise/passwords_controller.rb +++ b/app/controllers/devise/passwords_controller.rb @@ -55,7 +55,12 @@ class Devise::PasswordsController < DeviseController protected def after_resetting_password_path_for(resource) - Devise.sign_in_after_reset_password ? after_sign_in_path_for(resource) : new_session_path(resource_name) + session.delete(:user_return_to) + if Devise.sign_in_after_reset_password + root_path + else + new_session_path(resource_name) + end end # The path used after sending reset password instructions diff --git a/spec/features/devise/password_reset.rb b/spec/features/devise/password_reset.rb index c12396c..874f39a 100644 --- a/spec/features/devise/password_reset.rb +++ b/spec/features/devise/password_reset.rb @@ -50,5 +50,26 @@ RSpec.describe 'Password reset', type: :feature do expect(page).to have_content 'Your password has been changed successfully' expect(user.reload.reset_password_token).to be_nil end + + scenario "Ignores a stale return location left by an rs/oauth request" do + expect(Devise::LDAP::Adapter).to receive(:update_password) + .with(user.cn, 'catch me if you can').and_return(true) + + # Simulate an earlier rs/oauth authorization request that stored a + # return URL in the session (the original cause of issue #235). + visit new_rs_oauth_path(user.cn, + redirect_uri: "https://example.com", + client_id: "https://example.com", + scope: "documents") + + visit edit_user_password_path(reset_password_token: token) + fill_in :user_password, with: 'catch me if you can' + fill_in :user_password_confirmation, with: 'catch me if you can' + click_button 'Change my password' + + expect(page).to have_content 'Your password has been changed successfully' + expect(page).to have_current_path(root_path) + expect(page).not_to have_content 'Bad request' + end end end diff --git a/spec/requests/devise/passwords_spec.rb b/spec/requests/devise/passwords_spec.rb new file mode 100644 index 0000000..bdfae93 --- /dev/null +++ b/spec/requests/devise/passwords_spec.rb @@ -0,0 +1,36 @@ +require 'rails_helper' + +RSpec.describe "Devise password reset", type: :request do + let(:user) { create :user } + + describe "PUT /users/password" do + let(:token) { user.send(:set_reset_password_token) } + + before do + allow(Devise::LDAP::Adapter).to receive(:update_password).and_return(true) + end + + context "with a stale stored return location from an rs/oauth request" do + before do + get new_rs_oauth_url(user.cn, + redirect_uri: "https://example.com", + client_id: "https://example.com", + scope: "documents") + expect(session[:user_return_to]).to be_present + end + + it "redirects to the dashboard instead of the stored return URL" do + put user_password_path, params: { + user: { + reset_password_token: token, + password: "a brand new password", + password_confirmation: "a brand new password" + } + } + + expect(response).to redirect_to(root_path) + expect(session[:user_return_to]).to be_nil + end + end + end +end