Merge pull request #13357 from mfo/harden-api-token

mfo · web-flow · commit 4ff66145a3f6 · 2026-06-30T03:16:49.000Z
ETQ admin, je dois fournir un filtrage réseau pour avoir un jeton API
diff --git a/app/components/profile/api_token_component.rb b/app/components/profile/api_token_component.rb
@@ -20,6 +20,8 @@ def autorizations
   def network_filtering
     if @api_token.authorized_networks.present?
       "filtrage : #{@api_token.authorized_networks_for_ui}"
+    elsif @api_token.pending_auto_ip?
+      tag.span('en attente de détection IP (1er appel)', class: 'fr-badge fr-badge--sm fr-badge--info')
     else
       tag.span('aucun filtrage réseau', class: 'fr-text-default--warning')
     end
diff --git a/app/controllers/administrateurs/api_tokens_controller.rb b/app/controllers/administrateurs/api_tokens_controller.rb
@@ -26,7 +26,8 @@ def create
       @api_token, @packed_token = APIToken.generate(current_administrateur)
 
       @api_token.update!(name:, write_access:,
-                         allowed_procedure_ids:, authorized_networks:, expires_at:)
+                         allowed_procedure_ids:, authorized_networks:,
+                         expires_at:, requires_ip_filtering: true)
 
       @curl_command = curl_command(@packed_token, @api_token.procedure_ids.first)
     end
@@ -176,7 +177,7 @@ def expires_at
           Date.parse(params[:customLifetime]),
           1.year.from_now,
         ].min
-      in 'infinite' if authorized_networks.present?
+      in 'infinite'
         nil
       else
         1.week.from_now.to_date
diff --git a/app/controllers/api/v2/base_controller.rb b/app/controllers/api/v2/base_controller.rb
@@ -56,6 +56,7 @@ def authenticate_from_token
 
     if @api_token.present?
       @api_token.touch(:last_v2_authenticated_at)
+      @api_token.assign_first_ip!(request.remote_ip)
       @api_token.store_new_ip(request.remote_ip)
       @current_user = @api_token.administrateur.user
       Current.user = @current_user
diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb
@@ -33,6 +33,7 @@ def authenticate_from_token
 
     if @api_token.present?
       @api_token.touch(:last_v1_authenticated_at)
+      @api_token.assign_first_ip!(request.remote_ip)
       @api_token.store_new_ip(request.remote_ip)
       @current_user = @api_token.administrateur.user
     end
diff --git a/app/javascript/controllers/api_token_securite_controller.ts b/app/javascript/controllers/api_token_securite_controller.ts
@@ -4,15 +4,13 @@ export class ApiTokenSecuriteController extends ApplicationController {
   static targets = [
     'continueButton',
     'networkFiltering',
-    'infiniteLifetime',
     'customLifetime',
     'customLifetimeInput',
     'networks'
   ];
 
   declare readonly continueButtonTarget: HTMLButtonElement;
   declare readonly networkFilteringTarget: HTMLElement;
-  declare readonly infiniteLifetimeTarget: HTMLInputElement;
   declare readonly customLifetimeTarget: HTMLElement;
   declare readonly customLifetimeInputTarget: HTMLInputElement;
   declare readonly networksTarget: HTMLInputElement;
@@ -24,14 +22,11 @@ export class ApiTokenSecuriteController extends ApplicationController {
   showNetworkFiltering() {
     this.networkFilteringTarget.classList.remove('hidden');
     this.setContinueButtonState();
-    this.infiniteLifetimeTarget.disabled = false;
   }
 
   hideNetworkFiltering() {
     this.networkFilteringTarget.classList.add('hidden');
     this.setContinueButtonState();
-    this.infiniteLifetimeTarget.checked = false;
-    this.infiniteLifetimeTarget.disabled = true;
   }
 
   showCustomLifetime() {
@@ -45,34 +40,13 @@ export class ApiTokenSecuriteController extends ApplicationController {
   }
 
   setContinueButtonState() {
-    if (this.networkDefined() && this.lifetimeDefined()) {
+    if (this.lifetimeDefined()) {
       this.continueButtonTarget.disabled = false;
     } else {
       this.continueButtonTarget.disabled = true;
     }
   }
 
-  networkDefined() {
-    if (
-      this.element.querySelectorAll(
-        "[name='networkFiltering'][value='none']:checked"
-      ).length > 0
-    ) {
-      return true;
-    }
-
-    if (
-      this.element.querySelectorAll(
-        "[name='networkFiltering'][value='customNetworks']:checked"
-      ).length > 0 &&
-      this.networksTarget.value.trim() != ''
-    ) {
-      return true;
-    }
-
-    return false;
-  }
-
   lifetimeDefined() {
     if (
       this.element.querySelectorAll(
diff --git a/app/models/api_token.rb b/app/models/api_token.rb
@@ -87,6 +87,16 @@ def forbidden_network?(ip)
     authorized_networks.none? { |range| range.include?(ip) }
   end
 
+  def assign_first_ip!(ip)
+    return if !requires_ip_filtering? || authorized_networks.any?
+
+    update!(authorized_networks: [IPAddr.new(ip)])
+  end
+
+  def pending_auto_ip?
+    requires_ip_filtering? && authorized_networks.empty?
+  end
+
   def expired?
     expires_at&.past?
   end
diff --git a/app/views/administrateurs/api_tokens/edit.html.haml b/app/views/administrateurs/api_tokens/edit.html.haml
@@ -47,6 +47,11 @@
         - if @invalid_network_message.present?
           %p.fr-error-text= @invalid_network_message
 
+        - if @api_token.pending_auto_ip?
+          .fr-messages-group{ aria: { live: 'polite' } }
+            %p.fr-message.fr-message--info
+              Si aucun réseau n'est configuré, l'adresse IP sera automatiquement verrouillée lors du premier appel API.
+
     = form_with url: admin_api_token_path(@api_token), method: :patch, html: { class: 'fr-mt-2w' } do |f|
       .fr-mb-4w
         - if @api_token.full_access?
diff --git a/app/views/administrateurs/api_tokens/securite.html.haml b/app/views/administrateurs/api_tokens/securite.html.haml
@@ -28,15 +28,15 @@
 
     = render Dsfr::RadioButtonListComponent.new(form: f,
       target: :networkFiltering,
-      buttons: [ { label: 'Je veux spécifier les réseaux autorisées à utiliser mon jeton',
+      buttons: [ { label: 'Détection automatique au 1er appel API',
+        hint: "L'adresse IP de votre premier appel sera automatiquement autorisée",
+        value: :autoAssign,
+        checked: params[:networkFiltering] != 'customNetworks',
+        'data-action': 'click->api-token-securite#hideNetworkFiltering' },
+        { label: 'Saisie manuelle des réseaux autorisés',
         value: :customNetworks,
         checked: params[:networkFiltering] == 'customNetworks',
-        'data-action': 'click->api-token-securite#showNetworkFiltering' },
-        { label: 'Mon jeton peut être utilisé depuis nʼimporte quelle adresse IP dans le monde',
-        hint: 'dangereux',
-        value: :none,
-        checked: params[:networkFiltering] == 'none',
-        'data-action': 'click->api-token-securite#hideNetworkFiltering' }]) do
+        'data-action': 'click->api-token-securite#showNetworkFiltering' }]) do
       Filtrage réseau :
 
     .fr-input-group.fr-mb-4w{
@@ -68,11 +68,9 @@
         value: :custom,
         checked: params[:lifetime] == 'custom',
         'data-action': 'click->api-token-securite#showCustomLifetime'},
-        { label: 'Infini (le filtrage réseau doit être activé)',
+        { label: 'Infini',
         value: :infinite,
         checked: params[:lifetime] == 'infinite',
-        disabled: true,
-        'data-api-token-securite-target': 'infiniteLifetime',
         'data-action': 'click->api-token-securite#hideCustomLifetime' }]) do
       Durée de vie du jeton :
 
diff --git a/db/migrate/20260625153605_add_requires_ip_filtering_to_api_tokens.rb b/db/migrate/20260625153605_add_requires_ip_filtering_to_api_tokens.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class AddRequiresIPFilteringToAPITokens < ActiveRecord::Migration[8.0]
+  def change
+    add_column :api_tokens, :requires_ip_filtering, :boolean, default: false, null: false
+  end
+end
diff --git a/db/migrate/20260625153641_change_requires_ip_filtering_default.rb b/db/migrate/20260625153641_change_requires_ip_filtering_default.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class ChangeRequiresIPFilteringDefault < ActiveRecord::Migration[8.0]
+  def change
+    change_column_default :api_tokens, :requires_ip_filtering, from: false, to: true
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.2].define(version: 2026_06_22_131426) do
+ActiveRecord::Schema[8.0].define(version: 2026_06_25_153641) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_buffercache"
   enable_extension "pg_stat_statements"
@@ -124,6 +124,7 @@
     t.datetime "last_v1_authenticated_at"
     t.datetime "last_v2_authenticated_at"
     t.string "name", null: false
+    t.boolean "requires_ip_filtering", default: true, null: false
     t.inet "stored_ips", default: [], array: true
     t.datetime "updated_at", null: false
     t.integer "version", default: 3, null: false
diff --git a/spec/controllers/administrateurs/api_tokens_controller_spec.rb b/spec/controllers/administrateurs/api_tokens_controller_spec.rb
@@ -26,12 +26,13 @@
     context 'with write access, no filtering, one week' do
       let(:params) { default_params }
 
-      it 'creates a token' do
+      it 'creates a token with requires_ip_filtering' do
         expect(token.name).to eq('Test')
         expect(token.write_access?).to be true
         expect(token.full_access?).to be true
         expect(token.authorized_networks).to be_blank
         expect(token.expires_at).to eq(1.week.from_now.to_date)
+        expect(token.requires_ip_filtering).to be true
       end
     end
 
@@ -41,10 +42,10 @@
       it { expect(token.write_access?).to be false }
     end
 
-    context 'without network filtering but requiring infinite lifetime' do
+    context 'with infinite lifetime' do
       let(:params) { default_params.merge(lifetime: 'infinite') }
 
-      it { expect(token.expires_at).to eq(1.week.from_now.to_date) }
+      it { expect(token.expires_at).to be_nil }
     end
 
     context 'with bad network and infinite lifetime' do
diff --git a/spec/controllers/api/v2/graphql_controller_spec.rb b/spec/controllers/api/v2/graphql_controller_spec.rb
@@ -190,6 +190,41 @@ def compute_checksum_in_chunks(io)
           expect(gql_errors.first[:message]).to eq("An object of type Demarche was hidden due to permissions")
         }
       end
+
+      context 'when requires_ip_filtering is true and no networks defined (auto-assign)' do
+        before do
+          api_token.update!(requires_ip_filtering: true, authorized_networks: [])
+          request.remote_ip = '10.20.30.40'
+        end
+
+        it 'auto-assigns the IP on first call and succeeds' do
+          expect(gql_errors).to be_nil
+          expect(api_token.reload.authorized_networks).to eq([IPAddr.new('10.20.30.40')])
+        end
+      end
+
+      context 'when requires_ip_filtering is false and no networks defined (legacy)' do
+        before do
+          api_token.update!(requires_ip_filtering: false, authorized_networks: [])
+          request.remote_ip = '10.20.30.40'
+        end
+
+        it 'does not auto-assign' do
+          expect(gql_errors).to be_nil
+          expect(api_token.reload.authorized_networks).to be_empty
+        end
+      end
+
+      context 'when auto-assigned IP blocks subsequent call from different IP' do
+        before do
+          api_token.update!(requires_ip_filtering: true, authorized_networks: [IPAddr.new('10.20.30.40')])
+          request.remote_ip = '192.168.1.1'
+        end
+
+        it 'returns forbidden' do
+          expect(subject).to have_http_status(:forbidden)
+        end
+      end
     end
 
     describe "demarche" do
diff --git a/spec/controllers/api_controller_spec.rb b/spec/controllers/api_controller_spec.rb
@@ -73,6 +73,30 @@
         it { is_expected.to have_http_status(:unauthorized) }
       end
 
+      context 'when requires_ip_filtering is true and no networks defined (auto-assign)' do
+        before { token.update!(requires_ip_filtering: true, authorized_networks: []) }
+
+        context 'first call assigns the IP and succeeds' do
+          let(:remote_ip) { '10.20.30.40' }
+
+          it do
+            is_expected.to have_http_status(:ok)
+            expect(token.reload.authorized_networks).to eq([IPAddr.new('10.20.30.40')])
+          end
+        end
+      end
+
+      context 'when requires_ip_filtering is false and no networks defined (legacy)' do
+        before { token.update!(requires_ip_filtering: false, authorized_networks: []) }
+
+        let(:remote_ip) { '10.20.30.40' }
+
+        it 'does not auto-assign' do
+          is_expected.to have_http_status(:ok)
+          expect(token.reload.authorized_networks).to be_empty
+        end
+      end
+
       context 'when a single authorized network is defined' do
         before do
           token.update!(authorized_networks: [IPAddr.new('192.168.1.0/24')])
diff --git a/spec/models/api_token_spec.rb b/spec/models/api_token_spec.rb
@@ -199,6 +199,63 @@
     end
   end
 
+  describe '#assign_first_ip!' do
+    let(:api_token) { APIToken.generate(administrateur).first }
+
+    context 'when requires_ip_filtering is true and no networks defined' do
+      before { api_token.update!(requires_ip_filtering: true) }
+
+      it 'assigns the IP as authorized network' do
+        api_token.assign_first_ip!('192.168.1.1')
+        expect(api_token.reload.authorized_networks).to eq([IPAddr.new('192.168.1.1')])
+      end
+
+      it 'works with IPv6' do
+        api_token.assign_first_ip!('2001:41d0:304:400::52f')
+        expect(api_token.reload.authorized_networks).to eq([IPAddr.new('2001:41d0:304:400::52f')])
+      end
+    end
+
+    context 'when requires_ip_filtering is false' do
+      before { api_token.update!(requires_ip_filtering: false) }
+
+      it 'does nothing' do
+        api_token.assign_first_ip!('192.168.1.1')
+        expect(api_token.reload.authorized_networks).to be_empty
+      end
+    end
+
+    context 'when authorized_networks already set' do
+      before do
+        api_token.update!(requires_ip_filtering: true, authorized_networks: [IPAddr.new('10.0.0.1')])
+      end
+
+      it 'does not overwrite' do
+        api_token.assign_first_ip!('192.168.1.1')
+        expect(api_token.reload.authorized_networks).to eq([IPAddr.new('10.0.0.1')])
+      end
+    end
+  end
+
+  describe '#pending_auto_ip?' do
+    let(:api_token) { APIToken.generate(administrateur).first }
+
+    it 'returns true when requires_ip_filtering and no networks' do
+      api_token.update!(requires_ip_filtering: true, authorized_networks: [])
+      expect(api_token.pending_auto_ip?).to be true
+    end
+
+    it 'returns false when requires_ip_filtering is false' do
+      api_token.update!(requires_ip_filtering: false, authorized_networks: [])
+      expect(api_token.pending_auto_ip?).to be false
+    end
+
+    it 'returns false when networks are present' do
+      api_token.update!(requires_ip_filtering: true, authorized_networks: [IPAddr.new('10.0.0.1')])
+      expect(api_token.pending_auto_ip?).to be false
+    end
+  end
+
   describe '#expiring_within' do
     let(:api_token) { APIToken.generate(administrateur).first }
 
diff --git a/spec/system/administrateurs/api_token_spec.rb b/spec/system/administrateurs/api_token_spec.rb
