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" +} 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..9065aa45f1 --- /dev/null +++ b/app/mailers/password_reset_mailer.rb @@ -0,0 +1,45 @@ +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..051f30f5cc --- /dev/null +++ b/app/views/password_reset_mailer/password_changed.html.erb @@ -0,0 +1,97 @@ + + + + + + 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:

+ +
+ +
+

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..9cf892d6e4 --- /dev/null +++ b/app/views/password_reset_mailer/password_changed.text.erb @@ -0,0 +1,24 @@ +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..24bb1e5330 --- /dev/null +++ b/app/views/password_reset_mailer/reset_password.html.erb @@ -0,0 +1,101 @@ + + + + + + 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..c044d203b6 --- /dev/null +++ b/app/views/password_reset_mailer/reset_password.text.erb @@ -0,0 +1,19 @@ +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/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