Add CSRF protection #27

[mapeveri]

No description provided.

[josephmancuso]

Ok so the only reference of request should be getting the input and the cookie. Make all new methods relating to CSRF inside it's own class. There should really only be 3 new additions to the codebase.

1. a CSRF class
2. a CSRF service provider
3. a CSRF middleware

[josephmancuso]

Ok so let's move all these csrf methods out of the request class and into a new Csrf class.

[josephmancuso]

This needs to be inside it's own CSRFProvider file. This should also be:

ViewClass.share('csrf_field', "<input type='hidden' name='csrf_token' value='{0}' />".format(token))

then inside the template, we can do:

<form action="" method="">
    {{ csrf_field }}
    <input type=.....
</form>

[josephmancuso]

make sure the typo in befofe is before

[josephmancuso]

When you add the CSRFProvider then you'll need to also but CSRF in this constructor

[josephmancuso]

When you move these verify methods into it's own CSRF class make sure you use that here.

[josephmancuso]

Looks good to me. I'll test it out by hand in a few hours. Just some small changes I see here

[josephmancuso]

remove the self.app.bind('Request', request)

the request is already binded to the container in the AppProvider

You're overwriting it here

[josephmancuso]

Notice that you're getting the .input('csrftoken') but inside the input on line 19 of CsrfProvider it is csrf_token. I'd go with the underscore one

[josephmancuso]

This works really really well with a few minor changes. Remember that middleware also runs on every request (with the exception of Route Middleware). But all HTTP middleware runs on every request so we can use this to our advantage. Instead of making the provider run on every request AND the middleware run on every request, we can move the logic into the middleware.

I like the idea of making the providers stackable and would hate for the documentation to say "make sure you place this provider between 2 providers" which is easy enough but what if other third parties also want that too and now we have to place it between a third party provider AND a one of ours. It just gets really messy that way so let's make providers able to stack and be placed at the bottom and still work where applicable.

Only the core providers that actually build the framework (the first 5) need to run on every request and therefore need to be at the top.

Like always if you have any questions at all about the changes please let me know I'll be happy to answer them

[josephmancuso]

this should be .generate_csrf_token()

[josephmancuso]

this is going to return a byte string. We need a unicode string in order my make a cookie so this needs to be changed to:

token = bytes(binascii.b2a_hex(os.urandom(15))).decode('utf-8')

this will return a unicode string

[josephmancuso]

Let's call the csrf token csrf_token. Change where needed in the code. This way when I check my browser it should say csrf_token. Just a little cleaner as well as centralizes the naming scheme (since we now have csrftoken and csrf_token running around the code)

[josephmancuso]

csrf_token

[josephmancuso]

remove this. see comment below

[josephmancuso]

Ok let's move this share inside the middleware. If it's running on every request anyway then it should be fine. Since we are moving it into the middleware, we can remove everything inside this boot method. Since there is nothing in the boot method we can set wsgi = False and now we don't have this running on every request since we're just using this provider to load things into the container. The middleware will be the main thing that fires on every request as usual.

[josephmancuso]

so while testing, my middleware ended up looking like this:

from masonite.exceptions import InvalidCSRFToken


class CsrfMiddleware(object):
    """
    Verify csrf token middleware
    """

    exempt = []

    def __init__(self, Request, CSRF, ViewClass):
        self.request = Request
        self.csrf = CSRF
        self.view = ViewClass

    def before(self):
        token = self.__verify_csrf_token()

        self.view.share(
            {'csrf_field': "<input type='hidden' name='csrf_token' value='{0}' />".format(token)})

    def after(self):
        pass

    def __in_except(self):
        """
        Determine if the request has a URI that should pass
        through CSRF verification.
        """

        if self.request.path in self.exempt:
            return True
        else:
            return False

    def __verify_csrf_token(self):
        if self.request.is_post():
            token = self.request.input('csrf_token')
            if not self.csrf.verify_csrf_token(token) and not self.__in_except():
                raise InvalidCSRFToken("Invalid CSRF token.")
        else:
            token = self.csrf.generate_csrf_token()

        return token

I basically just copied and moved all the logic into it's own private method and then copied and pasted the viewclass.share method.