Move tokens from memory to DataStax DB #171
Conversation
test/features/auth.feature
Outdated
| | accessToken | "TYPE:JWT" | | ||
| | expiresIn | "TYPE:NUMBER" | | ||
| When add bearer token to the header | ||
| # And restart app |
There was a problem hiding this comment.
This causes the test to fail until the tokens are persisted in the DB
|
Do you want to merge this is branch 137c or do you want to merge this in main, when 137c is in main? |
|
we can do either, ideally into I think we have 2 options in preferred order:
|
src/auth/auth.service.ts
Outdated
| let tokens: string[]; | ||
| try { | ||
| tokens = await this.astraService | ||
| .get<string[]>('tokens', keyspace, 'tokens') |
There was a problem hiding this comment.
store the tokens as keys in the collection, not within a function
|
I'll take a look at this PR during the weekend, had quite much to do this week so I didnt find time for it :( |
|
No problem, that would be great 👍 I can look at it with you also |
src/auth/auth.service.ts
Outdated
| } catch (e) { | ||
| throw new HttpException('Invalid client id', HttpStatus.BAD_REQUEST); | ||
| this.jwtService.verify(token); | ||
| response.status(HttpStatus.OK).json({ valid: true }); |
There was a problem hiding this comment.
For now I'm just validating if the token is valid.
- Do we have to do more checks here?
- How should the response look like at this endpoint? Is it just returning
{valid: true/false}Or does it return more information? - Does this endpoint needs to be secured, or can anybody post data to it for checking if it is valid?
There was a problem hiding this comment.
Great questions, here are my thoughts
How should the response look like at this endpoint? Is it just returning {valid: true/false} Or does it return more information?
I think that is fine to have a simple { valid: true|false }
Does this endpoint needs to be secured, or can anybody post data to it for checking if it is valid?
I think this should be protected otherwise someone could try to brute force it. Is it possible to do an OR for either of the guards?
There was a problem hiding this comment.
Added the token-guard for using only with the bot to be consistent with all Auth-Endpoints
There was a problem hiding this comment.
response.status(HttpStatus.OK)
I think it might make more sense to have the response code decision made in the controller and the service to just return the boolean
|
Ready for merge in my opinion!! :D |
|
Looking great 💥 , 2 small comments 🤓 |
|
Awesome! Let's merge 🎉 |
closes #165
closes #173
Merge after #137c
Cahl Todos