Replies: 4 comments 3 replies
-
Thanks @glabrie10, this seems reasonable, and really the app should allow for cases where low cell masking is used but cohort obfuscation is not, as the former alone doesn't preclude visualizations unless the cohort count happens to be below the threshold. |
Beta Was this translation helpful? Give feedback.
-
Final Change proposal for allowing visualize and patient list while low cell masking and noise is enabled @ndobb :
void ThrowIfSettingsInvalid()
{
if (!clientOpts.PatientList.Enabled && !clientOpts.Visualize.Enabled)
{
throw new Exception("Both Visualize and Patient List are disabled");
}
/* if (deidentOpts.Cohort.Noise.Enabled)
{
throw new Exception("Demographics cannot be returned if Cohort De-identification Noise is enabled");
}
if (deidentOpts.Cohort.LowCellSizeMasking.Enabled)
{
throw new Exception("Demographics cannot be returned if Cohort De-identification Low Cell Size Masking is enabled");
} */
}
< void ThrowIfSettingsInvalid()
{
if (!clientOpts.PatientList.Enabled && !clientOpts.Visualize.Enabled)
{
throw new Exception("Both Visualize and Patient List are disabled");
}
/* if (deidentOpts.Cohort.Noise.Enabled)
{
throw new Exception("Demographics cannot be returned if Cohort De-identification Noise is enabled");
}
if (deidentOpts.Cohort.LowCellSizeMasking.Enabled)
{
throw new Exception("Demographics cannot be returned if Cohort De-identification Low Cell Size Masking is enabled");
} */
}
/**
* Blocks visualize if the query is 10 or smaller
*/
if (cohort.count.value < 11 ) {
return (
<div className={`${c}-error`}>
<p>
Sorry, visualize can't be seen for cohorts that are equal to or less than 10
</p>
</div>
);
}
/**
* Blocks patientlist if the query is 10 or smaller
*/
if (cohort.count.value < 11 ) {
return (
<div className={`${c}-error`}>
<p>
Sorry, patient list can't be seen for cohorts that are equal to or less than 10
</p>
</div>
);
} |
Beta Was this translation helpful? Give feedback.
-
Thanks @glabrie10, I'm trying to carve out some time to do a proper code review. At a glance the main thing I'd want to tweak is the hard-coding of the It looks like the API isn't currently sending the Given that, we'd first need to weave those into the server Finally the line |
Beta Was this translation helpful? Give feedback.
-
Actually this brings up a possible security vulnerability as well. If I'm reading correctly, the current code proposal above puts trust fully in the client to not show Visualize/Demographics data from the API if below a certain threshold (in fact, we'd also need to add a check to not even request demographics here). A bad actor however could still simply call the API (outside of Leaf) and get this data, as the server would not enforce any checks as to cohort size. If we needed the server to do a check of cohort size before retrieving demographics, we could add code to do so but it would first necessitate a database call to check, which is not ideal. So there are quite a few additional considerations. |
Beta Was this translation helpful? Give feedback.
-
Our group at HDC(University of Colorado) has lowcellmasking enabled. Due to university policy, this cannot be adjusted. With help from nic this is the proposed patch. With hopes that this becomes a feature of leaf. Proposed changes from the leaf team:
Comment out / remove this check in the API:
(
leaf/src/server/Model/Cohort/DemographicProvider.cs
Line 138 in 28b216e
Do the admin in API here:
https://github.com/uwrit/leaf/blob/master/src/server/Model/Cohort/DatasetProvider.cs#L115
In the web client, add a check similar to the line, to see if the cohort count is less than your threshold:
https://github.com/uwrit/leaf/blob/master/src/ui-client/src/containers/Visualize/Visualize.tsx#L108
Add this to your conditional:
'<div className={
${c}-error
}>Sorry Visualizations can't be shown for cohorts with 10 patients of less.
'https://github.com/uwrit/leaf/blob/master/src/ui-client/src/containers/PatientList/PatientList.tsx#L84
With this I will be adding additional changes for noise. When this is tested I will submit solution here.
Gabe
University of Colorado
Beta Was this translation helpful? Give feedback.
All reactions