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

Add API endpoint for destroying tokens #584

Merged
merged 8 commits into from
Feb 27, 2024
Merged

Conversation

JVT038
Copy link
Collaborator

@JVT038 JVT038 commented Feb 22, 2024

This PR adds an API endpoint that can destroy API tokens (manually generated by the user in the settings) and authentication tokens (generated upon login of the user).

Also important to mention that this PR is based on #575 , so that PR has to be merged before this one.

Part of #572

@JVT038 JVT038 linked an issue Feb 22, 2024 that may be closed by this pull request
74 tasks
@JVT038 JVT038 mentioned this pull request Feb 22, 2024
74 tasks
@leepeuker
Copy link
Owner

I have removed the IsAuthenticated middleware check from the new endpoint, I think we should not tell a user if a auth token really existed, only if the deletion process run without an error. Additionally, the IsAuthenticated middleware should in theory fail if the token is expired, which means you could not delete an expired token which seems not really practical.
Instead we will no return a 400 error if the Auth token header is missing.

@@ -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

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

@@ -0,0 +1,97 @@
POST http://127.0.0.1/api/authentication/token
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.

This is interesting feature in phpstorm, you can run assertions against the http requests.
To execute every request in a row press the green "Run all requests" button:
image

You can see the result in the Run window and which (if any) assertion failed, example with a failed assertion (wrong password):
image

This feature is a bit bulky to use, but at least we both can run the same tests in an automatic way locally.

leepeuker
leepeuker previously approved these changes Feb 25, 2024
@leepeuker leepeuker merged commit 8485c83 into main Feb 27, 2024
1 check passed
@leepeuker leepeuker deleted the add-logout-api-endpoint branch February 27, 2024 06:33
@JVT038 JVT038 removed a link to an issue Feb 28, 2024
74 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants