jbreckmckye / electron-auth0-login

Helper widget for Auth0 authentication in Electron desktop apps
MIT License
27 stars 19 forks source link

Added option to send custom auth0 parameters for authentication #21

Closed iamkhalidbashir closed 3 years ago

iamkhalidbashir commented 3 years ago

There are scenarios where we have to send custom params to the auth0 authorization page, an example includes the "connection_scope", so this PR created to make use of such params. @jbreckmckye

jbreckmckye commented 3 years ago

Hi Khalid - this is great. Can you upgrade the package.json version field to 1.4.0 also?

jbreckmckye commented 3 years ago

Also, whilst you're here, I was thinking this repository needs a bit of TLC.

My todo list with electron-auth-0-login is probably as follows

  1. Get a proper testbed so I can manually test this plugin again
  2. Fixup the issue you raised where the user is being remembered after logout
  3. Refactor the library

For the refactor I have a few ideas and wouldn't mind your input.

  1. At the moment, to use refresh tokens, we require the user to install a peer dependency of keytar, which is then called via a proxy called codependency. I've never felt very comfortable with this - codependency isn't in active development and in any case, not everyone may want to use keytar, as it appears there are security concerns on Windows.

What we could do is replace the keytar peer dependency with a keyService parameter. This would accept either keytar or a user-supplied module with a simple interface, e.g.

import keytar from 'keytar';

const auth = new ElectronAuth0Login({
  ...
  // using keytar
  keyService: keytar

  // OR, using user-supplied service
  keyService: {
     getRefreshToken: (): Promise<string> => { ... },
     deleteRefreshToken: (): Promise<void> => { ... }
  }
});
  1. Get rid of the dist folder from source control and set up a proper CI pipeline using Travis, including possibly automating NPM releases

  2. Open up the code to have HTTP and key storage injected, to enable proper unit tests. Right now the lack of good testability is what makes me most nervous when merging code.

Did you have any ideas or any feature requests?

iamkhalidbashir commented 3 years ago

Hi Khalid - this is great. Can you upgrade the package.json version field to 1.4.0 also?

Done

iamkhalidbashir commented 3 years ago

2. Open up the code to have HTTP and key storage injected, to enable proper unit tests. Right now the lack of good testability is what makes me most nervous when merging code.

Yes, this is a big concern. I have much on my plate for this month though, but surely making this repo stable would be awesome! Let me know what I can help you with, I'll try to find some time next month and do it. Also, keytar was a big no-no for when installing this repo but ended up using it because of a shortage of time. So that's what we should fix first.

jbreckmckye commented 3 years ago

Sounds great. Hopefully I should have some time next month too. If I start a new branch I'll tag you on a draft PR or something.

jbreckmckye commented 3 years ago

This was just released to NPM as version 1.4.0 😄 https://www.npmjs.com/package/electron-auth0-login