rubyforgood / human-essentials

Human Essentials is an inventory management system for diaper, incontinence, and period-supply banks. It supports them in distributing to partners, tracking inventory, and reporting stats and analytics.
https://humanessentials.app
MIT License
436 stars 450 forks source link

Unauthorized requests should result in 403/404 responses, not 500s, and basic login switching between bank/partner should direct to respectively authorized dashboards #4471

Open cielf opened 5 days ago

cielf commented 5 days ago

Summary

If people go to a URL that is the partner dashboard, but log in as a bank, they are getting a 500 error. We'd like to gently steer them where they should go, instead.

Details

Due to the security concerns below, this is definitely a core team onlhy thing, and we'd like to take care of it without exposing it to the world at large.

See support thread

Image

I'm not sure how often this can occur other than in switching between partner and bank.

It's concerning that requests that seem to require 403/404 responses are resulting in server errors, but solving that wouldn't entirely resolve the users' error.

When they're logging in, would it be possible to auto-detect that we're about to redirect them to a URL not valid for their currently active role and instead take them to the default dashboard for the active role?

Security

There's also a security implication here in that the Logout button we show when you're at the wrong page doesn't log you out (and doesn't show a warning). See https://rubyforgood.slack.com/archives/C029TFH5873/p1704333543464299?thread_ts=1704325981.838449&cid=C029TFH5873

Note: something that seems pretty problematic security-wise is that, if you're on the 500 error and click "Log out", you proceed to a blank screen with a 'logout'/'signout' URL, but you haven't been logged out (maybe because we tried to log you out as a partner?). You're still logged in as a bank. Pretty bad to present a logout button that you can click and seems to take you away from the logged-in app but which has left you logged in as a bank.

Additional info

This is showing up in bugsnag as "NoMethodError partners/dashboards#show undefined method `requests' for nil:NilClass @partner_requests = @partner.requests.order(created_at: :desc).limit(10) ^^^^^^^^^" -- we got about 10 of those in the period from Jan 2 -9, from 3 different users, so it's something people are doing

Criteria for completion