GSOC 2019: OpenMRS Atlas 3.1

Community GSoC
61

How about GET /markers?username={OpenMRS ID} to return a list of marker resources the given user can edit? (FYI – for now, it may only be the creator; in the future, we may add the ability to delegate edit access to others, so multiple people might be able to maintain a marker).

I think the exception would be for aminds. We wouldn’t want to return all markers (even though an admin could edit any marker); rather, we’d return those markers that the admin is specifically named as owner (or delegate).

We’ll need GET /types and GET /versions if we don’t have those yet. We may also need at least POST for types, versions, and distributions so an admin can manage the available values.

Using ids (UUIDs) as a “secret” feels like a poor design decision now. The goal was to ensure a module could send updates without having to be authenticated; however, it would probably be better to have an explicit “update token” distinct from a marker’s id that, if provided, would allow for unauthenticated updates of the marker. Then we wouldn’t have to worry about filtering ids, we could more easily support future read-only calls for specific markers (for reporting, etc.), and we’d never send the update token to clients. The legacy /ping.php endpoint could look up the marker by update token.

1 Like
62

Sounds good to me. :slight_smile:

It looks like there’s no single place whether types and versions are being stored. Marker types are being hardcoded in code multiple times, so I think we’ll have to store it in the database. As for versions, it isn’t being used much in the code, but I assume it isn’t more than a string.

What do you think is a good database schema for them? After seeing its usages, I can imagine them to be the following.

types(id, name, icon) Example: (1,“Clinical”,“https://maps.google.com/intl/en_us/mapfiles/ms/micons/purple-dot.png”)

versions(id, version) Example: (1,“1.9.0”)

@burke @harsha89 @cintiadr Is there anything else to add to the Add/Edit/Delete markers PR? I feel like a lot of changes to be made (like the REST API calls) depend on the PR. I could make the change in the same PR but I feel like it would be easier to review changes if I create a new PR after this one gets merged.

63

Those seem like reasonable suggestions.

I pulled down a local copy of your PR and tried to get it working, but couldn’t get the LDAP connection to work. I cloned the openmrs-contrib-ansible-docker-compose repo and started up a local copy of ldap-stg using docker-compose (I tweaked the docker-compose.yml to put its openldap service onto the same network as atlas. I was able to fire up Atlas via docker-compose, but I couldn’t seem to get the environment variables for the LDAP connection correct. I think the closest I got was using ldap://openldap:636 with credentials admin/admin, but it seemed to close the connection. I also tried ldap://openldap:389 but couldn’t get that to work. Port 3389 was refused. I could tell it was resolving the openldap address and reaching it and I could successfully run ldapsearch from within the openldap container for admin/admin. Then I tried manually running the atlas service container, using npm start to start the server, hoping I could debug the problem. Unfortunately, I’m not knowledgeable enough with ldap to troubleshoot.

I can try again when I get a chance… maybe coming at it fresh will help. If you can confirm the ldap URI, username, and password I should be using, it would help.

64

I tried it out and I’m having the same issue using docker. I kept getting ‘connection refused’ when I tried to sign in. I tried both openldap and localhospt, and ports 389 and 636.

I could quickly get atlas going using nodemon though. Here’s the steps I had to follow to get it working:

Using nodemon:

65

If we are changing the DB schema, we need to be careful, as I’d rather not lose data. On that case, if it’s a breaking change, we’ll need a migration script.

I’d love to have that merged this weekend, so I can get it out to staging easily and sort any teething problems.

Indeed, I didn’t fix the docker compose file inside the atlas repo. I fixed in 3.x now.

66

So, I added

    networks:
      - atlasnet

to the openldap service and put

networks:
  atlasnet:
    name: openmrs-contrib-atlas_default

at the bottom to bridge the openldap service to the atlas network.

Then I updated the PR’s docker-compose.yml with Cintia’s 3.x version. I’ve got

GOOGLE_MAPS_JS_API_KEY={My Google Maps Javascript API Key here}
LDAP_URI=ldap://openldap:3389

And I can confirm the openldap container is on the correct network via docker inspect, the environment variables are set correctly, and openldap is reachable from within the atlas container:

$ docker exec -it openmrs-contrib-atlas_atlas_1 bash
bash-4.4# ping openldap
PING openldap (172.31.0.2): 56 data bytes
64 bytes from 172.31.0.2: seq=0 ttl=64 time=0.140 ms

Once the server is listening, I got to http://localhost:3000 and Atlas loads. I click on the Sign In button and get to the Login screen. I enter credentials admin/admin and the Atlas server dies with this error:

GET /login 304 2.706 ms - -
GET /service-worker.js 404 1.085 ms - 30
uid=admin,ou=users,dc=openmrs,dc=org
events.js:183
      throw er; // Unhandled 'error' event
      ^

Error: connect ECONNREFUSED 172.31.0.2:3389
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1191:14)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! openmrs-contrib-atlas-node@0.0.1 start: `node ./bin/www`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the openmrs-contrib-atlas-node@0.0.1 start script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /root/.npm/_logs/2019-06-06T13_09_14_096Z-debug.log

That suggests openldap isn’t listening at 3389, right? So, I check

$ docker ps -f name=ldap
CONTAINER ID        IMAGE                   COMMAND                  CREATED             STATUS                    PORTS                                          NAMES
5088c9c11298        osixia/openldap:1.2.4   "/container/tool/run…"   19 minutes ago      Up 19 minutes (healthy)   0.0.0.0:3389->389/tcp, 0.0.0.0:6636->636/tcp   ldap-stg_openldap_1

The 0.0.0.0:3389->389/tcp does mean that ldap://openldap:3389 is the correct LDAP_URI setting, right? Thinking I’m reading it backwards, I try switching to port 389. Now, when I try logging in as admin/admin, I get

{ InvalidCredentialsError: Invalid Credentials
    at messageCallback (/node_modules/ldapjs/lib/client/client.js:1419:45)
    at Parser.onMessage (/node_modules/ldapjs/lib/client/client.js:1089:14)
    at emitOne (events.js:116:13)
    at Parser.emit (events.js:211:7)
    at Parser.write (/node_modules/ldapjs/lib/messages/parser.js:111:8)
    at Socket.onData (/node_modules/ldapjs/lib/client/client.js:1076:22)
    at emitOne (events.js:116:13)
    at Socket.emit (events.js:211:7)
    at addChunk (_stream_readable.js:263:12)
    at readableAddChunk (_stream_readable.js:250:11) lde_message: 'Invalid Credentials', lde_dn: null }

And the UI shows a pretty little Invalid Credentials error message, which seems more promising. So, I don’t know the admin credentials, but…

It works logging in with credentials test/test!

So, it looks like the LDAP_URI for docker-compose should be port 389 and I still manage to read docker port settings backwards. :slight_smile:

1 Like
67

Local testing looked pretty good. #45 has been merged! Good job, @heliostrike!

1 Like
68

I just realized that to implement GET /markers?username={OpenMRS ID} I’d have to put an if statement to see if there’s a username in the URL inside the GET /markers route. It feels a little off… what do you think?

After thinking about it for a while, I think it would be better to use the route you mentioned, if we needed to filter based on multiple conditions. I’ll get that done. :slight_smile:

69

There’s no admin user :smiley: Just test and testadmin, with passwords matching the username.

Latest merges deployed to STG again.

70

@burke @harsha89 @cintiadr I made a PR to allow admins to manage all markers(ATLAS-143). Local testing seemed fine, but I’m not sure how secure it is.

I made the requested changes for #48 (ATLAS-139). I made commits for ATLAS-141 and ATLAS-142, and will make PRs as soon as #48 gets merged.

I noticed an issue with our API. Since we changed ‘/markerSites’ to ‘/markers’ in ATLAS-135, the other /markers route to filter based on types, versions, and distributions is no longer accessible. I created a ticket(ATLAS-144) for it and will start working on it soon.

I think we can start working on administrative functions after all this is done. I will start writing the create, and update API for types and version, over the next few days. I’m not sure how to build the UI, so it will probably take some time.

1 Like
71

@burke @harsha89 @cintiadr I wrote the API to add/update/delete types, versions, and distributions. Unfortunately to test it(properly) we’ll probably need to write some UI for admins.

What would you imagine such a UI to look like? I was thinking of a few buttons on top of the map like ‘Update Types’, etc. Clicking on one of them should bring up a big boot-box on top of the map, and contains mini forms for each type/version/distribution. Does that sound alright?

72

To simplify things, I would suggest an admin choice that appears in the list of user actions for admins ( e.g., above “Locate Me”) that links to /admin and then a single page app (under an admin folder in the code) hosted there to perform administrative tasks. We could consider adding links in the map UI (e.g., edit icons on type/distribution/version menu items) that take you directly to the appropriate part of the admin app for convenience… but that’s not a priority.

1 Like
73

@burke Aye! :slight_smile:

I imagine the admin page contains links to view all types(/admin/types), versions(/admin/versions), and distributions(/admin/distributions).

We click a link, and it shows all entries under the category(types,versions.etc.). We’ll have edit and delete buttons beside each entry. The edit button leads up to /admin/types/<id>(etc.) where we can edit an entry.

Is that ok?

74

That’s fine; though, in a single page application (e.g., using React), it’s likely the application would run at https://atlas.openmrs,org/admin and the management pages would be routed as /admin#/types, /admin#/versions, etc. Unlike a REST api, it’s not important to have specific URLs for each resource used in the web application. As long was we can link directly to a page/tab within the admin app, that’s all we need from it.

1 Like
75

Will we be using React for the single page application? I never worked with a website like that, it was either Node, or it was React. I’ll have to read up on it.

76

The way I’ve typically organized repos for applications like this is something like:

/api   - node app
/web   - web app
/admin - web app
77

Could you provide me with a link to the repo of one such application, so that I could use it as reference? Or some resource explaining how its done?

I’m not sure how to run the 2 applications on the same port. Also, will they have to started separately?

78

I found some similar repos on your github account. :slight_smile:

I’m still curious about the application works, and how the Node and React app integrate. I think I’ll take a look at your repos for now. :slight_smile:

79

My question would be: except for the CSS for the login page, is anything required to replace the production atlas?

I mean, we have the security considerations for admin login, but I was planning on getting help on that this weekend.

80

Note: prd atlas is not really working