Merge remote-tracking branch 'origin/main' into deny-actors

Author: jnunemaker

lib/flipper/adapters/http.rb

@@ -65,7 +65,7 @@ def get_all(cache_bust: false)
         path += "&_cb=#{Time.now.to_i}" if cache_bust
         etag = @get_all_mutex.synchronize { @last_get_all_etag }
 
-        if etag
+        if etag && !cache_bust
           options[:headers] = { if_none_match: etag }
         end
 

lib/flipper/ui/actions/import.rb

@@ -8,6 +8,7 @@ class Import < UI::Action
         route %r{\A/settings\/import/?\Z}
 
         def post
+          render_read_only if read_only?
           contents = params['file'][:tempfile].read
           export = Flipper::Exporters::Json::Export.new(contents: contents)
           flipper.import(export)

lib/flipper/ui/configuration.rb

@@ -45,6 +45,11 @@ class Configuration
       # false and it will go away. Defaults to true.
       attr_accessor :cloud_recommendation
 
+      # Public: Set to false to disable the version check that fetches the
+      # latest release from flippercloud.io. Useful when a strict Content
+      # Security Policy is in place. Defaults to true.
+      attr_accessor :version_check_enabled
+
       # Public: What should show up in the form to add actors. This can be
       # different per application since flipper_id's can be whatever an
       # application needs. Defaults to "a flipper id".
@@ -114,6 +119,7 @@ def initialize
         @feature_removal_enabled = true
         @fun = true
         @cloud_recommendation = true
+        @version_check_enabled = true
         @add_actor_placeholder = "a flipper id"
         @descriptions_source = DEFAULT_DESCRIPTIONS_SOURCE
         @actor_names_source = DEFAULT_ACTOR_NAMES_SOURCE

lib/flipper/ui/views/layout.erb

@@ -18,8 +18,10 @@
       <%- end -%>
 
       <div class="text-muted small text-end">
-        <a href="#" class="badge text-bg-warning ms-2 d-none" style="font-size:100%" id="new-version-badge" data-version="<%= Flipper::VERSION %>">
-        </a>
+        <% if Flipper::UI.configuration.version_check_enabled %>
+          <a href="#" class="badge text-bg-warning ms-2 d-none" style="font-size:100%" id="new-version-badge" data-version="<%= Flipper::VERSION %>">
+          </a>
+        <% end %>
 
         <% if Flipper.deprecated_ruby_version? %>
           <a href="https://github.com/flippercloud/flipper/pull/776" class="badge text-bg-danger ms-2" style="font-size:100%">
@@ -83,6 +85,8 @@
     <script src="<%= script_name + popper_js[:src] %>" integrity="<%= popper_js[:hash] %>" crossorigin="anonymous"></script>
     <script src="<%= script_name + bootstrap_js[:src] %>" integrity="<%= bootstrap_js[:hash] %>" crossorigin="anonymous"></script>
     <script src="<%= script_name %>/js/application.js?v=<%= Flipper::VERSION %>"></script>
-    <script src="<%= script_name %>/js/version.js?v=<%= Flipper::VERSION %>"></script>
+    <% if Flipper::UI.configuration.version_check_enabled %>
+      <script src="<%= script_name %>/js/version.js?v=<%= Flipper::VERSION %>"></script>
+    <% end %>
   </body>
 </html>

lib/flipper/ui/views/settings.erb

@@ -38,6 +38,7 @@
   </div>
 </div>
 
+<% if write_allowed? %>
 <div class="card">
   <div class="card-header">
     <h4 class="m-0">Import</h4>
@@ -51,3 +52,4 @@
     </form>
   </div>
 </div>
+<% end %>

lib/flipper/version.rb

@@ -1,5 +1,5 @@
 module Flipper
-  VERSION = '1.4.0'.freeze
+  VERSION = '1.4.1'.freeze
 
   REQUIRED_RUBY_VERSION = '2.6'.freeze
   NEXT_REQUIRED_RUBY_VERSION = '3.0'.freeze

spec/flipper/adapters/http_spec.rb

@@ -278,6 +278,95 @@
       }.to raise_error(Flipper::Adapters::Http::Error)
     end
 
+    it "skips If-None-Match header when cache_bust is true" do
+      features_response = {
+        "features" => [
+          {
+            "key" => "search",
+            "gates" => [
+              {"key" => "boolean", "value" => true}
+            ]
+          }
+        ]
+      }
+
+      # First request - populate the ETag cache
+      stub_request(:get, "http://app.com/flipper/features?exclude_gate_names=true")
+        .to_return(
+          status: 200,
+          body: JSON.generate(features_response),
+          headers: { 'ETag' => '"abc123"' }
+        )
+
+      adapter = described_class.new(url: 'http://app.com/flipper')
+      adapter.get_all
+
+      # Second request with cache_bust - should NOT send If-None-Match
+      cache_bust_stub = stub_request(:get, %r{/flipper/features\?_cb=\d+&exclude_gate_names=true})
+        .to_return(
+          status: 200,
+          body: JSON.generate(features_response),
+          headers: { 'ETag' => '"def456"' }
+        )
+
+      adapter.get_all(cache_bust: true)
+
+      expect(cache_bust_stub).to have_been_requested.once
+      expect(
+        a_request(:get, %r{/flipper/features\?_cb=\d+&exclude_gate_names=true})
+        .with { |req| req.headers['If-None-Match'].nil? }
+      ).to have_been_made.once
+    end
+
+    it "returns fresh data on cache_bust even when ETag is cached" do
+      stale_response = {
+        "features" => [
+          {
+            "key" => "search",
+            "gates" => [
+              {"key" => "boolean", "value" => nil}
+            ]
+          }
+        ]
+      }
+
+      fresh_response = {
+        "features" => [
+          {
+            "key" => "search",
+            "gates" => [
+              {"key" => "boolean", "value" => true}
+            ]
+          }
+        ]
+      }
+
+      # First request - populate ETag cache with feature disabled
+      stub_request(:get, "http://app.com/flipper/features?exclude_gate_names=true")
+        .to_return(
+          status: 200,
+          body: JSON.generate(stale_response),
+          headers: { 'ETag' => '"abc123"' }
+        )
+
+      adapter = described_class.new(url: 'http://app.com/flipper')
+      stale_result = adapter.get_all
+
+      expect(stale_result["search"][:boolean]).to be_nil
+
+      # Cache bust request returns fresh data (feature now enabled)
+      stub_request(:get, %r{/flipper/features\?_cb=\d+&exclude_gate_names=true})
+        .to_return(
+          status: 200,
+          body: JSON.generate(fresh_response),
+          headers: { 'ETag' => '"def456"' }
+        )
+
+      fresh_result = adapter.get_all(cache_bust: true)
+
+      expect(fresh_result["search"][:boolean]).to eq("true")
+    end
+
     it "does not send If-None-Match for other endpoints" do
       stub_request(:get, "http://app.com/flipper/features/search")
         .to_return(status: 404)

spec/flipper/ui/actions/import_spec.rb

@@ -11,11 +11,12 @@
     { :csrf => token, 'csrf' => token, '_csrf_token' => token }
   end
 
+  let(:path) { FlipperRoot.join("spec", "fixtures", "flipper_pstore_1679087600.json") }
+
   describe "POST /settings/import" do
     before do
       flipper.enable :plausible
       flipper.disable :google_analytics
-      path = FlipperRoot.join("spec", "fixtures", "flipper_pstore_1679087600.json")
 
       post '/settings/import',
         {
@@ -37,5 +38,26 @@
       expect(last_response.status).to be(302)
       expect(last_response.headers['location']).to eq('/features')
     end
+
+    context "when in read only mode" do
+      before do
+        allow(flipper).to receive(:read_only?) { true }
+
+        post '/settings/import',
+          {
+            'authenticity_token' => token,
+            'file' => Rack::Test::UploadedFile.new(path, "application/json"),
+          },
+          'rack.session' => session
+      end
+
+      it 'returns 403' do
+        expect(last_response.status).to be(403)
+      end
+
+      it 'renders read only template' do
+        expect(last_response.body).to include('read only')
+      end
+    end
   end
 end

spec/flipper/ui/configuration_spec.rb

@@ -73,6 +73,17 @@
     end
   end
 
+  describe "#version_check_enabled" do
+    it "has default value" do
+      expect(configuration.version_check_enabled).to eq(true)
+    end
+
+    it "can be updated" do
+      configuration.version_check_enabled = false
+      expect(configuration.version_check_enabled).to eq(false)
+    end
+  end
+
   describe "#feature_removal_enabled" do
     it "has default value" do
       expect(configuration.feature_removal_enabled).to eq(true)