Finish improving Auth.pm control structure; fix other minor mistakes

both `register_do` and `login_do` in `CharmBoard::Controller::Auth` now
use two try/catch blocks instead of heavily nested if/elsif/else blocks.

I also reverted the change where I removed all of the
`use experimental 'feature-name'` declarations because, while
Mojolicious still whines about experimental features being experimental,
if you don't have `use experimental 'try'` set somewhere, then it just
does not work, unlike smartmatch which will work either way but cause
perl to get mad at you. plus, it feels weird not having those there
anyways!
This commit is contained in:
ngoomie 2023-05-08 04:26:11 -06:00
parent 07977292fe
commit f0ab7713cc
5 changed files with 90 additions and 67 deletions

View File

@ -1,5 +1,5 @@
-- --
-- File generated with SQLiteStudio v3.4.4 on Sun. May 7 22:15:23 2023 -- File generated with SQLiteStudio v3.4.4 on Mon. May 8 03:12:22 2023
-- --
-- Text encoding used: UTF-8 -- Text encoding used: UTF-8
-- --
@ -9,7 +9,7 @@ BEGIN TRANSACTION;
-- Table: categories -- Table: categories
DROP TABLE IF EXISTS categories; DROP TABLE IF EXISTS categories;
CREATE TABLE IF NOT EXISTS categories ( CREATE TABLE categories (
cat_id INTEGER NOT NULL ON CONFLICT ROLLBACK cat_id INTEGER NOT NULL ON CONFLICT ROLLBACK
UNIQUE ON CONFLICT ROLLBACK, UNIQUE ON CONFLICT ROLLBACK,
cat_name TEXT, cat_name TEXT,
@ -22,7 +22,7 @@ CREATE TABLE IF NOT EXISTS categories (
-- Table: posts -- Table: posts
DROP TABLE IF EXISTS posts; DROP TABLE IF EXISTS posts;
CREATE TABLE IF NOT EXISTS posts ( CREATE TABLE posts (
post_id INTEGER NOT NULL ON CONFLICT ROLLBACK post_id INTEGER NOT NULL ON CONFLICT ROLLBACK
UNIQUE ON CONFLICT ROLLBACK, UNIQUE ON CONFLICT ROLLBACK,
user_id INTEGER NOT NULL ON CONFLICT ROLLBACK, user_id INTEGER NOT NULL ON CONFLICT ROLLBACK,
@ -45,12 +45,12 @@ CREATE TABLE IF NOT EXISTS posts (
-- Table: sessions -- Table: sessions
DROP TABLE IF EXISTS sessions; DROP TABLE IF EXISTS sessions;
CREATE TABLE IF NOT EXISTS sessions ( CREATE TABLE sessions (
user_id INTEGER PRIMARY KEY
REFERENCES users (user_id)
NOT NULL,
session_key TEXT NOT NULL session_key TEXT NOT NULL
UNIQUE, UNIQUE
PRIMARY KEY,
user_id INTEGER REFERENCES users (user_id)
NOT NULL,
session_expiry NUMERIC NOT NULL, session_expiry NUMERIC NOT NULL,
is_ip_bound INTEGER (1, 1) NOT NULL is_ip_bound INTEGER (1, 1) NOT NULL
DEFAULT (0), DEFAULT (0),
@ -61,7 +61,7 @@ CREATE TABLE IF NOT EXISTS sessions (
-- Table: subforums -- Table: subforums
DROP TABLE IF EXISTS subforums; DROP TABLE IF EXISTS subforums;
CREATE TABLE IF NOT EXISTS subforums ( CREATE TABLE subforums (
subf_id INTEGER PRIMARY KEY subf_id INTEGER PRIMARY KEY
UNIQUE ON CONFLICT ROLLBACK UNIQUE ON CONFLICT ROLLBACK
NOT NULL ON CONFLICT ROLLBACK, NOT NULL ON CONFLICT ROLLBACK,
@ -76,7 +76,7 @@ CREATE TABLE IF NOT EXISTS subforums (
-- Table: threads -- Table: threads
DROP TABLE IF EXISTS threads; DROP TABLE IF EXISTS threads;
CREATE TABLE IF NOT EXISTS threads ( CREATE TABLE threads (
thread_id INTEGER NOT NULL ON CONFLICT ROLLBACK, thread_id INTEGER NOT NULL ON CONFLICT ROLLBACK,
thread_title TEXT NOT NULL ON CONFLICT ROLLBACK, thread_title TEXT NOT NULL ON CONFLICT ROLLBACK,
thread_subf INTEGER REFERENCES categories (cat_id), thread_subf INTEGER REFERENCES categories (cat_id),
@ -89,7 +89,7 @@ CREATE TABLE IF NOT EXISTS threads (
-- Table: users -- Table: users
DROP TABLE IF EXISTS users; DROP TABLE IF EXISTS users;
CREATE TABLE IF NOT EXISTS users ( CREATE TABLE users (
user_id INTEGER NOT NULL ON CONFLICT ROLLBACK user_id INTEGER NOT NULL ON CONFLICT ROLLBACK
UNIQUE ON CONFLICT ROLLBACK, UNIQUE ON CONFLICT ROLLBACK,
username TEXT NOT NULL ON CONFLICT ROLLBACK username TEXT NOT NULL ON CONFLICT ROLLBACK
@ -98,7 +98,7 @@ CREATE TABLE IF NOT EXISTS users (
NOT NULL ON CONFLICT ROLLBACK, NOT NULL ON CONFLICT ROLLBACK,
password TEXT NOT NULL ON CONFLICT ROLLBACK, password TEXT NOT NULL ON CONFLICT ROLLBACK,
salt TEXT NOT NULL ON CONFLICT ROLLBACK, salt TEXT NOT NULL ON CONFLICT ROLLBACK,
signup_date REAL NOT NULL, signup_date INTEGER NOT NULL,
PRIMARY KEY ( PRIMARY KEY (
user_id AUTOINCREMENT user_id AUTOINCREMENT
) )

View File

@ -1,4 +1,5 @@
package CharmBoard; package CharmBoard;
use experimental 'try', 'smartmatch';
use Mojo::Base 'Mojolicious', -signatures; use Mojo::Base 'Mojolicious', -signatures;
use CharmBoard::Schema; use CharmBoard::Schema;

View File

@ -1,4 +1,5 @@
package CharmBoard::Controller::Auth; package CharmBoard::Controller::Auth;
use experimental 'try', 'smartmatch';
use Mojo::Base 'Mojolicious::Controller', -signatures; use Mojo::Base 'Mojolicious::Controller', -signatures;
use CharmBoard::Crypt::Password; use CharmBoard::Crypt::Password;
use CharmBoard::Crypt::Seasoning; use CharmBoard::Crypt::Seasoning;
@ -8,52 +9,53 @@ sub register ($app) {
$app->render( $app->render(
template => 'register', template => 'register',
error => $app->flash('error'), error => $app->flash('error'),
message => $app->flash('message') message => $app->flash('message'))};
)};
# process submitted registration form # process submitted registration form
# TODO: implement email validation here at some point
sub register_do ($app) { sub register_do ($app) {
my $username = $app->param('username'); my $username = $app->param('username');
my $email = $app->param('email'); my $email = $app->param('email');
my $password = $app->param('password'); my $password = $app->param('password');
my $confirmPassword = $app->param('confirm-password'); my $confirmPassword = $app->param('confirm-password');
my $catchError;
# declare vars used through multiple try/catch blocks with
# 'our' so they work throughout the entire subroutine
our ($userCheck, $emailCheck, $salt, $hash);
try { # make sure registration info is valid
# TODO: implement email validation here at some point
# check to make sure all required fields are filled # check to make sure all required fields are filled
if (! $username || ! $password || ! $confirmPassword) { ($username, $email, $password, $confirmPassword) ne undef
$app->flash(error => 'All fields required.'); or die "Please fill out all required fields.";
$app->redirect_to('register')};
# check to make sure both passwords match # check to make sure both passwords match
# TODO: add check on frontend for this for people with JS enabled # TODO: add check on frontend for this for people with JS enabled
if ($confirmPassword ne $password) { $password eq $confirmPassword
$app->flash(error => 'Passwords do not match.'); or die "Passwords do not match";
$app->redirect_to('register')};
# check to make sure username and/or email isn't already in use; # check to make sure username and/or email isn't already in use;
# if not, continue with registration # if not, continue with registration
## search for input username and email in database ## search for input username and email in database
my $userCheck = $app->schema->resultset('Users')->search({username => $username})->single; $userCheck = $app->schema->resultset('Users')->search({username => $username})->single;
my $emailCheck = $app->schema->resultset('Users')->search({email => $email})->single; $emailCheck = $app->schema->resultset('Users')->search({email => $email})->single;
if ($userCheck || $emailCheck) { ($userCheck && $emailCheck) eq undef
if ($userCheck && $emailCheck) { # notify user that username and email are both already being used or die "Username already in use.\nemail already in use.";
$app->flash(error => 'Username and email already in use.'); ($userCheck) eq undef
$app->redirect_to('register')} or die "Username already in use.";
($emailCheck) eq undef
or die "email already in use."}
catch ($catchError) {
$app->flash(error => $catchError);
$app->redirect_to('register');}
elsif ($userCheck) { # notify user that only username is already in use try {
$app->flash(error => 'Username is already in use.');
$app->redirect_to('register')}
elsif ($emailCheck) { # notify user that only email is already in use
$app->flash(error => 'email is already in use.');
$app->redirect_to('register')}}
else { # TODO: add more error handling here, in case SQL transact fails
# append pepper to pass before hashing
$password = $app->pepper . ':' . $password; $password = $app->pepper . ':' . $password;
# return hashed result + salt # return hashed result + salt
my ($salt, $hash) = passgen($password); ($salt, $hash) = passgen($password) or die;
# add user info and pw/salt to DB # add user info and pw/salt to DB
$app->schema->resultset('Users')->create({ $app->schema->resultset('Users')->create({
@ -61,9 +63,15 @@ sub register_do ($app) {
email => $email, email => $email,
password => $hash, password => $hash,
salt => $salt, salt => $salt,
signup_date => time }); signup_date => time }) or die;
$app->flash(message => 'User registered successfully!'); $app->flash(message => 'User registered successfully!');
$app->redirect_to('register')}
catch ($catchError) {
print $catchError;
$app->flash(error => 'Your registration info was correct,
but a server error prevented you from registering.
This has been logged so your administrator can fix it.');
$app->redirect_to('register')}}; $app->redirect_to('register')}};
sub login ($app) { sub login ($app) {
@ -76,42 +84,56 @@ sub login_do ($app) {
my $username = $app->param('username'); my $username = $app->param('username');
my $password = $app->pepper . ':' . $app->param('password'); my $password = $app->pepper . ':' . $app->param('password');
try { my $catchError;
# declare vars used through multiple try/catch blocks with
# 'our' so they work throughout the entire subroutine
our ($userInfo, $passCheck, $userID, $sessionKey);
try { # check user credentials first
# check to see if user by entered username exists # check to see if user by entered username exists
my $userInfo = $app->schema->resultset('Users')->search({username => $username}); $userInfo = $app->schema->resultset('Users')->search({username => $username});
$userInfo or die; $userInfo or die;
# now check password validity # now check password validity
my $passCheck = passchk($userInfo->get_column('salt')->first, $passCheck = passchk($userInfo->get_column('salt')->first,
$userInfo->get_column('password')->first, $password); $userInfo->get_column('password')->first, $password);
$passCheck or die; $passCheck or die;}
catch ($catchError) { # redirect to login page on fail
print $catchError;
$app->flash(error => 'Username or password incorrect.');
$app->redirect_to('login');}
try { # now attempt to create session
# get user ID for session creation # get user ID for session creation
my $userID = $userInfo->get_column('user_id')->first; $userID = $userInfo->get_column('user_id')->first;
# gen session key and set expiry time # gen session key
my $sessionKey = seasoning(16); $sessionKey = seasoning(16);
my $sessionExpiry = time + 604800;
# add session to database # add session to database
$app->schema->resultset('Session')->create({ $app->schema->resultset('Session')->create({
user_id => $userID,
session_key => $sessionKey, session_key => $sessionKey,
session_expiry => $sessionExpiry, user_id => $userID,
session_expiry => time + 604800,
is_ip_bound => 0, is_ip_bound => 0,
bound_ip => undef }); bound_ip => undef }) or die;
# now create session cookie for user # now create session cookie for user
$app->session(is_auth => 1); $app->session(is_auth => 1);
$app->session(user_id => $userID); $app->session(user_id => $userID);
$app->session(session_key => $sessionKey); $app->session(session_key => $sessionKey);
$app->session(expires => $sessionExpiry); $app->session(expiration => 604800);
# redirect to index upon success # redirect to index upon success
$app->redirect_to('/')} $app->redirect_to('/')}
catch ($error) { # redir to login page on fail
print $error; catch ($catchError) { # redirect to login page on fail
$app->flash(error => 'Username or password incorrect.'); print $catchError;
$app->redirect_to('login')}}; $app->flash(error => 'Your username and password were correct,
but a server error prevented you from logging in.
This has been logged so your administrator can fix it.');
$app->redirect_to('login')}}
1; 1;

View File

@ -3,14 +3,14 @@ use base qw(DBIx::Class::Core);
__PACKAGE__->table('sessions'); __PACKAGE__->table('sessions');
__PACKAGE__->add_columns( __PACKAGE__->add_columns(
user_id => {
data_type => 'integer',
is_auto_increment => 0,
is_nullable => 0, },
session_key => { session_key => {
data_type => 'text', data_type => 'text',
is_auto_increment => 0, is_auto_increment => 0,
is_nullable => 0, }, is_nullable => 0, },
user_id => {
data_type => 'integer',
is_auto_increment => 0,
is_nullable => 0, },
session_expiry => { session_expiry => {
data_type => 'numeric', data_type => 'numeric',
is_auto_increment => 0, is_auto_increment => 0,
@ -24,7 +24,7 @@ __PACKAGE__->add_columns(
is_auto_increment => 0, is_auto_increment => 0,
is_nullable => 1, }); is_nullable => 1, });
__PACKAGE__->set_primary_key('user_id'); __PACKAGE__->set_primary_key('session_key');
__PACKAGE__->belongs_to( __PACKAGE__->belongs_to(
user_id => user_id =>

View File

@ -1,5 +1,5 @@
#!/usr/bin/env perl #!/usr/bin/env perl
use experimental 'try', 'smartmatch';
use strict; use strict;
use warnings; use warnings;
use utf8; use utf8;