fix vulnerabilities
This commit is contained in:
@@ -44,6 +44,12 @@ class Admin::InvitationsController < Admin::BaseController
|
|||||||
private
|
private
|
||||||
|
|
||||||
def invitation_params
|
def invitation_params
|
||||||
params.require(:user).permit(:email, :name, :role, :primary_language)
|
permitted = params.require(:user).permit(:email, :name, :primary_language)
|
||||||
|
|
||||||
|
if params[:user][:role].present? && User.roles.key?(params[:user][:role])
|
||||||
|
permitted[:role] = params[:user][:role]
|
||||||
|
end
|
||||||
|
|
||||||
|
permitted
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -3,14 +3,20 @@ class Admin::UsersController < Admin::BaseController
|
|||||||
|
|
||||||
def index
|
def index
|
||||||
@users = User.order(created_at: :desc)
|
@users = User.order(created_at: :desc)
|
||||||
@users = @users.where(role: params[:role]) if params[:role].present?
|
.by_role(params[:role])
|
||||||
@users = @users.where("email LIKE ?", "%#{params[:q]}%") if params[:q].present?
|
.search_email(params[:q])
|
||||||
end
|
end
|
||||||
|
|
||||||
def edit
|
def edit
|
||||||
end
|
end
|
||||||
|
|
||||||
def update
|
def update
|
||||||
|
# Prevent users from modifying their own role
|
||||||
|
if @user == current_user && user_params[:role].present?
|
||||||
|
redirect_to admin_users_path, alert: "You cannot modify your own role."
|
||||||
|
return
|
||||||
|
end
|
||||||
|
|
||||||
if @user.update(user_params)
|
if @user.update(user_params)
|
||||||
redirect_to admin_users_path, notice: "User updated successfully."
|
redirect_to admin_users_path, notice: "User updated successfully."
|
||||||
else
|
else
|
||||||
@@ -40,6 +46,13 @@ class Admin::UsersController < Admin::BaseController
|
|||||||
end
|
end
|
||||||
|
|
||||||
def user_params
|
def user_params
|
||||||
params.require(:user).permit(:name, :email, :role, :primary_language)
|
permitted = params.require(:user).permit(:name, :email, :primary_language)
|
||||||
|
|
||||||
|
# Only allow role if it's a valid role enum value
|
||||||
|
if params[:user][:role].present? && User.roles.key?(params[:user][:role])
|
||||||
|
permitted[:role] = params[:user][:role]
|
||||||
|
end
|
||||||
|
|
||||||
|
permitted
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -2,7 +2,7 @@ class EntriesController < ApplicationController
|
|||||||
before_action :set_entry, only: [ :show, :edit, :update ]
|
before_action :set_entry, only: [ :show, :edit, :update ]
|
||||||
|
|
||||||
def index
|
def index
|
||||||
@language_code = params[:language].presence
|
@language_code = validate_language_code(params[:language].presence)
|
||||||
@category = params[:category].presence
|
@category = params[:category].presence
|
||||||
@query = params[:q].to_s.strip
|
@query = params[:q].to_s.strip
|
||||||
@starts_with = params[:starts_with].presence
|
@starts_with = params[:starts_with].presence
|
||||||
@@ -79,4 +79,10 @@ class EntriesController < ApplicationController
|
|||||||
def entry_params
|
def entry_params
|
||||||
params.require(:entry).permit(:category)
|
params.require(:entry).permit(:category)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def validate_language_code(code)
|
||||||
|
return nil if code.blank?
|
||||||
|
|
||||||
|
SupportedLanguage.valid_codes.include?(code) ? code : nil
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
+1
-1
@@ -35,7 +35,7 @@ class Entry < ApplicationRecord
|
|||||||
return none unless valid_lang?(language_code)
|
return none unless valid_lang?(language_code)
|
||||||
|
|
||||||
where.not(language_code => [ nil, "" ])
|
where.not(language_code => [ nil, "" ])
|
||||||
.order(Arel.sql("#{language_code} ASC"))
|
.order(arel_table[language_code].asc)
|
||||||
end
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|||||||
@@ -21,6 +21,9 @@ class User < ApplicationRecord
|
|||||||
validates :email, presence: true, uniqueness: true
|
validates :email, presence: true, uniqueness: true
|
||||||
validates :password, length: { minimum: 12 }, if: -> { password.present? }
|
validates :password, length: { minimum: 12 }, if: -> { password.present? }
|
||||||
|
|
||||||
|
scope :by_role, ->(role) { where(role: role) if role.present? }
|
||||||
|
scope :search_email, ->(q) { where("email LIKE ?", "%#{sanitize_sql_like(q)}%") if q.present? }
|
||||||
|
|
||||||
# Invitation token expires after 14 days
|
# Invitation token expires after 14 days
|
||||||
INVITATION_TOKEN_EXPIRY = 14.days
|
INVITATION_TOKEN_EXPIRY = 14.days
|
||||||
|
|
||||||
|
|||||||
+20
-11
@@ -22,6 +22,24 @@
|
|||||||
- [ ] Contributor permissions enforcement (for entry editing)
|
- [ ] Contributor permissions enforcement (for entry editing)
|
||||||
- [ ] Reviewer permissions enforcement (for review queue)
|
- [ ] Reviewer permissions enforcement (for review queue)
|
||||||
|
|
||||||
|
## Security & Vulnerabilities
|
||||||
|
|
||||||
|
- [x] **Fixed user-controlled method execution** (HIGH)
|
||||||
|
- Added language code validation in EntriesController
|
||||||
|
- Prevents arbitrary method execution via `public_send()`
|
||||||
|
- [x] **Fixed SQL injection in Entry model** (MEDIUM)
|
||||||
|
- Replaced string interpolation with Arel safe column references
|
||||||
|
- Changed `Arel.sql("#{language_code} ASC")` to `arel_table[language_code].asc`
|
||||||
|
- [x] **Fixed mass assignment vulnerabilities** (MEDIUM)
|
||||||
|
- Added role validation in admin invitations and user management
|
||||||
|
- Only allows valid enum role values
|
||||||
|
- Prevents users from modifying their own role
|
||||||
|
- [x] **Fixed SQL LIKE injection** (MEDIUM)
|
||||||
|
- Added `sanitize_sql_like()` for email search in UsersController
|
||||||
|
- Prevents wildcard injection attacks
|
||||||
|
|
||||||
|
**Status:** All Brakeman security warnings resolved ✓
|
||||||
|
|
||||||
## Core Features
|
## Core Features
|
||||||
|
|
||||||
### Search & Browse
|
### Search & Browse
|
||||||
@@ -119,24 +137,15 @@
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Completed
|
## Completed (Not Tracked Above)
|
||||||
|
|
||||||
- [x] **Invitation system** (complete flow with email, acceptance, and expiry validation)
|
|
||||||
- [x] **Invitation acceptance flow** (users can accept invitations and set passwords)
|
|
||||||
- [x] **Invitation mailer** (HTML and text email templates with styled design)
|
|
||||||
- [x] **Token expiry validation** (14-day expiration for invitation links)
|
|
||||||
- [x] **Controller tests** (40 tests with 160+ assertions for authentication)
|
|
||||||
- [x] **Authentication system** (login/logout with session management)
|
|
||||||
- [x] **Admin layout design** updated to match entries page style
|
- [x] **Admin layout design** updated to match entries page style
|
||||||
- [x] **Dynamic navigation** (Admin button for logged-in admins, Sign In for guests)
|
- [x] **Dynamic navigation** (Admin button for logged-in admins, Sign In for guests)
|
||||||
- [x] **Authorization middleware** (Admin::BaseController with role checks)
|
- [x] **Controller tests** (40 tests with 160+ assertions for authentication)
|
||||||
- [x] **Invitation token generation** (secure token creation for new users)
|
|
||||||
- [x] **Search input loses focus on filter change**
|
- [x] **Search input loses focus on filter change**
|
||||||
- [x] **Mismatched enum syntax** in models
|
- [x] **Mismatched enum syntax** in models
|
||||||
- [x] **Replace hardcoded LANGUAGE_COLUMNS** with dynamic query
|
- [x] **Replace hardcoded LANGUAGE_COLUMNS** with dynamic query
|
||||||
- [x] **Improve fixture quality** (resolved foreign key violations)
|
- [x] **Improve fixture quality** (resolved foreign key violations)
|
||||||
- [x] **XLSX download** button for entries
|
|
||||||
- [x] **FTS5 integration** (migration added)
|
|
||||||
- [x] **Database schema** implementation (all models and migrations)
|
- [x] **Database schema** implementation (all models and migrations)
|
||||||
- [x] **Supported languages** table with seed data
|
- [x] **Supported languages** table with seed data
|
||||||
- [x] **Filters do not update with new search results**
|
- [x] **Filters do not update with new search results**
|
||||||
|
|||||||
Reference in New Issue
Block a user