search

opengovsg / sgid-client

The official TypeScript/JavaScript SDK for sgID
MIT License
11 stars 6 forks source link

fix(pkce): replace openid pkce generators with own implementations #52

Closed kschiew closed 1 year ago

kschiew commented 1 year ago

Problem

openid-client is a Nodejs library that is not mean to run in browser environments. Since it is technically possible that RP's generate their PKCE pairs on the frontend, our implementation of PKCE pair generation should not depend on openid-client. Closes #53 .

Solution

Referenced (read: copied) openid-client's implementation of PKCE pair generation

Tests

WIP

PrawiraGenestonlia commented 1 year ago

hmm...it seems like we are committing to support generatePkcePair in browser environments. but if we are making this commitment, we should have corresponding unit tests.

currently our unit tests only use a Node environment (see jest.config.js). I think we should set up separate tests to test for the browser environment. there are a few ways to do this, but we can start here: https://jestjs.io/docs/configuration#testenvironment-string

wdyt?

Agree. I have a follow up question. Should we go with

  1. Conditional import (import xx from "@opengovsg/sgid-client/browser)
  2. Import path remains the same as node js but within the client, we will need to export accordingly - see jose package.
mantariksh commented 1 year ago

another approach is to keep this repo for NodeJS, and have a completely separate one for browsers. advantage is separation of concerns and makes the package footprint smaller for client frontends.

raynerljm commented 1 year ago

i agree with @mantariksh - i'm more inclined towards having a separate repo intended for browsers so as not to blur the lines between Node.js and browser environments

kwajiehao commented 1 year ago

Discussed with @kschiew offline - my take is that we shouldn't make any rash decisions on updating our offering before we figure out whether we want to support a frontend PKCE generation flow. So in the meantime, we can use a third-party library like pkce-challenge for the dev portal while Wira and Kok Seng figure out what the best practice here is

kschiew commented 1 year ago

Closing PR until a new team decision is made. Pushed the code for custom implementation of browser-use PKCE generators for future reference.