cesium / atomic

⚛️ De-engineered bifurcarted intranet
MIT License
39 stars 8 forks source link

Security flaw with routes protection #414

Closed joaodiaslobo closed 11 months ago

joaodiaslobo commented 12 months ago

The current plug protecting the routes is only checking if the authenticated user has permissions inside the organization in the organization_id parameter, while this does work on all occasions and redirects inside the app, if a malicious user decides to change the organization_id parameter to one belonging to an organization that they have priveleges over, the plug authorizes the user. In other words, the object's association with the id parameter is not being checked.

image

292 Added AtomicWeb.MismatchError for this specific reason but currently it doesn't seem to be in use.

ruilopesm commented 12 months ago

I deleted Atomic.MismatchError while refactoring the app flow (for faster development) and forgot to add it back (or at least think about a new way of providing its functionality). Anyway, do you suggest getting it back on train?

joaodiaslobo commented 12 months ago

I deleted Atomic.MismatchError while refactoring the app flow (for faster development) and forgot to add it back (or at least think about a new way of providing its functionality). Anyway, do you suggest getting it back on train?

@RuiL1904 My initial implementation of AtomicWeb.MismatchError wasn't great because you had to write a function to check if the entities matched on every page file (index.ex, edit.ex, etc..) and call it on every handle_params. Ideally, this comparison logic would be done on the authorize plug, but unfortunately as of right now, I can't think of a way to check if an abstract object in the database has an association with an organization_id. Perhaps Ecto has something to handle these cases? I'll try to research this and come up with a better way to handle these situations. Any suggestions/ideas from the rest of the team would be greatly appreciated 🙏