Disable SSL certificate verification for private apps #369
Conversation
| // disable SSL check for private applications | ||
| if (!useDefaultSession && Number(appstoreApplicationId) > 1000000) { | ||
| services.defaultSession.disableSslCertVerification(partition); | ||
|
|
||
| // const appSession = remote.session.fromPartition(partition); | ||
| // appSession.setCertificateVerifyProc((_, callback) => { | ||
| // callback(0); | ||
| // }) | ||
| } |
There was a problem hiding this comment.
Hey @magne4000 , need your help here.
The issue could be fixed with 3 lines of code using @electrone/remote, but I think we should avoid to use it.
So I had tried to implement new method in SessionService and stuck 😞
There was a problem hiding this comment.
SessionServiceImpl was created at some point with the hope that it would be reusable with partitions, but clearly it's not the case.
What I would suggest would be to:
- simplify
SessionServiceImplconstructor andinitSessionby removingSessionOptionsand remove handling of partitions - rename others existing functions to
getDefaultUserAgent,getDefaultCookiesandgetDefaultSession - move
disableSslCertVerificationtoSessionServiceImpl, and make it so that it takes a partition as argument of the function
So instead of representing a particular session SessionService now represent a way to access all of them. Then we should finally rename all service.defaultSession to service.session.
What do you think?
There was a problem hiding this comment.
We are calling from renderer to main here. We have to use ipc.
That's the problem I'm struggling to. 😀
That's why all ProviderService magic here.
There was a problem hiding this comment.
I pushed a fix.
Generally, the way to think when implementing or using Services should be:
- Find the proper way to dispatch a message in the store:
- From a renderer, use redux store (our case here)
- From a worker, provide store in ServiceImpl init, then use it
- From main, provide a Worker Service in init (i.e. a Service whose impl runs in Worker), then use it (should be last resort, as its a complex pattern and difficult to debug)
- Then, no need to store the message in a state if its just a simple message passing
- Then, in a saga, call the right service
There was a problem hiding this comment.
That's a really complicated thing for me.
Thank you.
|
For security reasons, I would prefer that this parameter would be behind an opt-in config per app:
|
I totally agree with you. As for me the best way is to create parameter "Disable SSL check" or something like this. And enable-disable it at the first time with confirmation dialog like we do (should do 😄 ) with notifications. |
|
Feel free to merge it as-is, I understand that adding this feature in the UI and what not is not the focus here. |
What is this PR
Fixes Ability to trust self-signed or expired certificates with private apps? #278