ponzu-cms / ponzu

Headless CMS with automatic JSON API. Featuring auto-HTTPS from Let's Encrypt, HTTP/2 Server Push, and flexible server framework written in Go.
https://docs.ponzu-cms.org
BSD 3-Clause "New" or "Revised" License
5.7k stars 385 forks source link

Discussion: addon for client request authentication #136

Closed nilslice closed 5 years ago

nilslice commented 7 years ago

There has been some recent discussion about implementing an authentication addon to be used inside the various interface methods that can be added to content types. For example, when using the item.Hideable interface to determine if a type should be exposed or hidden from the content API, it would be reasonable to check the request for a auth token to verify that the data can be viewed or not.

The components that come to mind include:

I envision the typical use case for this would be something like:

// content/song.go

// ...
import "path/to/pkg/auth"

func (s *Song) Hide(res http.ResponseWriter, req *http.Request) error {
        ok, err := auth.Passes(req)
        if !ok {
                // nil error value returned will tell handler to hide the content and 404
                // optionally, could return the err itself so handler can check and return
                // a 401 Unauthorized instead
                return nil
        }

        // in the success case, the return could be something like the existing 
        // `item.ErrAllowHiddenItem` value or something else more generic to be
        // checked by the handler and respond accordingly. This ex. implementation
        // of auth.Passes(req) would require the error value to not be nil in any case.
        return err
 }

The reason for the ok, err := pattern and not just err := is so the package is useful outside of the Hide() method, and the ok bool can be used in cases where the error value is less important. I could be wrong here though, so I'd love to get other example uses / implementations if you have one that you like better!

This would require the auth package addon to know quite a bit about how Ponzu sets up the internal auth mechanism, and would not be very generalized -- I think this is probably good though, because it removes the requirement for the Ponzu developer to have to fetch the system's client secret and JWT/cookie implementation. Behind the scenes the auth package could check for the token in request from multiple sources: header authorization, cookie, URL query, and the the post form.

/cc @eticzon

eticzon commented 7 years ago

Nice write-up @nilslice! Some comments:

The reason for the ok, err := pattern and not just err := is so the package is useful outside of the Hide() method, and the ok bool can be used in cases where the error value is less important.

IMHO, we should just use err := here as auth.Passes(...) (or whatever the final name maybe) is a validation func. ok is mostly used in the "comma, ok" idiom to prevent run-time errors when checking map membership and doing type assertion. Also err := is more succinct.

This would require the auth package addon to know quite a bit about how Ponzu sets up the internal auth mechanism, and would not be very generalized ...

+1! I like the idea of the auth package handling the details behind the scenes.

As for the components, I would have to read more of ponzu internals to better comment on those.

nilslice commented 7 years ago

hey @eticzon -

IMHO, we should just use err := here as auth.Passes(...) (or whatever the final name maybe) is a validation func. ok is mostly used in the "comma, ok" idiom to prevent run-time errors when checking map membership and doing type assertion. Also err := is more succinct.

I agree it is a bit outside the idiomatic styling, but its just an artifact of naming the boolean return value "ok", instead of "passes" or something else - I'd be fine to name it something other than "ok". Plus, the map membership comma, ok pattern is reverse from this.

Further, since the interface methods have an error return, (like Hide(res, req) error, and Create(res, req) error) the error value is interpreted in different ways based on the method. This makes the error value coupled a bit too tightly to the handler which interprets it. For example, see hide.go and its implementation in the contentsHandler in handlers.go

The error value returned from Hide interface method determines if the handler responds with a 404 (in the case that Hideable is implemented), a 500 if Hide()'s error is non-nil and not special error item.ErrAllowHiddenItem, or the JSON content response if the error is item.ErrAllowHiddenItem - so, if we only return the error, and check for a nil value to determine the request passes auth-checks, then the auth addon in discussion is really only useful inside these interface methods (which return the error to a handler).

Conversely, keeping a boolean return value in addition to the error enables a caller to verify the auth-check (with the boolean) and then make a decision on how to proceed based on the error value.

If you think I am over-complicating this and have an example or thoughts which challenge mine, please share. I want to make sure this is done right :+1:

nilslice commented 7 years ago

@eticzon - hope all is well! I recently open sourced a version of this "access control" / auth concept we had discussed and I was hoping you'd take a look: https://github.com/bosssauce/access

I have added a new system/db package method, db.AddBucket(name string) so that a bucket from a package can be added at system initialization. If you'd like to try this out, you'll need to run a fresh go get on Ponzu to get the latest commits.

The main motivation is to make it easy to create tokens for auth based on requests, and then to check the access granted based on how the grant was initially configured (is it still valid? header or cookie? and so on).

requaos commented 7 years ago

I had fun playing with your https://github.com/bosssauce/access I extended it a bit in this fork, including a middleware function to guard endpoints; however, I implemented server-side user account approval, so it isn't your standard user registration process: https://github.com/Requaos/access

and a usage example with a postman collection for user creation, user authentication and making an authenticated call to an endpoint guarded by the middleware function: https://github.com/Requaos/ponzu-test

I didn't specialize in security, so please point out any obvious flaws in my implementation or even just suggestions for improving the process...

nilslice commented 7 years ago

I like your approach -- thanks for sharing this!

I'd suggest creating a new addon, which is either a fork of access, or builds on top of it as a dependency. The idea behind access was for it to be very general purpose, not necessarily just for login / sessions. However, a purpose-built "user" access addon would be great.

If you are able to, please tell me more about what you're building with Ponzu. Gathering this kind of feedback only helps make it better for everyone. Though, I totally understand if it is a project you can't publicly speak about.

requaos commented 7 years ago

Good idea, I see about separating the code and defining it as an ancillary plug-in, which may or may not depend on the 'access' plugin... As for Ponzu, I see it as the perfect micro-services building-block because you can arbitrarily plug your own code into any of the object lifecycle hooks:

  1. 'Authentication': Here you see the example service
  2. 'Authorization': This is minimally provided by the middleware service
  3. 'Monitoring': This requires a standard endpoint be on all nodes, to be called by this service. Achieved by 'uptime/status' object with Ponzu to return those stats on [PUT]
  4. 'Service Discovery': This would require a startup call to scan each ip in the current subnet for response on a given port+endpoint.
  5. 'Configuration': This is where the service needs are listed, this is where a service first calls on start to get a list of required services for a minimum viable product. Whether the service meets these requirements during service discovery is reported as a status on the monitoring endpoint.
  6. 'Storage': While Ponzu does have a datastore, one service would definitely need to provide access to either a filesystem, rmdbs or both.
  7. 'Deployment': Well it kinda just wrote itself by integrating 'Gogs'+'Drone'+'Docker'... basically, you have a Gogs repo for each of your Ponzu services, configure Drone to deploy on push and have it produce a fresh Docker image... given the architecture thus far, you should be able to stand that new image up right next to the currently running one then call shutdown on the old one, scripting it so to stops after servicing the last request.

Still leveraging the advantages of HTTP/2+push technology versus gPRC... I am told any modern microservices architecture should be using gPRC, but by abstracting the actual application details from the services definitions with Ponzu you gain the ability to quickly stand up any number of 'proof-of-concept' micro-services designs with an OOBE feel... pipe-dreams are still cool right?

olliephillips commented 5 years ago

Closing this issue. No activity. Please reopen if need to.