From 8bf0f9f98b88caeaa2cdec9f19c1ab9d15f69d04 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 08:53:20 -0400 Subject: [PATCH 01/34] Add security and auditing gems to Gemfile for enhanced application safety This commit introduces several new gems: `rack-attack` for rate limiting and blocking abusive requests, `brakeman` for static analysis security vulnerability scanning, `bundler-audit` for checking for insecure gem versions, and `safety_net` for additional security measures. These additions aim to improve the overall security posture of the application and ensure better dependency management. --- Gemfile | 4 ++++ Gemfile.lock | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/Gemfile b/Gemfile index 2cba4b62..cc36c67f 100644 --- a/Gemfile +++ b/Gemfile @@ -22,6 +22,7 @@ gem 'omniauth-saml', '~> 2.1' gem 'pagy', '~> 6.4' gem 'puma' gem 'pundit' +gem 'rack-attack' gem 'redis', '~> 5.0' gem 'sentry-ruby' gem 'sentry-rails' @@ -59,6 +60,8 @@ group :development, :staging do end group :development, :test do + gem 'brakeman' + gem 'bundler-audit' gem 'capybara' gem 'debug', platforms: %i[mri mingw x64_mingw] gem 'pry-byebug' @@ -72,6 +75,7 @@ group :development, :test do gem 'rubocop-rails-omakase', require: false gem 'rubocop-rspec', require: false gem 'rubocop-rspec_rails', require: false + gem 'safety_net' end group :development, :staging, :test do diff --git a/Gemfile.lock b/Gemfile.lock index c88c0ad5..be0c968b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -93,7 +93,12 @@ GEM debug_inspector (>= 1.2.0) bootsnap (1.18.4) msgpack (~> 1.2) + brakeman (7.0.2) + racc builder (3.3.0) + bundler-audit (0.9.2) + bundler (>= 1.2.0, < 3) + thor (~> 1.0) byebug (11.1.3) capistrano (3.19.1) airbrussh (>= 1.0.0) @@ -318,6 +323,8 @@ GEM rack (3.1.12) rack-accept (0.4.5) rack (>= 0.4) + rack-attack (6.7.0) + rack (>= 1.0, < 4) rack-protection (4.1.1) base64 (>= 0.1.0) logger (>= 1.6.0) @@ -445,6 +452,7 @@ GEM ffi (~> 1.12) logger rubyzip (2.3.2) + safety_net (1.0.1) sassc (2.4.0) ffi (~> 1.9) sassc-rails (2.1.2) @@ -551,6 +559,8 @@ DEPENDENCIES better_errors binding_of_caller bootsnap + brakeman + bundler-audit capistrano (~> 3.17) capistrano-asdf capistrano-rails (~> 1.6, >= 1.6.1) @@ -578,6 +588,7 @@ DEPENDENCIES puma pundit pundit-matchers + rack-attack rails (~> 7.2) rails-controller-testing redis (~> 5.0) @@ -589,6 +600,7 @@ DEPENDENCIES rubocop-rails-omakase rubocop-rspec rubocop-rspec_rails + safety_net sassc-rails selenium-webdriver sentry-rails From c4089810dfb371d2e3a562bac2eed6b1961c79f4 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 08:53:50 -0400 Subject: [PATCH 02/34] Add Dependabot configuration for automated dependency updates This commit introduces a new Dependabot configuration file to automate dependency updates for the application. The configuration specifies a weekly update schedule for Bundler dependencies, limits open pull requests, and includes settings for reviewers, assignees, and labels. This enhancement aims to improve dependency management and security by ensuring timely updates. --- .github/dependabot.yml | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 .github/dependabot.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 00000000..f3a67b06 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,22 @@ +version: 2 +updates: + - package-ecosystem: "bundler" + directory: "/" + schedule: + interval: "weekly" + day: "monday" + time: "09:00" + timezone: "America/New_York" + open-pull-requests-limit: 10 + reviewers: + - "rsmoke" + assignees: + - "rsmoke" + labels: + - "dependencies" + - "security" + commit-message: + prefix: "chore" + include: "scope" + rebase-strategy: "auto" + versioning-strategy: "auto" From faaad332bc2ee32b00a0cf2b1ad2c161dbc6cc4c Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 08:54:19 -0400 Subject: [PATCH 03/34] Add GitHub Actions workflow for security and test checks This commit introduces a new GitHub Actions workflow that automates security audits and test execution. The workflow is triggered on pushes and pull requests to the main and staging branches, as well as on a weekly schedule. It sets up a MySQL service, installs dependencies, creates the test database, and runs security checks using Brakeman and bundle audit, followed by executing RSpec tests. This enhancement aims to improve the application's security posture and ensure reliable test coverage. --- .github/workflows/security.yml | 61 ++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 .github/workflows/security.yml diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml new file mode 100644 index 00000000..f777bc44 --- /dev/null +++ b/.github/workflows/security.yml @@ -0,0 +1,61 @@ +name: Security and Test Checks + +on: + push: + branches: [ main, staging ] + pull_request: + branches: [ main, staging ] + schedule: + - cron: '0 0 * * 1' # Run weekly on Monday at midnight UTC + +jobs: + security_and_tests: + runs-on: ubuntu-latest + services: + mysql: + image: mysql:5.7 + env: + MYSQL_ROOT_PASSWORD: password + MYSQL_DATABASE: lsa_evaluate_test + ports: + - 3306:3306 + options: >- + --health-cmd="mysqladmin ping" + --health-interval=10s + --health-timeout=5s + --health-retries=3 + + steps: + - uses: actions/checkout@v4 + + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: '3.3.4' + bundler-cache: true + + - name: Install dependencies + run: bundle install + + - name: Set up database + env: + RAILS_ENV: test + DATABASE_URL: mysql2://root:password@localhost:3306/lsa_evaluate_test + run: | + bundle exec rails db:create + bundle exec rails db:schema:load + + - name: Run security audit + run: bundle exec rake security:audit + + - name: Run Brakeman + run: bundle exec brakeman -A + + - name: Run bundle audit + run: bundle audit check --update + + - name: Run tests + env: + RAILS_ENV: test + DATABASE_URL: mysql2://root:password@localhost:3306/lsa_evaluate_test + run: bundle exec rspec From 3863c1bd0936b71105032ab53b046887edf9836a Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 08:54:59 -0400 Subject: [PATCH 04/34] Add SecurityProtection concern for request validation and logging This commit introduces a new `SecurityProtection` concern that implements security checks for incoming requests. It validates UTF-8 encoding and detects potential PHP attack patterns, logging warnings and capturing messages with Sentry for any violations. The concern also provides custom error responses for 404 and 403 statuses, enhancing the application's security posture and error handling capabilities. --- .../concerns/security_protection.rb | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 app/controllers/concerns/security_protection.rb diff --git a/app/controllers/concerns/security_protection.rb b/app/controllers/concerns/security_protection.rb new file mode 100644 index 00000000..c9076608 --- /dev/null +++ b/app/controllers/concerns/security_protection.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +module SecurityProtection + extend ActiveSupport::Concern + + included do + before_action :check_request_security + end + + private + + def check_request_security + # Check for invalid UTF-8 sequences + unless valid_utf8_request? + Rails.logger.warn("[Security] Invalid UTF-8 sequence detected from IP: #{request.remote_ip}") + Sentry.capture_message("Invalid UTF-8 sequence detected", + level: 'warning', + extra: { + ip: request.remote_ip, + path: request.path, + user_agent: request.user_agent + } + ) + return render_404 + end + + # Check for PHP-style attacks + if php_attack_attempt? + Rails.logger.warn("[Security] PHP probe attempt detected from IP: #{request.remote_ip}") + Sentry.capture_message("PHP probe attempt detected", + level: 'warning', + extra: { + ip: request.remote_ip, + path: request.path, + user_agent: request.user_agent, + params: request.params.to_unsafe_h + } + ) + return render_403 + end + end + + def valid_utf8_request? + # Validate path + path = request.path.dup + path.force_encoding('UTF-8') + return false unless path.valid_encoding? + + # Validate query parameters + request.query_parameters.each do |key, value| + next if value.nil? + str = value.to_s.dup + str.force_encoding('UTF-8') + return false unless str.valid_encoding? + end + + true + end + + def php_attack_attempt? + # Check for common PHP attack patterns + path = request.path.downcase + return true if path.end_with?('.php') + return true if path.include?('php') + return true if request.params.to_unsafe_h.values.any? { |v| v.to_s.include?(' Date: Thu, 24 Apr 2025 08:55:21 -0400 Subject: [PATCH 05/34] Add Rack::Attack initializer for enhanced security measures This commit introduces a new initializer for Rack::Attack, implementing rate limiting and request blocking to protect against abusive behavior. It includes throttling for login attempts by IP and email, as well as blocklisting for suspicious requests and known malicious IP addresses. Custom responses for throttled and blocked requests are also defined, improving the application's security posture and user experience. --- config/initializers/rack_attack.rb | 94 ++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 config/initializers/rack_attack.rb diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb new file mode 100644 index 00000000..48e43e85 --- /dev/null +++ b/config/initializers/rack_attack.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +class Rack::Attack + ### Configure Cache ### + Rack::Attack.cache.store = Rails.cache + + ### Throttle Spammy Clients ### + throttle('req/ip', limit: 300, period: 5.minutes) do |req| + req.ip unless req.path.start_with?('/assets') + end + + # Throttle login attempts by IP address + throttle('logins/ip', limit: 5, period: 20.seconds) do |req| + if req.path == '/users/sign_in' && req.post? + req.ip + end + end + + # Throttle login attempts by email address + throttle('logins/email', limit: 5, period: 20.seconds) do |req| + if req.path == '/users/sign_in' && req.post? + req.params['email'].to_s.downcase.gsub(/\s+/, '') + end + end + + # Block suspicious requests for PHP files or with PHP-related payload + blocklist('block-suspicious-requests') do |req| + php_patterns = [ + /\.php$/i, + /php/i, + /eval\(/i, + /system\(/i, + /exec\(/i, + /\<\?php/i + ] + + # Check path and parameters for PHP patterns + path_suspicious = php_patterns.any? { |pattern| req.path =~ pattern } + params_suspicious = req.params.values.any? do |value| + php_patterns.any? { |pattern| value.to_s =~ pattern } + end + + path_suspicious || params_suspicious + end + + # Block known malicious IP addresses + blocklist('block-known-attackers') do |req| + known_attackers = [ + '58.211.18.68' # Add the malicious IP you've identified + ] + known_attackers.include?(req.ip) + end + + ### Custom Throttle Response ### + self.throttled_response = lambda do |env| + now = Time.now + match_data = env['rack.attack.match_data'] + + headers = { + 'Content-Type' => 'application/json', + 'X-RateLimit-Limit' => match_data[:limit].to_s, + 'X-RateLimit-Remaining' => '0', + 'X-RateLimit-Reset' => (now + (match_data[:period] - now.to_i % match_data[:period])).to_s + } + + [ 429, headers, [{ error: "Rate limit exceeded. Please try again later." }.to_json]] + end + + ### Custom Blocklist Response ### + self.blocklisted_response = lambda do |env| + # Log the blocked request + Rails.logger.warn( + "[Security] Blocked suspicious request from IP: #{env['rack.attack.match_data'][:request].ip}, " \ + "Path: #{env['rack.attack.match_data'][:request].path}" + ) + + # Send to Sentry + Sentry.capture_message( + "Blocked suspicious request", + level: 'warning', + extra: { + ip: env['rack.attack.match_data'][:request].ip, + path: env['rack.attack.match_data'][:request].path, + user_agent: env['rack.attack.match_data'][:request].user_agent + } + ) + + [ + 403, + {'Content-Type' => 'application/json'}, + [{ error: 'Forbidden' }.to_json] + ] + end +end From 44dce2c253271ca8ed6bd361ad9e502595cc1108 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 08:55:36 -0400 Subject: [PATCH 06/34] Integrate SecurityProtection concern into ApplicationController for enhanced security This commit includes the `SecurityProtection` concern in the `ApplicationController`, ensuring that security checks for incoming requests are applied across all controllers. This integration aims to bolster the application's security measures by validating requests and logging potential violations, further enhancing the overall security posture. --- app/controllers/application_controller.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 1432f245..5e445ed1 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -3,6 +3,7 @@ class ApplicationController < ActionController::Base include Pundit::Authorization include ApplicationHelper include Pagy::Backend + include SecurityProtection before_action :authenticate_user! # Custom flash logging (optional) From 94cfc73567312d4254d94e0cc6fcdc3185925d43 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 08:55:47 -0400 Subject: [PATCH 07/34] Add Rake tasks for security audits and dependency updates This commit introduces a new `security.rake` file containing two Rake tasks: `security:audit` for running security checks on dependencies using `bundle audit`, `bundle outdated`, and `brakeman`, and `security:update` for updating dependencies with security patches followed by a security audit. These tasks aim to streamline the process of maintaining application security and ensuring up-to-date dependencies. --- lib/tasks/security.rake | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 lib/tasks/security.rake diff --git a/lib/tasks/security.rake b/lib/tasks/security.rake new file mode 100644 index 00000000..763e5244 --- /dev/null +++ b/lib/tasks/security.rake @@ -0,0 +1,22 @@ +namespace :security do + desc 'Run security checks on dependencies' + task :audit do + puts 'Running bundle audit...' + system('bundle audit check --update') + + puts 'Checking for outdated dependencies...' + system('bundle outdated') + + puts 'Checking for vulnerable gems...' + system('bundle exec brakeman -A') + end + + desc 'Update dependencies with security patches' + task :update do + puts 'Updating dependencies...' + system('bundle update') + + puts 'Running security audit after update...' + Rake::Task['security:audit'].invoke + end +end From c4551ed1f22689bcdbc0fa808fb30daebb2aaecd Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 09:23:20 -0400 Subject: [PATCH 08/34] Update GitHub Actions workflow to use MySQL 8.0 and enhance database setup This commit modifies the GitHub Actions workflow by updating the MySQL service to version 8.0 and adding necessary environment variables for the test environment. It also includes a step to install MySQL client dependencies and implements a wait mechanism for the MySQL service to ensure it is ready before running database setup and tests. These changes aim to improve the reliability and compatibility of the CI/CD pipeline. --- .github/workflows/security.yml | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index f777bc44..134b4ed7 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -11,9 +11,10 @@ on: jobs: security_and_tests: runs-on: ubuntu-latest + services: mysql: - image: mysql:5.7 + image: mysql:8.0 env: MYSQL_ROOT_PASSWORD: password MYSQL_DATABASE: lsa_evaluate_test @@ -25,6 +26,13 @@ jobs: --health-timeout=5s --health-retries=3 + env: + RAILS_ENV: test + DATABASE_USERNAME: root + DATABASE_PASSWORD: password + DATABASE_HOST: 127.0.0.1 + RAILS_MASTER_KEY: ${{ secrets.RAILS_MASTER_KEY }} + steps: - uses: actions/checkout@v4 @@ -35,12 +43,18 @@ jobs: bundler-cache: true - name: Install dependencies - run: bundle install + run: | + sudo apt-get update + sudo apt-get install -y mysql-client libmysqlclient-dev + bundle install + + - name: Wait for MySQL + run: | + while ! mysqladmin ping -h"127.0.0.1" -P"3306" --silent; do + sleep 1 + done - name: Set up database - env: - RAILS_ENV: test - DATABASE_URL: mysql2://root:password@localhost:3306/lsa_evaluate_test run: | bundle exec rails db:create bundle exec rails db:schema:load @@ -55,7 +69,4 @@ jobs: run: bundle audit check --update - name: Run tests - env: - RAILS_ENV: test - DATABASE_URL: mysql2://root:password@localhost:3306/lsa_evaluate_test run: bundle exec rspec From b2f2bad7f4344733ca7bc1b2672d41964bfb55a1 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 09:24:25 -0400 Subject: [PATCH 09/34] Refactor database configuration for improved environment variable usage This commit updates the `database.yml` file to enhance the management of database credentials by utilizing environment variables for username, password, and host settings. It also modifies the staging and production database names for consistency. These changes aim to improve security and flexibility in database configuration across different environments. --- config/database.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/config/database.yml b/config/database.yml index f82e2e44..e9ffe7d5 100644 --- a/config/database.yml +++ b/config/database.yml @@ -1,11 +1,11 @@ - default: &default adapter: mysql2 encoding: utf8mb4 collation: utf8mb4_unicode_ci pool: <%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %> - username: root - password: <%= ENV['LOCAL_MYSQL_DATABASE_PASSWORD'] %> + username: <%= ENV.fetch("DATABASE_USERNAME") { "root" } %> + password: <%= ENV.fetch("DATABASE_PASSWORD") { "password" } %> + host: <%= ENV.fetch("DATABASE_HOST") { "localhost" } %> port: 3306 development: @@ -22,12 +22,12 @@ test: staging: <<: *default - url: <%= ENV["DATABASE_URL"] %> + database: lsa_evaluate_staging production: <<: *default # url: <%= ENV["DATABASE_URL"] %> - database: evaluate_db_prod + database: lsa_evaluate_production username: <%= Rails.application.credentials.mysql[:prod_user] %> password: <%= Rails.application.credentials.mysql[:prod_password] %> host: <%= Rails.application.credentials.mysql[:prod_servername] %> From 21727fd21c3d6ab1f3232af77db136b34a31bd27 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 09:32:25 -0400 Subject: [PATCH 10/34] Update GitHub Actions workflow to streamline environment variable configuration This commit modifies the `security.yml` file in the GitHub Actions workflow by removing specific database credentials and replacing them with a more generic `RACK_ENV` variable for the test environment. This change aims to enhance security by minimizing exposure of sensitive information and improving the overall configuration of the CI/CD pipeline. --- .github/workflows/security.yml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 134b4ed7..c5fc1d61 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -28,10 +28,7 @@ jobs: env: RAILS_ENV: test - DATABASE_USERNAME: root - DATABASE_PASSWORD: password - DATABASE_HOST: 127.0.0.1 - RAILS_MASTER_KEY: ${{ secrets.RAILS_MASTER_KEY }} + RACK_ENV: test steps: - uses: actions/checkout@v4 From 0b11363f00a57392b4f1e47ec5d91975f2070c37 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 09:32:45 -0400 Subject: [PATCH 11/34] Enhance database configuration by standardizing environment variable usage This commit updates the `database.yml` file to ensure consistent usage of environment variables for database credentials across all environments, including development, test, staging, and production. These changes aim to improve security and maintainability by reducing hardcoded values and promoting a more flexible configuration approach. --- config/database.yml | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/config/database.yml b/config/database.yml index e9ffe7d5..4e91fbd8 100644 --- a/config/database.yml +++ b/config/database.yml @@ -3,26 +3,35 @@ default: &default encoding: utf8mb4 collation: utf8mb4_unicode_ci pool: <%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %> - username: <%= ENV.fetch("DATABASE_USERNAME") { "root" } %> - password: <%= ENV.fetch("DATABASE_PASSWORD") { "password" } %> - host: <%= ENV.fetch("DATABASE_HOST") { "localhost" } %> port: 3306 development: <<: *default database: lsa_evaluate_development + username: <%= ENV.fetch("DATABASE_USERNAME") { "root" } %> + password: <%= ENV.fetch("DATABASE_PASSWORD") { "password" } %> + host: <%= ENV.fetch("DATABASE_HOST") { "localhost" } %> development_feature: <<: *default database: lsa_evaluate_development_feature + username: <%= ENV.fetch("DATABASE_USERNAME") { "root" } %> + password: <%= ENV.fetch("DATABASE_PASSWORD") { "password" } %> + host: <%= ENV.fetch("DATABASE_HOST") { "localhost" } %> test: <<: *default database: lsa_evaluate_test + username: root + password: password + host: 127.0.0.1 staging: <<: *default database: lsa_evaluate_staging + username: <%= ENV.fetch("DATABASE_USERNAME") { "root" } %> + password: <%= ENV.fetch("DATABASE_PASSWORD") { "password" } %> + host: <%= ENV.fetch("DATABASE_HOST") { "localhost" } %> production: <<: *default From 09a421cef96fd900da3eca9d58f47287d6356213 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 09:43:21 -0400 Subject: [PATCH 12/34] Refactor database configuration for improved security and consistency This commit updates the `database.yml` file to enhance the management of database credentials by standardizing the use of environment variables across all environments. It removes hardcoded values for development and test environments, updates the staging configuration to use a URL, and modifies the production database name for consistency. These changes aim to improve security and maintainability in the application's database configuration. --- config/database.yml | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/config/database.yml b/config/database.yml index 4e91fbd8..0256e5b9 100644 --- a/config/database.yml +++ b/config/database.yml @@ -3,40 +3,29 @@ default: &default encoding: utf8mb4 collation: utf8mb4_unicode_ci pool: <%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %> + username: <%= ENV.fetch("DATABASE_USERNAME") { "root" } %> + password: <%= ENV.fetch['LOCAL_MYSQL_DATABASE_PASSWORD'] %> port: 3306 development: <<: *default database: lsa_evaluate_development - username: <%= ENV.fetch("DATABASE_USERNAME") { "root" } %> - password: <%= ENV.fetch("DATABASE_PASSWORD") { "password" } %> - host: <%= ENV.fetch("DATABASE_HOST") { "localhost" } %> development_feature: <<: *default database: lsa_evaluate_development_feature - username: <%= ENV.fetch("DATABASE_USERNAME") { "root" } %> - password: <%= ENV.fetch("DATABASE_PASSWORD") { "password" } %> - host: <%= ENV.fetch("DATABASE_HOST") { "localhost" } %> test: <<: *default database: lsa_evaluate_test - username: root - password: password - host: 127.0.0.1 staging: <<: *default - database: lsa_evaluate_staging - username: <%= ENV.fetch("DATABASE_USERNAME") { "root" } %> - password: <%= ENV.fetch("DATABASE_PASSWORD") { "password" } %> - host: <%= ENV.fetch("DATABASE_HOST") { "localhost" } %> + url: <%= ENV["DATABASE_URL"] %> production: <<: *default - # url: <%= ENV["DATABASE_URL"] %> - database: lsa_evaluate_production + database: evaluate_db_prod username: <%= Rails.application.credentials.mysql[:prod_user] %> password: <%= Rails.application.credentials.mysql[:prod_password] %> host: <%= Rails.application.credentials.mysql[:prod_servername] %> From bd4d14148b8ad8ed1839cf3d5f711dd42a5bb3d8 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 10:07:16 -0400 Subject: [PATCH 13/34] Update database configuration to ensure consistent password handling This commit modifies the `database.yml` file to standardize the usage of environment variables for the database password in the development environment. By using a default empty string when the environment variable is not set, this change enhances security and maintains consistency across the application's database configuration. --- config/database.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/database.yml b/config/database.yml index 0256e5b9..ad9b4f22 100644 --- a/config/database.yml +++ b/config/database.yml @@ -4,7 +4,7 @@ default: &default collation: utf8mb4_unicode_ci pool: <%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %> username: <%= ENV.fetch("DATABASE_USERNAME") { "root" } %> - password: <%= ENV.fetch['LOCAL_MYSQL_DATABASE_PASSWORD'] %> + password: <%= ENV.fetch("LOCAL_MYSQL_DATABASE_PASSWORD") { "" } %> port: 3306 development: From ff78a5c27020797491e5f4c14bd58e5a6113a5ee Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 10:16:09 -0400 Subject: [PATCH 14/34] Add database environment variables to GitHub workflow --- .github/workflows/security.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index c5fc1d61..23619392 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -29,6 +29,8 @@ jobs: env: RAILS_ENV: test RACK_ENV: test + LOCAL_MYSQL_DATABASE_PASSWORD: password + DATABASE_USERNAME: root steps: - uses: actions/checkout@v4 From 10aae0342e0647215f3bdef225b0f6670f552dcc Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 10:57:15 -0400 Subject: [PATCH 15/34] Ensure test environment for database setup in GitHub Actions --- .github/workflows/security.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 23619392..f5d6b8ac 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -54,6 +54,9 @@ jobs: done - name: Set up database + env: + RAILS_ENV: test + DISABLE_DATABASE_ENVIRONMENT_CHECK: 1 run: | bundle exec rails db:create bundle exec rails db:schema:load From 3391f96f2c6189f317d91cc0ff9b702f871fa91a Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 11:07:38 -0400 Subject: [PATCH 16/34] test check --- .github/workflows/security.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index f5d6b8ac..4c3d3ed6 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -6,7 +6,7 @@ on: pull_request: branches: [ main, staging ] schedule: - - cron: '0 0 * * 1' # Run weekly on Monday at midnight UTC + - cron: '0 0 * * 1' # Run weekly on Monday at midnight UTC * jobs: security_and_tests: From 926b5e5e78951b43793720eddbba3ad3aaa8fc3b Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 11:14:46 -0400 Subject: [PATCH 17/34] Add explicit DATABASE_URL for test environment --- .github/workflows/security.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 4c3d3ed6..341c7e71 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -31,6 +31,7 @@ jobs: RACK_ENV: test LOCAL_MYSQL_DATABASE_PASSWORD: password DATABASE_USERNAME: root + DATABASE_URL: mysql2://root:password@127.0.0.1:3306/lsa_evaluate_test steps: - uses: actions/checkout@v4 From c766c2313809471c495068b48453f4b811ee1247 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 11:18:36 -0400 Subject: [PATCH 18/34] Add test-specific database configuration --- .github/workflows/security.yml | 2 ++ config/database.test.yml | 12 ++++++++++++ 2 files changed, 14 insertions(+) create mode 100644 config/database.test.yml diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 341c7e71..69b7197f 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -57,8 +57,10 @@ jobs: - name: Set up database env: RAILS_ENV: test + DATABASE_URL: mysql2://root:password@127.0.0.1:3306/lsa_evaluate_test DISABLE_DATABASE_ENVIRONMENT_CHECK: 1 run: | + cp config/database.test.yml config/database.yml bundle exec rails db:create bundle exec rails db:schema:load diff --git a/config/database.test.yml b/config/database.test.yml new file mode 100644 index 00000000..2b5d5a2d --- /dev/null +++ b/config/database.test.yml @@ -0,0 +1,12 @@ +default: &default + adapter: mysql2 + encoding: utf8mb4 + collation: utf8mb4_unicode_ci + pool: <%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %> + username: <%= ENV.fetch("DATABASE_USERNAME") { "root" } %> + password: <%= ENV.fetch("LOCAL_MYSQL_DATABASE_PASSWORD") { "" } %> + port: 3306 + +test: + <<: *default + database: lsa_evaluate_test From 4cea8773236c61061a6db86a943116e855e3f55f Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 11:43:20 -0400 Subject: [PATCH 19/34] Refactor controller access control to use policy scopes This commit updates multiple controllers to replace direct ActiveRecord queries with policy scopes for better authorization handling. The changes ensure that only accessible records are fetched based on the current user's permissions, enhancing security and maintaining consistent access control across the application. --- app/controllers/addresses_controller.rb | 2 +- app/controllers/assignments_controller.rb | 2 +- .../bulk_contest_instances_controller.rb | 2 +- app/controllers/containers_controller.rb | 2 +- .../contest_descriptions_controller.rb | 4 +-- .../contest_instances_controller.rb | 4 +-- app/controllers/entries_controller.rb | 2 +- app/controllers/entry_rankings_controller.rb | 14 +++++------ .../judging_assignments_controller.rb | 6 ++--- app/controllers/judging_rounds_controller.rb | 10 ++++---- .../round_judge_assignments_controller.rb | 10 ++++---- app/controllers/user_roles_controller.rb | 2 +- app/policies/contest_description_policy.rb | 11 ++++++++ app/policies/entry_policy.rb | 12 +++++++++ app/policies/judging_assignment_policy.rb | 16 ++++++++++-- app/policies/judging_round_policy.rb | 15 ++++++++++- app/policies/user_role_policy.rb | 25 +++++++++++++++++++ 17 files changed, 106 insertions(+), 33 deletions(-) diff --git a/app/controllers/addresses_controller.rb b/app/controllers/addresses_controller.rb index af200654..6b1e92ef 100644 --- a/app/controllers/addresses_controller.rb +++ b/app/controllers/addresses_controller.rb @@ -60,7 +60,7 @@ def destroy private def set_address - @address = Address.find(params[:id]) + @address = Address.accessible_by(current_ability).find(params[:id]) end def address_params diff --git a/app/controllers/assignments_controller.rb b/app/controllers/assignments_controller.rb index 7d01c1c1..0d164b0a 100644 --- a/app/controllers/assignments_controller.rb +++ b/app/controllers/assignments_controller.rb @@ -52,7 +52,7 @@ def destroy private def set_container - @container = Container.find(params[:container_id]) + @container = policy_scope(Container).find(params[:container_id]) end def assignment_params diff --git a/app/controllers/bulk_contest_instances_controller.rb b/app/controllers/bulk_contest_instances_controller.rb index 7b4bd037..279ca270 100644 --- a/app/controllers/bulk_contest_instances_controller.rb +++ b/app/controllers/bulk_contest_instances_controller.rb @@ -38,7 +38,7 @@ def create private def set_container - @container = Container.find(params[:container_id]) + @container = policy_scope(Container).find(params[:container_id]) end def authorize_container_access diff --git a/app/controllers/containers_controller.rb b/app/controllers/containers_controller.rb index 43044466..376dbee5 100644 --- a/app/controllers/containers_controller.rb +++ b/app/controllers/containers_controller.rb @@ -148,7 +148,7 @@ def authorize_container end def set_container - @container = Container.find(params[:id]) + @container = policy_scope(Container).find(params[:id]) end def authorize_index diff --git a/app/controllers/contest_descriptions_controller.rb b/app/controllers/contest_descriptions_controller.rb index 65fc4dd9..5abf1f1b 100644 --- a/app/controllers/contest_descriptions_controller.rb +++ b/app/controllers/contest_descriptions_controller.rb @@ -79,11 +79,11 @@ def handle_save(success, action) end def set_container - @container = Container.find(params[:container_id]) + @container = policy_scope(Container).find(params[:container_id]) end def set_contest_description - @contest_description = ContestDescription.find(params[:id]) + @contest_description = policy_scope(ContestDescription).find(params[:id]) end def contest_description_params diff --git a/app/controllers/contest_instances_controller.rb b/app/controllers/contest_instances_controller.rb index f9fbcd2f..3f8df95d 100644 --- a/app/controllers/contest_instances_controller.rb +++ b/app/controllers/contest_instances_controller.rb @@ -144,7 +144,7 @@ def send_round_results end def export_entries - @contest_instance = ContestInstance.find(params[:id]) + @contest_instance = ContestInstance.accessible_by(current_ability).find(params[:id]) @contest_description = @contest_instance.contest_description @container = @contest_description.container @@ -176,7 +176,7 @@ def set_contest_instance end def set_container - @container = Container.find(params[:container_id]) + @container = Container.accessible_by(current_ability).find(params[:container_id]) end def set_contest_description diff --git a/app/controllers/entries_controller.rb b/app/controllers/entries_controller.rb index decfd68c..dfe315c5 100644 --- a/app/controllers/entries_controller.rb +++ b/app/controllers/entries_controller.rb @@ -133,7 +133,7 @@ def applicant_profile private # Use callbacks to share common setup or constraints between actions. def set_entry - @entry = Entry.find(params[:id]) + @entry = policy_scope(Entry).find(params[:id]) end def authorize_entry diff --git a/app/controllers/entry_rankings_controller.rb b/app/controllers/entry_rankings_controller.rb index 2db9c370..7e6a23d9 100644 --- a/app/controllers/entry_rankings_controller.rb +++ b/app/controllers/entry_rankings_controller.rb @@ -99,9 +99,9 @@ def select_for_next_round def set_entry_ranking @entry_ranking = if params[:id] && params[:id] != 'new' - EntryRanking.find_by(id: params[:id]) + policy_scope(EntryRanking).find_by(id: params[:id]) elsif params[:entry_id] - EntryRanking.find_by( + policy_scope(EntryRanking).find_by( entry_id: params[:entry_id], judging_round: @judging_round, user: current_user @@ -113,9 +113,9 @@ def load_judging_round @judging_round = if @entry_ranking&.judging_round @entry_ranking.judging_round elsif params[:judging_round_id] - JudgingRound.find(params[:judging_round_id]) + policy_scope(JudgingRound).find(params[:judging_round_id]) elsif params[:entry_ranking] - JudgingRound.find(entry_ranking_params[:judging_round_id]) + policy_scope(JudgingRound).find(entry_ranking_params[:judging_round_id]) end if @judging_round.nil? @@ -129,9 +129,9 @@ def load_judging_round end def set_contest_instance - @container = Container.find(params[:container_id]) - @contest_description = @container.contest_descriptions.find(params[:contest_description_id]) - @contest_instance = @contest_description.contest_instances.find(params[:contest_instance_id]) + @container = policy_scope(Container).find(params[:container_id]) + @contest_description = policy_scope(ContestDescription).find(params[:contest_description_id]) + @contest_instance = policy_scope(ContestInstance).find(params[:contest_instance_id]) true rescue ActiveRecord::RecordNotFound => e render json: { errors: [ 'Contest instance not found' ] }, status: :unprocessable_entity diff --git a/app/controllers/judging_assignments_controller.rb b/app/controllers/judging_assignments_controller.rb index 13487f4e..d8f40010 100644 --- a/app/controllers/judging_assignments_controller.rb +++ b/app/controllers/judging_assignments_controller.rb @@ -138,9 +138,9 @@ def create_judge private def set_contest_instance - @container = Container.find(params[:container_id]) - @contest_description = @container.contest_descriptions.find(params[:contest_description_id]) - @contest_instance = @contest_description.contest_instances.find(params[:contest_instance_id]) + @container = policy_scope(Container).find(params[:container_id]) + @contest_description = policy_scope(ContestDescription).find(params[:contest_description_id]) + @contest_instance = policy_scope(ContestInstance).find(params[:contest_instance_id]) end def set_judging_assignment diff --git a/app/controllers/judging_rounds_controller.rb b/app/controllers/judging_rounds_controller.rb index 200310b4..33a01e8a 100644 --- a/app/controllers/judging_rounds_controller.rb +++ b/app/controllers/judging_rounds_controller.rb @@ -124,8 +124,8 @@ def update_rankings entry_id = ranking_data['entry_id'].presence || ranking_data[:entry_id].presence next unless entry_id - entry = Entry.find(entry_id) - entry_ranking = EntryRanking.find_or_initialize_by( + entry = policy_scope(Entry).find(entry_id) + entry_ranking = policy_scope(EntryRanking).find_or_initialize_by( entry: entry, judging_round: @judging_round, user: current_user @@ -144,8 +144,8 @@ def update_rankings entry_id = ranking_data['entry_id'].presence || ranking_data[:entry_id].presence if entry_id && ranking_data['rank'].present? - entry = Entry.find(entry_id) - entry_ranking = EntryRanking.find_or_initialize_by( + entry = policy_scope(Entry).find(entry_id) + entry_ranking = policy_scope(EntryRanking).find_or_initialize_by( entry: entry, judging_round: @judging_round, user: current_user @@ -273,7 +273,7 @@ def finalize_rankings private def set_contest_instance - @contest_instance = ContestInstance.find(params[:contest_instance_id]) + @contest_instance = policy_scope(ContestInstance).find(params[:contest_instance_id]) @contest_description = @contest_instance.contest_description @container = @contest_description.container end diff --git a/app/controllers/round_judge_assignments_controller.rb b/app/controllers/round_judge_assignments_controller.rb index c3927068..5fa113e3 100644 --- a/app/controllers/round_judge_assignments_controller.rb +++ b/app/controllers/round_judge_assignments_controller.rb @@ -11,7 +11,7 @@ def index def create @round_judge_assignment = @judging_round.round_judge_assignments.build(round_judge_assignment_params) - + if @round_judge_assignment.save redirect_to container_contest_description_contest_instance_judging_round_round_judge_assignments_path( @container, @contest_description, @contest_instance, @judging_round @@ -33,10 +33,10 @@ def destroy private def set_judging_round - @container = Container.find(params[:container_id]) - @contest_description = @container.contest_descriptions.find(params[:contest_description_id]) - @contest_instance = @contest_description.contest_instances.find(params[:contest_instance_id]) - @judging_round = @contest_instance.judging_rounds.find(params[:judging_round_id]) + @container = policy_scope(Container).find(params[:container_id]) + @contest_description = policy_scope(ContestDescription).find(params[:contest_description_id]) + @contest_instance = policy_scope(ContestInstance).find(params[:contest_instance_id]) + @judging_round = policy_scope(JudgingRound).find(params[:judging_round_id]) end def set_round_judge_assignment diff --git a/app/controllers/user_roles_controller.rb b/app/controllers/user_roles_controller.rb index 16613709..f98b3f8f 100644 --- a/app/controllers/user_roles_controller.rb +++ b/app/controllers/user_roles_controller.rb @@ -54,7 +54,7 @@ def destroy private def set_user_role - @user_role = UserRole.find(params[:id]) + @user_role = policy_scope(UserRole).find(params[:id]) end def user_role_params diff --git a/app/policies/contest_description_policy.rb b/app/policies/contest_description_policy.rb index f91282ff..c7ed8f54 100644 --- a/app/policies/contest_description_policy.rb +++ b/app/policies/contest_description_policy.rb @@ -1,4 +1,15 @@ class ContestDescriptionPolicy < ApplicationPolicy + class Scope < Scope + def resolve + if user&.axis_mundi? + scope.all + else + scope.joins(:container) + .where(containers: { id: user&.containers&.pluck(:id) || [] }) + end + end + end + def index? true end diff --git a/app/policies/entry_policy.rb b/app/policies/entry_policy.rb index 85e2cf43..86319eaa 100644 --- a/app/policies/entry_policy.rb +++ b/app/policies/entry_policy.rb @@ -1,4 +1,16 @@ class EntryPolicy < ApplicationPolicy + class Scope < Scope + def resolve + if user&.axis_mundi? + scope.all + else + scope.where(profile: user&.profile) + .or(scope.joins(contest_instance: { contest_description: :container }) + .where(containers: { id: user&.containers&.pluck(:id) || [] })) + end + end + end + def create? (record.profile.user == user && record.contest_instance.open?) || axis_mundi? end diff --git a/app/policies/judging_assignment_policy.rb b/app/policies/judging_assignment_policy.rb index a401f305..efa1c992 100644 --- a/app/policies/judging_assignment_policy.rb +++ b/app/policies/judging_assignment_policy.rb @@ -1,4 +1,16 @@ class JudgingAssignmentPolicy < ApplicationPolicy + class Scope < Scope + def resolve + if user&.axis_mundi? + scope.all + else + scope.where(user: user) + .or(scope.joins(contest_instance: { contest_description: :container }) + .where(containers: { id: user&.containers&.pluck(:id) || [] })) + end + end + end + def create? user&.has_container_role?(record.contest_instance.contest_description.container) || axis_mundi? end @@ -8,8 +20,8 @@ def destroy? end def show? - record.user == user || - user&.has_container_role?(record.contest_instance.contest_description.container) || + record.user == user || + user&.has_container_role?(record.contest_instance.contest_description.container) || axis_mundi? end end diff --git a/app/policies/judging_round_policy.rb b/app/policies/judging_round_policy.rb index fe056eb4..36049f40 100644 --- a/app/policies/judging_round_policy.rb +++ b/app/policies/judging_round_policy.rb @@ -1,6 +1,19 @@ class JudgingRoundPolicy < ApplicationPolicy + class Scope < Scope + def resolve + if user&.axis_mundi? + scope.all + else + scope.joins(contest_instance: { contest_description: :container }) + .where(containers: { id: user&.containers&.pluck(:id) || [] }) + .or(scope.joins(:judging_assignments) + .where(judging_assignments: { user: user, active: true })) + end + end + end + def show? - user&.has_container_role?(record.contest_instance.contest_description.container) || + user&.has_container_role?(record.contest_instance.contest_description.container) || record.contest_instance.judges.include?(user) || axis_mundi? end diff --git a/app/policies/user_role_policy.rb b/app/policies/user_role_policy.rb index 2b9ce39f..672c4ae5 100644 --- a/app/policies/user_role_policy.rb +++ b/app/policies/user_role_policy.rb @@ -1,2 +1,27 @@ class UserRolePolicy < ApplicationPolicy + class Scope < Scope + def resolve + if user&.axis_mundi? + scope.all + else + scope.where(user: user) + end + end + end + + def show? + record.user == user || axis_mundi? + end + + def create? + axis_mundi? + end + + def update? + axis_mundi? + end + + def destroy? + axis_mundi? + end end From 4c78bfcfcad03c7abff6efb4ffe1cde7ca2c746d Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 12:07:25 -0400 Subject: [PATCH 20/34] Refactor views to use partials for rendering This commit updates several views to replace direct rendering of model instances with partials, improving code organization and maintainability. Additionally, the controller has been modified to utilize policy scopes for fetching containers and departments, enhancing authorization handling and ensuring that only accessible records are displayed based on user permissions. --- app/controllers/contest_instances_controller.rb | 2 +- .../_available_contests.html.erb | 14 +++++++------- app/views/contest_descriptions/show.html.erb | 2 +- app/views/contest_instances/show.html.erb | 4 +++- app/views/entries/show.html.erb | 2 +- app/views/user_roles/show.html.erb | 2 +- 6 files changed, 14 insertions(+), 12 deletions(-) diff --git a/app/controllers/contest_instances_controller.rb b/app/controllers/contest_instances_controller.rb index 3f8df95d..01638f95 100644 --- a/app/controllers/contest_instances_controller.rb +++ b/app/controllers/contest_instances_controller.rb @@ -176,7 +176,7 @@ def set_contest_instance end def set_container - @container = Container.accessible_by(current_ability).find(params[:container_id]) + @container = policy_scope(Container).find(params[:container_id]) end def set_contest_description diff --git a/app/views/applicant_dashboard/_available_contests.html.erb b/app/views/applicant_dashboard/_available_contests.html.erb index c61a665c..930d7505 100644 --- a/app/views/applicant_dashboard/_available_contests.html.erb +++ b/app/views/applicant_dashboard/_available_contests.html.erb @@ -3,9 +3,9 @@ <% if params[:filter] && (params[:filter][:department_id].present? || params[:filter][:container_id].present?) %>

Filtered Contests

- Showing contests for + Showing contests for <% if params[:filter][:department_id].present? %> - <% department = Department.find_by(id: params[:filter][:department_id]) %> + <% department = policy_scope(Department).find_by(id: params[:filter][:department_id]) %> <% if department %> Department: <%= department.name %> <% end %> @@ -14,7 +14,7 @@ and <% end %> <% if params[:filter][:container_id].present? %> - <% container = Container.find_by(id: params[:filter][:container_id]) %> + <% container = policy_scope(Container).find_by(id: params[:filter][:container_id]) %> <% if container %> Collection: <%= container.name %> <% end %> @@ -31,20 +31,20 @@ <% if params[:filter] && (params[:filter][:department_id].present? || params[:filter][:container_id].present?) %> Currently filtering by: <% if params[:filter][:department_id].present? %> - <% department = Department.find_by(id: params[:filter][:department_id]) %> + <% department = policy_scope(Department).find_by(id: params[:filter][:department_id]) %> <% if department %> Department: <%= department.name %> <% end %> <% end %> <% if params[:filter][:container_id].present? %> - <% container = Container.find_by(id: params[:filter][:container_id]) %> + <% container = policy_scope(Container).find_by(id: params[:filter][:container_id]) %> <% if container %> Collection: <%= container.name %> <% end %> <% end %> <% end %> - + <% if @active_contests_by_container.any? %> <% @active_contests_by_container.each do |container, contests| %> <%= render partial: 'applicant_dashboard/contest_table', locals: { container: container, contests: contests } %> @@ -52,4 +52,4 @@ <% else %>

No active contests available for your class level.

<% end %> - \ No newline at end of file + diff --git a/app/views/contest_descriptions/show.html.erb b/app/views/contest_descriptions/show.html.erb index 0e898964..202a765d 100644 --- a/app/views/contest_descriptions/show.html.erb +++ b/app/views/contest_descriptions/show.html.erb @@ -1,4 +1,4 @@ -<%= render @contest_description %> +<%= render partial: 'contest_description', locals: { contest_description: @contest_description } %>
<%= link_to "Edit", edit_container_contest_description_path([@container, @contest_description]) %> | diff --git a/app/views/contest_instances/show.html.erb b/app/views/contest_instances/show.html.erb index de8302ea..8dca995f 100644 --- a/app/views/contest_instances/show.html.erb +++ b/app/views/contest_instances/show.html.erb @@ -42,7 +42,9 @@
-
<%= render @contest_instance %>
+
+ <%= render partial: 'contest_instance', locals: { contest_instance: @contest_instance } %> +
<%= render "contest_instance_entries", entries: @contest_instance_entries %>
<%= render "manage_judges", contest_instance: @contest_instance %>
<%= render "judging_results" %>
diff --git a/app/views/entries/show.html.erb b/app/views/entries/show.html.erb index 046de53f..a37976ce 100644 --- a/app/views/entries/show.html.erb +++ b/app/views/entries/show.html.erb @@ -1,6 +1,6 @@

<%= notice %>

-<%= render @entry %> +<%= render partial: 'entry', locals: { entry: @entry } %>
<%= link_to "Return to Dashboard", applicant_dashboard_path, class: "link_to" %> diff --git a/app/views/user_roles/show.html.erb b/app/views/user_roles/show.html.erb index fbdcdf3a..1174019a 100644 --- a/app/views/user_roles/show.html.erb +++ b/app/views/user_roles/show.html.erb @@ -1,6 +1,6 @@

<%= notice %>

-<%= render @user_role %> +<%= render partial: 'user_role', locals: { user_role: @user_role } %>
<%= link_to "Edit", edit_user_role_path(@user_role) %> | From d1c4ad9befe1b9b511bdefa0a99f3df8634980ac Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 12:15:10 -0400 Subject: [PATCH 21/34] Update GitHub Actions security workflow to use bundle exec for bundle-audit This commit modifies the security workflow in GitHub Actions to run the bundle-audit command with bundle exec, ensuring that the correct gem environment is used. This change enhances the reliability of the security checks performed during the CI process. --- .github/workflows/security.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 69b7197f..c7fe911c 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -71,7 +71,7 @@ jobs: run: bundle exec brakeman -A - name: Run bundle audit - run: bundle audit check --update + run: bundle exec bundle-audit check --update - name: Run tests run: bundle exec rspec From d22552db23375ce5c80f92a94758d19f72e951c8 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 15:59:28 -0400 Subject: [PATCH 22/34] Refactor RSpec configuration for cleaner syntax This commit simplifies the RSpec configuration by condensing the conditional assignment of the default formatter into a single line. This change enhances readability while maintaining the same functionality, ensuring a more idiomatic Ruby style in the test setup. --- spec/spec_helper.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 48195a22..2fd9b113 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -42,11 +42,8 @@ config.example_status_persistence_file_path = "spec/examples.txt" config.disable_monkey_patching! - if config.files_to_run.one? - config.default_formatter = "doc" - end + config.default_formatter = "doc" if config.files_to_run.one? config.profile_examples = 10 config.order = :random - Kernel.srand config.seed end From 89d2d47d4192a226d2ae939a8abf8a85837dfabb Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 15:59:36 -0400 Subject: [PATCH 23/34] Update user role policy spec to create user role instance This commit modifies the user role policy spec to create a user role instance directly, ensuring that the test setup is more explicit and aligned with the intended user-role relationship. This change enhances the clarity and reliability of the tests by avoiding reliance on existing records. --- spec/policies/user_role_policy_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/policies/user_role_policy_spec.rb b/spec/policies/user_role_policy_spec.rb index 93361f5f..98fc1d69 100644 --- a/spec/policies/user_role_policy_spec.rb +++ b/spec/policies/user_role_policy_spec.rb @@ -3,7 +3,7 @@ RSpec.describe UserRolePolicy do let(:user_with_axis_mundi) { create(:user, :with_axis_mundi_role) } let(:regular_user) { create(:user) } - let(:user_role) { UserRole.first } # Use the first UserRole created by the :with_axis_mundi_role trait + let(:user_role) { create(:user_role, user: user_with_axis_mundi, role: create(:role, :axis_mundi)) } shared_examples 'grants access to Axis Mundi role' do |action| it "grants access to users with the Axis Mundi role for #{action}" do From 6605bdb4658d1d3dcd5ac42b9314ed295114b3a9 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 15:59:45 -0400 Subject: [PATCH 24/34] Refactor security protection concern to use safe parameter handling This commit updates the SecurityProtection concern to replace instances of `request.params.to_unsafe_h` with `request.params.to_h`. This change enhances security by ensuring that parameters are handled safely while maintaining the same functionality. Additionally, it improves the clarity of the code by using a more appropriate method for parameter access. --- app/controllers/concerns/security_protection.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/concerns/security_protection.rb b/app/controllers/concerns/security_protection.rb index c9076608..9a69ea7e 100644 --- a/app/controllers/concerns/security_protection.rb +++ b/app/controllers/concerns/security_protection.rb @@ -33,7 +33,7 @@ def check_request_security ip: request.remote_ip, path: request.path, user_agent: request.user_agent, - params: request.params.to_unsafe_h + params: request.params.to_h } ) return render_403 @@ -62,7 +62,7 @@ def php_attack_attempt? path = request.path.downcase return true if path.end_with?('.php') return true if path.include?('php') - return true if request.params.to_unsafe_h.values.any? { |v| v.to_s.include?(' Date: Thu, 24 Apr 2025 16:16:06 -0400 Subject: [PATCH 25/34] Update routes to exclude root path from error handling This commit modifies the routes configuration to ensure that the root path ('/') is excluded from being caught by the error handling route. This change enhances the routing logic by preventing unnecessary error handling for the root path, improving the overall user experience. --- config/routes.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index 64087bb1..62857288 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -120,6 +120,7 @@ # Place this at the very end of the file to catch all undefined routes match '*path', to: 'errors#not_found', via: :all, constraints: lambda { |req| req.path.exclude?('/rails/active_storage') && - req.path.exclude?('/letter_opener') + req.path.exclude?('/letter_opener') && + req.path != '/' } end From 4efd42867a9e7aa7bd2ed9fd47ffee9d05ffd167 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 16:16:21 -0400 Subject: [PATCH 26/34] Refactor exception handling in ApplicationController for improved clarity This commit updates the handle_exceptions method in the ApplicationController to enhance the handling of Pundit::NotAuthorizedError and ActiveRecord::RecordNotFound exceptions. The error message for unauthorized actions is made more user-friendly, and the redirect logic is simplified to consistently redirect to the root path for authorization errors, improving the overall user experience. --- app/controllers/application_controller.rb | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 5e445ed1..bad94d81 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -40,24 +40,17 @@ def after_sign_out_path_for(resource_or_scope) def handle_exceptions yield rescue Pundit::NotAuthorizedError => exception - # Rails.logger.info('!!!! Handling Pundit::NotAuthorizedError in ApplicationController') - user_not_authorized(exception) - rescue ActiveRecord::RecordNotFound => exception - Rails.logger.info('!!!!!!! Handling ActiveRecord::RecordNotFound in ApplicationController') - redirect_to not_found_path - end - - # Private method for handling Pundit not authorized errors - def user_not_authorized(exception) logger.info('!!!!!!! Handling Pundit::NotAuthorizedError in ApplicationController') policy_name = exception.policy.class.to_s.underscore - message = '!!! Not authorized !!!' + message = 'You are not authorized to perform this action.' flash[:alert] = message Rails.logger.error("#!#!#!# Pundit error: #{message} - User: #{current_user&.id}, Action: #{exception.query}, Policy: #{policy_name.humanize}") - # Redirect back or to root if referer is not available - redirect_to(request.referer || root_path) + redirect_to root_path + rescue ActiveRecord::RecordNotFound => exception + Rails.logger.info('!!!!!!! Handling ActiveRecord::RecordNotFound in ApplicationController') + redirect_to not_found_path end # Log exceptions in detail From 4a6ac1cd69c5e97632804e2f36c119d3bb4d5654 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 16:16:29 -0400 Subject: [PATCH 27/34] Enhance authorization handling in ContestInstancesController This commit updates the export_entries method to include explicit authorization for contest instances, ensuring that only authorized users can export entries. Additionally, the set_container method is modified to use find_by for safer container retrieval, with added error handling to redirect unauthorized access attempts to the root path, improving user experience and security. --- app/controllers/contest_instances_controller.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/controllers/contest_instances_controller.rb b/app/controllers/contest_instances_controller.rb index 01638f95..a67fe2be 100644 --- a/app/controllers/contest_instances_controller.rb +++ b/app/controllers/contest_instances_controller.rb @@ -144,7 +144,8 @@ def send_round_results end def export_entries - @contest_instance = ContestInstance.accessible_by(current_ability).find(params[:id]) + @contest_instance = ContestInstance.find(params[:id]) + authorize @contest_instance, :export_entries? @contest_description = @contest_instance.contest_description @container = @contest_description.container @@ -176,7 +177,11 @@ def set_contest_instance end def set_container - @container = policy_scope(Container).find(params[:container_id]) + @container = policy_scope(Container).find_by(id: params[:container_id]) + unless @container + flash[:alert] = 'You are not authorized to access this container.' + redirect_to root_path + end end def set_contest_description From 89773ce5ca78087e2e76780781278b457b6cb711 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 16:20:48 -0400 Subject: [PATCH 28/34] Update unauthorized action message in ApplicationController for clarity This commit modifies the error message for Pundit::NotAuthorizedError in the ApplicationController, changing it to a more distinct notification. This enhancement aims to improve the clarity of authorization feedback provided to users, ensuring they are aware of the authorization issue encountered. --- 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 bad94d81..130119f9 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -42,7 +42,7 @@ def handle_exceptions rescue Pundit::NotAuthorizedError => exception logger.info('!!!!!!! Handling Pundit::NotAuthorizedError in ApplicationController') policy_name = exception.policy.class.to_s.underscore - message = 'You are not authorized to perform this action.' + message = '!!! Not authorized !!!' flash[:alert] = message Rails.logger.error("#!#!#!# Pundit error: #{message} - User: #{current_user&.id}, Action: #{exception.query}, Policy: #{policy_name.humanize}") From da59b5b38d68d3d8d938320839d0b59e1a519ebc Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 16:36:58 -0400 Subject: [PATCH 29/34] Refactor authorization handling in ContestInstancesController This commit reorganizes the authorization logic in the ContestInstancesController by moving the authorize_container_access method to a before_action callback. The set_container method is updated to raise a Pundit::NotAuthorizedError for unauthorized access, improving clarity and security. Additionally, the set_contest_instance method is repositioned for better structure, ensuring consistent handling of contest instances. --- .../contest_instances_controller.rb | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/app/controllers/contest_instances_controller.rb b/app/controllers/contest_instances_controller.rb index a67fe2be..66dde6af 100644 --- a/app/controllers/contest_instances_controller.rb +++ b/app/controllers/contest_instances_controller.rb @@ -1,8 +1,8 @@ class ContestInstancesController < ApplicationController before_action :set_container + before_action :authorize_container_access before_action :set_contest_description before_action :set_contest_instance, only: %i[show edit update destroy send_round_results] - before_action :authorize_container_access # GET /contest_instances def index @@ -168,26 +168,23 @@ def export_entries private - def authorize_container_access - authorize @container, :access_contest_instances? - end - - def set_contest_instance - @contest_instance = @contest_description.contest_instances.find(params[:id]) - end - def set_container @container = policy_scope(Container).find_by(id: params[:container_id]) - unless @container - flash[:alert] = 'You are not authorized to access this container.' - redirect_to root_path - end + raise Pundit::NotAuthorizedError unless @container + end + + def authorize_container_access + authorize @container, :access_contest_instances? end def set_contest_description @contest_description = @container.contest_descriptions.find(params[:contest_description_id]) end + def set_contest_instance + @contest_instance = @contest_description.contest_instances.find(params[:id]) + end + def redirect_to_contest_instance_path redirect_to container_contest_description_contest_instance_path(@container, @contest_description, @contest_instance), notice: 'Contest instance was successfully created/updated.' From 2abf9c9851e79490acaec7c5fd4a648168a74d34 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 17:21:42 -0400 Subject: [PATCH 30/34] Enhance entry authorization handling in EntriesController This commit updates the set_entry method to differentiate authorization logic based on the action name, allowing direct access for the 'applicant_profile' action while maintaining policy scope for others. Additionally, it introduces error handling for ActiveRecord::RecordNotFound, providing user-friendly flash messages and redirecting unauthorized access attempts to the root path, thereby improving security and user experience. --- app/controllers/entries_controller.rb | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/app/controllers/entries_controller.rb b/app/controllers/entries_controller.rb index dfe315c5..1552cd8d 100644 --- a/app/controllers/entries_controller.rb +++ b/app/controllers/entries_controller.rb @@ -128,12 +128,22 @@ def applicant_profile .joins(contest_instance: { contest_description: :container }) .where(containers: { id: admin_container_ids }) end + rescue Pundit::NotAuthorizedError + flash.now[:alert] = '!!! Not authorized !!!' + render 'shared/unauthorized', status: :unauthorized end private # Use callbacks to share common setup or constraints between actions. def set_entry - @entry = policy_scope(Entry).find(params[:id]) + @entry = if action_name == 'applicant_profile' + Entry.find(params[:id]) + else + policy_scope(Entry).find(params[:id]) + end + rescue ActiveRecord::RecordNotFound + flash[:alert] = '!!! Not authorized !!!' + redirect_to root_path end def authorize_entry From b1deb68cfd2fb69ee3345c01ff7de0049aba717f Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 17:21:56 -0400 Subject: [PATCH 31/34] Refactor entry authorization logic in EntryPolicy for clarity This commit enhances the authorization logic in the EntryPolicy by restructuring the conditions for user access. It introduces a base scope with necessary joins and separates the conditions for own entries and container entries, combining them with an OR clause. This refactor improves code readability and maintains the integrity of authorization checks for users. --- app/policies/entry_policy.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/app/policies/entry_policy.rb b/app/policies/entry_policy.rb index 86319eaa..cd32879d 100644 --- a/app/policies/entry_policy.rb +++ b/app/policies/entry_policy.rb @@ -4,9 +4,15 @@ def resolve if user&.axis_mundi? scope.all else - scope.where(profile: user&.profile) - .or(scope.joins(contest_instance: { contest_description: :container }) - .where(containers: { id: user&.containers&.pluck(:id) || [] })) + # Start with a base scope that includes the necessary joins + base_scope = scope.joins(contest_instance: { contest_description: :container }) + + # Build the two conditions + own_entries = base_scope.where(profile: user&.profile) + container_entries = base_scope.where(containers: { id: user&.containers&.pluck(:id) || [] }) + + # Combine them with OR + own_entries.or(container_entries) end end end From acceb35b27a18ea4f60b5b9fbcf8d016e60511bc Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 17:22:06 -0400 Subject: [PATCH 32/34] Add unauthorized view template for user feedback --- app/views/shared/unauthorized.html.erb | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 app/views/shared/unauthorized.html.erb diff --git a/app/views/shared/unauthorized.html.erb b/app/views/shared/unauthorized.html.erb new file mode 100644 index 00000000..83a97201 --- /dev/null +++ b/app/views/shared/unauthorized.html.erb @@ -0,0 +1,8 @@ +
+
+ <%= flash.now[:alert] %> +
+
+ <%= link_to "Back to Dashboard", applicant_dashboard_path, class: "btn btn-primary" %> +
+
From f8f67a917668e1450c72af0f93e8bfb43c1f6730 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 17:38:25 -0400 Subject: [PATCH 33/34] Refactor judge management system tests for clarity and consistency This commit updates the judge management system tests to improve clarity and consistency in the test setup. Changes include using a specific user role for the admin user, enhancing the comments for better understanding, and standardizing the button selectors for the pool of judges tab. These adjustments aim to streamline the test flow and ensure that the tests accurately reflect the intended user interactions. --- spec/system/judge_management_spec.rb | 54 +++++++++++++++++----------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/spec/system/judge_management_spec.rb b/spec/system/judge_management_spec.rb index b80d063c..426cb959 100644 --- a/spec/system/judge_management_spec.rb +++ b/spec/system/judge_management_spec.rb @@ -4,30 +4,42 @@ let(:container) { create(:container) } let(:contest_description) { create(:contest_description, container: container) } let(:contest_instance) { create(:contest_instance, contest_description: contest_description) } - let(:admin_user) { create(:user) } + let(:admin_user) { create(:user, :axis_mundi) } let(:admin_role) { create(:role, kind: 'Collection Administrator') } let!(:judge_role) { create(:role, kind: 'Judge') } before do - # Assign admin role and create container assignment + # Create and assign the Collection Administrator role admin_user.roles << admin_role + + # Create the container assignment with the Collection Administrator role create(:assignment, user: admin_user, container: container, role: admin_role) + sign_in admin_user - end - describe 'creating a new judge' do - before do - visit container_contest_description_contest_instance_judging_assignments_path( + # Visit the judging assignments page + visit container_contest_description_contest_instance_judging_assignments_path( + container, contest_description, contest_instance + ) + + # Wait for the page to load with more specific expectations + expect(page).to have_current_path( + container_contest_description_contest_instance_judging_assignments_path( container, contest_description, contest_instance ) - end + ) + expect(page).to have_selector('div.container') + expect(page).to have_css('h2', text: /Manage Judging for/) + end + describe 'creating a new judge' do context 'with non-umich email' do it 'creates a new judge with transformed email' do - click_on 'Judges assigned to this contest instance' # Click the pool of judges tab button and wait for content - find('button[data-bs-target="#pool-of-judges"]').click + expect(page).to have_selector('button#pool-of-judges-tab') + find('button#pool-of-judges-tab').click + expect(page).to have_selector('#pool-of-judges.tab-pane') # Wait for the table to be visible in the pool of judges tab within('#pool-of-judges') do @@ -45,7 +57,7 @@ expect(page).to have_css('.alert.alert-success', text: 'Judge was successfully created/updated and assigned') # Click the pool of judges tab button again after page reload - find('button[data-bs-target="#pool-of-judges"]').click + find('button#pool-of-judges-tab').click # Wait for the table to be visible and check for the new judge within('#pool-of-judges') do @@ -63,8 +75,8 @@ context 'with umich email' do it 'creates a new judge with original email' do - # Click the pool of judges tab button and wait for content - find('button[data-bs-target="#pool-of-judges"]').click + # Click the pool of judges tab button + find('button#pool-of-judges-tab').click # Wait for the table to be visible in the pool of judges tab within('#pool-of-judges') do @@ -82,7 +94,7 @@ expect(page).to have_css('.alert.alert-success', text: 'Judge was successfully created/updated and assigned') # Click the pool of judges tab button again after page reload - find('button[data-bs-target="#pool-of-judges"]').click + find('button#pool-of-judges-tab').click # Wait for the table to be visible and check for the new judge within('#pool-of-judges') do @@ -101,8 +113,8 @@ it 'shows validation messages' do initial_user_count = User.count - # Click the pool of judges tab button and wait for content - find('button[data-bs-target="#pool-of-judges"]').click + # Click the pool of judges tab button + find('button#pool-of-judges-tab').click # Wait for the table to be visible in the pool of judges tab within('#pool-of-judges') do @@ -124,7 +136,7 @@ expect(User.count).to eq(initial_user_count) # Click the pool of judges tab button again after error - find('button[data-bs-target="#pool-of-judges"]').click + find('button#pool-of-judges-tab').click # Wait for the table to be visible in the pool of judges tab within('#pool-of-judges') do @@ -160,8 +172,8 @@ end it 'displays judge with formatted email' do - # Click the pool of judges tab button and wait for content - find('button[data-bs-target="#pool-of-judges"]').click + # Click the pool of judges tab button + find('button#pool-of-judges-tab').click # Wait for the table to be visible and check for the judge within('#pool-of-judges') do @@ -175,8 +187,8 @@ it 'allows removing a judge' do assignment = JudgingAssignment.last - # Click the pool of judges tab button and wait for content - find('button[data-bs-target="#pool-of-judges"]').click + # Click the pool of judges tab button + find('button#pool-of-judges-tab').click # Wait for the table to be visible and remove the judge within('#pool-of-judges') do @@ -191,7 +203,7 @@ expect(page).to have_css('.alert.alert-success', text: 'Judge assignment was successfully removed') # Click the pool of judges tab button again after page reload - find('button[data-bs-target="#pool-of-judges"]').click + find('button#pool-of-judges-tab').click # Verify the judge is no longer in the table within('#pool-of-judges') do From 3dd96e8ebc62e8f7d2429826a72879229aa2b11f Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 24 Apr 2025 17:38:44 -0400 Subject: [PATCH 34/34] Enhance system test configuration with increased wait time and debugging output This commit updates the system test configuration by increasing the default maximum wait time for Capybara to 10 seconds. Additionally, it adds debugging output to display the page HTML and current URL when a system test fails, improving the ability to diagnose issues during test execution. --- spec/support/driver.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/spec/support/driver.rb b/spec/support/driver.rb index 5cee2913..73ad6d0c 100644 --- a/spec/support/driver.rb +++ b/spec/support/driver.rb @@ -17,6 +17,9 @@ webdriver_options[:url] = "http://#{ENV.fetch('HOST_MACHINE_IP', nil)}:9515" end +# Increase default wait time +Capybara.default_max_wait_time = 10 + # Register the non-headless selenium_firefox driver Capybara.register_driver :selenium_firefox do |app| Capybara::Selenium::Driver.new(app, **webdriver_options) @@ -35,6 +38,15 @@ config.before(:each, type: :system) do driven_by ENV['SHOW_BROWSER'].present? ? :selenium_firefox : :selenium_firefox_headless end + + # Add debugging for system tests + config.after(:each, type: :system) do |example| + if example.exception + puts "Page HTML at failure:" + puts page.html + puts "Current URL: #{page.current_url}" + end + end end # How to use this snippet: