Resolve _verify logic for Security module

[avaidyam]

(Please see prior email for details.)

[Linoy339]

@avaidyam . We assume that the idea is to remove the vulnerability in LAMP-server which results in creating resource like activities, sensors, participants from invalid parent id.( _verifywill incorrectly allow the user to insert activity to a study that does not exist. )

Can you confirm ?

[avaidyam]

That's correct. @michaelmenon mentioned a solution was already prepared/ready so I am creating an issue to formally track it (so we don't forget to include this in the next production release).

[Linoy339]

We believe that we have given 2 suggestions or solutions : 1) The _verify function need to be tweaked to check against the authObject (study/researcher id) supplied and allow only if the same exists in the database. 2) As we publish data to nats, we can collect the data from LAMP-worker and check with the resource id and there by deleting it if no owner exists for this resource.

Can you please suggest which one we should implement ?

[avaidyam]

Option 1 is the correct one here. Please do that.

[Linoy339]

Sure @avaidyam . We can do that

[avaidyam]

Has this been completed yet, @Linoy339 ?

[Linoy339]

No @avaidyam . We will push this week

[Linoy339]

@avaidyam . While we are in the progression, we came to see something which seems to be corrected: https://github.com/BIDMCDigitalPsychiatry/LAMP-server/blob/master/src/repository/couch/TypeRepository.ts#L47

Here, the data is returned as nullfor always. Can we tweak this to return the data as other blocks does?(eg:https://github.com/BIDMCDigitalPsychiatry/LAMP-server/blob/master/src/repository/couch/TypeRepository.ts#L51)

[avaidyam]

The owner of a researcher is always null (i.e. root/there is no parent). Please do not tweak this (correct) code!

[Linoy339]

@avaidyam Got it. Actually, we are working the strategy of validating the authObject with the code: let _owner = await TypeRepository._owner(authObject ?? "") So with this above implementation, when, someone calls the api, @POST http://api-staging.lamp.digital/study/invalidstudy/activity where invalidstudy is an invalid study id, this would not results in creating new resource(activity), as we checks for the study given( invalidstudy ) with the code: let _owner = await TypeRepository._owner(authObject ?? ""). Here, _owner would be null as the parent id(invalidstudy) given is invalid. So, it works

But, if we call GET https://api-staging.lamp.digital/researcher/validresearcher where validresearcheris an valid researcher id, it checks for validity of researcher id with the same code: let _owner = await TypeRepository._owner(authObject ?? ""). Unfortunately, _owner here would also be null(because researcher id is the authObject and _owner function returns null if researcher id is passed as authObject ). So, it doesn't work

To resolve the above, the following can be done :

• Can we write a similar function resembling the _owner function in TypeRepository and the only tweak which we would have is that we return researcher data also in that function?

• We can introduce ResearcherRepositoryin _verifyfunction and check for valid researcher if the _owner is null. So, even if we get _owner as null in case of a valid researcher id, ResearcherRepository._select(authobject) would give a valid response, so that we can return as valid - i.e if((_owner!==null) || await ResearcherRepository._select(authObject as any).length)

• Skip this whole validation in _verify function and check this in individual services. For example: when, someone calls the api, @POST http://api-staging.lamp.digital/study/invalidstudy/activity , we can check whether the study id given is valid or not from the ActivityService.

Can you suggest ?

[avaidyam]

The simplest solution would be to throw a context error (i.e. throw new Error("404.resource-not-found")) or something at the return null line at the bottom of that function. That error would propagate upwards to the service function and return the HTTP 404 error automatically.

[Linoy339]

@avaidyam Thanks for the reply. So we have to throw an error in repo function: https://github.com/BIDMCDigitalPsychiatry/LAMP-server/blob/master/src/repository/mongo/TypeRepository.ts#L63

We can do it now

[Linoy339]

@avaidyam Can you check this : Can you check this : https://github.com/BIDMCDigitalPsychiatry/LAMP-server/pull/200

[divyav2020]

We have updated in staging