Developing the "OCL for OpenMRS" Application


(Alex Mochu) #202

Hello, This is a reminder that we will be holding our seventh splint planning session in the next next 30 minutes. To join please follow the link below.

https://hangouts.google.com/hangouts/_/aya4nfmr7jhsxajbijdn72eax4e

cc Emmanuel Ndukwe{@emasys } Emmanuel Abaye {@emmabaye } Kelvin Kariuki {@nesh } @darius @dkayiwa @paynejd


(Daniel Kayiwa) #203

Is it possible to use anything else other than google hangout?


(Alex Mochu) #204

We can use zoom it’s a good alternative to google hangout. But for this call I think we will just go with google hangout, since it’s a bit late to change now.


(Alex Mochu) #205

Hello everyone,

Thank you for attending the Seventh Sprint Planning session. In case you missed it you can follow the link below:

cc @dkayiwa @akanter @darius @ball


(Cintia Del Rio) #206

Uberconference should be available for that too, no?

@dkayiwa you have as much access as I do :smiley:


(Cintia Del Rio) #207

Sorry @raff , I somehow missed this mention. No, sorry, no idea. But I don’t mind at all whatever you prefer.


(Daniel Kayiwa) #208

Oh yes @cintiadr ! :smile:


(Kelvin Kariuki) #209

Hello community, Hope you are well. I am currently facing a blocker working on OCLOMRS-159 which is about changing the login system to enable a user to login with their username and password. From the OCL API documentation on how to retrieve a user’s auth token, we are required to provide the user’s username and hashed password to the endpoint. While we can provide the username, we cannot provide the hashed password because when a user provides a password we are not able to encrypt/hash the password. I am therefore requesting your support especially in terms of how other clients are able to provide the hashed password to the API as I continue to look for a solution. cc @dkayiwa @alexmochu @emasys @emmabaye


(Burke Mamlin) #210

Odd. Perhaps there is some obfuscation of the password expected (i.e., a reversible hash)? Passwords should be salted & hashed on the server and it wouldn’t make sense to be passing around that hashed value since providing the password that results in the hashed value (i.e., proving you know a secret not even the server knows) is how one authenticates.


(Darius Jazayeri) #211

@nesh, Take a look at posts 19-33 earlier in this thread, as they discuss this issue a bit. The key point is that:

Therefore, the person who works on this ticket should have some python/django experience (or be interested in learning about them)

So, the things I would do are:

  1. find the code in oclapi where this is done, and poke around there a bit
  2. explore how authentication is typically done in django REST applications, and if there’s a typical way to do this using username/password (in addition to token)
  3. invite @raff and @paynejd to a design discussion about this

Basically we need to add new functionality to oclapi, such that it can support user/password authentication.


(Rafal Korytkowski) #212

There’s a REST API endpoint for login at /login supported by this view https://github.com/OpenConceptLab/oclapi/blob/master/ocl/users/views.py#L87 When posted username and password it returns a token. However, it seems to be broken :slight_smile:

I’ll look into fixing that.


(Jonathan Payne) #213

How did that break? Is this a good time to make some changes to the authentication approach? Or do you think this approach is still acceptable for the time being?


(Rafal Korytkowski) #214

It seems to be requiring a token for requesting a token :slight_smile: Maybe it never worked or someone changed permissions for that URL.

The approach we use is pretty standard. In the future it would be good to have OAuth 2.0. Not sure how much time it would take to set it up for the django REST framework.


(Kelvin Kariuki) #215

@darius, I will have a deeper look at the OCL REST API and try to figure out how we can go about the authentication. I looked at the codebase today and found the part of the code where we can get a user’s token by providing the username and the password, however, the incoming password is being compared to an encrypted password hence the error “Passwords did not match.”. Thank you for the support I will post any progress made. cc @dkayiwa @alexmochu @emmabaye


(Kelvin Kariuki) #216

It’s not really broken, it requires you to provide an already encrypted password as the password you input is directly compared to an encrypted password.


(Darius Jazayeri) #217

@nesh, I haven’t looked at the OCL code for this, but typical server-side code for checking a password would do something like:

def test = hash(userEntered + salt);
if (test.equals(storedInDb)) {
    // pass
} else {
    // fail
}

Relying on the caller to hash their own password does not make sense (and is not appropriate from a security standpoint, since it means not using a salt). And now that I look at the code I see that there is no salt…

So, I do think it really is broken, in the sense that it doesn’t behave as you would expect a modern application to behave.

@raff, I don’t know how much time you have available now. Are you saying that you’re going to prioritize fixing this in the short run, so that @nesh should work on another ticket for now? Or do you want to provide some guidance for how he could fix the underlying oclapi? Or something else?


(Alex Mochu) #218

Hello Community,

While working on this task OCLOMRS-158(A user should be able to delete a concept in a Dictionary) I have encountered this two blockers.

  1. Currently, concepts in a Dictionary are being fetched from Collections which should not be the case. Concepts in a Dictionary should be fetched from Sources. More info on this is here OCLAPI wiki. I have raised the issue on Jira OCLOMRS-177. (This issue when fixed is going to introduce a couple of changes to the current UI state)

  2. Also, the “Purge” parameter used to Delete a concept after it has been retired seems not to be working. I have tested several times using Postman and the results are the same. This means currently we can only retire concepts but cannot delete them. OclApi Wiki on how to Retire/Delete a Concept. The issue on Jira OCLOMRS-176.

cc @darius @dkayiwa @nesh @emmabaye @raff

I would look to get your opinion on this.


(Darius Jazayeri) #219

@alexmochu,

(1) please make sure to read the What is a “Dictionary” and how does this compare to OCL’s existing domain model? section of the google doc about this project. Maybe this need to be stated more clearly, but the idea is that if I create a custom concept in my dictionary, it is created in source and then added to a collection. I would expect that the query that shows you “all concepts in a dictionary” would be looking at the collection.

So I believe that OCLOMRS-177 is incorrect.

(2) I don’t know the OCL API code well, but a quick search on github suggests that purging concepts is not implemented https://github.com/OpenConceptLab/oclapi/search?q=purge&unscoped_q=purge @paynejd, is that the case?

If this is the case, I can imagine two quick-fix approaches:

  • (a) deprioritize OCLOMRS-158 until after the MVP (e.g. maybe users don’t really need to delete at first…)
  • (b) when the user clicks Delete, we do a Retire instead of a Purge, and we document this as a problem to fix for later.

(Alex Mochu) #220

Thanks for your response @darius

I now have a more clear understanding on what is a Dictionary. This is after going through the resource that you have shared above. I must confess it was quite confusing at first.

Concepts being added to the Collection would mean that they are being referenced. That is according to the OclApi wiki. The other thing we can do with a referenced concept in a Collection is Delete it but we cannot Edit. If that is the case it would make this OCLOMRS-157 ticket also incorrect. This is because you cannot edit a referenced concept.

Concepts can only be edited through their Sources.


(Darius Jazayeri) #221

@alexmochu also, let me take a step back and make a point about process…

The OCLOMRS-158 ticket specifically says

(Need some business analysis about the exact use case and workflow here.)

I wrote that sentence because I thought it would clearly indicate that there is a need for more business analysis before this ticket can be marked as Ready For Dev.

I see that you’re doing tech analysis, which is not the same thing. As a general point the first step is to understand what a user wants to be able to do and why, before you start trying to solution it.

In a larger team that has business analysts on it, this work would typically be done by one of them, but since our team is small, the devs get to (have to) do this also. The recommended way to do this is by writing out the ticket in the form of a User Story, because it forces you to describe the end-user value. In this case I would expect the ticket to say

As a dictionary maintainer I want to delete a concept that I just created (but did not release yet) so that I can undo a mistake.

I would expect a different ticket and user story for:

As a dictionary maintainer I want to retire a concept that has been released into production but we no longer want to use so that (my implementation sees that this concept is not to be used going forwards) or (future implementation using this dictionary do not have the deprecated concept deployed)

Does this make sense? (And, people like myself, @ball, and @akanter can help you understand what the user – i.e. dictionary maintainer – is trying to do, and why.)