-
Notifications
You must be signed in to change notification settings - Fork 274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Require private code to edit a project settings, document better the invite options #1204
Conversation
That makes sense to me, go ahead with this :-) |
Ok :) The good thing I had not anticipated: it also restricts changing the settings through the API if you just have a token (which is the same as the invite token). So, you also need the current private code to change settings:
I need to update tests, add new tests that check this, and update the API doc. |
Crap, you can still delete a project through the API with just an auth token, so it's not strictly equivalent :( |
241d511
to
25e588e
Compare
This is something we had documented in our security documentation [1], but we didn't actually do it... As mentioned in [1], this has good security properties: you can invite somebody with an invitation link, and they will be able to access the project but not change the private code (because they don't know the current private code). This new check also applies to all other settings (email address, history settings, currency), which is desirable. Only somebody with knowledge of the private code can now change these settings. [1] https://ihatemoney.readthedocs.io/en/latest/security.html#giving-access-to-a-project
We now display a success flash message, and we stay on the same page (instead of redirecting to the list of bills, which makes little sense)
Also move the "invitation link" option first, because it's the preferred way to give access to people that only need to handle participants and bills. Sharing the identifier and private becomes the last option, because it gives full access to changing settings.
25e588e
to
451640e
Compare
OK, I'm going to merge this, and leave the project deletion issue for #1206. This is already an improvement. |
Thanks, it's good that we don't let things rot ;) |
This is something we had documented in our security documentation, but we didn't actually do it...
As mentioned in this document, this has good security properties: you can invite somebody with an invitation link, and they will be able to access the project but not change the private code (because they don't know the current private code).
This new check also applies to all other settings (email address, history settings, currency), which is desirable. Only somebody with knowledge of the private code can now change these settings.
Finally, document these security properties in the invitation page.