This makes perfect sense. And I agree that we can handle this merely by using a naming convention. For sake of not changing the names of the bulk of existing privileges (and privilege.name is the pk of that table), I propose that we go with:
(api-level privileges, stick with the same naming) - View Xyz (for metadata and data) - Manage Xyz (for metadata) - Add Xyz, Edit Xyz, Delete Xyz, Purge Xyz (for data) (application-level privileges, follow the pattern from patientDashboardForm.jsp) - Page Name - View Xyz Section (Ironically patientDashboardForm.jsp is the *only* jsp page I'm aware of where we've actually done this right. So yes, good choice of example. :-) ) So, I guess the proper solution is to look at all of our jsp and jsp-like files (.tag, and anything else?) for <openmrs:hasPrivilege, and identify every one of those in core (and ideally also bundled modules) that uses an API-level privilege. (This will be 90% of them.) Then, for this list of privilege checks, we need to: 1. create a new, matching application-layer privilege 2. switch the jsp to check against the new privilege 3. write a liquibase changeset that will grant the new application-level privilege to every role that has the old api-level privilege. -Darius On Wed, May 9, 2012 at 11:16 PM, Dave Thomas <pihd...@gmail.com> wrote: > First I apologise for using the dashboard as an example -- it was > hypothetical, and therefore crappy. > > And yes, your suggestion is exactly right, but it's a pattern that I think > should be applied globally, and not just to the patient dashboard > components. > > Here's another example: 'View Concepts' is required by gutter.jsp to show > the Dictionary gutter item. There's no reason why our data officers or lab > technicians need to be looking up concepts through the UI. They just need > to be able to fill out forms and look at patient records for data quality. > We've also had instances where research staff have looked for Concepts > associated with specific forms without really knowing the correct way to do > this, which resulted in bad data being exported. Without access to the > Dictionary, they would have been forced to ask us for help. > > So there are a couple of groups of people for whom i'd like to hide the > Dictionary gutter item. > > But if you take away 'View Concepts' for a user, this basically breaks all > of OpenMRS for that user. More than 1/2 the ConceptService requires this > privilege. > > This is kind of an extreme example in terms of the functionality that > would be lost by removing the privilege. I feel like we've run into more > subtle 'accidental' restrictions mistakes that might only manifest > themselves in 1 or 2 unexpected places, sometimes in a module, or maybe in > the various reporting mechanisms we've used. > > What i think would be a good step forward would be a complete separation > between privileges used by the app layer and privileges used by the api, > which I think could be handled through naming conventions for privileges. > This doesn't require any new architecture, but would greatly increase the > granularity of what you can restrict and for whom. Privileges used > concurrently in app and api layers are too blunt of an instrument, and lead > to (sometimes strikingly) sub-optimal role configurations. > > I know that increased granularity seems like it would lead to greater > complexity for implementations, but i'd argue that at least in our > experience its the opposite. > > Does this make sense? > > d > > > On Wed, May 9, 2012 at 5:24 PM, Darius Jazayeri > <djazayeri+...@gmail.com>wrote: > >> All dashboard tabs *do* have application-level privileges already. At >> least in 1.9.x, patientDashboardForm.jsp has: >> >> <openmrs:hasPrivilege privilege="Patient Dashboard - View Overview >> Section"> >> <openmrs:hasPrivilege privilege="Patient Dashboard - View Regimen >> Section"> >> <openmrs:hasPrivilege privilege="Patient Dashboard - View Visits Section"> >> <openmrs:hasPrivilege privilege="Patient Dashboard - View Encounters >> Section"> >> <openmrs:hasPrivilege privilege="Patient Dashboard - View Demographics >> Section"> >> <openmrs:hasPrivilege privilege="Patient Dashboard - View Graphs Section"> >> <openmrs:hasPrivilege privilege="Form Entry"> >> >> >> However looking at patientOverview.jsp I see we refer to the underlying >> API-level privileges: >> >> <openmrs:hasPrivilege privilege="View Patient Programs"> >> <openmrs:hasPrivilege privilege="View Relationships"> >> <openmrs:hasPrivilege privilege="View Allergies"> >> <openmrs:hasPrivilege privilege="View Problems"> >> >> >> Seems like we should write a ticket to: >> >> 1. introduce new privileges like "Patient Overview - View Programs >> Section", etc >> 2. use those in all dashboard portlets rather than the API >> counterparts >> 3. do a liquibase changeset so that every role that has "View Patient >> Programs" also gets "Patient Overview - View Programs Section", etc. (That >> way nobody loses the ability to see something they currently see.) >> >> Dave, would that help? >> >> Daniel, want to do this? >> >> -Darius >> >> >> On Wed, May 9, 2012 at 2:33 PM, Dave Thomas <pihd...@gmail.com> wrote: >> >>> Burke, sorry for not being clear before. I think what you said in #1 is >>> pretty much what i'm asking for. >>> >>> The terminology can be worked out (to use your example 'Access' could >>> imply API-level privileges, and 'View' could imply app-level privileges), >>> but the important thing to me would be to not have 'Access Patient Data' >>> sometimes be applied at the API level and sometimes applied at the app >>> level. With 'Access Patient Data' used at both the API and app layers, if >>> I restrict 'Access Patient Data' for a user, it is incredibly difficult to >>> tell what i've actually restricted. This is when pieces of the >>> application, other than the one I was trying to restrict, start throwing >>> errors unexpectedly (per mark's example). >>> >>> Part two of this argument (and maybe this is a different discussion) >>> would be that once there was a clear distinction between api and app level >>> privileges, the app level privileges could start to reflect actual UI >>> components a little more. If each portlet on the patient dashboard had its >>> own privilege for example, like 'view regimen tab', the UI would be a LOT >>> more easy to customise in a meaningful way using privileges and roles. >>> >>> d >>> >>> >>> On Wed, May 9, 2012 at 1:43 PM, Burke Mamlin <bmam...@regenstrief.org>wrote: >>> >>>> Mark, >>>> >>>> This is an interesting point. Your example helps me understand Dave's >>>> point and suggests that we should distinguish between "Access Patient Data" >>>> and "View Patient Data" – i.e., the use of "View" in the privilege term >>>> describing API-level access to data per our conventions implies that >>>> getting data at the API level also means that the user should be able to >>>> view it within the application, which is not always the case. In your >>>> example (a person being able to view aggregate patient data without viewing >>>> individual patient records), we have two choices: >>>> >>>> 1. Grant the user "Access Patient Data" as the API-level privilege, >>>> meaning that they would be permitted to execute API calls that return >>>> patient information, but would not imply that the application should let >>>> them view the data directly. >>>> >>>> 2. Allow the methods that need to access patient data to proxy >>>> privileges for the user. >>>> >>>> The first seems like the better option to me. Proxying privileges >>>> seems like a hacky way to override privileges that are there for a reason. >>>> The first option will take some effort (maybe more than the upcoming >>>> sprint can muster), but seems like a better long-term solution. Maybe we >>>> can come up with a way to start heading that direction. >>>> >>>> -Burke >>>> >>>> >>>> On Wed, May 9, 2012 at 3:55 PM, Mark Goodrich <mgoodr...@pih.org>wrote: >>>> >>>>> +1 for this. We’ve run into issues with this a lot. For instance, >>>>> say you have a user who should not be able to view patients, but should >>>>> have access to a report of aggregate patient data that calls getPatients() >>>>> behind the scenes to perform the necessary calculations. If you take away >>>>> the “view patient” privilege from the user, they’ll get a stack trace when >>>>> they attempt to execute the report.**** >>>>> >>>>> ** ** >>>>> >>>>> This issue has been prevalent enough for us that we basically are >>>>> unable to use privileges and roles for access control … **** >>>>> >>>>> ** ** >>>>> >>>>> Mark**** >>>>> >>>>> ** ** >>>>> >>>>> ** ** >>>>> >>>>> *From:* dev@openmrs.org [mailto:dev@openmrs.org] *On Behalf Of *Dave >>>>> Thomas >>>>> *Sent:* Wednesday, May 09, 2012 2:51 PM >>>>> *To:* openmrs-deve...@listserv.iupui.edu >>>>> *Subject:* Re: [OPENMRS-DEV] Roles and Privileges Sprint**** >>>>> >>>>> ** ** >>>>> >>>>> If i can weigh in on a discussion of roles and privileges briefly, >>>>> I've found (and maybe things are improved in versions greater than 1.6.x) >>>>> that we've run into trouble when a privilege is used both at the api and >>>>> web layer. This happens when we want to hide something in the UI that is >>>>> wrapped in a <privilege/> tag, but then when we remove that privilege for >>>>> a >>>>> user, it turns out that the same privilege was wrapped around a couple of >>>>> API methods, causing unexpected and hard-to-track-down UI problems across >>>>> all of openmrs. >>>>> >>>>> If there's a dedicated pass at roles+privileges, has there been any >>>>> thought to separating api privileges from ui privileges? I kind of feel >>>>> like this is low-hanging fruit. This wouldn't even need re-architecting, >>>>> maybe just privilege naming conventions? >>>>> >>>>> Or is the general sense that this type of problem has disappeared in >>>>> newer versions? >>>>> >>>>> d**** >>>>> >>>>> On Wed, May 9, 2012 at 10:29 AM, Burke Mamlin <bu...@openmrs.org> >>>>> wrote:**** >>>>> >>>>> (bringing this conversation about preparing for the Roles & Privileges >>>>> sprint onto the dev list)**** >>>>> >>>>> ** ** >>>>> >>>>> Reviewing the notes <http://notes.openmrs.org/2012-roadmap> on Roles >>>>> & Permissions section from Jembi & PIH, it looks like there are some >>>>> fundamental improvements requested:**** >>>>> >>>>> - Ship OpenMRS with pre-defined roles**** >>>>> - Better documentation on managing roles (avoiding pitfalls)**** >>>>> - More informative handling of privilege exceptions (make it >>>>> easier to understand where/which privileges are missing)**** >>>>> - Data-level permissions (restricting access to specific data >>>>> based on privileges)**** >>>>> >>>>> We've had prior conversations about improving roles/privileges:**** >>>>> >>>>> - Avoiding the common pitfall of conflating organizational roles >>>>> (job title) with application roles (authorization within OpenMRS); >>>>> they may >>>>> align early on in simple systems, but exceptions are common over time >>>>> or as >>>>> a a system grows.**** >>>>> - Creating privilege groups vs. programmatically defined roles – >>>>> e.g., a web page wants to limit access to those who have a set of >>>>> privileges.**** >>>>> - Introducing location-based privileges**** >>>>> >>>>> There seem to be some potential short-term wins that could be done in >>>>> the sprint:**** >>>>> >>>>> - Improve our documentation to better introduce people to roles & >>>>> privileges and cover the common pitfalls.**** >>>>> - Improve privilege error messages in core and/or create a module >>>>> that makes it easier to troubleshoot privilege errors (e.g., log all >>>>> privilege checks during an operation and present the unique list of >>>>> privileges and/or roles that would cover the operation, allowing >>>>> someone to >>>>> step through a workflow as superuser and then see the list of >>>>> privileges >>>>> required to complete the workflow).**** >>>>> - Come up with some basic application roles that can be >>>>> pre-defined within OpenMRS (ship with the application)**** >>>>> - Design (and, if possible, implement) an approach for privilege >>>>> groups or system roles (i.e., uneditable sets of privileges that >>>>> applications can program against)**** >>>>> >>>>> Data-level privileges (limiting access to data based on privileges) >>>>> would be a terrific addition, but I'm afraid it will take more design that >>>>> we can muster between now & the beginning of this sprint. Maybe we could >>>>> come up with some small but useful first attempts at solving this problem >>>>> (e.g., a module requiring permissions to access certain observations … or >>>>> a >>>>> module that limits access to specific patients based on permissions).* >>>>> *** >>>>> >>>>> ** ** >>>>> >>>>> Cheers,**** >>>>> >>>>> ** ** >>>>> >>>>> -Burke**** >>>>> >>>>> ** ** >>>>> >>>>> On Wed, May 9, 2012 at 9:49 AM, Burke Mamlin <bu...@openmrs.org> >>>>> wrote:**** >>>>> >>>>> Looking back at notes from AMPATH, the only reference to anything >>>>> close to roles & privileges I found was the desire for the Data Entry >>>>> Statistics Module to have a basic view privilege that allows a data >>>>> assistant to see only his/her own statistics.**** >>>>> >>>>> ** ** >>>>> >>>>> -Burke**** >>>>> >>>>> ** ** >>>>> >>>>> On Wed, May 9, 2012 at 9:44 AM, Ben Wolfe <b...@openmrs.org> wrote:**** >>>>> >>>>> Dawn found this link for me: >>>>> http://notes.openmrs.org/2012-roadmap >>>>> >>>>> Is has the (mostly raw) notes from the calls we had with >>>>> Jembi/PIH/AMPATH. >>>>> >>>>> Daniel, can you tease out the topics from that and the other text >>>>> below in the next 4 hours? >>>>> >>>>> Ben**** >>>>> >>>>> ** ** >>>>> ------------------------------ >>>>> >>>>> Click here to >>>>> unsubscribe<lists...@listserv.iupui.edu?body=SIGNOFF%20openmrs-devel-l>from >>>>> OpenMRS Developers' mailing list >>>>> **** >>>>> >>>>> ** ** >>>>> ------------------------------ >>>>> >>>>> Click here to >>>>> unsubscribe<lists...@listserv.iupui.edu?body=SIGNOFF%20openmrs-devel-l>from >>>>> OpenMRS Developers' mailing list >>>>> **** >>>>> ------------------------------ >>>>> Click here to >>>>> unsubscribe<lists...@listserv.iupui.edu?body=SIGNOFF%20openmrs-devel-l>from >>>>> OpenMRS Developers' mailing list >>>>> >>>> >>>> ------------------------------ >>>> Click here to >>>> unsubscribe<lists...@listserv.iupui.edu?body=SIGNOFF%20openmrs-devel-l>from >>>> OpenMRS Developers' mailing list >>>> >>> >>> ------------------------------ >>> Click here to >>> unsubscribe<lists...@listserv.iupui.edu?body=SIGNOFF%20openmrs-devel-l>from >>> OpenMRS Developers' mailing list >>> >> >> ------------------------------ >> Click here to >> unsubscribe<lists...@listserv.iupui.edu?body=SIGNOFF%20openmrs-devel-l>from >> OpenMRS Developers' mailing list >> > > ------------------------------ > Click here to > unsubscribe<lists...@listserv.iupui.edu?body=SIGNOFF%20openmrs-devel-l>from > OpenMRS Developers' mailing list > _________________________________________ To unsubscribe from OpenMRS Developers' mailing list, send an e-mail to lists...@listserv.iupui.edu with "SIGNOFF openmrs-devel-l" in the body (not the subject) of your e-mail. [mailto:lists...@listserv.iupui.edu?body=SIGNOFF%20openmrs-devel-l]
