Conversation
georgebejan
requested review from
RaduCStefanescu,
aniri,
catileptic and
mihairinzis
as code owners
|
This pull request is being automatically deployed with ZEIT Now (learn more). 🔍 Inspect: https://zeit.co/code4romania/next-door/pbg2b3zwm |
georgebejan
force-pushed
the
refresh_token
branch
from
ca652c2 to
a25ba1d
Compare
georgebejan
force-pushed
the
refresh_token
branch
from
a25ba1d to
c12c98b
Compare
georgebejan
force-pushed
the
refresh_token
branch
from
c12c98b to
530a2a3
Compare
|
|
||
| final String jwt = tokenProvider.generateToken(authentication); | ||
| return ResponseEntity.ok(new JwtAuthenticationResponse(jwt)); | ||
| final UserPrincipal userPrincipal = (UserPrincipal) authentication.getPrincipal(); |
There was a problem hiding this comment.
Why is a custom implementation for this and not a normal authentication flow?
Default SpringSecurity Authentication using JWT is super simple to implement without extra work needed.
And for the long run is the way to go
|
|
||
| return Jwts.builder() | ||
| .setSubject(userPrincipal.getId().toString()) | ||
| .setSubject(userId.toString()) |
There was a problem hiding this comment.
The safest way to sign the access token is using Private key and implementing RSA algorithm
Also, for the current implementation the signing key is too short
| @@ -60,6 +84,7 @@ public boolean validateToken(final String authToken) { | |||
| } catch (IllegalArgumentException ex) { | |||
| LOGGER.error("JWT claims string is empty."); | |||
There was a problem hiding this comment.
it's a best practice to pass exception to the logger to have it's stacktrace, event if you have a custom logging message the stacktrace provides valuable info
| jwtExpirationInMs: 1000 | ||
| jwt: | ||
| secret: test | ||
| accessTokenExpirationInMs: 86400000 |
2 participants
What does it fix?
#44
How has it been tested?
Tested the whole register-> login -> request access token flow with Postman