Typesafe Client -> Header Builder overwrites @AuthorizationHeader

[ftrossbach]

I've encountered an issue in HeaderBuilder which might be fairly specific to the way we have implemented custom config sources, but nonetheless could be considered an issue. If I use @AuthorizationHeader with the Bearer type, it will be resolved at first correctly, but in the end this will be overwritten by a call to "configuredCredentials"

Take HeaderBuilder's build method:

 public Map<String, String> build() {
        Map<String, String> headers = new LinkedHashMap<>();
        addDefaultHeaders(headers);
        if (method != null) {
            method.getResolvedAnnotations(api, Header.class)
                    // getResolvedAnnotations returns class-level annotations first
                    // so if there is something on class level, it will be overwritten
                    // by a header on the method
                    .forEach(annotation -> resolve(annotation).apply(headers));
            method.headerParameters().forEach(parameter -> resolve(parameter).apply(headers));
            method.getResolvedAnnotations(api, AuthorizationHeader.class)
                    // getResolvedAnnotations returns class-level annotations first, then method-level annotations,
                    // so we need to take the last element of this stream.
                    // This `reduce` operation is basically 'find the last element'
                    .reduce((first, second) -> second)
                    .map(this::resolveAuthHeader)
                    .ifPresent(auth -> headers.put("Authorization", auth));
        }
        if (additionalHeaders != null) {
            headers.putAll(additionalHeaders);
        }
        headers.putAll(configuredCredentials());
        return headers;
    }

First, it resolves the header based on @AuthorizationHeader. This works correctly in my debugger. Then, just before the return, it calls headers.putAll(configuredCredentials());, which overwrites it in my case as we have a custom config source that provides default credentials even if the specific config key cannot be found. This is desirable for us in most cases, but not in this one.

I would suggest that headers.putAll(configuredCredentials()); should only be called if headers does not already contain an Authorization header. Would you agree?

[t1]

Thanks for the feedback, and after thinking it through, I come to this reasoning about MP Config: dynamic settings like the ones from config sources generally overwrite the static settings from source code. If we would change this here, the general principle of priorities in MP Config would be violated.

So I suppose you'll have to find a different solution, but I don't yet have an idea as to how this could work codewise. In theory, we could change the code so that the annotations are also provided with a priority so that your default credentials can be even lower prio. But that would be a big change.

Maybe you can instead simply configure the specific credentials with MP Config instead, so the default credentials don't get used?

[ftrossbach]

The thing is: i don't have specific credentials as I need to use a JWT. So I'll have to find another way. However, what I don't understand is: The code currently works like this:

• it evaluates @Header and @AuthorizationHeader annotations
• if /mp-graphql/username and password are set, overwrite any Authorization header evaluated in the first step without warning.

Is this really the desired behaviour?

[t1]

You're right, if the code provides dynamical credentials (e.g. a bearer token), then it doesn't make sense to still consider MP Config. I guess, I'll have to rethink this.

[ftrossbach]

I guess a simple solution could be to change the order: do headers.putAll(configuredCredentials()); first, then evaluate custom headers, so the overwriting goes the other way. But there could be unwanted side-effects to this that I haven't considered.

[t1]

But that would also change static headers in the code. It needs more than that.