Scope laboratory assay run template lookups to the appropriate container#290
Scope laboratory assay run template lookups to the appropriate container#290labkey-martyp wants to merge 3 commits into
Conversation
The assay_run_templates lookups in DefaultAssayParser.saveTemplate, DefaultAssayParser.getTemplateRowMap, and AssayHelper.saveTemplate filtered only by rowid, which is the table's global primary key. Because Table.update also operates by global PK, a user could pass a templateId belonging to another container and have that foreign template loaded and updated (runid/status), bypassing container authorization. Each lookup now adds a container condition scoped to the request container before reading or updating the row. Follow-up to #286.
The prior container-scoping fix (7bd95a9) filtered the assay_run_templates lookups by strict equality on the request container, which breaks imports performed inside a workbook: the run-template picker sources templates from the parent container (mirroring Laboratory.Utils.getQueryContainerPath), so the selected templateId points at a parent-owned row that strict scoping rejects with "Unknown template". Widen the three lookups in DefaultAssayParser.saveTemplate, DefaultAssayParser.getTemplateRowMap, and AssayHelper.saveTemplate to an IN filter matching the request container or, when the request runs in a workbook, its parent. This still never reaches an unrelated container, so the original authorization boundary holds. Also stop writing the container into the AssayHelper.saveTemplate update row so an existing parent-owned template is not moved into the workbook by Table.update; the container is now set only on insert.
Replace the duplicated workbook-resolution snippet in DefaultAssayParser.saveTemplate, DefaultAssayParser.getTemplateRowMap, and AssayHelper.saveTemplate with a single DefaultAssayParser.getTemplateContainerIds(Container) helper, so the container-scoping rule for assay_run_templates lookups lives in one place. Pure refactor; no behavior change.
|
Hi @bbimber, I'm not sure of the intended cross-container use here. I scoped it to the current container and parent container for workbooks. I did not see anything about assay run templates in the Shared container. Let me know if this looks scoped correctly. Thanks. |
OK. This is quite old code that I havent thought about in ages. I will need to sit down and look this over more carefully. |
|
I think one simple solution is actually to make this use UserSchema, not raw DB schema, and let ContainerFilter solve things for us. I will have a look and possibly propose some changes to use this |
| //validate the template exists in (or above) this container | ||
| TableInfo ti = DbSchema.get("laboratory").getTable("assay_run_templates"); | ||
| TableSelector ts = new TableSelector(ti, new SimpleFilter(FieldKey.fromString("rowid"), templateId), null); | ||
| SimpleFilter filter = new SimpleFilter(FieldKey.fromString("rowid"), templateId); |
There was a problem hiding this comment.
Similar to the comment I just posted: this code is basically replicating ContainerFilter. I think we should switch this to use UserSchema, which looks like it might just handle the container aspects seamlessly for us
2 participants
Rationale
The
assay_run_templateslookups inDefaultAssayParser.saveTemplate,DefaultAssayParser.getTemplateRowMap, andAssayHelper.saveTemplatefiltered only byrowid, which is the table's global primary key. BecauseTable.updatealso operates by global PK, a user could pass atemplateIdbelonging to another container and have that foreign template read or updated (runid/status), bypassing container authorization. Each lookup is now scoped to the request's own folder tree: the request container, and additionally its parent when the request runs in a workbook, since a template referenced from a workbook import may reside in either the workbook or its parent (the run-template picker reaches up to the parent viaLaboratory.Utils.getQueryContainerPath). This preserves the existing workbook import workflow while never reaching an unrelated container.Related Pull Requests
None.
Changes
assay_run_templateslookups to the set{ request container, its parent when a workbook }via anINfilter, replacing therowid-only filter.AssayHelper.saveTemplate, setcontaineronly on insert and omit it from the update row so an existing parent-owned template is not moved into the workbook byTable.update.DefaultAssayParser.getTemplateContainerIds(Container)helper used by all three call sites.