From 68b66d431c12f300521638816bc105b8f3babad2 Mon Sep 17 00:00:00 2001 From: Tom Bloor Date: Tue, 18 Apr 2017 22:31:08 +0100 Subject: [PATCH] Reworked api login to just need a session_key for everything --- lib/Pear/LocalLoop.pm | 71 +++---- lib/Pear/LocalLoop/Controller/Api/Auth.pm | 176 +++++++++--------- lib/Pear/LocalLoop/Controller/Api/Register.pm | 7 +- .../LocalLoop/Schema/Result/SessionToken.pm | 2 +- lib/Pear/LocalLoop/Schema/Result/User.pm | 15 +- lib/Test/Pear/LocalLoop.pm | 41 ++++ t/api/login.t | 73 ++++++++ t/login.t | 73 ++++---- t/register.t | 27 +-- 9 files changed, 271 insertions(+), 214 deletions(-) create mode 100644 lib/Test/Pear/LocalLoop.pm create mode 100644 t/api/login.t diff --git a/lib/Pear/LocalLoop.pm b/lib/Pear/LocalLoop.pm index f6fac56..9d08712 100644 --- a/lib/Pear/LocalLoop.pm +++ b/lib/Pear/LocalLoop.pm @@ -7,6 +7,7 @@ use Email::Valid; use Authen::Passphrase::BlowfishCrypt; use Scalar::Util qw(looks_like_number); use Pear::LocalLoop::Schema; +use DateTime; has schema => sub { my $c = shift; @@ -23,7 +24,7 @@ sub startup { $self->plugin('Config', { default => { sessionTimeSeconds => 60 * 60 * 24 * 7, - sessionTokenJsonName => 'sessionToken', + sessionTokenJsonName => 'session_key', sessionExpiresJsonName => 'sessionExpires', }, }); @@ -54,52 +55,27 @@ sub startup { $r->get('/register')->to('register#index'); $r->post('/register')->to('register#register'); $r->any('/logout')->to('root#auth_logout'); - my $api = $r->under('/api' => sub { - my $c = shift; - #See if logged in. - my $sessionToken = $c->get_session_token(); + # Always available api routes + $r->post('api/login')->to('api-auth#post_login'); + $r->post('api/register')->to('api-register#post_register'); + $r->post('api/logout')->to('api-auth#post_logout'); - #0 = no session, npn-0 is has updated session - my $hasBeenExtended = $c->extend_session($sessionToken); + my $api = $r->under('/api')->to('api-auth#auth'); - my $path = $c->req->url->to_abs->path; - - #Has valid session - if ($hasBeenExtended) { - #If logged in and requestine the login page redirect to the main page. - if ($path eq '/api/login') { - #Force expire and redirect. - $c->res->code(303); - $c->redirect_to('/api'); - return undef; - } - } - #Has expired or did not exist in the first place and the path is not login - elsif ($path ne '/api/login' && $path ne '/api/register') { - $c->res->code(303); - $c->redirect_to('/api/login'); - return undef; - } - return 1; - }); - - $api->post("/register")->to('api-register#post_register'); - $api->post("/upload")->to('api-upload#post_upload'); - $api->post("/search")->to('api-upload#post_search'); - $api->post("/admin-approve")->to('api-admin#post_admin_approve'); - $api->post("/admin-merge")->to('api-admin#post_admin_merge'); - $api->get("/login")->to('api-auth#get_login'); - $api->post("/login")->to('api-auth#post_login'); - $api->post("/logout")->to('api-auth#post_logout'); - $api->post("/edit")->to('api-api#post_edit'); - $api->post("/fetchuser")->to('api-api#post_fetchuser'); - $api->post("/user-history")->to('api-user#post_user_history'); - - $api->any( '/' => sub { - my $self = shift; - return $self->render(json => { success => Mojo::JSON->true }); + $api->post('/' => sub { + return shift->render( json => { + success => Mojo::JSON->true, + message => 'Successful Auth', + }); }); + $api->post('/upload')->to('api-upload#post_upload'); + $api->post('/search')->to('api-upload#post_search'); + $api->post('/admin-approve')->to('api-admin#post_admin_approve'); + $api->post('/admin-merge')->to('api-admin#post_admin_merge'); + $api->post('/edit')->to('api-api#post_edit'); + $api->post('/fetchuser')->to('api-api#post_fetchuser'); + $api->post('/user-history')->to('api-user#post_user_history'); my $admin_routes = $r->under('/admin')->to('admin#under'); @@ -200,15 +176,11 @@ $self->helper(generate_session => sub { my ($self, $userId) = @_; my $sessionToken = $self->generate_session_token(); - my $expireDateTime = $self->session_token_expiry_date_time(); my $insertStatement = $self->db->prepare('INSERT INTO SessionTokens (SessionTokenName, UserIdAssignedTo_FK, ExpireDateTime) VALUES (?, ?, ?)'); - my $rowsAdded = $insertStatement->execute($sessionToken, $userId, $expireDateTime); - - $self->session(expires => $expireDateTime); - $self->session->{$self->app->config->{sessionTokenJsonName}} = $sessionToken; + my $rowsAdded = $insertStatement->execute($sessionToken, $userId, DateTime->now()->add( years => 1 )); - return {$self->app->config->{sessionTokenJsonName} => $sessionToken, $self->app->config->{sessionExpiresJsonName} => $expireDateTime}; + return $sessionToken; }); $self->helper(generate_session_token => sub { @@ -282,6 +254,7 @@ $self->helper(get_session_expiry => sub { sessiontokenname => $sessionToken, })->delete_all; + ## TODO Does this need a seperate session cookie? $self->session(expires => 1); $self->session->{$self->app->config->{sessionTokenJsonName}} = $sessionToken; diff --git a/lib/Pear/LocalLoop/Controller/Api/Auth.pm b/lib/Pear/LocalLoop/Controller/Api/Auth.pm index 9615096..302a169 100644 --- a/lib/Pear/LocalLoop/Controller/Api/Auth.pm +++ b/lib/Pear/LocalLoop/Controller/Api/Auth.pm @@ -3,121 +3,113 @@ use Mojo::Base 'Mojolicious::Controller'; use Data::Dumper; use Mojo::JSON; +has error_messages => sub { + return { + email => { + required => { message => 'No email sent.', status => 400 }, + email => { message => 'Email is invalid.', status => 400 }, + }, + password => { + required => { message => 'No password sent.', status => 400 }, + }, + }; +}; -#FIXME placeholders -#Because of "before_dispatch" this will never be accessed unless the user is not logged in. -sub get_login { - my $self = shift; - return $self->render( success => Mojo::JSON->true, text => 'This will be the login page.', status => 200 ); +sub auth { + my $c = shift; + + my $session_key = $c->req->json( '/session_key' ); + + my $session_result = $c->schema->resultset('SessionToken')->find({ sessiontokenname => $session_key }); + + if ( defined $session_result ) { + $c->stash( api_user => $session_result->user ); + return 1; + } + + $c->render( + json => { + success => Mojo::JSON->false, + message => 'Invalid Session', + }, + status => 401, + ); + return 0; } -#TODO set session cookie and add it to the database. -#FIXME This suffers from replay attacks, consider a challenge response. Would TLS solve this, most likely. -#SessionToken -#Because of "before_dispatch" this will never be accessed unless the user is not logged in. sub post_login { - my $self = shift; + my $c = shift; - my $json = $self->req->json; - $self->app->log->debug( "\n\nStart of login"); - $self->app->log->debug( "JSON: " . Dumper $json ); + my $validation = $c->validation; + + my $json = $c->req->json; if ( ! defined $json ){ - $self->app->log->debug('Path Error: file:' . __FILE__ . ', line: ' . __LINE__); - return $self->render( json => { + return $c->render( json => { success => Mojo::JSON->false, message => 'No json sent.', }, - status => 400,); #Malformed request + status => 400); #Malformed request } - my $email = $json->{email}; - if ( ! defined $email ){ - $self->app->log->debug('Path Error: file:' . __FILE__ . ', line: ' . __LINE__); - return $self->render( json => { - success => Mojo::JSON->false, - message => 'No email sent.', - }, - status => 400,); #Malformed request - } - elsif ( ! $self->valid_email($email) ) { - $self->app->log->debug('Path Error: file:' . __FILE__ . ', line: ' . __LINE__); - return $self->render( json => { - success => Mojo::JSON->false, - message => 'email is invalid.', - }, - status => 400,); #Malformed request + $validation->input( $json ); + $validation->required('email')->email; + $validation->required('password'); + + my $email = $validation->param('email'); + my $password = $validation->param('password'); + + if ( $validation->has_error ) { + my $failed_vals = $validation->failed; + for my $val ( @$failed_vals ) { + my $check = shift @{ $validation->error($val) }; + return $c->render( + json => { + success => Mojo::JSON->false, + message => $c->error_messages->{$val}->{$check}->{message}, + }, + status => $c->error_messages->{$val}->{$check}->{status}, + ); + } } - my $password = $json->{password}; - if ( ! defined $password ){ - $self->app->log->debug('Path Error: file:' . __FILE__ . ', line: ' . __LINE__); - return $self->render( json => { - success => Mojo::JSON->false, - message => 'No password sent.', - }, - status => 400,); #Malformed request - } + my $user_result = $c->schema->resultset('User')->find({ email => $email }); + + if ( defined $user_result ) { + if ( $user_result->check_password($password) ) { + my $session_key = $c->generate_session( $user_result->userid ); - - #FIXME There is a timing attack here determining if an email exists or not. - if ($self->does_email_exist($email) && $self->check_password_email($email, $password)) { - #Match. - $self->app->log->debug('Path Success: file:' . __FILE__ . ', line: ' . __LINE__); - - my $userId = $self->get_userid_foreign_key($email); - - #Generates and stores - my $hash = $self->generate_session($userId); - - $self->app->log->debug('session dump:' . Dumper ($hash)); - - return $self->render( json => { - success => Mojo::JSON->true, - $self->config->{sessionTokenJsonName} => $hash->{$self->config->{sessionTokenJsonName}}, - $self->config->{sessionExpiresJsonName} => $hash->{$self->config->{sessionExpiresJsonName}}, - }); - } - else{ - #Mismatch - $self->app->log->debug('Path Error: file:' . __FILE__ . ', line: ' . __LINE__); - return $self->render( json => { - success => Mojo::JSON->false, - message => 'Email or password is invalid.', - }, - status => 401,); #Unauthorized request + return $c->render( json => { + success => Mojo::JSON->true, + session_key => $session_key, + }); + } + } else { + return $c->render( + json => { + success => Mojo::JSON->false, + message => 'Email or password is invalid.', + }, + status => 401 + ); } } sub post_logout { - my $self = shift; + my $c = shift; - my $json = $self->req->json; - $self->app->log->debug( "\n\nStart of logout"); - $self->app->log->debug( "JSON: " . Dumper $json ); + my $session_key = $c->req->json( '/session_key' ); - #If the session token exists. - if ($self->expire_current_session()) { - $self->app->log->debug('Path Success: file:' . __FILE__ . ', line: ' . __LINE__); - return $self->render( json => { - success => Mojo::JSON->true, - message => 'you were successfully logged out.', - }); - } - #Due to the "before_dispatch" hook, this most likely will not be called. i.e. race conditions. - #FIXME untested. - #An invalid token was presented, most likely because it has expired. - else { - $self->app->log->debug('Path Error: file:' . __FILE__ . ', line: ' . __LINE__); - return $self->render( json => { - success => Mojo::JSON->false, - message => 'the session has expired or did not exist in the first place.', - }, - status => 401,); #Unauthorized request + my $session_result = $c->schema->resultset('SessionToken')->find({ sessiontokenname => $session_key }); + + if ( defined $session_result ) { + $session_result->delete; } + $c->render( json => { + success => Mojo::JSON->true, + message => 'Logged Out', + }); } - - 1; diff --git a/lib/Pear/LocalLoop/Controller/Api/Register.pm b/lib/Pear/LocalLoop/Controller/Api/Register.pm index 924ebdc..4bbe97a 100644 --- a/lib/Pear/LocalLoop/Controller/Api/Register.pm +++ b/lib/Pear/LocalLoop/Controller/Api/Register.pm @@ -105,9 +105,6 @@ sub post_register{ my $postcode = $validation->param('postcode'); my $password = $validation->param('password'); - # TODO Replace with Password Column encoding - my $hashedPassword = $c->generate_hashed_password($password); - if ($usertype eq 'customer'){ # TODO replace with actually using the value on the post request my $ageForeignKey = $c->get_age_foreign_key( $validation->param('age') ); @@ -124,7 +121,7 @@ sub post_register{ postcode => $postcode, }, email => $email, - hashedpassword => $hashedPassword, + hashedpassword => $password, joindate => DateTime->now, }); }); @@ -145,7 +142,7 @@ sub post_register{ postcode => $postcode, }, email => $email, - hashedpassword => $hashedPassword, + hashedpassword => $password, joindate => DateTime->now, }); }); diff --git a/lib/Pear/LocalLoop/Schema/Result/SessionToken.pm b/lib/Pear/LocalLoop/Schema/Result/SessionToken.pm index 3458fdb..fddf569 100644 --- a/lib/Pear/LocalLoop/Schema/Result/SessionToken.pm +++ b/lib/Pear/LocalLoop/Schema/Result/SessionToken.pm @@ -107,7 +107,7 @@ Related object: L =cut __PACKAGE__->belongs_to( - "useridassignedto_fk", + "user", "Pear::LocalLoop::Schema::Result::User", { userid => "useridassignedto_fk" }, { is_deferrable => 0, on_delete => "NO ACTION", on_update => "NO ACTION" }, diff --git a/lib/Pear/LocalLoop/Schema/Result/User.pm b/lib/Pear/LocalLoop/Schema/Result/User.pm index 31ab082..c814194 100644 --- a/lib/Pear/LocalLoop/Schema/Result/User.pm +++ b/lib/Pear/LocalLoop/Schema/Result/User.pm @@ -25,7 +25,7 @@ use base 'DBIx::Class::Core'; =cut -__PACKAGE__->load_components("InflateColumn::DateTime"); +__PACKAGE__->load_components("InflateColumn::DateTime", "PassphraseColumn"); =head1 TABLE: C @@ -82,7 +82,18 @@ __PACKAGE__->add_columns( "joindate", { data_type => "datetime", is_nullable => 0 }, "hashedpassword", - { data_type => "text", is_nullable => 0 }, + { + data_type => "varchar", + is_nullable => 0, + size => 100, + passphrase => 'crypt', + passphrase_class => 'BlowfishCrypt', + passphrase_args => { + salt_random => 1, + cost => 8, + }, + passphrase_check_method => 'check_password', + }, ); =head1 PRIMARY KEY diff --git a/lib/Test/Pear/LocalLoop.pm b/lib/Test/Pear/LocalLoop.pm new file mode 100644 index 0000000..f18e24e --- /dev/null +++ b/lib/Test/Pear/LocalLoop.pm @@ -0,0 +1,41 @@ +package Test::Pear::LocalLoop; +use Mojo::Base -base; + +use File::Temp; +use Test::Mojo; + +has config => sub { + my $file = File::Temp->new; + + print $file <<'END'; +{ + dsn => "dbi:SQLite::memory:", + user => undef, + pass => undef, +} +END + + $file->seek( 0, SEEK_END ); + return $file; +}; + +has framework => sub { + my $self = shift; + + $ENV{MOJO_CONFIG} = $self->config->filename; + + my $t = Test::Mojo->new('Pear::LocalLoop'); + my $schema = $t->app->schema; + $schema->deploy; + + $schema->resultset('AgeRange')->populate([ + [ qw/ agerangestring / ], + [ '20-35' ], + [ '35-50' ], + [ '50+' ], + ]); + + return $t; +}; + +1; diff --git a/t/api/login.t b/t/api/login.t new file mode 100644 index 0000000..f3382c0 --- /dev/null +++ b/t/api/login.t @@ -0,0 +1,73 @@ +use Mojo::Base -strict; + +use Test::More; +use Mojo::JSON; +use Test::Pear::LocalLoop; + +my $framework = Test::Pear::LocalLoop->new; +my $t = $framework->framework; +my $schema = $t->app->schema; + +my $account_token = 'a'; +my $email = 'rufus@shinra.energy'; +my $password = 'MakoGold'; + +$schema->resultset('AccountToken')->create({ + accounttokenname => $account_token +}); + +my $test_json = { + 'usertype' => 'customer', + 'token' => $account_token, + 'username' => 'RufusShinra', + 'email' => $email, + 'postcode' => 'LA1 1AA', + 'password' => $password, + 'age' => '20-35' +}; +$t->post_ok('/api/register' => json => $test_json) + ->status_is(200) + ->json_is('/success', Mojo::JSON->true); +use Data::Dumper; + +is $schema->resultset('User')->count, 1, 'found a user'; + +$t->post_ok('/api' => json => { + session_key => 'invalid', + }) + ->status_is(401) + ->json_is('/success', Mojo::JSON->false) + ->json_like('/message', qr/Invalid Session/); + +$t->post_ok('/api/login' => json => { + email => 'nonexistant@test.com', + password => 'doesnt matter', + }) + ->status_is(401) + ->json_is('/success', Mojo::JSON->false); + +$t->post_ok('/api/login' => json => { + email => $email, + password => $password, + }) + ->status_is(200) + ->json_is('/success', Mojo::JSON->true) + ->json_has('/session_key'); + +my $session_key = $t->tx->res->json->{session_key}; + +$t->post_ok('/api' => json => { session_key => $session_key }) + ->status_is(200) + ->json_is('/success', Mojo::JSON->true) + ->json_like('/message', qr/Successful Auth/); + +is $schema->resultset('SessionToken')->count, 1, 'One Session'; + +$t->post_ok('/api/logout' => json => { session_key => $session_key }) + ->status_is(200) + ->json_is('/success', Mojo::JSON->true) + ->json_like('/message', qr/Logged Out/); + +is $schema->resultset('SessionToken')->count, 0, 'No Sessions'; + +done_testing; diff --git a/t/login.t b/t/login.t index e03b5ae..f579599 100644 --- a/t/login.t +++ b/t/login.t @@ -1,43 +1,55 @@ +use Mojo::Base -strict; + +use File::Temp; use Test::More; use Test::Mojo; use Mojo::JSON; use Time::Fake; -use FindBin; +my $file = File::Temp->new; -BEGIN { - $ENV{MOJO_MODE} = 'testing'; - $ENV{MOJO_LOG_LEVEL} = 'debug'; +print $file <<'END'; +{ + dsn => "dbi:SQLite::memory:", + user => undef, + pass => undef, } +END +$file->seek( 0, SEEK_END ); -my $t = Test::Mojo->new("Pear::LocalLoop"); +$ENV{MOJO_CONFIG} = $file->filename; -my $dbh = $t->app->db; +my $t = Test::Mojo->new('Pear::LocalLoop'); +my $schema = $t->app->schema; +$schema->deploy; -#Dump all pf the test tables and start again. -my $sqlDeployment = Mojo::File->new("$FindBin::Bin/../dropschema.sql")->slurp; -for (split ';', $sqlDeployment){ - $dbh->do($_) or die $dbh->errstr; -} +$schema->resultset('AgeRange')->populate([ + [ qw/ agerangestring / ], + [ '20-35' ], + [ '35-50' ], + [ '50+' ], +]); -my $sqlDeployment = Mojo::File->new("$FindBin::Bin/../schema.sql")->slurp; -for (split ';', $sqlDeployment){ - $dbh->do($_) or die $dbh->errstr; -} +$schema->resultset('AccountToken')->create({ + accounttokenname => 'a', +}); my $accountToken = 'a'; -my $tokenStatement = $dbh->prepare('INSERT INTO AccountTokens (AccountTokenName) VALUES (?)'); -$tokenStatement->execute($accountToken); my $sessionTimeSeconds = 60 * 60 * 24 * 7; #1 week. -my $sessionTokenJsonName = 'sessionToken'; +my $sessionTokenJsonName = 'session_key'; my $sessionExpiresJsonName = 'sessionExpires'; +my $location_is = sub { + my ($t, $value, $desc) = @_; + $desc ||= "Location: $value"; + local $Test::Builder::Level = $Test::Builder::Level + 1; + return $t->success(is($t->tx->res->headers->location, $value, $desc)); +}; #This depends on "register.t" working #Valid customer, this also tests that redirects are disabled for register. -print "test 1 - Initial create user account\n"; my $email = 'rufus@shinra.energy'; my $password = 'MakoGold'; my $testJson = { @@ -53,9 +65,7 @@ $t->post_ok('/api/register' => json => $testJson) ->status_is(200) ->json_is('/success', Mojo::JSON->true); - #Test login, this also checks that redirects are disabled for login when logged out. -print "test 2 - Login (cookies)\n"; $testJson = { 'email' => $email, 'password' => $password, @@ -63,26 +73,7 @@ $testJson = { $t->post_ok('/api/login' => json => $testJson) ->status_is(200) ->json_is('/success', Mojo::JSON->true) - ->json_has("/$sessionTokenJsonName") - ->json_has("/$sessionExpiresJsonName"); - -print "test 3 - Login, no redirect on login paths (cookies)\n"; -#No redirect, as you're logged in. -$t->get_ok('/api/') - ->status_is(200); - -my $location_is = sub { - my ($t, $value, $desc) = @_; - $desc ||= "Location: $value"; - local $Test::Builder::Level = $Test::Builder::Level + 1; - return $t->success(is($t->tx->res->headers->location, $value, $desc)); -}; - -print "test 4 - Login, redirect to root as already logged in (cookies)\n"; -#Check for redirect to root when logged in. -$t->get_ok('/api/login') - ->status_is(303) - ->$location_is('/api'); + ->json_has("/$sessionTokenJsonName"); #Does login/logout work with a cookie based session. diff --git a/t/register.t b/t/register.t index b337852..511a382 100644 --- a/t/register.t +++ b/t/register.t @@ -1,33 +1,12 @@ use Mojo::Base -strict; -use File::Temp; use Test::More; -use Test::Mojo; use Mojo::JSON; +use Test::Pear::LocalLoop; -my $file = File::Temp->new; - -print $file <<'END'; -{ - dsn => "dbi:SQLite::memory:", - user => undef, - pass => undef, -} -END -$file->seek( 0, SEEK_END ); - -$ENV{MOJO_CONFIG} = $file->filename; - -my $t = Test::Mojo->new('Pear::LocalLoop'); +my $framework = Test::Pear::LocalLoop->new; +my $t = $framework->framework; my $schema = $t->app->schema; -$schema->deploy; - -$schema->resultset('AgeRange')->populate([ - [ qw/ agerangestring / ], - [ '20-35' ], - [ '35-50' ], - [ '50+' ], -]); #Variables to be used for uniqueness when testing. my @names = ('a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z');