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

Add designated folder for extensions #1245

Merged
merged 2 commits into from Nov 24, 2020
Merged

Add designated folder for extensions #1245

merged 2 commits into from Nov 24, 2020

Conversation

Nokel81
Copy link
Collaborator

@Nokel81 Nokel81 commented Nov 5, 2020

This PR adds a method onto LensExtension for getting a unique path to a folder that the extension can use for its own purposes.

The folder is based on the extension ID but is not passed in so it cannot be faked. The folder name is salted and then hashed to obfuscate it and the mapping is stored in a lens store which is, by design, not otherwise exposed to extension writers. This is done so that the folder's name is not able to be determined without calling this function.

No security is applied to these folders.

fixes #1179

@Nokel81 Nokel81 requested a review from a team November 5, 2020 20:53
@Nokel81 Nokel81 self-assigned this Nov 5, 2020
@github-actions github-actions bot added the area/extension Something to related to the extension api label Nov 5, 2020
@ixrock
Copy link
Member

ixrock commented Nov 5, 2020

This should be merged first #1233

@Nokel81
Copy link
Collaborator Author

Nokel81 commented Nov 5, 2020

okay, ping me when that is merged and I will rebase again

package.json Outdated Show resolved Hide resolved
src/main/menu.ts Outdated Show resolved Hide resolved
integration/__tests__/app.tests.ts Outdated Show resolved Hide resolved
src/extensions/extension-loader.ts Show resolved Hide resolved
src/main/extension-filesystem.ts Outdated Show resolved Hide resolved
src/main/extension-filesystem.ts Outdated Show resolved Hide resolved
src/main/extension-filesystem.ts Outdated Show resolved Hide resolved
src/main/index.ts Outdated Show resolved Hide resolved
@Nokel81 Nokel81 requested a review from ixrock November 13, 2020 20:29
Copy link
Contributor

@jakolehm jakolehm left a comment

Choose a reason for hiding this comment

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

filesystemProvisionerStore itself looks good but this PR introduces bunch of other changes that are not related to PR title at all. We should keep PRs small and focused for easier review process. Could you split non-related changes to separate PRs with descriptive title/description why those changes are done.

integration/__tests__/app.tests.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
src/common/base-store.ts Outdated Show resolved Hide resolved
src/common/utils/app-version.ts Outdated Show resolved Hide resolved
@Nokel81
Copy link
Collaborator Author

Nokel81 commented Nov 18, 2020

@jakolehm You are correct about the sidecar changes. I have moved them to a separate branch. Are you able to review again?

Copy link
Contributor

@jakolehm jakolehm left a comment

Choose a reason for hiding this comment

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

LGTM

@Nokel81 Nokel81 force-pushed the issue-1179 branch 2 times, most recently from b17c9ed to bdb66b8 Compare November 23, 2020 15:21
- add extension mechinism for getting a folder to save files to

- add test to make sure that extensions are being loaded

- skip extension loading test

Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
@Nokel81 Nokel81 merged commit 4e6b8ee into master Nov 24, 2020
@Nokel81 Nokel81 deleted the issue-1179 branch November 24, 2020 15:28
@jakolehm jakolehm mentioned this pull request Nov 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/extension Something to related to the extension api
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide easy way for extensions to store files
4 participants