Security framework for the API #2461
Conversation
| private Map<String, Long> tokens = new HashMap<>(); | ||
|
|
||
| public TokenRegistry() { | ||
| this(() -> UUID.randomUUID().toString(), System::currentTimeMillis); |
There was a problem hiding this comment.
I think you have to use SecureRandom for cryptographically secure randomness.
There was a problem hiding this comment.
UUID.randomUUID uses SecureRandom by default.
| } | ||
|
|
||
| public TokenRegistry(RandomStringGenerator randomStringGenerator, TimeProvider timeProvider) { | ||
| this.timeProvider = timeProvider; |
There was a problem hiding this comment.
I think you have to use SecureRandom for cryptographically secure randomness.
There was a problem hiding this comment.
Time provider is meant to provide actual time. It is used as abstraction for unit tests.
|
|
||
|
|
||
|
|
||
| import com.github.javafaker.Faker; |
There was a problem hiding this comment.
I dont find that class in the source code at https://github.com/Javafaker.
Do we really need that? It is one of those examples where I don't want to get new dependencies (unknown developer, small functionality)
There was a problem hiding this comment.
Ah sorry, just saw its a test class. So then I dont care so much about that dependency anymore, but still would like to know better what the class provides.
There was a problem hiding this comment.
Faker is random data generator. On the top level it has methods with categories.
| private static String getRandomPasswordDifferentThan(String otherPassword) { | ||
| String newPassword; | ||
| do { | ||
| newPassword = Faker.instance().internet().password(); |
There was a problem hiding this comment.
What is that supposed to do? internet() is a weird method name...
As said above I prefer to not add that dependency.
There was a problem hiding this comment.
Ah sorry, just saw its a test class. So then I dont care so much about that dependency anymore, but still would like to know better what the class provides.
|
I've substituted |
|
I will merge master after the PR. Please update your branch afterwards to be in sync. |
No description provided.