http.basicauth credentials are attached to every request, whatever the host

[rzo1]

What happens

HttpProtocol.configure() builds one list of request headers that includes the Authorization header derived from http.basicauth.user and http.basicauth.password, plus every entry of http.custom.headers. getProtocolOutput() applies that list to each request, including robots.txt fetches, with no reference to the host being fetched. There is no key to bind a credential to an origin. The behaviour is described in extending.adoc:407, which points at metadata-driven headers as the per-site alternative.

Where

core/src/main/java/org/apache/stormcrawler/protocol/okhttp/HttpProtocol.java:216-229 and :368-372 on main. Config keys: http.basicauth.user, http.basicauth.password, http.custom.headers.

final Builder rb = new Request.Builder().url(url);
customRequestHeaders.forEach(
        (k) -> {
            rb.header(k.getKey(), k.getValue());
        });

Why it matters

The keys exist for crawling a site that requires authentication, and a crawler follows outlinks, so the moment a crawled page links off the authenticated site the credential goes to the linked host. Pages on an internal site are often editable by their users, so the outlink need not be one the operator chose. The alternative named in the documentation works, but the simple configuration that the reference table describes has no scoping at all, and nothing warns about it.

Reproduction

Save as core/src/test/java/org/apache/stormcrawler/protocol/OkHttpBasicAuthScopeTest.java:

/*
 * Licensed to the Apache Software Foundation (ASF) under one or more
 * contributor license agreements.  See the NOTICE file distributed with
 * this work for additional information regarding copyright ownership.
 * The ASF licenses this file to you under the Apache License, Version 2.0
 * (the "License"); you may not use this file except in compliance with
 * the License.  You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package org.apache.stormcrawler.protocol;

import jakarta.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.io.OutputStream;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
import org.apache.storm.Config;
import org.apache.stormcrawler.Metadata;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

/** Records which requests carry the configured Authorization header. */
class OkHttpBasicAuthScopeTest extends AbstractProtocolTest {

    static final List<String> authorizationSeen = new ArrayList<>();

    @Override
    protected Handler[] getHandlers() {
        return new Handler[] {
            new AbstractHandler() {
                @Override
                public void handle(
                        String target,
                        Request baseRequest,
                        jakarta.servlet.http.HttpServletRequest request,
                        HttpServletResponse response)
                        throws IOException {
                    baseRequest.setHandled(true);
                    authorizationSeen.add(String.valueOf(request.getHeader("Authorization")));
                    response.setStatus(HttpServletResponse.SC_OK);
                    response.setContentType("text/html");
                    final byte[] content = "hello".getBytes(StandardCharsets.UTF_8);
                    response.setContentLength(content.length);
                    try (OutputStream out = response.getOutputStream()) {
                        out.write(content);
                    }
                }
            }
        };
    }

    @Test
    void credentialsGoToEveryHost() throws Exception {
        authorizationSeen.clear();
        Config conf = new Config();
        conf.put("http.agent.name", "this_is_only_a_test");
        conf.put("http.basicauth.user", "wikiuser");
        conf.put("http.basicauth.password", "wikipass");
        org.apache.stormcrawler.protocol.okhttp.HttpProtocol protocol =
                new org.apache.stormcrawler.protocol.okhttp.HttpProtocol();
        protocol.configure(conf);
        // the credentials were configured for some other site; this fetch is an
        // outlink to an unrelated server and the robots.txt that precedes it
        protocol.getProtocolOutput(
                "http://127.0.0.1:" + HTTP_PORT + "/robots.txt", new Metadata());
        protocol.getProtocolOutput("http://127.0.0.1:" + HTTP_PORT + "/page.html", new Metadata());
        protocol.cleanup();

        Assertions.assertEquals(2, authorizationSeen.size());
        // documents the current behaviour: there is no way to bind the header to
        // an origin, so every request carries it. Both values should be "null"
        // once the credentials are scoped to the site they belong to.
        for (String seen : authorizationSeen) {
            Assertions.assertTrue(
                    seen.startsWith("Basic "),
                    "expected the Authorization header on every request, got " + seen);
        }
    }
}

Run it:

mvn -pl core test -Dtest=OkHttpBasicAuthScopeTest

A local Jetty server records the Authorization header of each request. The test passes on main and documents the present behaviour, because there is no origin binding to assert against yet.

[INFO] Running org.apache.stormcrawler.protocol.OkHttpBasicAuthScopeTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.493 s -- in org.apache.stormcrawler.protocol.OkHttpBasicAuthScopeTest

Both requests, the robots.txt fetch and the page fetch, carry Basic ... to a server the credentials were never meant for.

Suggested fix

Add a host list alongside the credentials, for example http.basicauth.hosts, and attach the Authorization header in getProtocolOutput only when the request host matches. Do the same for http.custom.headers, either with a per-entry host or by moving credential-bearing headers to a separate key that takes a host. Requiring the host is a behaviour change for existing configurations, so either land it in a major release or accept an unset list as "everywhere" for one release with a WARN at startup.