Add token support #738
Add token support #738
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #738 +/- ##
===========================================
- Coverage 82.30% 81.96% -0.34%
===========================================
Files 58 65 +7
Lines 1000 1181 +181
Branches 57 74 +17
===========================================
+ Hits 823 968 +145
- Misses 175 211 +36
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
DerMolly
force-pushed
the
private/tokens
branch
from
573927f
to
0797c14
Compare
This PR is still a Draft 😉 |
|
We should hash the auth token in database because it's working like a password. |
yes I only want to mention it because it can easiely be forgotten. |
DerMolly
force-pushed
the
private/tokens
branch
2 times, most recently
from
1ffef32
to
1c6dd14
Compare
DerMolly
force-pushed
the
private/tokens
branch
from
1c6dd14
to
7034cdf
Compare
DerMolly
force-pushed
the
private/tokens
branch
from
7034cdf
to
53f824d
Compare
DerMolly
force-pushed
the
private/tokens
branch
from
dbbce9f
to
88eddbc
Compare
|
I feel using a JWT is a bit overcomplicated here.
When we get a token from the user, we can then get the username and only check the database for tokens that correspond to the user. The only downside of this approach is that check-time scales linearly with the amount of API tokens a user has, but we could just limit that to a reasonable number (20?). |
|
Inspired by @davidmehren's idea: What about having a key-secret pair? I could imagine the following process:
With that approach you won't have a linear scale on the process to find the user for a token. |
|
If we use the id of the token anyway (for deletion purposes), we could just use that instead of a new second random string. |
|
Let's not invent an own session management. The OWASP has some quite clear guidelines: https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html We should also see if there might be a library which just takes care of that, as it appears to be a quite common use case. |
Your linking to something talking about sessions, when we don't implement any sessions (in this PR). I'm a bit confused what we should take from that post? |
A point against this made by @InnayTool is that an incrementing id would tell every user exactly how many tokens were at one point issued on the instance. An information we may want to not disclose. |
|
To make it clear:
The token is made from An API user authenticates via the This doesn't involve any session logic. It's just about the tokens for the API. A user has a list of their API keyids to delete them any time. After generation a user can not reclaim the secret as it's only stored hashed. In some terms this would be similar to GitHub's PATs. |
adds identifier and createdAt to AuthToken renames authToken in User to authTokens Signed-off-by: Philip Molares <philip.molares@udo.edu>
Signed-off-by: Philip Molares <philip.molares@udo.edu>
this seems very unnecessary as the chance of this is 1 / 2^512 Signed-off-by: Philip Molares <philip.molares@udo.edu>
Signed-off-by: Philip Molares <philip.molares@udo.edu>
adds auth service adds auth module adds token-auth strategy adds token-auth to all public api calls Signed-off-by: Philip Molares <philip.molares@udo.edu>
See: https://docs.nestjs.com/openapi/security Signed-off-by: Philip Molares <philip.molares@udo.edu>
adds MockAuthGuard which always return user 'hardcoded' Signed-off-by: Philip Molares <philip.molares@udo.edu>
Fix token deletion Update plantuml docs Add token validUntil and lastUsed fields Signed-off-by: Philip Molares <philip.molares@udo.edu>
Move AuthTokens to auth folder Signed-off-by: Philip Molares <philip.molares@udo.edu>
add test for BufferToBase64Url and toAuthTokenDto Signed-off-by: Philip Molares <philip.molares@udo.edu>
Add number type alias TimestampMillis Remove solved ToDos Change AuthToken and AuthTokenDto to use Date Rename authService unit tests Signed-off-by: Philip Molares <philip.molares@udo.edu>
As suggested by @InnayTool Signed-off-by: Philip Molares <philip.molares@udo.edu>
Rename AuthToken.identifier to label Signed-off-by: Philip Molares <philip.molares@udo.edu>
adds auth service adds auth module adds token-auth strategy adds token-auth to all public api calls Signed-off-by: Philip Molares <philip.molares@udo.edu>
adds MockAuthGuard which always return user 'hardcoded' Signed-off-by: Philip Molares <philip.molares@udo.edu>
Fix token deletion Update plantuml docs Add token validUntil and lastUsed fields Signed-off-by: Philip Molares <philip.molares@udo.edu>
This is a very high ceiling unlikely to hinder legitimate usage, but should prevent possible attack vectors Signed-off-by: Philip Molares <philip.molares@udo.edu>
This should prevent problem with the AuthToken purge on Sundays, as the service is either running on sunday or will be restarted there after. Also move base64url comment to right function Signed-off-by: Philip Molares <philip.molares@udo.edu>
Signed-off-by: Philip Molares <philip.molares@udo.edu>
This should prevent problem with the AuthToken purge on Sundays, as the service is either running on sunday or will be restarted there after. Also move base64url comment to right function Signed-off-by: Philip Molares <philip.molares@udo.edu>
DerMolly
dismissed stale reviews from davidmehren, InnayTool, and SISheogorath
via
720fcce
DerMolly
force-pushed
the
private/tokens
branch
from
3e8ce26
to
720fcce
Compare
Signed-off-by: Philip Molares <philip.molares@udo.edu>
DerMolly
force-pushed
the
private/tokens
branch
from
720fcce
to
2c38bd5
Compare
Component/Part
private api
Description
This PR adds the private api and the token routes.
This can be merged after #733 was merged.
Steps
signed-off my commits to accept the DCO.