Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added code and setup for password reset #1

Merged
merged 1 commit into from Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions composer.json
@@ -0,0 +1,5 @@
{
"require": {
"phpmailer/phpmailer": "^6.9"
}
}
100 changes: 100 additions & 0 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions hid/mail.php
@@ -0,0 +1,11 @@
<?php

// Configuration of Mail Server

return [
'smtp_host' => 'sandbox.smtp.mailtrap.io', // STMP Host Server
'smtp_port' => '2525', // STMP Port
'smtp_username' => 'xxxx', // STMP Username
'smtp_password' => 'xxxx' // STMP Password
];

67 changes: 67 additions & 0 deletions index.php
Expand Up @@ -66,6 +66,8 @@ function ErrorHandler($type, $message, $file, $line)
//error_reporting(E_ALL);

//ini_set('zlib.output_compression', 0);
require 'vendor/autoload.php';
use PHPMailer\PHPMailer\PHPMailer;

$allowed_hosts = array('172.31.9.201', '34.234.225.210');
if(substr_count( $_SERVER["HTTP_ACCEPT_ENCODING"] , "gzip" ) )
Expand Down Expand Up @@ -95,6 +97,71 @@ function ErrorHandler($type, $message, $file, $line)
# ]


# ••••••••••••••••••••••••••••••••••••••••••••••••••••••••••• [ _FORGOT
function configureMail() {

$config = include '../../cghost/hid/config/mail.php';

$mail = new PHPMailer(true);
$mail->isSMTP();
$mail->Host = $config['smtp_host'];
$mail->Port = $config['smtp_port'];
$mail->SMTPAuth = true;
$mail->Username = $config['smtp_username'];
$mail->Password = $config['smtp_password'];
$mail->SMTPSecure = 'tls';

return $mail;
}

Flight::route("/_forgot", function() {
$safePost = filter_input_array(INPUT_POST);
check_token($safePost);

$thismember = R::getRow(
'SELECT * FROM user WHERE email = :username',
[':username' => $safePost['email']]
);

if (empty($thismember)) {
em( "Invalid email address!" );
forward();
} else {

if (strtolower($thismember['nick']) != strtolower($safePost['nick'])) {
em( "Invalid nickname!" );
forward();
}

$reload = R::load("user", $thismember['id']);

$plainpass = uniqid();
$newpass = password_hash($plainpass, PASSWORD_DEFAULT);
$reload->pass = $newpass;
$update = R::store($reload);
$to = $reload->email;
$subject = 'New Password for cogdemos.ink';
$message = 'Your password for cogdemos.ink has been reset to: ' . $plainpass;

$mail = configureMail();

$mail->setFrom('no-reply@cogdemos.ink', 'CoG Demos Ink');
$mail->addAddress($to);
$mail->Subject = $subject;
$mail->Body = $message;

if ($mail->send()) {
sm( "A new password has been sent to your email address." );
forward();
} else {
em( "Error sending email:" . $mail->ErrorInfo);
forward();
}
}

die("err.forgot");
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait i left a comment where did it go?

Copy link

@codeby22 codeby22 Apr 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i left a whole ASS comment and it just didnt send, man i really hate github, whatever ill just summarize what i said before

Your code is good however

Sending plaintext passwords is well bad, as anyone with access to the user's email can read the password. This can lead to unauthorized access if the user's email account is compromised.

My suggestion was;

Instead of generating a new password, the system generates a unique, cryptographically secure token. This token is sent to the user's email address and used to authenticate the password reset process. The token is temporarily stored in the user's record in the database, associating it with their account. A password reset URL is generated, which includes the token. The URL points to a new route (/_reset) that handles the actual password reset process. In this route, the user enters a new password, which is then hashed and stored in the database. The reset token is removed, disabling the password reset functionality for that token.

Pros(yay);
Users(people like me) can easily update their passwords without having to manage or remember the previous one.
Users(people like me) can set their own

Cons(yes it has);
Implementing a token-based password reset process is more complex than simply generating and sending a new password. (might be? idk, depends on skill level i guess)

and well if tokens are not set to expire after a certain time or after they have been used, they could be used maliciously if intercepted. Therefore, it is essential to implement token expiration or a limited number of uses to ensure security. 👍

(theres probably more)

but your code is good(tested website) and like i said before i dont think anyone is that desperate(hopefully) and this is just a suggestion because yes your current code is good

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good suggestion, and would definitely be better security-wise. I will add it to the "to-do" list for this code. Right now, I just wanted to get it up and running so people could reset their own passwords without me having to do it for them (also bad security!).

If you can code this, please feel free to do so and send a PR for it. Otherwise, it will have to wait just a bit until I can get a few other things sorted.

I think I will add a note to the password reset, informing the user to please change their password after logging in, and not keep the temporary password. The "change password" code is another thing I eventually need to change, because the "old password" is hashed, but the new password is shown in the form as plain text, which isn't a good idea.


# ••••••••••••••••••••••••••••••••••••••••••••••••••••••••••• [ WIP
Flight::route( "/wip(/@page)" , function( $page ){
Expand Down