@cielf to clarify you mean something like:

I log in a partner and go to http://localhost:3000/reports/annual_reports/2023 and get hit with

?

I think the particular error that's coming up in that case is because they don't have a 'current organization" because they are signed in as a partner. I think that's going to be the kind of thing that currently happens on most of the relevant cases, but/and they shouldn't be able to access any of the views that require org_user or org_admin status when logged in as a partner. Does that help?

Hey all,

With apologies for forging rashly ahead (I took a look at this to see if I could figure it out, and by the time I figured out I could, well, it was mostly done) I'm working on a draft for this one: #4650

I still have some tests to write, and per the reqs need to bubble up a slightly more helpful message than Access Denied, but put up a draft bc I find it it helpful to see in diff view (and so folks can chime in if I've totally misunderstood something here!)

It might be a day or two before anyone has a chance to take a look at what you've done, but I've assigned it to you.

thanks @cielf! I'll try to get the rest of my ducks in a row here meanwhile :)

@therufs not sure I understand what's changed in that PR. As far as I can tell you renamed a method, but I don't really see how that fixes this problem?

This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.

Hey @dorner -- sorry I missed this when it was fresher! The renamed method is now being called as a before_action in the annual_reports_controller and reports_controller (which other reports inherit from). As in the screenshot elasticspoon and cielf were discussing above, the 500 error we're currently seeing is due to those reports trying to look things up on nil orgs, so the before methods should ascertain that the user persona who's trying to access the report has the necessary information to successfully find an org.

I'm still a bit confused.

ApplicationController has this as a filter:

  def authorize_user
    return unless params[:controller] # part of omniauth controller flow
    verboten! unless params[:controller].include?("devise") ||
      current_user.has_role?(Role::SUPER_ADMIN) ||
      current_user.has_role?(Role::ORG_USER, current_organization) ||
      current_user.has_role?(Role::ORG_ADMIN, current_organization) ||
      current_user.has_role?(Role::PARTNER, current_partner)
  end

Are you saying that in the case of the 500 error, some of the other checks are stopping this from doing verboten!? If so, which? I can't see the logic that this would help with.

tl;dr: No, I think verboten! still runs, but since the conditionals we're checking before we run it still permit users who currently have the PARTNER role, and users who currently have the PARTNER role have a current_organization of nil, we still end up having problems. Or, put another way, the "user" that's being authorized in this authorize_user is too broad a net for these specific cases, so we need to check specifically that the user is an ORG_USER or admin.

Deeper exploration, following through the stack trace above from the bottom:

1. The user is in the PARTNER role, so the current_organization helper doesn't find anything (and does not raise an error).
2. the annual_reports_controller asks Reports to retrieve_report, passing organization: current_organization (nil).
3. AnnualReport.find_or_create_by is called with report_attributes that include the nil organization. (On the topic of not raising errors, AnnualReport does not seem to immediately care that its organization is nil, which I would have guessed belongs_to would validate -- possibly the error ends up getting bubbled up before the validation occurs?)
4. Reports#all_reports generates an array of new different types of Report, with organization: organization (nil)
5. the new report in question finally tries to ask something of the nil organization, specifically in the example for the total_disposable_diapers_distributed in the time specified
6. Nil cannot respond to the inquiry, NoMethodErrors (500s to prod users) ensue

As a meta observation, while this issue is specifically about reports, the same error is generated when a partner visits any page that is only accessible to org users (e.g. /purchases). This problem doesn't go the other way, because the BaseController that partner-related controllers inherit from already handles a redirect for users without appropriate credentials. If we wanted to commit to the bit, we could refactor the controllers that handle org actions to inherit from a similar specialized class that could implement and call a require_org?

Yep, that's what I think we need to do. Now I get why this is happening, but your current fix doesn't go far enough. Your suggestion is what I would go with. Rather than inheriting, maybe we could create a module that all the org controllers include (because otherwise I think we'd have to mess with all the routes to make it make sense).