SolutionGuidance / psm

Welcome to the Medicare/Medicaid Provider Enrollment Screening Portal
http://projectpsm.org/
Other
26 stars 20 forks source link

Access denied error on application save and submit #1105

Closed PaulMorris closed 5 years ago

PaulMorris commented 5 years ago

Saving an application as a draft and then submitting it produces a server error with the message "Access Denied". (On current master.) See background discussion on zulip and the error log.

A similar error is produced when saving an individual provider application as a draft, and then saving it as a draft a second time.

Also a similar error is produced when attempting to edit a submitted application as a provider (error log).

frankduncan commented 5 years ago

This is caused because of how the profiles are now loaded. Technical brain dump incoming:

When we used to load up attachments, there was a check to see if a user had access to that attachment. That was done via a check on the ownerId on the provider_profile table. However, this check was NOT done if the ticket id was 0, meaning that the provider_profile was not yet attached to an approved ticket (we used to use 0 as a marker for unapproved providers). This may have actually been an information leak because of how drafts were overridden with regards to security because we didn't actually look at things too closely throughout the application if it hadn't been approved.

In the new world, attachments are associated with, and only with, profiles. And profiles are always associated with existing enrollments. Which means the override no longer makes sense, and was removed. So we always check permissions on attachments when loading up for draft editing/submitting.

But! The ownerId field is not set until an enrollment is approved. This is another marker flag for both permissions, and how to fill out the "My Profile" table for a provider, and whether an application is available for renewal/editing. The "My Profile" is actually no longer needed because of the changes in PR #967, and we should start evaluating whether something can be edited/renewed based on enrollment status, not the ownerId marker field.

However, making that change may be too substantial at this point, and so the long winded discussion is here.