Skip to content

Commit 7fd6049

Browse files
author
taylor.smock
committed
Fix #23083: SocketTimeoutException in OAuth20Authorization$OAuth20AuthorizationHandler.handleRequest
git-svn-id: https://josm.openstreetmap.de/svn/trunk@18786 0c6e7542-c601-0410-84e7-c038aed88b3b
1 parent 27a0d06 commit 7fd6049

File tree

3 files changed

+60
-15
lines changed

3 files changed

+60
-15
lines changed

‎src/org/openstreetmap/josm/data/oauth/OAuth20Authorization.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,13 @@ public AuthorizationHandler.ResponseRecord handleRequest(String sender, String r
101101
consumer.accept(Optional.of(oAuth20Token));
102102
} catch (IOException | OAuth20Exception e) {
103103
consumer.accept(Optional.empty());
104-
throw new JosmRuntimeException(e);
104+
throw new RequestHandler.RequestHandlerErrorException(e);
105105
} finally {
106106
tradeCodeForToken.disconnect();
107107
}
108108
} catch (MalformedURLException e) {
109-
throw new JosmRuntimeException(e);
109+
consumer.accept(Optional.empty());
110+
throw new RequestHandler.RequestHandlerBadRequestException(e);
110111
}
111112
return null;
112113
}

‎src/org/openstreetmap/josm/gui/preferences/server/OAuthAuthenticationPreferencesPanel.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import javax.swing.JButton;
2121
import javax.swing.JCheckBox;
2222
import javax.swing.JLabel;
23+
import javax.swing.JOptionPane;
2324
import javax.swing.JPanel;
2425

2526
import org.openstreetmap.josm.actions.ExpertToggleAction;
@@ -374,6 +375,12 @@ public void actionPerformed(ActionEvent arg0) {
374375
OAuthAccessTokenHolder.getInstance().setAccessToken(OsmApi.getOsmApi().getServerUrl(), token.orElse(null));
375376
OAuthAccessTokenHolder.getInstance().save(CredentialsManager.getInstance());
376377
GuiHelper.runInEDT(OAuthAuthenticationPreferencesPanel.this::refreshView);
378+
if (!token.isPresent()) {
379+
GuiHelper.runInEDT(() -> JOptionPane.showMessageDialog(MainApplication.getMainPanel(),
380+
tr("Authentication failed, please check browser for details."),
381+
tr("OAuth Authentication Failed"),
382+
JOptionPane.ERROR_MESSAGE));
383+
}
377384
}, OsmScopes.read_gpx, OsmScopes.write_gpx,
378385
OsmScopes.read_prefs, OsmScopes.write_prefs,
379386
OsmScopes.write_api, OsmScopes.write_notes);

‎test/unit/org/openstreetmap/josm/data/oauth/OAuth20AuthorizationTest.java‎

Lines changed: 50 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
// License: GPL. For details, see LICENSE file.
22
package org.openstreetmap.josm.data.oauth;
33

4+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
45
import static org.junit.jupiter.api.Assertions.assertEquals;
56
import static org.junit.jupiter.api.Assertions.assertNotNull;
67
import static org.junit.jupiter.api.Assertions.assertNull;
78
import static org.junit.jupiter.api.Assertions.assertTrue;
89

910
import java.io.IOException;
11+
import java.net.URL;
1012
import java.util.HashMap;
1113
import java.util.Map;
1214
import java.util.Optional;
@@ -19,6 +21,7 @@
1921
import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
2022
import com.github.tomakehurst.wiremock.extension.Parameters;
2123
import com.github.tomakehurst.wiremock.extension.ResponseTransformer;
24+
import com.github.tomakehurst.wiremock.http.FixedDelayDistribution;
2225
import com.github.tomakehurst.wiremock.http.HttpHeader;
2326
import com.github.tomakehurst.wiremock.http.HttpHeaders;
2427
import com.github.tomakehurst.wiremock.http.QueryParameter;
@@ -61,8 +64,14 @@ class OAuth20AuthorizationTest {
6164
private static final String CODE_CHALLENGE_METHOD_VALUE = "S256";
6265
private static final String CODE_CHALLENGE = "code_challenge";
6366

67+
private enum ConnectionProblems {
68+
NONE,
69+
SOCKET_TIMEOUT
70+
}
71+
6472
private static class OAuthServerWireMock extends ResponseTransformer {
6573
String stateToReturn;
74+
ConnectionProblems connectionProblems = ConnectionProblems.NONE;
6675
@Override
6776
public Response transform(Request request, Response response, FileSource files, Parameters parameters) {
6877
try {
@@ -88,7 +97,15 @@ private Response tokenRequest(Request request, Response response) {
8897
|| !queryParameters.containsKey("code") || !queryParameters.containsKey("code_verifier")) {
8998
return Response.Builder.like(response).but().status(500).build();
9099
}
91-
return Response.Builder.like(response).but().body("{\"token_type\": \"bearer\", \"access_token\": \"test_access_token\"}").build();
100+
switch (connectionProblems) {
101+
case SOCKET_TIMEOUT:
102+
return Response.Builder.like(response).but().configureDelay(null, null,
103+
10_000, new FixedDelayDistribution(0)).build();
104+
case NONE:
105+
default:
106+
return Response.Builder.like(response).but()
107+
.body("{\"token_type\": \"bearer\", \"access_token\": \"test_access_token\"}").build();
108+
}
92109
}
93110

94111
private Response authorizationRequest(Request request, Response response) {
@@ -135,6 +152,7 @@ void setup() {
135152
OpenBrowserMocker.getCalledURIs().clear();
136153
RemoteControl.stop(); // Ensure remote control is stopped
137154
oauthServer.stateToReturn = null;
155+
oauthServer.connectionProblems = ConnectionProblems.NONE;
138156
}
139157

140158
/**
@@ -163,17 +181,22 @@ public String getDefaultOsmApiUrl() {
163181
wireMockRuntimeInfo.getWireMock().register(WireMock.post(WireMock.urlPathEqualTo("/oauth2/token")));
164182
}
165183

166-
@Test
167-
void testAuthorize(WireMockRuntimeInfo wireMockRuntimeInfo) throws IOException {
184+
private HttpClient generateClient(WireMockRuntimeInfo wireMockRuntimeInfo, AtomicReference<Optional<IOAuthToken>> consumer) {
168185
final OAuth20Authorization authorization = new OAuth20Authorization();
169-
final AtomicReference<Optional<IOAuthToken>> consumer = new AtomicReference<>();
170186
OAuth20Parameters parameters = (OAuth20Parameters) OAuthParameters.createDefault(OsmApi.getOsmApi().getBaseUrl(), OAuthVersion.OAuth20);
171187
RemoteControl.start();
172188
authorization.authorize(new OAuth20Parameters(parameters.getClientId(), parameters.getClientSecret(),
173189
wireMockRuntimeInfo.getHttpBaseUrl() + "/oauth2", wireMockRuntimeInfo.getHttpBaseUrl() + "/api",
174190
parameters.getRedirectUri()), consumer::set, OsmScopes.read_gpx);
175191
assertEquals(1, OpenBrowserMocker.getCalledURIs().size());
176-
HttpClient client = HttpClient.create(OpenBrowserMocker.getCalledURIs().get(0).toURL());
192+
final URL url = assertDoesNotThrow(() -> OpenBrowserMocker.getCalledURIs().get(0).toURL());
193+
return HttpClient.create(url);
194+
}
195+
196+
@Test
197+
void testAuthorize(WireMockRuntimeInfo wireMockRuntimeInfo) throws IOException {
198+
final AtomicReference<Optional<IOAuthToken>> consumer = new AtomicReference<>();
199+
final HttpClient client = generateClient(wireMockRuntimeInfo, consumer);
177200
try {
178201
HttpClient.Response response = client.connect();
179202
assertEquals(200, response.getResponseCode());
@@ -190,15 +213,8 @@ void testAuthorize(WireMockRuntimeInfo wireMockRuntimeInfo) throws IOException {
190213
@Test
191214
void testAuthorizeBadState(WireMockRuntimeInfo wireMockRuntimeInfo) throws IOException {
192215
oauthServer.stateToReturn = "Bad_State";
193-
final OAuth20Authorization authorization = new OAuth20Authorization();
194216
final AtomicReference<Optional<IOAuthToken>> consumer = new AtomicReference<>();
195-
OAuth20Parameters parameters = (OAuth20Parameters) OAuthParameters.createDefault(OsmApi.getOsmApi().getBaseUrl(), OAuthVersion.OAuth20);
196-
RemoteControl.start();
197-
authorization.authorize(new OAuth20Parameters(parameters.getClientId(), parameters.getClientSecret(),
198-
wireMockRuntimeInfo.getHttpBaseUrl() + "/oauth2", wireMockRuntimeInfo.getHttpBaseUrl() + "/api",
199-
parameters.getRedirectUri()), consumer::set, OsmScopes.read_gpx);
200-
assertEquals(1, OpenBrowserMocker.getCalledURIs().size());
201-
HttpClient client = HttpClient.create(OpenBrowserMocker.getCalledURIs().get(0).toURL());
217+
final HttpClient client = generateClient(wireMockRuntimeInfo, consumer);
202218
try {
203219
HttpClient.Response response = client.connect();
204220
assertEquals(400, response.getResponseCode());
@@ -209,4 +225,25 @@ void testAuthorizeBadState(WireMockRuntimeInfo wireMockRuntimeInfo) throws IOExc
209225
}
210226
assertNull(consumer.get(), "The OAuth consumer should not be called since the state does not match");
211227
}
228+
229+
@Test
230+
void testSocketTimeout(WireMockRuntimeInfo wireMockRuntimeInfo) throws Exception {
231+
// 1s before timeout
232+
Config.getPref().putInt("socket.timeout.connect", 1);
233+
Config.getPref().putInt("socket.timeout.read", 1);
234+
oauthServer.connectionProblems = ConnectionProblems.SOCKET_TIMEOUT;
235+
236+
final AtomicReference<Optional<IOAuthToken>> consumer = new AtomicReference<>();
237+
final HttpClient client = generateClient(wireMockRuntimeInfo, consumer)
238+
.setConnectTimeout(15_000).setReadTimeout(30_000);
239+
try {
240+
HttpClient.Response response = client.connect();
241+
assertEquals(500, response.getResponseCode());
242+
String content = response.fetchContent();
243+
assertTrue(content.contains("java.net.SocketTimeoutException: Read timed out"));
244+
} finally {
245+
client.disconnect();
246+
}
247+
assertEquals(Optional.empty(), consumer.get());
248+
}
212249
}

0 commit comments

Comments
 (0)