Skip to content

Add API endpoint for destroying tokens #584

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

Merged
merged 8 commits into from
Feb 27, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions docs/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -1151,6 +1151,31 @@
"$ref": "#/components/responses/401"
}
}
},
"delete" : {
"tags": [
"Authentication"
],
"description": "Delete the authentication token provided in the X-Auth-Token header value.",
"parameters": [
{
"in": "header",
"name": "X-Auth-Token",
"schema": {
"type": "string"
},
"required": true
}
],
"requestBody": {},
"responses": {
"204": {
"description": "Returned if authentication token does no longer exist"
},
"400": {
"$ref": "#/components/responses/400"
}
}
}
}
},
Expand Down
8 changes: 8 additions & 0 deletions public/js/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -498,3 +498,11 @@ function addAlert(parentDivId, message, color, withCloseButton = true, marginBot
function removeAlert(parentDivId) {
document.getElementById(parentDivId).innerHTML = ''
}

async function logout() {
await fetch('/api/authentication/token', {
method: 'DELETE',
});

window.location.href = '/'
Copy link
Owner

@leepeuker leepeuker Feb 25, 2024

Choose a reason for hiding this comment

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

info: I only wanted to reload the current page initially on logout, so that the user is only redirect to the login page if he was on a protected page, but that this would display the Sign in to access page alert on the login page, which seems weird if the user purposefully logged out, so I decided for the redirect on default

}
5 changes: 5 additions & 0 deletions public/js/login.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ async function submitCredentials() {
return;
}

const forbiddenPageAlert = document.getElementById('forbiddenPageAlert');
Copy link
Owner

Choose a reason for hiding this comment

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

info: while testing the logout I noticed that the Sign in to access page did not vanish when other error messages are displayed (e.g. Invalid credentials). Displaying two alerts seems weird, so I added this

if (forbiddenPageAlert) {
forbiddenPageAlert.classList.add('d-none');
}

if (response.status === 400) {
const error = await response.json();

Expand Down
3 changes: 1 addition & 2 deletions settings/routes.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ function addWebRoutes(RouterService $routerService, FastRoute\RouteCollector $ro

$routes->add('GET', '/', [Web\LandingPageController::class, 'render'], [Web\Middleware\UserIsUnauthenticated::class, Web\Middleware\ServerHasNoUsers::class]);
$routes->add('GET', '/login', [Web\AuthenticationController::class, 'renderLoginPage'], [Web\Middleware\UserIsUnauthenticated::class]);
$routes->add('POST', '/login', [Web\AuthenticationController::class, 'login']);
$routes->add('GET', '/logout', [Web\AuthenticationController::class, 'logout']);
$routes->add('POST', '/create-user', [Web\CreateUserController::class, 'createUser'], [
Web\Middleware\UserIsUnauthenticated::class,
Web\Middleware\ServerHasUsers::class,
Expand Down Expand Up @@ -203,6 +201,7 @@ function addApiRoutes(RouterService $routerService, FastRoute\RouteCollector $ro

$routes->add('GET', '/openapi', [Api\OpenApiController::class, 'getSchema']);
$routes->add('POST', '/authentication/token', [Api\AuthenticationController::class, 'createToken']);
$routes->add('DELETE', '/authentication/token', [Api\AuthenticationController::class, 'destroyToken']);

$routeUserHistory = '/users/{username:[a-zA-Z0-9]+}/history/movies';
$routes->add('GET', $routeUserHistory, [Api\HistoryController::class, 'getHistory'], [Api\Middleware\IsAuthorizedToReadUserData::class]);
Expand Down
6 changes: 3 additions & 3 deletions src/Domain/User/Service/Authentication.php
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public function getUserIdByApiToken(Request $request) : ?int
return $this->userApi->findByToken($apiToken)?->getId();
}

public function isUserAuthenticated() : bool
public function isUserAuthenticatedWithCookie() : bool
{
$token = filter_input(INPUT_COOKIE, self::AUTHENTICATION_COOKIE_NAME);

Expand Down Expand Up @@ -152,11 +152,11 @@ public function isUserPageVisibleForCurrentUser(int $privacyLevel, int $userId)
return true;
}

if ($privacyLevel === 1 && $this->isUserAuthenticated() === true) {
if ($privacyLevel === 1 && $this->isUserAuthenticatedWithCookie() === true) {
return true;
}

return $this->isUserAuthenticated() === true && $this->getCurrentUserId() === $userId;
return $this->isUserAuthenticatedWithCookie() === true && $this->getCurrentUserId() === $userId;
}

public function isValidToken(string $token) : bool
Expand Down
6 changes: 3 additions & 3 deletions src/Domain/User/Service/UserPageAuthorizationChecker.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public function __construct(

public function fetchAllHavingWatchedMovieVisibleUsernamesForCurrentVisitor(int $movieId) : array
{
if ($this->authenticationService->isUserAuthenticated() === false) {
if ($this->authenticationService->isUserAuthenticatedWithCookie() === false) {
return $this->userApi->fetchAllHavingWatchedMoviePublicVisibleUsernames($movieId);
}

Expand All @@ -24,7 +24,7 @@ public function fetchAllHavingWatchedMovieVisibleUsernamesForCurrentVisitor(int

public function fetchAllHavingWatchedMovieWithPersonVisibleUsernamesForCurrentVisitor(int $personId) : array
{
if ($this->authenticationService->isUserAuthenticated() === false) {
if ($this->authenticationService->isUserAuthenticatedWithCookie() === false) {
return $this->userApi->fetchAllHavingWatchedMovieWithPersonPublicVisibleUsernames($personId);
}

Expand All @@ -33,7 +33,7 @@ public function fetchAllHavingWatchedMovieWithPersonVisibleUsernamesForCurrentVi

public function fetchAllVisibleUsernamesForCurrentVisitor() : array
{
if ($this->authenticationService->isUserAuthenticated() === false) {
if ($this->authenticationService->isUserAuthenticatedWithCookie() === false) {
return $this->userApi->fetchAllPublicVisibleUsernames();
}

Expand Down
2 changes: 1 addition & 1 deletion src/Factory.php
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ public static function createTwigEnvironment(ContainerInterface $container) : Tw
$currentRequest = $container->get(Request::class);
$routeUsername = $currentRequest->getRouteParameters()['username'] ?? null;

$userAuthenticated = $container->get(Authentication::class)->isUserAuthenticated();
$userAuthenticated = $container->get(Authentication::class)->isUserAuthenticatedWithCookie();

$twig->addGlobal('loggedIn', $userAuthenticated);

Expand Down
27 changes: 26 additions & 1 deletion src/HttpController/Api/AuthenticationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use Movary\Domain\User\Exception\InvalidTotpCode;
use Movary\Domain\User\Exception\MissingTotpCode;
use Movary\Domain\User\Service\Authentication;
use Movary\Domain\User\UserApi;
use Movary\Util\Json;
use Movary\ValueObject\Http\Header;
use Movary\ValueObject\Http\Request;
Expand All @@ -22,7 +23,7 @@ public function createToken(Request $request) : Response
{
$tokenRequestBody = Json::decode($request->getBody());

if ($tokenRequestBody['email'] === null || $tokenRequestBody['password'] === null) {
if (isset($tokenRequestBody['email']) === false || isset($tokenRequestBody['password']) === false) {
return Response::createBadRequest(
Json::encode([
'error' => 'MissingCredentials',
Expand Down Expand Up @@ -89,4 +90,28 @@ public function createToken(Request $request) : Response
]),
);
}

public function destroyToken(Request $request) : Response
{
if ($this->authenticationService->isUserAuthenticatedWithCookie() === true) {
$this->authenticationService->logout();

return Response::CreateNoContent();
}

$apiToken = $request->getHeaders()['X-Auth-Token'] ?? null;
if ($apiToken === null) {
return Response::createBadRequest(
Json::encode([
'error' => 'MissingAuthToken',
'message' => 'Authentication token to delete in headers missing'
]),
[Header::createContentTypeJson()],
);
}

$this->authenticationService->deleteToken($apiToken);

return Response::CreateNoContent();
}
}
2 changes: 1 addition & 1 deletion src/HttpController/Web/ActorsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public function renderPage(Request $request) : Response
$requestData = $this->requestMapper->mapRenderPageRequest($request);

$currentUserId = null;
if ($this->authenticationService->isUserAuthenticated() === true) {
if ($this->authenticationService->isUserAuthenticatedWithCookie() === true) {
$currentUserId = $this->authenticationService->getCurrentUserId();
}

Expand Down
17 changes: 0 additions & 17 deletions src/HttpController/Web/AuthenticationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,7 @@
namespace Movary\HttpController\Web;

use Movary\Domain\SessionService;
use Movary\Domain\User\Exception\InvalidCredentials;
use Movary\Domain\User\Exception\InvalidTotpCode;
use Movary\Domain\User\Exception\MissingTotpCode;
use Movary\Domain\User\Service;
use Movary\Util\SessionWrapper;
use Movary\ValueObject\Http\Header;
use Movary\ValueObject\Http\Request;
use Movary\ValueObject\Http\Response;
use Movary\ValueObject\Http\StatusCode;
Expand All @@ -18,22 +13,10 @@ class AuthenticationController
{
public function __construct(
private readonly Environment $twig,
private readonly Service\Authentication $authenticationService,
private readonly SessionWrapper $sessionWrapper,
) {
}

public function logout() : Response
{
$this->authenticationService->logout();

return Response::create(
StatusCode::createSeeOther(),
null,
[Header::createLocation('/')],
);
}

public function renderLoginPage(Request $request) : Response
{
$failedLogin = $this->sessionWrapper->has('failedLogin');
Expand Down
2 changes: 1 addition & 1 deletion src/HttpController/Web/DashboardController.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public function render(Request $request) : Response
}

$currentUserId = null;
if ($this->authenticationService->isUserAuthenticated() === true) {
if ($this->authenticationService->isUserAuthenticatedWithCookie() === true) {
$currentUserId = $this->authenticationService->getCurrentUserId();
}

Expand Down
2 changes: 1 addition & 1 deletion src/HttpController/Web/DirectorsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public function renderPage(Request $request) : Response
$requestData = $this->requestMapper->mapRenderPageRequest($request);

$currentUserId = null;
if ($this->authenticationService->isUserAuthenticated() === true) {
if ($this->authenticationService->isUserAuthenticatedWithCookie() === true) {
$currentUserId = $this->authenticationService->getCurrentUserId();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public function __construct(

public function __invoke() : ?Response
{
if ($this->authenticationService->isUserAuthenticated() === true) {
if ($this->authenticationService->isUserAuthenticatedWithCookie() === true) {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public function __construct(

public function __invoke() : ?Response
{
if ($this->authenticationService->isUserAuthenticated() === false) {
if ($this->authenticationService->isUserAuthenticatedWithCookie() === false) {
return null;
}

Expand Down
2 changes: 1 addition & 1 deletion src/HttpController/Web/Movie/MovieController.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public function renderPage(Request $request) : Response
}

$currentUser = null;
if ($this->authenticationService->isUserAuthenticated() === true) {
if ($this->authenticationService->isUserAuthenticatedWithCookie() === true) {
$currentUser = $this->authenticationService->getCurrentUser();
}

Expand Down
2 changes: 1 addition & 1 deletion src/HttpController/Web/PersonController.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public function renderPage(Request $request) : Response
}

$isHiddenInTopLists = false;
if ($this->authenticationService->isUserAuthenticated() === true) {
if ($this->authenticationService->isUserAuthenticatedWithCookie() === true) {
$userId = $this->authenticationService->getCurrentUserId();
$isHiddenInTopLists = $this->userApi->hasHiddenPerson($userId, $personId);
}
Expand Down
4 changes: 2 additions & 2 deletions src/HttpController/Web/UserController.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public function __construct(

public function createUser(Request $request) : Response
{
if ($this->authenticationService->isUserAuthenticated() === false
if ($this->authenticationService->isUserAuthenticatedWithCookie() === false
&& $this->authenticationService->getCurrentUser()->isAdmin() === false) {
return Response::createForbidden();
}
Expand Down Expand Up @@ -65,7 +65,7 @@ public function deleteUser(Request $request) : Response

public function fetchUsers() : Response
{
if ($this->authenticationService->isUserAuthenticated() === false
if ($this->authenticationService->isUserAuthenticatedWithCookie() === false
&& $this->authenticationService->getCurrentUser()->isAdmin() === false) {
return Response::createForbidden();
}
Expand Down
2 changes: 1 addition & 1 deletion templates/component/navbar.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
</li>
{% if loggedIn == true %}
<li><a class="dropdown-item {% if requestUrlPath starts with '/settings/' %}active{% endif %}" href="/settings/account/general">Settings</a></li>
<li><a class="dropdown-item" href="/logout">Logout</a></li>
<li><button class="dropdown-item" onclick="logout()" style="cursor: pointer">Logout</button></li>
{% else %}
<li><a class="dropdown-item" href="/login">Login</a></li>
{% endif %}
Expand Down
2 changes: 1 addition & 1 deletion templates/page/login.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
</div>
{% endif %}
{% if redirect != false %}
<div class="alert alert-danger" role="alert" style="margin-bottom: 0.7rem!important;">
<div class="alert alert-danger" role="alert" id="forbiddenPageAlert">
Sign in to access page
</div>
<input type="hidden" value="{{ redirect }}" name="redirect" />
Expand Down
Loading