From 293b6924e9758725b521edc1d667f3abe76efe82 Mon Sep 17 00:00:00 2001 From: Andrew Cain Date: Fri, 14 Jun 2024 11:01:58 +1000 Subject: [PATCH 1/3] chore: ensure lf file endings --- .gitattributes | 2 ++ .gitignore | 1 - .vscode/settings.json | 3 +++ 3 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 .gitattributes create mode 100644 .vscode/settings.json diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000000..d56abbf304 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,2 @@ +# Set the default behavior, in case people don't have core.autocrlf set. +* text=auto eol=lf diff --git a/.gitignore b/.gitignore index 481e82b6c2..3ef1f7a03a 100644 --- a/.gitignore +++ b/.gitignore @@ -36,5 +36,4 @@ student-work/ .idea/ .byebug_history coverage/ -.vscode _history diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000000..7e6882bfa3 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,3 @@ +{ + "files.eol": "\n" +} From b998fc0b1a9da2ed7b7fe04aafc2bec6e70dd0dd Mon Sep 17 00:00:00 2001 From: Millicent Amolo Date: Wed, 17 Sep 2025 22:09:53 +1000 Subject: [PATCH 2/3] Implement password management API updates --- Gemfile.lock | 4 + app/api/authentication_api.rb | 170 +++++++++++++++ app/mailers/password_reset_mailer.rb | 44 ++++ app/models/user.rb | 52 ++++- .../password_changed.html.erb | 96 +++++++++ .../password_changed.text.erb | 23 ++ .../reset_password.html.erb | 100 +++++++++ .../reset_password.text.erb | 18 ++ docs/password_management_security.md | 138 ++++++++++++ test/api/password_management_test.rb | 202 ++++++++++++++++++ 10 files changed, 841 insertions(+), 6 deletions(-) create mode 100644 app/mailers/password_reset_mailer.rb create mode 100644 app/views/password_reset_mailer/password_changed.html.erb create mode 100644 app/views/password_reset_mailer/password_changed.text.erb create mode 100644 app/views/password_reset_mailer/reset_password.html.erb create mode 100644 app/views/password_reset_mailer/reset_password.text.erb create mode 100644 docs/password_management_security.md create mode 100644 test/api/password_management_test.rb diff --git a/Gemfile.lock b/Gemfile.lock index 58ca5d5924..000d70b11a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -168,6 +168,7 @@ GEM faraday-net_http (3.1.0) net-http ffi (1.17.0-aarch64-linux-gnu) + ffi (1.17.0-x86_64-linux-gnu) fugit (1.11.0) et-orbi (~> 1, >= 1.2.11) raabro (~> 1.4) @@ -265,6 +266,8 @@ GEM nio4r (2.7.3) nokogiri (1.16.5-aarch64-linux) racc (~> 1.4) + nokogiri (1.16.5-x86_64-linux) + racc (~> 1.4) observer (0.1.2) orm_adapter (0.5.0) parallel (1.24.0) @@ -490,6 +493,7 @@ GEM PLATFORMS aarch64-linux + x86_64-linux DEPENDENCIES better_errors diff --git a/app/api/authentication_api.rb b/app/api/authentication_api.rb index 3b43b10cf6..693f2b9f00 100644 --- a/app/api/authentication_api.rb +++ b/app/api/authentication_api.rb @@ -277,6 +277,176 @@ class AuthenticationApi < Grape::API present response, with: Grape::Presenters::Presenter end + # + # Password management endpoints - only available for database auth + # + if !AuthenticationHelpers.aaf_auth? && !AuthenticationHelpers.saml_auth? && !AuthenticationHelpers.ldap_auth? + + # + # User registration endpoint + # + desc 'Register a new user' + params do + requires :username, type: String, desc: 'User username' + requires :email, type: String, desc: 'User email' + requires :password, type: String, desc: 'User password' + requires :password_confirmation, type: String, desc: 'Password confirmation' + requires :first_name, type: String, desc: 'User first name' + requires :last_name, type: String, desc: 'User last name' + optional :nickname, type: String, desc: 'User nickname' + end + post '/register' do + username = params[:username].downcase + email = params[:email] + password = params[:password] + password_confirmation = params[:password_confirmation] + + # Check if user already exists + if User.exists?(username: username) + error!({ error: 'Username already exists.' }, 409) + end + + if User.exists?(email: email) + error!({ error: 'Email already exists.' }, 409) + end + + # Create new user + user = User.new( + username: username, + email: email, + password: password, + password_confirmation: password_confirmation, + first_name: params[:first_name], + last_name: params[:last_name], + nickname: params[:nickname] || params[:first_name], + role_id: Role.student.id, + login_id: username + ) + + if user.save + logger.info "User registered: #{username} from #{request.ip}" + present :user, user, with: Entities::UserEntity + present :auth_token, user.generate_authentication_token!(false).authentication_token + present :message, 'User registered successfully.' + else + error!({ error: 'Registration failed.', details: user.errors.full_messages }, 422) + end + end + + # + # Password reset request endpoint + # + desc 'Request password reset' + params do + requires :email, type: String, desc: 'User email' + end + post '/password/reset' do + email = params[:email] + user = User.find_by(email: email) + + if user + user.generate_password_reset_token! + + # Send password reset email + begin + PasswordResetMailer.reset_password(user).deliver_now + logger.info "Password reset email sent to #{email}" + rescue => e + logger.error "Failed to send password reset email to #{email}: #{e.message}" + # Don't fail the request if email sending fails + end + + present :message, 'If an account with that email exists, a password reset link has been sent.' + else + # Don't reveal whether email exists for security + present :message, 'If an account with that email exists, a password reset link has been sent.' + end + end + + # + # Password reset confirmation endpoint + # + desc 'Reset password with token' + params do + requires :token, type: String, desc: 'Password reset token' + requires :password, type: String, desc: 'New password' + requires :password_confirmation, type: String, desc: 'Password confirmation' + end + post '/password/reset/confirm' do + token = params[:token] + password = params[:password] + password_confirmation = params[:password_confirmation] + + user = User.find_by(reset_password_token: token) + + unless user && user.password_reset_token_valid? + error!({ error: 'Invalid or expired reset token.' }, 400) + end + + user.password = password + user.password_confirmation = password_confirmation + + if user.save + user.clear_password_reset_token! + logger.info "Password reset completed for user: #{user.username} from #{request.ip}" + + # Send password changed notification email + begin + PasswordResetMailer.password_changed(user).deliver_now + logger.info "Password changed notification email sent to #{user.email}" + rescue => e + logger.error "Failed to send password changed notification email to #{user.email}: #{e.message}" + # Don't fail the request if email sending fails + end + + present :message, 'Password has been reset successfully.' + else + error!({ error: 'Password reset failed.', details: user.errors.full_messages }, 422) + end + end + + # + # Change password endpoint (requires authentication) + # + desc 'Change password' + params do + requires :current_password, type: String, desc: 'Current password' + requires :password, type: String, desc: 'New password' + requires :password_confirmation, type: String, desc: 'Password confirmation' + end + post '/password/change' do + authenticate! + + current_password = params[:current_password] + password = params[:password] + password_confirmation = params[:password_confirmation] + + unless current_user.valid_password?(current_password) + error!({ error: 'Current password is incorrect.' }, 400) + end + + current_user.password = password + current_user.password_confirmation = password_confirmation + + if current_user.save + logger.info "Password changed for user: #{current_user.username} from #{request.ip}" + + # Send password changed notification email + begin + PasswordResetMailer.password_changed(current_user).deliver_now + logger.info "Password changed notification email sent to #{current_user.email}" + rescue => e + logger.error "Failed to send password changed notification email to #{current_user.email}: #{e.message}" + # Don't fail the request if email sending fails + end + + present :message, 'Password has been changed successfully.' + else + error!({ error: 'Password change failed.', details: current_user.errors.full_messages }, 422) + end + end + end + # # Returns the current auth signout URL # diff --git a/app/mailers/password_reset_mailer.rb b/app/mailers/password_reset_mailer.rb new file mode 100644 index 0000000000..c96df157bb --- /dev/null +++ b/app/mailers/password_reset_mailer.rb @@ -0,0 +1,44 @@ +class PasswordResetMailer < ActionMailer::Base + def reset_password(user) + @doubtfire_host = Doubtfire::Application.config.institution[:host] + @doubtfire_product_name = Doubtfire::Application.config.institution[:product_name] + + @user = user + @reset_url = "#{@doubtfire_host}/#/reset-password?token=#{user.reset_password_token}" + @expiry_hours = 24 + + # Set the default from address + institution_email_domain = Doubtfire::Application.config.institution[:email_domain] + default_from = "noreply@#{institution_email_domain}" + + # Create email with user's name + email_with_name = %("#{@user.name}" <#{@user.email}>) + + mail( + to: email_with_name, + from: default_from, + subject: "[#{@doubtfire_product_name}] Password Reset Request" + ) + end + + def password_changed(user) + @doubtfire_host = Doubtfire::Application.config.institution[:host] + @doubtfire_product_name = Doubtfire::Application.config.institution[:product_name] + + @user = user + + # Set the default from address + institution_email_domain = Doubtfire::Application.config.institution[:email_domain] + default_from = "noreply@#{institution_email_domain}" + + # Create email with user's name + email_with_name = %("#{@user.name}" <#{@user.email}>) + + mail( + to: email_with_name, + from: default_from, + subject: "[#{@doubtfire_product_name}] Password Changed Successfully" + ) + end +end + diff --git a/app/models/user.rb b/app/models/user.rb index 6e016badfd..1afdc9c345 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -61,18 +61,58 @@ def valid_jwt?(jws) end # - # We incorporate password details for local dev server - needed to keep devise happy + # Password management methods # - def password - 'password' + attr_accessor :password, :password_confirmation + + # Password validation + validates :password, presence: true, length: { minimum: 8 }, on: :create + validates :password, presence: true, length: { minimum: 8 }, on: :update, if: :password_required? + validates :password_confirmation, presence: true, if: :password_required? + validate :password_confirmation_match, if: :password_required? + + def password_required? + password.present? || password_confirmation.present? end - def password_confirmation - 'password' + def password_confirmation_match + if password != password_confirmation + errors.add(:password_confirmation, "doesn't match password") + end end def password=(value) - self.encrypted_password = BCrypt::Password.create(value) + @password = value + if value.present? + self.encrypted_password = BCrypt::Password.create(value) + end + end + + # Check if provided password matches stored password + def valid_password?(password) + return false if encrypted_password.blank? + BCrypt::Password.new(encrypted_password) == password + end + + # Generate password reset token + def generate_password_reset_token! + self.reset_password_token = SecureRandom.urlsafe_base64 + self.reset_password_sent_at = Time.current + save!(validate: false) + end + + # Clear password reset token + def clear_password_reset_token! + self.reset_password_token = nil + self.reset_password_sent_at = nil + save!(validate: false) + end + + # Check if password reset token is valid and not expired (24 hours) + def password_reset_token_valid? + reset_password_token.present? && + reset_password_sent_at.present? && + reset_password_sent_at > 24.hours.ago end # diff --git a/app/views/password_reset_mailer/password_changed.html.erb b/app/views/password_reset_mailer/password_changed.html.erb new file mode 100644 index 0000000000..7a1e3e305a --- /dev/null +++ b/app/views/password_reset_mailer/password_changed.html.erb @@ -0,0 +1,96 @@ + + + + + + Password Changed - <%= @doubtfire_product_name %> + + + +
+ +
+ +
+

Password Successfully Changed

+ +

Hello <%= @user.name %>,

+ +
+ ✓ Password Changed Successfully
+ Your password for <%= @doubtfire_product_name %> has been successfully updated. +
+ +

This is a confirmation that your password was changed at <%= Time.current.strftime("%B %d, %Y at %I:%M %p") %>.

+ +
+ Security Notice: If you did not make this change, please contact your system administrator immediately as your account may have been compromised. +
+ +

You can now log in to <%= @doubtfire_product_name %> using your new password.

+ +

For security reasons, we recommend:

+
    +
  • Using a strong, unique password
    • +
  • Not sharing your password with anyone
    • +
  • Changing your password regularly
    • +
+
+ +
+

This email was sent from <%= @doubtfire_product_name %>

+

If you have any questions or concerns, please contact your system administrator.

+
+ + + diff --git a/app/views/password_reset_mailer/password_changed.text.erb b/app/views/password_reset_mailer/password_changed.text.erb new file mode 100644 index 0000000000..38b6004cd1 --- /dev/null +++ b/app/views/password_reset_mailer/password_changed.text.erb @@ -0,0 +1,23 @@ +Password Changed Successfully - <%= @doubtfire_product_name %> + +Hello <%= @user.name %>, + +✓ PASSWORD CHANGED SUCCESSFULLY + +Your password for <%= @doubtfire_product_name %> has been successfully updated. + +This is a confirmation that your password was changed at <%= Time.current.strftime("%B %d, %Y at %I:%M %p") %>. + +SECURITY NOTICE: If you did not make this change, please contact your system administrator immediately as your account may have been compromised. + +You can now log in to <%= @doubtfire_product_name %> using your new password. + +For security reasons, we recommend: +- Using a strong, unique password +- Not sharing your password with anyone +- Changing your password regularly + +--- +This email was sent from <%= @doubtfire_product_name %> +If you have any questions or concerns, please contact your system administrator. + diff --git a/app/views/password_reset_mailer/reset_password.html.erb b/app/views/password_reset_mailer/reset_password.html.erb new file mode 100644 index 0000000000..46c5987dc1 --- /dev/null +++ b/app/views/password_reset_mailer/reset_password.html.erb @@ -0,0 +1,100 @@ + + + + + + Password Reset - <%= @doubtfire_product_name %> + + + +
+ +
+ +
+

Password Reset Request

+ +

Hello <%= @user.name %>,

+ +

We received a request to reset your password for your <%= @doubtfire_product_name %> account. If you made this request, click the button below to reset your password:

+ +

+ Reset My Password +

+ +
+ Important: This link will expire in <%= @expiry_hours %> hours for security reasons. +
+ +

If the button doesn't work, you can copy and paste this link into your browser:

+

+ <%= @reset_url %> +

+ +

If you didn't request a password reset, you can safely ignore this email. Your password will remain unchanged.

+ +

If you continue to have problems, please contact your system administrator.

+
+ +
+

This email was sent from <%= @doubtfire_product_name %>

+

If you have any questions, please contact your system administrator.

+
+ + + diff --git a/app/views/password_reset_mailer/reset_password.text.erb b/app/views/password_reset_mailer/reset_password.text.erb new file mode 100644 index 0000000000..7d587d7c49 --- /dev/null +++ b/app/views/password_reset_mailer/reset_password.text.erb @@ -0,0 +1,18 @@ +Password Reset Request - <%= @doubtfire_product_name %> + +Hello <%= @user.name %>, + +We received a request to reset your password for your <%= @doubtfire_product_name %> account. If you made this request, please click the link below to reset your password: + +<%= @reset_url %> + +IMPORTANT: This link will expire in <%= @expiry_hours %> hours for security reasons. + +If you didn't request a password reset, you can safely ignore this email. Your password will remain unchanged. + +If you continue to have problems, please contact your system administrator. + +--- +This email was sent from <%= @doubtfire_product_name %> +If you have any questions, please contact your system administrator. + diff --git a/docs/password_management_security.md b/docs/password_management_security.md new file mode 100644 index 0000000000..ed1c571b9a --- /dev/null +++ b/docs/password_management_security.md @@ -0,0 +1,138 @@ +# Password Management Security Documentation + +## Overview + +This document outlines the security measures implemented for the password management system in Doubtfire. + +## Security Features + +### Password Storage +- **Bcrypt Hashing**: All passwords are hashed using BCrypt with salt +- **No Plain Text Storage**: Passwords are never stored in plain text +- **Salt Generation**: Each password gets a unique salt automatically + +### Password Validation +- **Minimum Length**: Passwords must be at least 8 characters long +- **Confirmation Matching**: Password confirmation must match the original password +- **Required Fields**: Password is required during registration and password changes + +### Password Reset Security +- **Token Generation**: Reset tokens are generated using `SecureRandom.urlsafe_base64` +- **Token Expiration**: Reset tokens expire after 24 hours +- **Single Use**: Tokens are cleared after successful password reset +- **Email Privacy**: The system doesn't reveal whether an email exists in the system + +### Authentication Security +- **Token-Based Auth**: Uses secure authentication tokens for API access +- **Token Expiration**: Authentication tokens have configurable expiration times +- **Current Password Verification**: Password changes require current password verification +- **Rate Limiting**: Consider implementing rate limiting for authentication attempts + +## API Endpoints Security + +### Registration Endpoint (`POST /api/register`) +- Validates all required fields +- Checks for existing username/email +- Validates password strength +- Creates user with student role by default +- Returns authentication token on success + +### Password Reset Request (`POST /api/password/reset`) +- Accepts email address +- Generates secure reset token +- Logs reset request (in production, send email) +- Doesn't reveal if email exists + +### Password Reset Confirmation (`POST /api/password/reset/confirm`) +- Validates reset token +- Checks token expiration +- Validates new password +- Clears reset token after successful reset + +### Change Password (`POST /api/password/change`) +- Requires authentication +- Verifies current password +- Validates new password +- Updates password hash + +## Security Considerations + +### Environment-Specific Behavior +- Password management endpoints are only available when using database authentication +- LDAP, AAF, and SAML authentication methods bypass password management +- This ensures compatibility with existing institutional authentication systems + +### Error Handling +- Generic error messages to prevent information disclosure +- Detailed validation errors only for legitimate users +- Proper HTTP status codes for different error conditions + +### Logging +- All password-related actions are logged with IP addresses +- Failed authentication attempts are logged +- Password reset requests are logged with tokens (for development) + +## Implementation Notes + +### Database Schema +The following fields are used for password management: +- `encrypted_password`: Stores the bcrypt hash +- `reset_password_token`: Stores reset tokens +- `reset_password_sent_at`: Tracks when reset was requested + +### Frontend Security +- Form validation on both client and server side +- Secure token handling in Angular services +- Proper error message display +- Password confirmation matching + +## Recommendations for Production + +### Email Integration +- Implement email sending for password reset links +- Use secure email templates +- Include expiration time in email +- Consider email rate limiting + +### Rate Limiting +- Implement rate limiting for authentication attempts +- Limit password reset requests per IP/email +- Implement account lockout after failed attempts + +### Monitoring +- Monitor for suspicious authentication patterns +- Log and alert on multiple failed attempts +- Track password reset request patterns + +### Additional Security +- Consider implementing password strength requirements +- Add CAPTCHA for registration and password reset +- Implement session management improvements +- Consider two-factor authentication for sensitive accounts + +## Testing + +Comprehensive tests have been implemented covering: +- User registration with valid/invalid data +- Password reset token generation and validation +- Password change with authentication +- Error handling and edge cases +- Security boundary conditions + +Run tests with: +```bash +rails test test/api/password_management_test.rb +``` + +## Configuration + +### Environment Variables +- `DF_AUTH_METHOD`: Set to 'database' to enable password management +- Other authentication methods (ldap, aaf, saml) will disable password management endpoints + +### Database Migrations +Ensure the following migrations are applied: +- Devise user creation migration +- Password reset token fields migration +- Any additional user fields migration + diff --git a/test/api/password_management_test.rb b/test/api/password_management_test.rb new file mode 100644 index 0000000000..384529bc03 --- /dev/null +++ b/test/api/password_management_test.rb @@ -0,0 +1,202 @@ +require 'test_helper' + +class PasswordManagementTest < ActionDispatch::IntegrationTest + include ActiveJob::TestHelper + setup do + @user = User.create!( + username: 'testuser', + email: 'test@example.com', + first_name: 'Test', + last_name: 'User', + password: 'password123', + password_confirmation: 'password123', + role_id: Role.student.id, + login_id: 'testuser' + ) + end + + test "should register new user with valid data" do + post '/api/register', params: { + username: 'newuser', + email: 'newuser@example.com', + password: 'password123', + password_confirmation: 'password123', + first_name: 'New', + last_name: 'User' + } + + assert_response :success + assert_equal 'newuser', JSON.parse(response.body)['user']['username'] + assert_not_nil JSON.parse(response.body)['auth_token'] + end + + test "should not register user with existing username" do + post '/api/register', params: { + username: 'testuser', + email: 'different@example.com', + password: 'password123', + password_confirmation: 'password123', + first_name: 'Different', + last_name: 'User' + } + + assert_response :conflict + assert_includes JSON.parse(response.body)['error'], 'Username already exists' + end + + test "should not register user with existing email" do + post '/api/register', params: { + username: 'differentuser', + email: 'test@example.com', + password: 'password123', + password_confirmation: 'password123', + first_name: 'Different', + last_name: 'User' + } + + assert_response :conflict + assert_includes JSON.parse(response.body)['error'], 'Email already exists' + end + + test "should not register user with short password" do + post '/api/register', params: { + username: 'newuser', + email: 'newuser@example.com', + password: 'short', + password_confirmation: 'short', + first_name: 'New', + last_name: 'User' + } + + assert_response :unprocessable_entity + assert_includes JSON.parse(response.body)['details'], 'Password is too short' + end + + test "should not register user with mismatched passwords" do + post '/api/register', params: { + username: 'newuser', + email: 'newuser@example.com', + password: 'password123', + password_confirmation: 'different123', + first_name: 'New', + last_name: 'User' + } + + assert_response :unprocessable_entity + assert_includes JSON.parse(response.body)['details'], "doesn't match password" + end + + test "should request password reset for existing user" do + assert_emails 1 do + post '/api/password/reset', params: { email: 'test@example.com' } + end + + assert_response :success + assert_includes JSON.parse(response.body)['message'], 'password reset link has been sent' + + @user.reload + assert_not_nil @user.reset_password_token + assert_not_nil @user.reset_password_sent_at + + # Check that the email was sent + email = ActionMailer::Base.deliveries.last + assert_equal 'test@example.com', email.to[0] + assert_includes email.subject, 'Password Reset Request' + end + + test "should not reveal if email exists for password reset" do + post '/api/password/reset', params: { email: 'nonexistent@example.com' } + + assert_response :success + assert_includes JSON.parse(response.body)['message'], 'password reset link has been sent' + end + + test "should reset password with valid token" do + @user.generate_password_reset_token! + token = @user.reset_password_token + + assert_emails 1 do + post '/api/password/reset/confirm', params: { + token: token, + password: 'newpassword123', + password_confirmation: 'newpassword123' + } + end + + assert_response :success + assert_includes JSON.parse(response.body)['message'], 'Password has been reset' + + @user.reload + assert_nil @user.reset_password_token + assert_nil @user.reset_password_sent_at + assert @user.valid_password?('newpassword123') + + # Check that the password changed notification email was sent + email = ActionMailer::Base.deliveries.last + assert_equal 'test@example.com', email.to[0] + assert_includes email.subject, 'Password Changed Successfully' + end + + test "should not reset password with invalid token" do + post '/api/password/reset/confirm', params: { + token: 'invalid_token', + password: 'newpassword123', + password_confirmation: 'newpassword123' + } + + assert_response :bad_request + assert_includes JSON.parse(response.body)['error'], 'Invalid or expired reset token' + end + + test "should change password with correct current password" do + auth_token = @user.generate_authentication_token!(false).authentication_token + + assert_emails 1 do + post '/api/password/change', params: { + current_password: 'password123', + password: 'newpassword123', + password_confirmation: 'newpassword123' + }, headers: { + 'Auth-Token' => auth_token, + 'Username' => @user.username + } + end + + assert_response :success + assert_includes JSON.parse(response.body)['message'], 'Password has been changed' + + @user.reload + assert @user.valid_password?('newpassword123') + + # Check that the password changed notification email was sent + email = ActionMailer::Base.deliveries.last + assert_equal 'test@example.com', email.to[0] + assert_includes email.subject, 'Password Changed Successfully' + end + + test "should not change password with incorrect current password" do + auth_token = @user.generate_authentication_token!(false).authentication_token + + post '/api/password/change', params: { + current_password: 'wrongpassword', + password: 'newpassword123', + password_confirmation: 'newpassword123' + }, headers: { + 'Auth-Token' => auth_token, + 'Username' => @user.username + } + + assert_response :bad_request + assert_includes JSON.parse(response.body)['error'], 'Current password is incorrect' + end + + test "should require authentication for password change" do + post '/api/password/change', params: { + current_password: 'password123', + password: 'newpassword123', + password_confirmation: 'newpassword123' + } + + assert_response :unauthorized + end +end From 2adfc23ecea9ce42ac6458fe6d35649a94436fcc Mon Sep 17 00:00:00 2001 From: Millicent Amolo Date: Sun, 28 Sep 2025 04:37:27 +1000 Subject: [PATCH 3/3] Update password reset email templates and remove deprecated security doc --- app/mailers/password_reset_mailer.rb | 1 + .../password_changed.html.erb | 1 + .../password_changed.text.erb | 1 + .../reset_password.html.erb | 1 + .../reset_password.text.erb | 1 + docs/password_management_security.md | 138 ------------------ 6 files changed, 5 insertions(+), 138 deletions(-) delete mode 100644 docs/password_management_security.md diff --git a/app/mailers/password_reset_mailer.rb b/app/mailers/password_reset_mailer.rb index c96df157bb..9065aa45f1 100644 --- a/app/mailers/password_reset_mailer.rb +++ b/app/mailers/password_reset_mailer.rb @@ -42,3 +42,4 @@ def password_changed(user) end end + diff --git a/app/views/password_reset_mailer/password_changed.html.erb b/app/views/password_reset_mailer/password_changed.html.erb index 7a1e3e305a..051f30f5cc 100644 --- a/app/views/password_reset_mailer/password_changed.html.erb +++ b/app/views/password_reset_mailer/password_changed.html.erb @@ -94,3 +94,4 @@ + diff --git a/app/views/password_reset_mailer/password_changed.text.erb b/app/views/password_reset_mailer/password_changed.text.erb index 38b6004cd1..9cf892d6e4 100644 --- a/app/views/password_reset_mailer/password_changed.text.erb +++ b/app/views/password_reset_mailer/password_changed.text.erb @@ -21,3 +21,4 @@ For security reasons, we recommend: This email was sent from <%= @doubtfire_product_name %> If you have any questions or concerns, please contact your system administrator. + diff --git a/app/views/password_reset_mailer/reset_password.html.erb b/app/views/password_reset_mailer/reset_password.html.erb index 46c5987dc1..24bb1e5330 100644 --- a/app/views/password_reset_mailer/reset_password.html.erb +++ b/app/views/password_reset_mailer/reset_password.html.erb @@ -98,3 +98,4 @@ + diff --git a/app/views/password_reset_mailer/reset_password.text.erb b/app/views/password_reset_mailer/reset_password.text.erb index 7d587d7c49..c044d203b6 100644 --- a/app/views/password_reset_mailer/reset_password.text.erb +++ b/app/views/password_reset_mailer/reset_password.text.erb @@ -16,3 +16,4 @@ If you continue to have problems, please contact your system administrator. This email was sent from <%= @doubtfire_product_name %> If you have any questions, please contact your system administrator. + diff --git a/docs/password_management_security.md b/docs/password_management_security.md deleted file mode 100644 index ed1c571b9a..0000000000 --- a/docs/password_management_security.md +++ /dev/null @@ -1,138 +0,0 @@ -# Password Management Security Documentation - -## Overview - -This document outlines the security measures implemented for the password management system in Doubtfire. - -## Security Features - -### Password Storage -- **Bcrypt Hashing**: All passwords are hashed using BCrypt with salt -- **No Plain Text Storage**: Passwords are never stored in plain text -- **Salt Generation**: Each password gets a unique salt automatically - -### Password Validation -- **Minimum Length**: Passwords must be at least 8 characters long -- **Confirmation Matching**: Password confirmation must match the original password -- **Required Fields**: Password is required during registration and password changes - -### Password Reset Security -- **Token Generation**: Reset tokens are generated using `SecureRandom.urlsafe_base64` -- **Token Expiration**: Reset tokens expire after 24 hours -- **Single Use**: Tokens are cleared after successful password reset -- **Email Privacy**: The system doesn't reveal whether an email exists in the system - -### Authentication Security -- **Token-Based Auth**: Uses secure authentication tokens for API access -- **Token Expiration**: Authentication tokens have configurable expiration times -- **Current Password Verification**: Password changes require current password verification -- **Rate Limiting**: Consider implementing rate limiting for authentication attempts - -## API Endpoints Security - -### Registration Endpoint (`POST /api/register`) -- Validates all required fields -- Checks for existing username/email -- Validates password strength -- Creates user with student role by default -- Returns authentication token on success - -### Password Reset Request (`POST /api/password/reset`) -- Accepts email address -- Generates secure reset token -- Logs reset request (in production, send email) -- Doesn't reveal if email exists - -### Password Reset Confirmation (`POST /api/password/reset/confirm`) -- Validates reset token -- Checks token expiration -- Validates new password -- Clears reset token after successful reset - -### Change Password (`POST /api/password/change`) -- Requires authentication -- Verifies current password -- Validates new password -- Updates password hash - -## Security Considerations - -### Environment-Specific Behavior -- Password management endpoints are only available when using database authentication -- LDAP, AAF, and SAML authentication methods bypass password management -- This ensures compatibility with existing institutional authentication systems - -### Error Handling -- Generic error messages to prevent information disclosure -- Detailed validation errors only for legitimate users -- Proper HTTP status codes for different error conditions - -### Logging -- All password-related actions are logged with IP addresses -- Failed authentication attempts are logged -- Password reset requests are logged with tokens (for development) - -## Implementation Notes - -### Database Schema -The following fields are used for password management: -- `encrypted_password`: Stores the bcrypt hash -- `reset_password_token`: Stores reset tokens -- `reset_password_sent_at`: Tracks when reset was requested - -### Frontend Security -- Form validation on both client and server side -- Secure token handling in Angular services -- Proper error message display -- Password confirmation matching - -## Recommendations for Production - -### Email Integration -- Implement email sending for password reset links -- Use secure email templates -- Include expiration time in email -- Consider email rate limiting - -### Rate Limiting -- Implement rate limiting for authentication attempts -- Limit password reset requests per IP/email -- Implement account lockout after failed attempts - -### Monitoring -- Monitor for suspicious authentication patterns -- Log and alert on multiple failed attempts -- Track password reset request patterns - -### Additional Security -- Consider implementing password strength requirements -- Add CAPTCHA for registration and password reset -- Implement session management improvements -- Consider two-factor authentication for sensitive accounts - -## Testing - -Comprehensive tests have been implemented covering: -- User registration with valid/invalid data -- Password reset token generation and validation -- Password change with authentication -- Error handling and edge cases -- Security boundary conditions - -Run tests with: -```bash -rails test test/api/password_management_test.rb -``` - -## Configuration - -### Environment Variables -- `DF_AUTH_METHOD`: Set to 'database' to enable password management -- Other authentication methods (ldap, aaf, saml) will disable password management endpoints - -### Database Migrations -Ensure the following migrations are applied: -- Devise user creation migration -- Password reset token fields migration -- Any additional user fields migration -