From 68e5f6dc20347ebc84c440c34f1460d0368d9820 Mon Sep 17 00:00:00 2001
From: Sylvain <sylvain@sleede.com>
Date: Mon, 9 Jan 2023 13:13:24 +0100
Subject: [PATCH] (security) restrict access to stats endpoint

---
 CHANGELOG.md                                |  2 ++
 app/controllers/api/analytics_controller.rb |  2 ++
 app/policies/admin_policy.rb                |  3 ++
 app/policies/age_range_policy.rb            |  5 ++-
 app/policies/analytics_policy.rb            |  8 +++++
 test/integration/analytics_test.rb          | 37 +++++++++++++++++++++
 6 files changed, 56 insertions(+), 1 deletion(-)
 create mode 100644 app/policies/analytics_policy.rb
 create mode 100644 test/integration/analytics_test.rb

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 732dcac75..209338311 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,7 @@
 # Changelog Fab-manager
 
+- Fix a security issue: logged users but non-admins can access to analytics data throught the API
+
 ## v5.6.2 2023 January 9
 
 - Improved fix_invoice_item task
diff --git a/app/controllers/api/analytics_controller.rb b/app/controllers/api/analytics_controller.rb
index 36ea8ca0f..12128202f 100644
--- a/app/controllers/api/analytics_controller.rb
+++ b/app/controllers/api/analytics_controller.rb
@@ -5,6 +5,8 @@ class API::AnalyticsController < API::ApiController
   before_action :authenticate_user!
 
   def data
+    authorize :analytics
+
     render json: HealthService.row_stats
   end
 end
diff --git a/app/policies/admin_policy.rb b/app/policies/admin_policy.rb
index 3d76f15fe..4c8521281 100644
--- a/app/policies/admin_policy.rb
+++ b/app/policies/admin_policy.rb
@@ -1,3 +1,6 @@
+# frozen_string_literal: true
+
+# Check the access policies for API::AdminsController
 class AdminPolicy < ApplicationPolicy
   def index?
     user.admin? || user.manager?
diff --git a/app/policies/age_range_policy.rb b/app/policies/age_range_policy.rb
index a6e3b7cb4..d43be22a2 100644
--- a/app/policies/age_range_policy.rb
+++ b/app/policies/age_range_policy.rb
@@ -1,5 +1,8 @@
+# frozen_string_literal: true
+
+# Check the access policies for API::AgeRangesController
 class AgeRangePolicy < ApplicationPolicy
-  %w(create update destroy show).each do |action|
+  %w[create update destroy show].each do |action|
     define_method "#{action}?" do
       user.admin?
     end
diff --git a/app/policies/analytics_policy.rb b/app/policies/analytics_policy.rb
new file mode 100644
index 000000000..c830f4ce8
--- /dev/null
+++ b/app/policies/analytics_policy.rb
@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+# Check the access policies for API::AnalyticsController
+class AnalyticsPolicy < ApplicationPolicy
+  def data?
+    user.admin?
+  end
+end
diff --git a/test/integration/analytics_test.rb b/test/integration/analytics_test.rb
new file mode 100644
index 000000000..a5f456f9a
--- /dev/null
+++ b/test/integration/analytics_test.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class AnalyticsTest < ActionDispatch::IntegrationTest
+  def setup
+    @admin = User.find_by(username: 'admin')
+    @jdupond = User.find_by(username: 'jdupond')
+  end
+
+  test 'fetch analytics data' do
+    login_as(@admin, scope: :user)
+
+    get '/api/analytics/data'
+
+    # Check response format & status
+    assert_equal 200, response.status, response.body
+    assert_equal Mime[:json], response.content_type
+
+    # Check the resulting data was created
+    res = json_response(response.body)
+    assert_not_nil res[:version]
+    assert_not_nil res[:members]
+    assert_not_nil res[:admins]
+    assert_not_nil res[:managers]
+    assert_not_nil res[:availabilities]
+    assert_not_nil res[:reservations]
+    assert_not_nil res[:orders]
+  end
+
+  test 'non-admin cannot fetch analytics data' do
+    login_as(@jdupond, scope: :user)
+    get '/api/analytics/data'
+
+    assert_response :forbidden
+  end
+end