Add OpaS3SecurityFacade

[pranavr12]

Related to #148

[Randgalt]

add requireNonNull

[Randgalt]

add requireNonNull

[Randgalt]

Apply Trino style - implements should be indented on next line

[Randgalt]

I'm -1 on this change. The goal, as I understand it, is to be able to short-circuit the OPA request and return a Security response without querying the OPA server.

IMO a better way of doing this is adding an OPA query mechanism that is separate from the OPA mapping. If users want to short-circuit, they can inject a query adaptor that does that.

E.g.

public interface OpaClient
{
    SecurityResponse requestSecurityResponse(OpaRequest request);
}

The default implementation would be the body of OpaS3SecurityFacadeProvider.securityFacadeForRequest().

The OpaModule would optionally bind a default to the default OpaClient which would look like this:

public class DefaultOpaClient
        implements OpaClient
{
    private static final JsonCodec<Map<String, Object>> CODEC = mapJsonCodec(String.class, Object.class);
    private final HttpClient httpClient;

    @Inject
    public DefaultOpaClient(@ForOpa HttpClient httpClient)
    {
        this.httpClient = requireNonNull(httpClient, "httpClient is null");
    }

    @Override
    public SecurityResponse requestSecurityResponse(OpaRequest opaRequest)
    {
        Map<String, Object> inputDocument = toInputDocument(opaRequest.document());

        Request.Builder builder = preparePost()
                .setUri(opaRequest.opaServerUri())
                .addHeader(CONTENT_TYPE, APPLICATION_JSON_TYPE.getType())
                .setBodyGenerator(jsonBodyGenerator(CODEC, inputDocument));
        opaRequest.additionalHeaders().forEach((name, values) -> values.forEach(value -> builder.addHeader(name, value)));

        Map<String, Object> responseDocument = httpClient.execute(builder.build(), createJsonResponseHandler(CODEC));
        return toSecurityResponse(responseDocument);
    }

    private static Map<String, Object> toInputDocument(Map<String, Object> document)
    {
        /*
            Default formats standard input:

            {
                "input": {
                    ... nested document ...
                }
            }
        */

        return ImmutableMap.of("input", document);
    }

    private static SecurityResponse toSecurityResponse(Map<String, Object> responseDocument)
    {
        /*
            Default handles standard response:

            {
                "result": true/false
            }
        */

        return extractBoolean(responseDocument, "result")
                .map(allowed -> allowed ? SUCCESS : FAILURE)
                .orElse(FAILURE);
    }


    private static Optional<Boolean> extractBoolean(Map<?, ?> map, String key)
    {
        return switch (map.get(key)) {
            case Boolean b -> Optional.of(b);
            case null, default -> Optional.empty();
        };
    }
}

You'll need to add keyInBucket to OpaRequest

cc @vagaerg and @mosiac1 for your opinion

[mosiac1]

re @Randgalt

I think the OpaClient approach somewhat defeats the point of having direct support for OPA. If you bind both OpaClient and OpaS3SecurityMapper (which is required) with custom implementations all the core proxy does is:

mapper.toSecurityResponse(opaClient.requestSecurityResponse(mapper.toOparequest(...)))

In this case I think its better off to just implement a custom S3SecurityFacadeProvider and not use any of the provided OPA utility.

This approach doesn't quite provide the short-circuit functionality. The mapper would have to return some special OpaRequest that the OpaClient would recognise as a short-circuit and not fire a request for. This seems quite convoluted and I personally prefer returning a SecurityResponse from the mapper

[mosiac1]

Could we make the constructors of OpaRequestAdapter, OpaRequestWrapper and SecurityResponseWrapper private/protected and provide 2 static functions in OpaRequestAdapter: ofOpaRequest and ofSecurityResponse?

[mosiac1]

WDYT of OpaRequestOrSecurityResponse as the interface name? A bit wordy but very descriptive