Skip to content

Commit

Permalink
179: remember me feature
Browse files Browse the repository at this point in the history
- allow user to stay connected
- implement long-lived user tokens
  • Loading branch information
Atmos4 committed Mar 24, 2024
1 parent 7ac03f3 commit ddb3df5
Show file tree
Hide file tree
Showing 12 changed files with 173 additions and 24 deletions.
2 changes: 1 addition & 1 deletion app/pages/index.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?php
if (empty($_SESSION['user_id'])) {
if (empty ($_SESSION['user_id'])) {
redirect("login");
} else {
//redirect("accueil");
Expand Down
4 changes: 3 additions & 1 deletion app/pages/login.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
$v = new Validator();
$login = $v->text("login")->placeholder("Login")->required();
$password = $v->password("password")->placeholder("Password")->autocomplete("current-password")->required();
$rememberMe = $v->switch("remember_me")->label("Rester connecté");
if ($v->valid()) {
AuthService::create()->tryLogin($login->value, $password->value, $v);
AuthService::create()->tryLogin($login->value, $password->value, $rememberMe->value, $v);
}

page("Login")->css("login.css")->disableNav()->heading(false);
Expand All @@ -18,6 +19,7 @@
data-placement="left"><i class="fas fa-circle-info"></i></ins>
</div>
<?= $password->render() ?>
<?= $rememberMe->render() ?>
<?= $v->render_validation() ?>
<button type="submit">Se connecter</button>
</form>
Expand Down
2 changes: 1 addition & 1 deletion app/pages/tokens/reset_password.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?php
$token = AccessToken::retrieve($_GET['token'] ?? "");
$token = AccessToken::retrieveOrFail($_GET['token'] ?? "");
$v = new Validator(["username" => $token->user->login], action: "reset_password_form");
$username = $v->text("username")->autocomplete("username")->label("Login")->readonly();
$new_password = $v->password("new_password")->autocomplete("new-password")->placeholder("Nouveau mot de passe")->required()->secure();
Expand Down
2 changes: 1 addition & 1 deletion app/pages/tokens/user_activation.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?php
$token = AccessToken::retrieve($_GET['token'] ?? "");
$token = AccessToken::retrieveOrFail($_GET['token'] ?? "");
page("Activer le compte")->disableNav()->heading(false);
$v = new Validator(["username" => $token->user->login], action: "validate_form");
$username = $v->text("username")->autocomplete("username")->label("Votre nom d'utilisateur")->readonly();
Expand Down
92 changes: 89 additions & 3 deletions app/services/AuthService.php
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
<?php
class AuthService extends FactoryDependency
{
function tryLogin(string $login, string $password, Validator &$v)
private const REMEMBERME_COOKIE = 'rememberme';

function tryLogin(string $login, string $password, bool|null $rememberMe, Validator &$v)
{
$user = User::getByLogin($login);
if ($user) {
Expand All @@ -25,8 +27,11 @@ function tryLogin(string $login, string $password, Validator &$v)
case $user?->status == UserStatus::ACTIVE:
if (password_verify($password, $user->password)) {
logger()->info("Login successful for user {login}", ["login" => $user->login]);
$_SESSION['user_id'] = $user->id;
$_SESSION['user_permission'] = $user->permission;
$this->loginUserSession($user);
if ($rememberMe) {
$this->createRememberMeToken($user);
}

redirect("/");
return;
}
Expand All @@ -45,11 +50,92 @@ function tryLogin(string $login, string $password, Validator &$v)
$v->set_error("Utilisateur non trouvé");
}

private function loginUserSession(User $user)
{
// prevent session fixation/hijack
session_regenerate_id();
$_SESSION['user_id'] = $user->id;
$_SESSION['user_permission'] = $user->permission;
}

function logout()
{
// Remove session data.
$this->deleteUserTokens($_SESSION['user_id']);
$_SESSION = [];
setcookie(self::REMEMBERME_COOKIE, '', -1);
session_destroy();
}

function createActivationLink(User $user): string
{
$token = new AccessToken($user, AccessTokenType::ACTIVATE, new DateInterval('PT15M'));
em()->persist($token);
em()->flush();
return env("BASE_URL") . "/activation?token=$token->id";
}

function createRememberMeToken(User $user)
{
// Dele
$this->deleteUserTokens($user->id);

$token = new AccessToken($user, AccessTokenType::REMEMBER_ME, new DateInterval('P1M')); // 1month
$validator = $token->createHashedValidator();
em()->persist($token);
em()->flush();

setcookie(
self::REMEMBERME_COOKIE,
"$token->id:$validator",
$token->expiration->getTimestamp(),
httponly: true
);
}

/** Always delete all user tokens when invalidating.
* If you want more custom behavior, write another function. But maybe you shouldn't. */
function deleteUserTokens(string $userId)
{
em()->createQueryBuilder()
->delete(AccessToken::class, 'a')
->where('a.user = :user_id')
->setParameter("user_id", $userId)
->getQuery()->execute();
}

function isUserLoggedIn()
{
if (isset ($_SESSION['user_permission'])) {
return true;
}

$sessionString = filter_input(INPUT_COOKIE, self::REMEMBERME_COOKIE, FILTER_SANITIZE_STRING);
if (!$sessionString)
return false;

[$selector, $validator] = self::parseSessionToken($sessionString);
$token = AccessToken::retrieve($selector);
if (!$token || $token->type !== AccessTokenType::REMEMBER_ME || !$token->hashed_validator)
return false;

// AP 2024-03: we may want to throw an alert here. If this password_verify fails, it is very concerning.
if (!password_verify($validator, $token->hashed_validator))
return false;

// At this point, the user has been verified and can be logged in!
logger()->info("User {userId} logged in with long-lived session token", ["userId" => $token->user->id]);
$this->loginUserSession($token->user);
return true;
}

private static function parseSessionToken(string $token): ?array
{
$parts = explode(':', $token);
if ($parts && count($parts) == 2) {
return [$parts[0], $parts[1]];
}
return null;
}

}
2 changes: 1 addition & 1 deletion app/template/layout.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<!doctype html>
<html lang="en">

<head>
<head hx-head=merge>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<title>
Expand Down
31 changes: 31 additions & 0 deletions database/migrations/Version20240324194709.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

declare(strict_types=1);

namespace intranose\Migrations;

use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;

/**
* Auto-generated Migration: Please modify to your needs!
*/
final class Version20240324194709 extends AbstractMigration
{
public function getDescription(): string
{
return 'add hashed_validator to access tokens';
}

public function up(Schema $schema): void
{
// this up() migration is auto-generated, please modify it to your needs
$this->addSql('ALTER TABLE orm_access_tokens ADD hashed_validator VARCHAR(255) NOT NULL');
}

public function down(Schema $schema): void
{
// this down() migration is auto-generated, please modify it to your needs
$this->addSql('ALTER TABLE orm_access_tokens DROP hashed_validator');
}
}
31 changes: 26 additions & 5 deletions database/models/users.db.php
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ static function getCurrent(): User|null
if (!has_session("user_id")) {
return null;
}
if (isset($_SESSION['controlled_user_id'])) {
if (isset ($_SESSION['controlled_user_id'])) {
Page::getInstance()->controlled();
}
self::$currentUser ??= em()->find(User::class, $_SESSION['controlled_user_id'] ?? $_SESSION['user_id']);
Expand Down Expand Up @@ -247,6 +247,7 @@ enum AccessTokenType: string
case ACTIVATE = 'ACTIVATE';
case INVITE = 'INVITE';
case RESET_PASSWORD = 'RESET_PASSWORD';
case REMEMBER_ME = 'REMEMBER_ME';
}

#[Entity, Table(name: 'access_tokens')]
Expand All @@ -259,6 +260,9 @@ class AccessToken
#[ManyToOne]
public User|null $user = null;

#[Column]
public string|null $hashed_validator = null;

#[Column]
public DateTime $expiration;

Expand All @@ -273,20 +277,37 @@ function __construct(User $user, AccessTokenType $type, DateInterval $duration =
$this->expiration = date_create()->add($duration);
}

static function retrieve(string $uuid): AccessToken
/** This is added security against
* - compromised DB with the hash
* - timing attacks
* Use this for long lived tokens */
function createHashedValidator(): string
{
$validator = bin2hex(random_bytes(32));
$this->hashed_validator = password_hash($validator, PASSWORD_DEFAULT);
return $validator;
}

static function retrieve(string $uuid, bool $forceExit = false): ?AccessToken
{
if (!Uuid::isValid($uuid)) {
force_404("invalid token");
return $forceExit ? force_404("invalid token") : null;
}
// TODO: optimize with DQL if perf problems.
$token = em()->find(AccessToken::class, $uuid);
if (!$token) {
force_404("token not found");
return $forceExit ? force_404("token not found") : null;
}
if (date_create() > $token->expiration) {
em()->remove($token);
em()->flush();
force_404("token expired");
return $forceExit ? force_404("token expired") : null;
}
return $token;
}

static function retrieveOrFail(string $uuid): AccessToken
{
return self::retrieve($uuid, true) ?? throw new Exception("Token not found");
}
}
7 changes: 4 additions & 3 deletions engine/core/Router.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,18 @@ private function cleanBuffer(): self
static function abort(string $message = null, int $code = 404)
{
http_response_code($code);
Page::reset();
$instance = self::getInstance()->cleanBuffer();
if (env('DEVELOPMENT')) {
echo $message;
}
Page::reset();
self::getInstance()->cleanBuffer()->render();
$instance->render();
}

static function getParameter($param, $strict = true, $numeric = true, $pattern = null)
{
$router = self::getInstance();
if (empty($router->dynamicSegments[$param])) {
if (empty ($router->dynamicSegments[$param])) {
if ($strict) {
self::abort(message: "Route parameter $param was not found");
}
Expand Down
15 changes: 9 additions & 6 deletions engine/helpers.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ function page(string $title)
*/
function check_auth($levels = [])
{
if (!isset($_SESSION['user_permission'])) {
redirect("login");
if (!AuthService::create()->isUserLoggedIn()) {
redirect("/");
}
$ulevel = $_SESSION['user_permission'];
foreach ($levels as $level) {
Expand All @@ -118,7 +118,10 @@ function check_auth($levels = [])
*/
function restrict_access($permissions = [])
{
if (!isset($_SESSION['user_permission']) || (count($permissions) && !in_array($_SESSION['user_permission'], $permissions))) {

if (!AuthService::create()->isUserLoggedIn())
redirect("/");
if (!isset ($_SESSION['user_permission']) || (count($permissions) && !in_array($_SESSION['user_permission'], $permissions))) {
$permission = $_SESSION['user_permission']?->value ?? "non authenticated user";
if (get_header('HX-Request')) {
// This is a bit lazy but it's the idea
Expand All @@ -135,7 +138,7 @@ function force_404($message = "")

function has_session($key): bool
{
return isset($_SESSION[$key]);
return isset ($_SESSION[$key]);
}

function restrict_dev()
Expand All @@ -160,14 +163,14 @@ function format_date($date, string $format = null)
/** CSRF Protection */
function set_csrf()
{
if (!isset($_SESSION["csrf"])) {
if (!isset ($_SESSION["csrf"])) {
$_SESSION["csrf"] = bin2hex(random_bytes(50));
}
return '<input type="hidden" name="csrf" value="' . $_SESSION["csrf"] . '">';
}
function is_csrf_valid()
{
if (!isset($_SESSION['csrf']) || !isset($_POST['csrf'])) {
if (!isset ($_SESSION['csrf']) || !isset ($_POST['csrf'])) {
return false;
}
if ($_SESSION['csrf'] != $_POST['csrf']) {
Expand Down
4 changes: 2 additions & 2 deletions routes.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@

// Logout
Router::add('/logout', function () {
session_destroy();
redirect("login");
AuthService::create()->logout();
redirect("/login");
});

// Special route, see router
Expand Down
5 changes: 5 additions & 0 deletions tests/integration/AuthServiceTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?php declare(strict_types=1);

final class AuthServiceTest extends BaseTestCase
{
}

0 comments on commit ddb3df5

Please sign in to comment.