Implementing access policies

[Tectu]

Most real-life applications have the need of implementing access policies to check whether a client is allowed to perform a certain request. As a basic example: A web application which provides an admin backend. A user should not be able to access any of the endpoints of the admin backend without prior authentication.
Other use cases might include things such as rate limiting (eg. only 100 requests per day).

This should be functionality baked into malloy.

I think it would be best to have some sort of security-policy-check interface in the endpoint class. This might be something as simple as:

struct access_policy
{
    /**
     * Checks whether a request satisfies the policy constrains.
     *
     * @return `true` if the request is clear. `false` if not. The response will be sent back to the client if `false`.
     */
    [[nodiscard]]
    virtual
    std::pair<bool, std::optional<response<>>>
    check(const request<>& req) = 0;
};

The router would check the policy constrains on each request. If the check passes (returns true), the endpoint handler will be invoked. If the check fails (returns false), the endpoint handler will not be invoked and the router instead responds with the response provided by the checker. This allows a user of the library to respond in whatever way is sensible for the application.

This is just to illustrate the idea roughly. The interface might look differently - it's mainly about functionality at this point.
Especially the cost of doing this via virtual functions might not be desirable in this case.
Also std::pair<bool, std::optional<response<>>> is most likely not the way to go. Again: just for illustrating functionality.

A router should have the capabilities of getting a "default security policy" assigned which will be propagated/used by all endpoints if an endpoint didn't receive a specific policy.

@0x00002a thoughts?

[0x00002a]

Not sure if I'm misunderstanding but what would the advantage of this be over the user simply changing their response in the route handler? The file serving endpoint might benefit from this but I can't think of any other cases, am I missing something?

[Tectu]

The advantage is exactly that: An application using malloy will not have to manually perform this check on every endpoint.

Think of a simple CMS (eg. something like WordPress): The admin backend can be implemented using it's own sub-router on /admin. Every time that sub-router receives a request the developer needs to make sure that the application specific policy check is performed to prevent access to admin-relevant functionalities to an unauthorized user (client). If there's even just one endpoint in that sub-router (and nested sub-routers) where that check was omitted (bug) a user can get access to that endpoint without being authenticated.

Having some sort of access policy baked into malloy allows to simply set an access policy on the /admin sub-router and the router will automatically call the access policy checker (which is implemented by the application) on every endpoint request.
The application can implement this access policy checker in a way that it checks for the user authentication. If it passes, the router will simply continue handling the request by invoking the corresponding endpoint's handler. If the authentication check fails, the application can provide a corresponding http::response which might be something along the lines of "Access Denied - Not authenticated".

This vastly simplifies the life of a developer implementing such an admin backend as there is one local point where the access policy check is assigned to the /admin sub-router. There is no need to manually perform this check on every endpoint added to the /admin sub-router (and nested sub-routers).

I know this because I am running exactly into this issue: Implementing a web application with an admin backend and making sure that every endpoint of that /admin sub-router has the authentication/access check is cumbersome, error prone and potentially fatal if not done properly.
Simply adding a handler (callable of some sorts) to the main /admin sub-router which checks every single request automatically is the preferred way of doing this.

[0x00002a]

I see your point. Why not something like how preflights are currently done then? iirc django allows defining an authentication backend, we could do that with a template (and then some type hiding), which could be registered in the same way preflights are currently. Adding a callable could work I guess, that would let us define preset backends which just overloaded ()

[Tectu]

Would you care to draft up some code snippets?
I'm not sure how the add_preflight() paradigm would work in this case as multiple endpoints might exist on the same resource with different authentication requirements (eg. GET /gallery might have a different authentication requirement than POST /gallery).

[0x00002a]

hmm, that could be done at the implementation level. e.g.:

struct post_auth {
    auto operator()(const malloy::http::request_header<>& h) const -> std::optional<malloy::http::response<>> {
        if (h.method() != malloy::http::method::post) return std::nullopt;
        
        /* do some authentication */
    }
};

Potentially the return type of this could be the same as for the route handlers, just wrapped in an optional (allowing any type of bodies to be used). I've used std::optional alone rather than a bool pair since an empty optional is falsy anyway.

It also might be better to have this as something like router::add_check, or something, since it is not necessarily just for authentication, and ideally it would try to match all from the most specific to the least specific but I'm not sure if that can be done with std::regex.

[Tectu]

I see the following possibilities:

• Per end-point policies: That would probably be the approach providing the most flexibility. Not sure how the interface for setting an endpoints policy would look like. Extending the various add_() versions/overloads seems like a bitch.
• pre-dispatch policy checks: Basically we'd have some base type policy. The router would have a list of these policies and check every incoming request against that policy list/table/what ever. The interface would probably look something like router::add_policy(std::string&& resource, policy&& p);. The resource could be a regex again. There might also be an additional overload to specify the HTTP method. This might still provide the user with the opportunity to screw up by providing an incorrect resource introducing very hard-to-spot security problems.
• only sub-router level policies: We could keep it simple and only have per-sub-router policies.

[0x00002a]

I like 2 personally, it fits best with the existing interface imo.

I agree we need some sort of polymorphism on the policies, but I suggest we do the type erasure as an implementation detail. Let the user pass a concept to add_policy (or whichever method is used to register them), rather than needing to inherit.

[Tectu]

I like 2 personally, it fits best with the existing interface imo.

I was hoping you'd say that - I fully agree.

Shall we start a branch for this? Do you feel like drafting up the necessary code? :)

[0x00002a]

Shall we start a branch for this? Do you feel like drafting up the necessary code? :)

hmm, I wouldn't mind doing it but I was going to work on #55 and #70 so this is gonna have to wait a bit if I'm doing it :p.

Also, not sure if we should offer stuff like web tokens out of the box, but I certainly think the API needs to be built with cases like it in mind. I think for starters we should just offer http auth and then have an example using something more complex.

[Tectu]

hmm, I wouldn't mind doing it but I was going to work on #55 and #70 so this is gonna have to wait a bit if I'm doing it :p.

That's completely fine of course :)
I do think #55 shouldn't take too much work anymore. With MinGW I am able to build & use shared libraries successfully in the current configuration (eg. main branch). I do think that MSVC needs the export macros but I leave that one up to you as discussed in #55.

Also, not sure if we should offer stuff like web tokens out of the box, but I certainly think the API needs to be built with cases like it in mind. I think for starters we should just offer http auth and then have an example using something more complex.

Yeah I fully agree. Having different built-in authentication methods such as JWT is certainly what I'm after.

[0x00002a]

Right so the current implementation plan is:

• interface like add_policy with policy invocable passed. Invocable takes the http method and request header and returns an optional
• Policies are checked before being handed off to the route handlers, if the response is non std::nullopt then its written to the peer and message processing stops. This also means however that we never read the body of the message which may be an issue. If we do end up reading the body the policy should specify how much max imo, for optimisation and to stop possible attacks

Anything I'm missing before I start implementing this?

[Tectu]

This also means however that we never read the body of the message which may be an issue

Which scenario do you see this becoming a problem? Reading the body might often not be necessary by a policy implementation.
I am thinking of an extreme case where one adds a policy that just always returns generator::bad_request(). No need to read the request body ever.

If we do end up reading the body the policy should specify how much max imo, for optimisation and to stop possible attacks

This one I do see. However, that too should be left to be up to the policy implementation.

[0x00002a]

I'm working on this but I've run into a bit of an issue. I need base64 decoding for basic http authentication. I suppose I could implement it myself but I'd rather not, its a real pain and I don't really want to be responsible for testing it and such. OpenSSL has base64 support iirc but that'd make basic http auth support dependent on building with TLS which seems a little weird. We could always just depend on a base64 lib, but casually adding dependencies isn't something I like to make a habit of.

I could also just not add the basic http auth support for now, and leave it up to users to create themselves.

Thoughts?

[Tectu]

I understand that base64 decoding is required for HTTP basic auth. However, I don't understand why this would be a requirement to implement the policy interface? The idea would be to have an interface as discussed above which allows a user to implement their own policy (or rather: policy access checks). Therefore, it shouldn't be necessary to do the HTTP basic auth checking.

For the future it might be nice to have some default policies built into the library. Something like HTTP basic auth might be useful but that would be on top of the interface itself.
Once we get there I think it's a decent approach to just have an interface/hook for base64 decoding so the user can supply his own implementation.
I'm doing something similar in an application which uses malloy. In that particular application, I am using botan which also provides base64 capabilities. Therefore, it would be as simple as using the built-in HTTP auth policy supplied by malloy and simply adding/implementing the provided interface for base64 decoding which would in that case be done via botan.

I could also just not add the basic http auth support for now, and leave it up to users to create themselves.
Exactly :)
Although I would still have a built-in policy for HTTP basic auth - just with an interface for the base64 decoding. I agree that at least of today it would be outside of the scope of malloy to manage this.

Or am I missing / misunderstanding something?