ENG-2825
Keyserver needs to authenticate with the push service and to do that we need to generate a key pair and send the public one to the web app, which it will use during notification subscription. The keys were generated using web-push package in a json format. This diff injects the push key into redux.
Details
Details
Check if the key is available in redux in the web app.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| keyserver/src/responders/website-responders.js | ||
|---|---|---|
| 104 ↗ | (On Diff #22464) | It looks like we're introducing one use case right now where we need to open this file to access the publicKey parameter. Will we need to access other parameters in the future? Should this code be factored out into another file that can be used for accessing this config? |
| 110 ↗ | (On Diff #22464) | By circumventing the importJSON framework that we use for all other imports, you're making this impossible to configure on prod. Whenever you are doing something that has been done before, please spend some time researching how it's done and why |
| 113 ↗ | (On Diff #22464) | Should we be printing some error? |
Comment Actions
Moved the code to the push/providers.js and use importJSON. I'm no longer caching the config myself because importJSON does the caching. Added logging and invariant.
| keyserver/src/responders/website-responders.js | ||
|---|---|---|
| 309–311 ↗ | (On Diff #22670) | I don't think we need to print this in development... it's normal for a developer to not have this configured. On the other hand, I think we should print something in production (instead of the invariant below) |
| 314–317 ↗ | (On Diff #22670) | This is a dangerous invariant, and I'm not sure what it accomplishes vs. just printing a warning |
Comment Actions
Don't print in development, print warning instead of invariant in production (the previous code was inspired by our apn handling of secrets)
Comment Actions
Please rebase on master, re-run yarn cleaninstall, update this diff, and confirm all CI gates are passing before landing!
Comment Actions
Rebase, move the web-push dependency from future diff to this one, and init it during config loading.
Comment Actions
Not familiar with this library, but it seems reasonable. Unfortunately the author doesn't seem to keep an up-to-date changelog / GitHub releases, but it seems like a popular library so it probably isn't completely broken