Skip to content

Comments

Added Login and Device Registration#8

Open
Rocketeer007 wants to merge 6 commits intonbrownus:masterfrom
Rocketeer007:loginAndRegister
Open

Added Login and Device Registration#8
Rocketeer007 wants to merge 6 commits intonbrownus:masterfrom
Rocketeer007:loginAndRegister

Conversation

@Rocketeer007
Copy link

I've added functionality to handle the proper Login and Device Registration procedures documented in the Pushover Open Client API: https://pushover.net/api/client

Initially, the user should save the userEmail and userPassword in either the settings json or in environment variables. Using these details, the library will now fetch the userSecret needed, and register a new deviceId; both of these values are then saved back to the settings json.

The user's password is never saved to disk - only the userSecret and deviceId are saved, and once these are available the rest of the package continues to work exactly as per your original design.

@nbrownus
Copy link
Owner

nbrownus commented May 1, 2016

I haven't had time to properly review but my thoughts so far; A first run where the user is prompted for their username/password would be far more preferable to every saving those credentials to disk.

In general this looks like a complete rewrite, disregarding existing style. You may be better off just starting another client.

Nick Price added 4 commits May 2, 2016 08:48
Improve the way settings are saved to disk (no value stored at all for username & password; pretty print used)
@Rocketeer007
Copy link
Author

I've tried to keep to the patterns you established in the original project; however, on review I realise that I have used semicolons where you have not, and this may be contributing to the perception that it's a complete rewrite.

In fact, I have only added two new functions to your main class - one to do a "login" (gets the user Secret from Pushover based on the email address and password), and the other to register the device.
Both of these functions serve to populate the "secret" and "deviceId" values that you currently advise users to scrape from the Pushover Desktop Client manually.

The pushover-desktop-client script has by necessity changed a lot, so that the login function and register function can be used - however, I have updated this again to try and better match your style, and make it clearer which are changes to your design, and where it (mostly) stays the same.

Finally, in this revision, I have reverted back to "secret" rather than "userSecret", so existing settings files will work without requiring the user to login again

@JayBrown
Copy link

I installed the version by @Rocketeer007 just now, on macOS 12 (Monterey), and it throws some errors during install:

npm WARN deprecated mkdirp@0.5.0: Legacy versions of mkdirp are no longer supported.
Please update to mkdirp 1.x. (Note that the API surface has changed to use Promises in 1.x.)

4 vulnerabilities (2 moderate, 1 high, 1 critical)

npm audit results in:

npm ERR! code ENOLOCK
npm ERR! audit This command requires an existing lockfile.
npm ERR! audit Try creating one first with: npm i --package-lock-only
npm ERR! audit Original error: loadVirtual requires existing shrinkwrap file

npm ERR! A complete log of this run can be found in:
npm ERR!     ~/.npm/_logs/2022-09-14T18_50_46_644Z-debug-0.log

pushover-desktop-client runs fine, though, but only on first run, adding the secret etc. to the user .config directory, registering the client with Pushover. However, notifications do not work during subsequent executions. Looking at the installation directly (in /usr/local/lib/node_modules/pushover-desktop-client) and running npm audit results in:

npm WARN deprecated minimatch@0.4.0: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
npm WARN deprecated mkdirp@0.5.0: Legacy versions of mkdirp are no longer supported. Please update to mkdirp 1.x. (Note that the API surface has changed to use Promises in 1.x.)
npm WARN deprecated minimatch@0.3.0: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
npm WARN deprecated mkdirp@0.3.5: Legacy versions of mkdirp are no longer supported. Please update to mkdirp 1.x. (Note that the API surface has changed to use Promises in 1.x.)
npm WARN deprecated istanbul@0.3.0: This module is no longer maintained, try this instead:
npm WARN deprecated   npm i nyc
npm WARN deprecated Visit https://istanbul.js.org/integrations for other alternatives.

added 37 packages, changed 13 packages, and audited 51 packages in 10s

13 vulnerabilities (2 moderate, 7 high, 4 critical)

npm audit results in a couple of packages that are out-of-date:

Package        Current  Wanted  Latest  Location                    Depended by
istanbul         0.3.0   0.3.0   0.4.5  node_modules/istanbul       pushover-desktop-client
mkdirp           0.5.0   0.5.0   1.0.4  node_modules/mkdirp         pushover-desktop-client
node-notifier    3.0.6   3.0.6  10.0.1  node_modules/node-notifier  pushover-desktop-client
should           4.0.4   4.0.4  13.2.3  node_modules/should         pushover-desktop-client
ws              0.4.31  0.4.31   8.8.1  node_modules/ws             pushover-desktop-client

So it looks like it needs some overhaul and updating. Or am I missing something?

@JayBrown
Copy link

Update – installed from my own fork with updated dependencies here – https://github.com/JayBrown/pushover-desktop-client –, and the above warnings/errors are gone. However, the index.js is now throwing an error upon execution. Any idea how to remedy the TypeError? Tried a couple of things, but it's not my language.

❯ pushover-desktop-client
Attempting to load settings from ~/.config/pushover-dc/settings.json
Initializing image cache directory ~/.cache/pushover-dc
/usr/local/lib/node_modules/pushover-desktop-client/index.js:36
    this.notifier = new Notification()
                    ^

TypeError: Notification is not a constructor
    at new Client (/usr/local/lib/node_modules/pushover-desktop-client/index.js:36:21)
    at startClient (/usr/local/lib/node_modules/pushover-desktop-client/bin/pushover-desktop-client:35:15)
    at Object.<anonymous> (/usr/local/lib/node_modules/pushover-desktop-client/bin/pushover-desktop-client:28:5)
    at Module._compile (node:internal/modules/cjs/loader:1119:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1173:10)
    at Module.load (node:internal/modules/cjs/loader:997:32)
    at Module._load (node:internal/modules/cjs/loader:838:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:18:47

Node.js v18.9.0

@JayBrown
Copy link

I seem to have solved it… the index.js needs to have an additional .NotificationCenter:

var ws = require('ws')
  , fs = require('fs')
  , querystring = require('querystring')
  , https = require('https')
  , Notification = require('node-notifier').NotificationCenter
  , path = require('path')

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.

3 participants