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

Possible new way to use the library ?? #11

Open
sheeyang opened this issue May 28, 2022 · 1 comment
Open

Possible new way to use the library ?? #11

sheeyang opened this issue May 28, 2022 · 1 comment

Comments

@sheeyang
Copy link

Hi, I saw your comment on how to have secure IPC in Electron applications, and that led me to look into your profile, which then led me to look into this library, but realised something weird about how the library is used,

For example, in order to write to the config, you do:

import { writeConfigRequest } from "secure-electron-store";

window.api.store.send(writeConfigRequest, "myvalue", "14");

But in my opinion doing something like this would be better

window.api.store.send.writeConfigRequest("myvalue", "14");

since this means we dont have to import writeConfigRequest, so I wrote a solution to that and I would like to know what you think

I will use the example you used in your comment to demonstrate my method

From your the comment you did this:

preload.js

contextBridge.exposeInMainWorld(
    "api", {
        send: (channel, data) => {
            // whitelist channels
            let validChannels = ["toMain"];
            if (validChannels.includes(channel)) {
                ipcRenderer.send(channel, data);
            }
        },
        receive: (channel, func) => {
            let validChannels = ["fromMain"];
            if (validChannels.includes(channel)) {
                // Deliberately strip event as it includes `sender`
                ipcRenderer.on(channel, (event, ...args) => func(...args));
            }
        }
    }
);

I would like to suggest doing it this way:

preload.js

// whitelist channels
let validSendChannels = ["toMain"];
let validReceiveChannels = ["fromMain"];

contextBridge.exposeInMainWorld("api", {
  send: validSendChannels.reduce((prev, channel) => {
    return {
      ...prev,
      [channel]: (data) => {
        return ipcRenderer.send(channel, data);
      },
    };
  }, {}),
  receive: validReceiveChannels.reduce((prev, channel) => {
    return {
      ...prev,
      [channel]: (func) => {
        ipcRenderer.on(channel, (event, ...args) => {
          func(...args);
        });
      },
    };
  }, {}),
});

This means instead of doing:

index.html

window.api.receive("fromMain", (data) => {
    console.log(`Received ${data} from main process`);
});
window.api.send("toMain", "some data");

We can do:

index.html

window.api.receive.fromMain((data) => {
    console.log(`Received ${data} from main process`);
});
window.api.send.toMain("some data");

In my opinion, this is better, but I would like to know what you think

Also, thank you for your work on making it easier for us to make our electron apps secure, I really appreciate it 😊

@reZach
Copy link
Owner

reZach commented Jun 27, 2022

Thanks for contributing this idea to the library @sheeyang. I do like your idea, and the way you propose it is implemented is clever. I think there's some things to weigh against this:

current approach

  • calls out importance of safelisting channel names when using IPC

proposed approach

  • hides implementation details to consumers of the library

I think I agree with your suggestion. Are you able to garner support for this change from others, or alternatively, submit a PR? This will be a breaking change and I'll have to update documentation in a number of places.

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

No branches or pull requests

2 participants