From 8fae099c12dea290c56968e19e977510875b1a83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Tue, 6 Oct 2026 15:10:00 +0200 Subject: [PATCH] Gate remoteStorage by serviceEnabled instead of Flipper The admin "Default services" setting writes the LDAP serviceEnabled attribute, but remoteStorage access was also gated behind an unused per-user Flipper flag that nothing ever enabled, so remoteStorage was inaccessible for everyone. Gate remoteStorage on service_enabled? like the other services, and make the admin toggle reflect the LDAP attribute (this also fixes it reading current_user instead of the user being viewed). E-Mail keeps its Flipper gate for now. --- .../services/remotestorage_controller.rb | 6 +++--- .../services/rs_auths_controller.rb | 7 +++---- app/views/admin/users/show.html.erb | 2 +- app/views/dashboard/index.html.erb | 3 +-- app/views/shared/_sidenav_settings.html.erb | 3 +-- .../services/rs_auths_controller_spec.rb | 18 +++++++++++++++++- 6 files changed, 26 insertions(+), 13 deletions(-) diff --git a/app/controllers/services/remotestorage_controller.rb b/app/controllers/services/remotestorage_controller.rb index 4a7801e..cfa18a6 100644 --- a/app/controllers/services/remotestorage_controller.rb +++ b/app/controllers/services/remotestorage_controller.rb @@ -1,7 +1,7 @@ class Services::RemotestorageController < Services::BaseController before_action :authenticate_user! before_action :require_service_available - before_action :require_feature_enabled + before_action :require_service_enabled # Dashboard def show @@ -17,8 +17,8 @@ class Services::RemotestorageController < Services::BaseController http_status :not_found unless Setting.remotestorage_enabled? end - def require_feature_enabled - unless Flipper.enabled?(:remotestorage, current_user) + def require_service_enabled + unless current_user.service_enabled?(:remotestorage) http_status :forbidden end end diff --git a/app/controllers/services/rs_auths_controller.rb b/app/controllers/services/rs_auths_controller.rb index d47208d..f9376d8 100644 --- a/app/controllers/services/rs_auths_controller.rb +++ b/app/controllers/services/rs_auths_controller.rb @@ -1,8 +1,7 @@ class Services::RsAuthsController < Services::BaseController before_action :authenticate_user! - before_action :require_feature_enabled + before_action :require_service_enabled before_action :require_service_available - # before_action :require_service_enabled before_action :find_rs_auth, only: [:destroy, :launch_app] def index @@ -34,8 +33,8 @@ class Services::RsAuthsController < Services::BaseController private - def require_feature_enabled - unless Flipper.enabled?(:remotestorage, current_user) + def require_service_enabled + unless current_user.service_enabled?(:remotestorage) http_status :forbidden end end diff --git a/app/views/admin/users/show.html.erb b/app/views/admin/users/show.html.erb index 01aaf12..5dd83ae 100644 --- a/app/views/admin/users/show.html.erb +++ b/app/views/admin/users/show.html.erb @@ -305,7 +305,7 @@ remoteStorage <%= render FormElements::ToggleComponent.new( - enabled: Flipper.enabled?(:remotestorage, current_user) && @services_enabled.include?("remotestorage"), + enabled: @services_enabled.include?("remotestorage"), input_enabled: false ) %> diff --git a/app/views/dashboard/index.html.erb b/app/views/dashboard/index.html.erb index fc44512..3c97380 100644 --- a/app/views/dashboard/index.html.erb +++ b/app/views/dashboard/index.html.erb @@ -43,8 +43,7 @@ <% end %> <% end %> - <% if Setting.remotestorage_enabled? && - Flipper.enabled?(:remotestorage, current_user) %> + <% if Setting.remotestorage_enabled? %>
diff --git a/app/views/shared/_sidenav_settings.html.erb b/app/views/shared/_sidenav_settings.html.erb index c8f17a5..ada77aa 100644 --- a/app/views/shared/_sidenav_settings.html.erb +++ b/app/views/shared/_sidenav_settings.html.erb @@ -25,8 +25,7 @@ active: @settings_section.to_s == "lightning" ) %> <% end %> -<% if Setting.remotestorage_enabled? && - Flipper.enabled?(:remotestorage, current_user) %> +<% if Setting.remotestorage_enabled? %> <%= render SidenavLinkComponent.new( name: "Storage", path: setting_path(:remotestorage), icon: "remotestorage", active: @settings_section.to_s == "remotestorage" diff --git a/spec/controllers/services/rs_auths_controller_spec.rb b/spec/controllers/services/rs_auths_controller_spec.rb index 1284f68..1c619c2 100644 --- a/spec/controllers/services/rs_auths_controller_spec.rb +++ b/spec/controllers/services/rs_auths_controller_spec.rb @@ -6,7 +6,9 @@ RSpec.describe Services::RsAuthsController, type: :controller do before do allow_any_instance_of(AppCatalog::WebApp).to receive(:update_metadata).and_return(true) allow_any_instance_of(RemoteStorageAuthorization).to receive(:remove_token_expiry_job).and_return(nil) - allow_any_instance_of(Flipper).to receive(:enabled?).and_return(true) + allow_any_instance_of(LdapService).to receive(:fetch_users).and_return([ + { services_enabled: ["remotestorage"] } + ]) end describe "GET /services/storage/rs_auths/:id/launch_app" do @@ -35,6 +37,20 @@ RSpec.describe Services::RsAuthsController, type: :controller do expect(response).to redirect_to(launch_url) end end + + context "when remoteStorage is not enabled for the user" do + before do + allow_any_instance_of(LdapService).to receive(:fetch_users).and_return([ + { services_enabled: [] } + ]) + + get :launch_app, params: { id: 1 } + end + + it "responds with forbidden" do + expect(response).to have_http_status(:forbidden) + end + end end end end -- 2.50.1