Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

handle permissions #13

Merged
merged 18 commits into from
Sep 5, 2023
Merged

handle permissions #13

merged 18 commits into from
Sep 5, 2023

Conversation

ETLaurent
Copy link
Contributor

@ETLaurent ETLaurent commented Aug 29, 2023

  • scope export and import options into a importExport one
  • use the manager of the docs and related docs to filter them out if the user does not have sufficient permission
  • use apos.util.clonePermanent to strip the docs from the scalar properties (except for _id)
    --> We're not fetching docs directly from the mongo anymore
  • refactor code a little bit so it does not become too hard to maintain

@linear
Copy link

linear bot commented Aug 29, 2023

PRO-4261 As an editor, I can download a ZIP file of my exported documents

When clicking on the download button of the export modal, after I selected the related docs that I want to include in the export, the process must be triggered.

image.png

Two new routes export must exist in the modules piece-type and page. Both routes will call a method export from the module doc-type .

This method should take the IDs of the documents the user want to export, as well as the related types that he wants to get. Only the types, not the IDs, since we decided that it would be too heavy for the export modal to get the amount of related docs when we batch a large amount of documents.

Generate files

This method will need to use the relate method from the module schema, this method fills documents with related ones.

Since we need to store documents in a flat EJSON array, it might be a good idea to add an option to this method in order to get a flat array of related documents instead of filling requested documents with their related docs, see how it could potentially be used with query builders.

We want one EJSON file for each db collection, for example aposDocs.json and aposAttachment.json, containing arrays of documents..

We want, for each document to export the draft and live versions (only draft if there is no live). The draft version must appear first to avoid situation where a live exists without draft which is forbidden.

Same thing for related documents, they must be written first in the EJSON file, otherwise the doc manager will yell when we'll import it.

We will create an attachment folder when actual attachments will be stored (images, files…).

Generate Zip

To generate the ZIP file we would prefer using the native node library.

Progress bar notification

This progress bar notification is triggered by wrapping your export function with the run method of the job module. Here is an example from the piece-type-importer module:

return self.apos.modules['apostrophecms/job'].run(
  req,
  (req, reporting, { notificationId }) => self.importRun(req, {
    progressNotifId: notificationId,
    filePath: file.path,
    reporting,
    totalPieces
  }),
  {}
);

The reporting object passed from the second argument function allows to set the total of the documents you want to import with the method setTotal, as well as to call the methods success or failure for every document that we tried to export or import.

Finally the ZIP file will be sent back to the client and the download triggered from the user's browser.

Permissions

For exporting documents, a user that has at least one per-type permission should be able to export anything. He won’t be able to see adminOnly documents, therefore should not be able to export these documents. It matches the way it works currently because in this case the user can see all drafts. In core, admin should be able to export adminOnly content, except for users.

For Reference

https://github.com/apostrophecms/tech-designs/blob/main/3.x/share-documents-across-sites/design.md

Acceptance Criteria

  • I'm an editor or admin on with the Download modal open, and my documents selected, and I hit "Download" and when I do, a ZIP file begins downloading.
  • The zip file downloads and I open it and I see the the draft and live versions (only draft if there is no live) in my unarchived folder.

Comment on lines -23 to -28
if (self.options.export !== false) {
self.apos.asset.iconMap['apos-import-export-download-icon'] = 'Download';
}
if (self.options.import !== false) {
self.apos.asset.iconMap['apos-import-export-upload-icon'] = 'Upload';
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't want to check self.options.import or self.options.export here, because this index.js is not the improvement of the modules that set these options.

@ETLaurent ETLaurent force-pushed the pro-4261-export-docs-permissions branch from 434a51a to 15e7708 Compare August 29, 2023 16:15
@ETLaurent ETLaurent mentioned this pull request Aug 30, 2023
10 tasks
@ETLaurent ETLaurent force-pushed the pro-4261-export-docs-permissions branch 2 times, most recently from dcbf3fd to ee3750f Compare August 30, 2023 14:43
Comment on lines 173 to 200
if (!self.apos.permission.can(req, 'view-draft', docType)) {
return false;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure

@ETLaurent ETLaurent marked this pull request as ready for review August 31, 2023 14:14
}
})
.project(projection)
// .permission('view-draft') // TODO: add this?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

@ETLaurent ETLaurent requested review from ValJed and removed request for haroun and ValJed August 31, 2023 15:29
lib/apiRoutes.js Outdated
Comment on lines 35 to 36
// TODO: handle permission here: exclude types that the user cannot access to:
return self.canExport(obj.withType) && obj.withType;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will be done in a next PR soon

@@ -1,3 +1,5 @@
const _ = require('lodash');
Copy link
Contributor Author

@ETLaurent ETLaurent Aug 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I know... but straightforward and it does the job

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like jquery? (joke from Harouna 😄 )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least just import the feature you need:

const { uniqBy } = require('lodash');

if (!format) {
throw self.apos.error('invalid');
}

const allIds = self.getAllModesIds(ids);
const docs = await self.getDocs(req, ids, manager, reporting);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the reporting is now handled by getDocs

// Get docs via their manager in order to populate them
// so that we can retrieve their relationships IDs later,
// and to let the manager handle permissions.
// TODO: get draft and live in the same call
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wil tend to that in another PR

return [];
}

// TODO: test with `importExport: { export: false }` option in the attachment module:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To test with attachments @ValJed

formatArchiveData(docs, attachments = [], urls = {}) {
return {
json: {
// TODO: use BSON
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a separate ticket for that

@ETLaurent ETLaurent force-pushed the pro-4261-export-docs-permissions branch from c7cd9c7 to 013e1bd Compare September 4, 2023 09:47
@ETLaurent ETLaurent changed the title Pro 4261 export docs permissions handle permissions Sep 5, 2023
@ETLaurent ETLaurent force-pushed the pro-4261-export-docs-permissions branch from f5747ff to f9f9a8c Compare September 5, 2023 07:40
@@ -1,3 +1,5 @@
const _ = require('lodash');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like jquery? (joke from Harouna 😄 )

@@ -1,3 +1,5 @@
const _ = require('lodash');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least just import the feature you need:

const { uniqBy } = require('lodash');

@ETLaurent ETLaurent merged commit e812bf5 into main Sep 5, 2023
0 of 9 checks passed
@ETLaurent ETLaurent deleted the pro-4261-export-docs-permissions branch September 5, 2023 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants