Changeset 18437 in josm for trunk/test/functional


Ignore:
Timestamp:
2022-04-27T21:26:39+02:00 (3 years ago)
Author:
taylor.smock
Message:

Fix #21935: Avoid leaking Authorization headers on redirects in HttpClient

This was found due to a change in where OSM stores GPX data files.
OSM now uses s3 buckets, and redirects using a signed URL. S3 does
not like multiple authentication methods.

File:
1 edited

Legend:

Unmodified
Added
Removed
  • trunk/test/functional/org/openstreetmap/josm/tools/HttpClientTest.java

    r18106 r18437  
    1616import static org.hamcrest.MatcherAssert.assertThat;
    1717import static org.hamcrest.text.IsEqualIgnoringCase.equalToIgnoringCase;
     18import static org.junit.jupiter.api.Assertions.assertAll;
    1819import static org.junit.jupiter.api.Assertions.assertEquals;
     20import static org.junit.jupiter.api.Assertions.assertFalse;
    1921import static org.junit.jupiter.api.Assertions.assertThrows;
    2022import static org.junit.jupiter.api.Assertions.assertTrue;
     
    3032import java.util.Collections;
    3133import java.util.Map;
     34import java.util.UUID;
    3235import java.util.logging.Handler;
    3336import java.util.logging.LogRecord;
     
    3841import org.junit.jupiter.api.Test;
    3942import org.junit.jupiter.api.Timeout;
     43import org.junit.jupiter.params.ParameterizedTest;
     44import org.junit.jupiter.params.provider.ValueSource;
    4045import org.openstreetmap.josm.TestUtils;
    4146import org.openstreetmap.josm.data.Version;
     
    4752
    4853import com.github.tomakehurst.wiremock.WireMockServer;
     54import com.github.tomakehurst.wiremock.admin.model.ServeEventQuery;
    4955import com.github.tomakehurst.wiremock.http.HttpHeader;
    5056import com.github.tomakehurst.wiremock.http.HttpHeaders;
    5157import com.github.tomakehurst.wiremock.matching.UrlPattern;
     58import com.github.tomakehurst.wiremock.stubbing.ServeEvent;
    5259
    5360/**
     
    234241        mockRedirects(false, 3);
    235242        assertThrows(IOException.class, () -> HttpClient.create(url("/relative-redirect/3")).setMaxRedirects(2).connect(progress));
     243    }
     244
     245    /**
     246     * Ensure that we don't leak authorization headers
     247     * See <a href="https://josm.openstreetmap.de/ticket/21935">JOSM #21935</a>
     248     * @param authorization The various authorization configurations to test
     249     */
     250    @ParameterizedTest
     251    @ValueSource(strings = { "Basic dXNlcm5hbWU6cGFzc3dvcmQ=", "Digest username=test_user",
     252            /* OAuth 1.0 for OSM as implemented in JOSM core */
     253            "OAuth oauth_consumer_key=\"test_key\", oauth_nonce=\"1234\", oauth_signature=\"test_signature\", "
     254                    + "oauth_signature_method=\"HMAC-SHA1\", oauth_timestamp=\"0\", oauth_token=\"test_token\", "
     255                    + "oauth_version=\"1.0\"",
     256            /* OAuth 2.0, not yet implemented in JOSM core */
     257            "Bearer some_random_token"
     258        })
     259    void testRedirectsToDifferentSite(String authorization) throws IOException {
     260        final String localhost = "localhost";
     261        final String localhostIp = "127.0.0.1";
     262        final String otherServer = this.localServer.baseUrl().contains(localhost) ? localhostIp : localhost;
     263        final UUID redirect = this.localServer.stubFor(get(urlEqualTo("/redirect/other-site"))
     264                .willReturn(aResponse().withStatus(302).withHeader(
     265                        "Location", localServer.url("/same-site/other-site")))).getId();
     266        final UUID sameSite = this.localServer.stubFor(get(urlEqualTo("/same-site/other-site"))
     267                .willReturn(aResponse().withStatus(302).withHeader(
     268                        "Location", localServer.url("/other-site")
     269                                .replace(otherServer == localhost ? localhostIp : localhost, otherServer)))).getId();
     270        final UUID otherSite = this.localServer.stubFor(get(urlEqualTo("/other-site"))
     271                .willReturn(aResponse().withStatus(200).withBody("other-site-here"))).getId();
     272        final HttpClient client = HttpClient.create(url("/redirect/other-site"));
     273        client.setHeader("Authorization", authorization);
     274        try {
     275            client.connect();
     276            this.localServer.getServeEvents();
     277            final ServeEvent first = this.localServer.getServeEvents(ServeEventQuery.forStubMapping(redirect)).getRequests().get(0);
     278            final ServeEvent second = this.localServer.getServeEvents(ServeEventQuery.forStubMapping(sameSite)).getRequests().get(0);
     279            final ServeEvent third = this.localServer.getServeEvents(ServeEventQuery.forStubMapping(otherSite)).getRequests().get(0);
     280            assertAll(() -> assertEquals(3, this.localServer.getServeEvents().getRequests().size()),
     281                    () -> assertEquals(authorization, first.getRequest().getHeader("Authorization"),
     282                    "Authorization is expected for the first request: " + first.getRequest().getUrl()),
     283                    () -> assertEquals(authorization, second.getRequest().getHeader("Authorization"),
     284                            "Authorization is expected for the second request: " + second.getRequest().getUrl()),
     285                    () -> assertFalse(third.getRequest().containsHeader("Authorization"),
     286                    "Authorization is not expected for the third request: " + third.getRequest().getUrl()));
     287        } finally {
     288            client.disconnect();
     289        }
    236290    }
    237291
Note: See TracChangeset for help on using the changeset viewer.