<p><b>@ManfredKarrer</b> requested changes on this pull request.</p>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/2461#discussion_r259628692">api/src/main/java/bisq/api/http/service/auth/TokenRegistry.java</a>:</p>
<pre style='color:#555'>> +package bisq.api.http.service.auth;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+
+public class TokenRegistry {
+
+    public static final long TTL = 30 * 60 * 1000;
+
+    private final TimeProvider timeProvider;
+    private final RandomStringGenerator randomStringGenerator;
+    private Map<String, Long> tokens = new HashMap<>();
+
+    public TokenRegistry() {
+        this(() -> UUID.randomUUID().toString(), System::currentTimeMillis);
</pre>
<p>I think you have to use SecureRandom for ryptographically secure randomness.</p>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/2461#discussion_r259628699">api/src/main/java/bisq/api/http/service/auth/TokenRegistry.java</a>:</p>
<pre style='color:#555'>> +import java.util.UUID;
+
+public class TokenRegistry {
+
+    public static final long TTL = 30 * 60 * 1000;
+
+    private final TimeProvider timeProvider;
+    private final RandomStringGenerator randomStringGenerator;
+    private Map<String, Long> tokens = new HashMap<>();
+
+    public TokenRegistry() {
+        this(() -> UUID.randomUUID().toString(), System::currentTimeMillis);
+    }
+
+    public TokenRegistry(RandomStringGenerator randomStringGenerator, TimeProvider timeProvider) {
+        this.timeProvider = timeProvider;
</pre>
<p>I think you have to use SecureRandom for ryptographically secure randomness.</p>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/2461#discussion_r259628870">api/src/test/java/bisq/api/http/service/auth/ApiPasswordManagerTest.java</a>:</p>
<pre style='color:#555'>> +
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+
+
+import com.github.javafaker.Faker;
</pre>
<p>I dont find that class in the source code at <a href="https://github.com/Javafaker">https://github.com/Javafaker</a>.<br>
Do we really need that? It is one of those examples where I don't want to get new dependencies (unknown developer, small functionality)</p>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/2461#discussion_r259628902">api/src/test/java/bisq/api/http/service/auth/ApiPasswordManagerTest.java</a>:</p>
<pre style='color:#555'>> +import com.github.javafaker.Faker;
+
+public class ApiPasswordManagerTest {
+
+    @Rule
+    public ExpectedException expectedException = ExpectedException.none();
+
+    private String dataDir;
+    private BisqEnvironment bisqEnvironmentMock;
+    private TokenRegistry tokenRegistryMock;
+    private ApiPasswordManager apiPasswordManager;
+
+    private static String getRandomPasswordDifferentThan(String otherPassword) {
+        String newPassword;
+        do {
+            newPassword = Faker.instance().internet().password();
</pre>
<p>What is that supposed to do? internet() is a weird method name...<br>
As said above I prefer to not add that dependency.</p>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/2461#discussion_r259628953">api/src/test/java/bisq/api/http/service/auth/ApiPasswordManagerTest.java</a>:</p>
<pre style='color:#555'>> +
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+
+
+import com.github.javafaker.Faker;
</pre>
<p>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.</p>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/2461#discussion_r259628963">api/src/test/java/bisq/api/http/service/auth/ApiPasswordManagerTest.java</a>:</p>
<pre style='color:#555'>> +import com.github.javafaker.Faker;
+
+public class ApiPasswordManagerTest {
+
+    @Rule
+    public ExpectedException expectedException = ExpectedException.none();
+
+    private String dataDir;
+    private BisqEnvironment bisqEnvironmentMock;
+    private TokenRegistry tokenRegistryMock;
+    private ApiPasswordManager apiPasswordManager;
+
+    private static String getRandomPasswordDifferentThan(String otherPassword) {
+        String newPassword;
+        do {
+            newPassword = Faker.instance().internet().password();
</pre>
<p>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.</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/bisq-network/bisq/pull/2461#pullrequestreview-207160069">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AkpZtiFedasXnwS1TKD7BB_AcAOxLMhHks5vQr-jgaJpZM4bOkt3">mute the thread</a>.<img src="https://github.com/notifications/beacon/AkpZto2D2-EXtkuAwhyDS6g_rAFjvleuks5vQr-jgaJpZM4bOkt3.gif" height="1" width="1" alt="" /></p>
<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/bisq-network/bisq","title":"bisq-network/bisq","subtitle":"GitHub repository","main_image_url":"https://github.githubassets.com/images/email/message_cards/header.png","avatar_image_url":"https://github.githubassets.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/bisq-network/bisq"}},"updates":{"snippets":[{"icon":"PERSON","message":"@ManfredKarrer requested changes on #2461"}],"action":{"name":"View Pull Request","url":"https://github.com/bisq-network/bisq/pull/2461#pullrequestreview-207160069"}}}</script>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/bisq-network/bisq/pull/2461#pullrequestreview-207160069",
"url": "https://github.com/bisq-network/bisq/pull/2461#pullrequestreview-207160069",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>