Location based Access

Hi,

A large humanitarian organisation is involved in a couple of OpenMRS implementations and one of the aspects they are very interested in is improving security in OpenMRS especially when it comes to restricting who can access patient records and data associated to them. Therefore, they are looking at starting with implementing location based access to such data.

@mksd is collaborating with this organisation, they reached out and asked me to help with guiding these efforts outside of my day job and we’re already in the initial stages of working on it.

They are aware of the location based access module that was developed as part of GSoC last summer, thanks to @suthagar23! Given that what they intend to be implementing addresses what could be deemed as a security issue, they feel that they need to take another approach that’s not AOP based and will address its current limitations e.g filtering out data before returning it from service methods via AOP throws off the paging mechanism built into them, I’m also not sure if getCountOfXXX() methods in services are taken care of to stay consistent with their counterparts that return the actual lists of matches.

I just wanted to reach out to the community and make you aware of these efforts, I think it’s key to keep the community involved and try to implement this in a collaborative and transparent way with input from everyone interested.

Currently, I’m spiking on use of hibernate filters and possibly along with an interceptor to see if we can use them to implement location based access, so please feel free to share your ideas, we would love to hear them.

I’m really excited about this because it’s going to help keep me even more involved in the development process of OpenMRS.

Regards,

Wyclif

11 Likes

@wyclif Please do keep us posted on your progress. I would really love to see such functionality incorporated into one of the up and coming releases of OMRS.

1 Like

Thanks @wyclif for sharing and it’s great to keep you involved! :smile:

Is this going to be a new module? Hopefully API only module without a GUI.

Hi,

Just to give a little background of what we’re are trying to achieve with these efforts, we want an implementation to be able to set up their installation such that they can filter out patient records and any clinical data associated to them by a given criteria for each user, our first focus will be to limit access to Patients, Encounters, Observations and possibly Visits by location.

We plan to use person attributes to tie each patient to a specific location, possibly we will support multiple locations per patient. For the first pass, we will have a simple mapping table between users and locations, in fact, if we are faced by time constraints, we could just employ user properties to define locations each user has access to.

We plan on using hibernate filters to limit access along with an interceptor that will be the fall back to block other CRUD operations e.g. loading of an entity because it’s possible that someone might know the DB primary key or uuid.

For us to achieve our goals, we will need the following changes to be made in core and we are ready to work on them ourselves in the interest of time.

  • Change from xml to annotation based mappings, the reason for this is because with xml, the filter and filter defs can only be defined in the hbm files themselves so modules can’t introduce filters externally, on the other hand, with annotations they can via some fancy reflection to mutate the annotation metadata for a given class object of a persistent type, we will need to do this for Person, Patient, Encounters and Obs.

  • We will need to apply some spring AOP around SessionFactory.getCurrentSession() and FullTextSession.createFullTextQuery calls so that we can enable the filters on them in a single centralized place from the custom module. For this to be possible, Search.getFullTextSession needs to be reimplemented to return an instance from some sort of FactoryBean in order to be able to apply spring AOP based advice around it that way, we don’t have to deal with the more invasive aspectj alternative that would require extra work by system admins.

That’s the architectural overview at a glance, there are several other things we plan to address from the custom module.

Since these proposed changes to core don’t seem to be changing any logic, we assume we can include them as part of a minor release.

Your feedback is welcome.

Wyclif

1 Like

Anybody read this?

I forgot to mention that we intend to name the custom module something along the lines of a filtering module e.g openmrs-module-datafilter

This seems to me a very brilliant idea. cc @c.antwi @dkayiwa

i hope your team is also aware of Location Based Access Control Phase 2 ,. i havent got into details of what phase 2 is trying to Enhance or add to phase 1 , but your team should also take that into consideration not to re-invent the wheels.

1 Like

Basing on the module name, do you intend to filter by more than just location?

Any reason why, given time, you would prefer a new table instead of simply using the existing user properties?

I have not yet understood why you would need to AOP around SessionFactory.getCurrentSession(). May be i will understand after looking at the code. :slight_smile:

What is your planned timeline to have this ready for testing?

Thanks again for sharing with us up to this level of detail. :slight_smile:

3 Likes

@mozzy in theory this is going to be a generic datafilter module, the only reason why we are saying location based is because that’s going to be our first main focus, so it’s not just location based access, we envision filtering by say Program, Person Attribute, creator column in the DB etc. Also note the current location based access module is AOP based which has several drawbacks, I touched a little on some of them in my initial post.

I think I have partially answered this, it’s going to be more than just filtering by location, location is just our first primary focus.

I used the user properties as part of the proof of concept but in the future if we’re to model this as a more generic solution to support other filtering capabilities possibly with a domain object backing up the user-location mapping then using user properties can quickly get challenged, there is still a possibility that we might go with user properties in the beginning and evolve with time out to a separate table.

One of the drawbacks of the location-based access module is you have to keep adding advice around each and every method that return patients which means you have to continuously keep adding it to other methods where it is missing or new ones, in fact, how do you deal with module methods with that approach? There is no way. Also, filters can only be applied to session and full text query objects, this poses a challenge that would put us in the same position, so to avoid it, we want to wrap AOP around the 2 factory methods that create session and full text query objects so we can apply our filters in a single place.

Possibly sometime in June.

Thanks for your questions.

2 Likes

I will be creating a new module repo in the OpenMRS space named openmrs-module-datafilter, any objections to the module name?

2 Likes

No objection from me. :slight_smile:

Hi,

We’ve made some changes to core to support the datafilter module’s functionality, here is a list of the issues that we worked on. We were hoping to back port these changes and have them included in platform 2.2.1 release, does anyone have any objection to this?

1 Like

No objections from me.

Is there any reason why we should not instead do a platform 2.3.0 release?

@dkayiwa, i think its our way of doing things, to back port major changes to atleast three previous releases https://wiki.openmrs.org/pages/viewpage.action?pageId=3346649

so i think barkporting to 2.2.1 is pretty much fine

We’ve been running our test and UAT servers on 2.2.x for a couple of months, and we feel confident that things are ok with Core 2.2.x. Upgrading to 2.3.x is something I’d rather keep out of the way for now.

1 Like

@mksd did you notice that these are mostly the changes that have happened on master since the release of 2.2.0? Making the release of 2.3.0 with these changes safer than a back port that even includes some database changes? :slight_smile:

@mozzy we commonly back port bug fixes.

1 Like

thats true.

@jsibley this all conversation means that we need to upgrade Core to 2.3.0-SNAPSHOT asap in our distro. The longer we sit on it the safer we’ll feel.

Cc @lilian

1 Like