feat: adds PKCE support

rbarilani commented 2 years ago

PR Checklist

Please check if your PR fulfills the following requirements:

[x] The commit message follows our guidelines
[ ] Tests for the changes have been added (for bug fixes / features)
[ ] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Adding Tests
[ ] Build related changes
[ ] CI related changes
[ ] Documentation changes
[ ] Other... Please describe:

What is the current behavior?

The middleware doesn't support PKCE, so it's always necessary to use a client secret. See also community request #27.

Issue Number: #27

What is the new behavior?

The middleware optionally supports PKCE by passing usePKCE flag during initialisation, eg:

const oidc = new ExpressOIDC({
  issuer: 'https://{yourOktaDomain}/oauth2/default',
  client_id: '{clientId}',
  // client_secret: '{clientSecret}', # not required when using PKCE
  usePKCE: true,
  appBaseUrl: '{appBaseUrl}',
  scope: 'openid'
});

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

Hi everybody I am opening this draft to understand if you are willing to add necessary changes to support PKCE as requested by #27. If yes I can add related unit, integration and e2e test and as well documentation. I guess I might need some support from the maintainers to setup the e2e test, if you are running them on some CI environment as we should then probably have an app without client secret, with PKCE enabled and auth code flow enabled.

Reviewers

rbarilani commented 2 years ago

@jaredperreault-okta @oleksandrpravosudko-okta WDYT?

jaredperreault-okta commented 2 years ago

@rbarilani Thanks for the PR! We will review this internally

Internal Ref: OKTA-491178

rbarilani commented 2 years ago

Hi @jaredperreault-okta any updates?

jaredperreault-okta commented 2 years ago

@rbarilani could you explain your use case for PKCE in your node app?

PKCE is not a replacement for a client secret, and PKCE is recommended even if a client is using a client secret.

source: https://oauth.net/2/pkce/

Adding PKCE to a web app would increase the security, but should not replace the client secret. PKCE is used as a "replacement" for client secret in SPA or mobile apps where providing the client secret to the client risks exposing the secret

rbarilani commented 2 years ago

Hi @jaredperreault-okta, the use case is "development ease". We have a use case for a web application is started by multiple teams for their local development, by using PKCE we can avoid to share the client secret with multiple developers. In this regard the use case is really similar to an SPA or mobile app, where providing client secret to the client (aka the developer machine) risks exposing the secret. When the application will be running instead on cloud server we will use client secret and "traditional" authorization code flow. The use case is really similar to the one highlighted by #27 so I guess it might be useful also to other people.

Thanks in advance.

jaredperreault-okta commented 2 years ago

@rbarilani this module is designed to be easy to use (low config) and to accomplish this we makes certain assumptions. The main assumption being clients will be confidential (private) client, meaning they require a client secret.

To accomplish your use case, I suggest taking a look at okta-auth-js or a different oidc client module like openid-client

rbarilani commented 2 years ago

@jaredperreault-okta thanks for have taken the time to consider the change.

okta / okta-oidc-middleware