Auth using before_request decorator

[RenaudLN]

This PR changes the behaviour of Auth (and BasicAuth) to work on every single route by default. Until now, only the routes that were created before the instantiation of Auth were protected.

To ensure every route goes through the auth check, a before_request decorator is used.

Bonus: Users can whitelist endpoints by marking them as public via Flask's config using the PUBLIC_ROUTES key. The public routes should follow the same format as regular Flask routes.

[alexcjohnson]

I like this a lot! I wonder if we can (optionally) combine the public routes into the constructor, ie instead of two calls:

add_public_routes(app, ["/home", "/user/<user_id>/public"])
BasicAuth(app, USERS)

you could write it as:

BasicAuth(app, USERS, public_routes=["/home", "/user/<user_id>/public"])

[RenaudLN]

Good idea, added public_routes to the constructor (which under the hood just calls add_public_routes).
Also updated the test to check that both paths work.

[fkuschel]

Hey Renaud,

I was looking exactly for the new feature "integration of public routes" for my multi page dash app.
I installed your latest branch and somehow the public routes pages still require a password.

Would you mind having a short look into this minimal example? Do you see something wrong here?

import dash_auth
from dash import Dash, html, dcc
import dash_bootstrap_components as dbc


app = Dash(__name__)
VALID_USERNAME_PASSWORD_PAIRS = {'fkl': 'fkl'}

dash_auth.BasicAuth(app, VALID_USERNAME_PASSWORD_PAIRS,
                    public_routes=["/"])

app.layout = dbc.Container([
    dcc.Store(id="table_edits"),
    dbc.Row([
        dbc.Col(html.H2(["Test"]), width=12, xl={"size": 4})])])

if __name__ == "__main__":
    app.run_server()

Thanks in advance :)
Best regards
Florian

[RenaudLN]

Yes you are right it actually doesn't work in its current state... The issues are:

• All the dash dependencies, layout route etc. need to be whitelisted, that can be done easily
• Some or all the assets need to be whitelisted, for js scripts and css it's fine to whitelist them all but for other assets it might be an issue to make them public (e.g. hosting some static documentation)
• Dash's callback all use a single /_dash-update-component, that is also the case for the navigation of multi-page apps. There might be workarounds to catch the callback that does the navigation but the issue would then be to whitelist the callbacks that are accessible on those public routes. Maybe we could use the request.referrer to decide whether a callback is allowed but I have a feeling this is not safe as someone on a public route could do a post request to /_dash-update-component with the right set of arguments and get the data they want.

Given the above, I think I will remove the public routes part of the PR to keep only the change to a @before_request, this will give the benefit of protecting all routes, even ones added after the auth is instantiated.

@alexcjohnson happy to have your thoughts on this or if you see some workarounds I may not have thought about.

[RenaudLN]

@fkuschel I gave it a bit more thought and I believe I have a working solution. It does require to mark all callbacks on public routes as public_callbacks but I don't see any other way around it.

Checkout the updated Readme for usage and example in a multi-page app.

[fkuschel]

@RenaudLN,
for me this approach works perfectly!

Thanks a lot!

[RenaudLN]

I reverted this PR to just being about using the before_request decorator. I will create another PR for the public routes once this is merged.

@alexcjohnson Would you be able to give this a final review?

@fkuschel If you were using the branch from my fork, you should switch to the new branch feature/whitelist-routes

Also looks like the tests are failing due to issues with installing the latest version of the Chrome driver in CircleCI.

[alexcjohnson]

@RenaudLN this looks great. Can you update the browser-tools orb like in https://github.com/plotly/dash/pull/2603/files? That should get the tests running again.

[RenaudLN]

Done and tests passed :)

[alexcjohnson]

💃 Thanks very much @RenaudLN - and @fkuschel and @olivier-lacroix thank you for your feedback :)

[fkuschel]

@RenaudLN , thank you very much for your suggestions!

[alexcjohnson]

@RenaudLN, @fkuschel, and @olivier-lacroix - my apologies, I had forgotten to create a release for this work! But v2.1.0 is now published.

[alexcjohnson]

@RenaudLN this PR includes whitelisting, does that mean we can close #142 or is there additional functionality there?

[RenaudLN]

@alexcjohnson I had split out the whitelisting from this PR as this one was originally about protecting all routes with before_request and whitelisting was starting to muddy up the changes. #142 is still required to add the whitelist functionality.

[alexcjohnson]

Ah ok, I had forgotten that in the history here. If you want to update that PR I can make another release!

[avegancafe]

Is there no way anymore to have unprotected paths? This seems like a breaking change

[RenaudLN]

#142 will add a proper way to have public routes. Previously it was purely based on the order of definition of routes vs the auth which isn't intuitive (or safe in a lot of cases).
#142 will also bring public routes for multi-page apps.