Conversation
dash that run on multiple workers need to be stateless
- secret key is by default the user’s API key. can be over-ridden by setting `app.server.secret_key` (flask’s convention) - an access cookie is created with a time-expiring signature. this is hard to guess without the user’s secret key. - if an invalid access cookie is provided or if the signature has expired, we check if the user has access to the file with the plotly api key and their oauth token (also stored as a cookie). - if they are granted access, then a new time expiring signature is set as a cookie - if access wasn’t granted, the request returns with a 403
i’m pretty sure that it’s part of flask’s required packages but putting it in just to be safe
`gunicorn` usage: ``` gunicorn usage_plotly_auth:server —workers 4 ```
|
@scjody - Could you review this one? I reread the This PR simplifies things keeping the auth module immutable. See 366b309 for the new approach.
|
scjody
left a comment
scjody
left a comment
There was a problem hiding this comment.
Looks good overall! (Nice commit history.)
I'd prefer not to use the user's API key in this way. As a general principle it's equivalent to a Plotly password so the less we use it the better.
In this specific case, the secret key will be the same for all of the apps owned by a given user, so that leads to the possibility of an attacker generating a token with one app (that they're allowed to access) and using it with another (that they aren't).
It looks like you could avoid that risk by adding the app name as a "salt": http://pythonhosted.org/itsdangerous/#the-salt
Suggested longer term alternatives:
- For local testing, is it possible to read a hidden file from the app directory (and create it if it doesn't exist)? If this file lives in
.gitignorethe user won't be able to commit it by accident. - For On-Prem, we could add a random string in 2.4.0 and make it available via an environment variable.
At this point we're all busy so I'd go ahead with the salt-based approach and log an issue re: the improvements (or just add to the discussion in #10776).
Good call 👍
Yup, this works for most cases. Unfortunately heroku doesn't allow you to write to the filesystem, so it won't work for those users deploying to Heroku. However, they can just manually supply a
Yeah, great idea. I'll log an issue. |
this prevents malicious users from using an access token from one dash app (that they have access to) to access another dash app (that they shouldn’t have access to)
2 participants
No description provided.