medic / cht-core

The CHT Core Framework makes it faster to build responsive, offline-first digital health apps that equip health workers to provide better care in their communities. It is a central resource of the Community Health Toolkit.
https://communityhealthtoolkit.org
GNU Affero General Public License v3.0
441 stars 211 forks source link

/api/v2/users look up users by `facility_id` and/or `contact_id` #8877

Closed kennsippell closed 6 months ago

kennsippell commented 8 months ago

Is your feature request related to a problem? Please describe. I want to look up the details for a single user by username I want to look up the details for users with a specific facility_id or contact_id

Describe the solution you'd like GET /api/v2/users/username (Edit: split out retrieving data for a single user into https://github.com/medic/cht-core/issues/8986) GET /api/v2/users?facility_id=foo GET /api/v2/users?contact_id=bar

Describe alternatives you've considered Admin users (only) can query the database directly and use /_users/_find You can add mm-online role to the _users/_security document

Additional context

garethbowen commented 8 months ago

One workaround would be to install a custom ddoc with a map/reduce view to facilitate this. Obviously the API is better but this would work on all versions of the CHT until the API is released.

Something like...

function (doc) {
  if (doc.type === 'user-settings') {
    if (doc.facility_id) emit('facility:' + doc.facility_id);
    if (doc.contact_id) emit('contact:' + doc.contact_id);
  }
}

Use a bespoke ddoc so it doesn't get overwritten on upgrade.

Then for the GET /api/v2/users/username just fetch the org.couchdb.user:username doc from the medic db, which (probably?) has everything you need.

I think this will work but haven't actually tried it out yet...

kennsippell commented 8 months ago

Sadly I don't think this will work above 4.4 because non-admin users can't send GET requests to the _users database after the CouchDB 3 bump. I'll double check.

garethbowen commented 8 months ago

That's the beauty of this solution... the ddoc above is in the medic db, not the _users db.

kennsippell commented 8 months ago

Yes true. Can use a _find request with payload instead of the ddoc:

{
  "selector": {
    "type": "user-settings",
    "facility_id": facilityId,
    "$or": [
      { "inactive": false },
      { "inactive": { "$exists": false } }
    ]
  }
}

It's a good suggestion. Curious what you think of this --

Hopefully the data in the user-settings doc matches the data in the user doc, but it can be freely edited by users.

  1. Malicious user logs in
  2. Malicious user changes facility_id in their user-settings doc (in chrome console or otherwise)
    const myDoc = await CHTCore.DB.get().get('org.couchdb.user:malicious');
    myDoc.facility_id = 'fake';
    await CHTCore.DB.get().put(myDoc);
  3. cht-user-management user deletes all users with malicious user's expected facility_id
  4. Malicious user is not deleted and can still login. cht-user-manager doesn't know something unexpected happened.

We have been trying not to rely on user-settings docs for any cht-user-management logic. But perhaps we should do it until this is resolved.

garethbowen commented 8 months ago

Yes a find request will be easier to set up, but may not perform as well. Depending on your use case this might be fine!

Yes you're right there is potential for a user to hide themselves with this approach. I'm now wondering why we allow users to modify their own user-settings doc at all. Other than their name I can't see anything in there that we should allow them to update so it should be in the validate_doc_update for the db. Historically it hasn't been a problem because in API we only rely on the doc from _users so it doesn't change their ability through APIs but there's no need to allow the medic version and the _users version to get out of sync.

@dianabarsan What do you think? Can we lock it down? If there is some optional field I'm forgetting we could at least lock down facility and contact updates.

kennsippell commented 8 months ago

Unsure if user edits of user-settings contribute to https://github.com/medic/cht-core/issues/8337, but could lock down roles.

dianabarsan commented 8 months ago

Other than their name

I believe some things are stored on the user doc, for example:

If we block updates to this doc through validate_doc_update, we have to rethink how we do privacy policies at the least.

My opinion is that users should not be allowed to edit their user doc, and everything should be done through APIs. With online first I know this is not possible. If validate_doc_update works in PouchDb, I think we should use it. If we rely on CouchDb only, and expect the replication to be denied, we have to make sure no important edits should ever be made on this doc.

garethbowen commented 8 months ago

My opinion is that users should not be allowed to edit their user doc

Issue raised: #8886

garethbowen commented 8 months ago

Additional APIs that would be useful for user management: https://github.com/medic/cht-user-management/blob/960bf86ca938460209e08498a789975763f6bc19/src/lib/cht-api.ts

Adding here for tracking purposes not necessarily blocking the APIs requested in the description.