Skip to content
This repository has been archived by the owner on Jan 26, 2025. It is now read-only.

fix: remove connect-ensure-login to avoid implicit req.session.returnTo change - OKTA-255316 #731

Conversation

shuowu-okta
Copy link
Contributor

@shuowu-okta shuowu-okta commented Apr 3, 2020

PR Checklist

Please check if your PR fulfills the following requirements:

  • 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
  • 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?

Currently oidc-middleware use unmaintained package connect-ensure-login as dependency, which implicitly changes req.session.returnTo value to req.orginUrl || req.url.
It cause loginCallback.afterCallback URL cannot be properly consumed by passport

Issue Number: #209

What is the new behavior?

Remove oidc-middleware, then reimplement oidcUtil.ensureAuthenticated function w/o changing 'req.session`

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

connect-ensure-login

Reviewers

@aarongranick-okta

@shuowu-okta shuowu-okta force-pushed the sw-oidc-middleware-remove-connect-ensure-login-dependency-OKTA-255316 branch 2 times, most recently from 5870e43 to b43d2ec Compare April 6, 2020 16:12
@@ -35,7 +35,6 @@
"dependencies": {
"@okta/configuration-validation": "^0.4.1",
"body-parser": "^1.18.2",
"connect-ensure-login": "^0.1.1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per https://oktawiki.atlassian.net/wiki/spaces/eng/pages/814532965/DevEx+JS+Repo+Notes this should update the version and include a CHANGELOG.

(Note that sign-in-widget follows different rules than the rest of our JS repos)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

@shuowu-okta shuowu-okta requested a review from swiftone April 6, 2020 18:21
@shuowu-okta shuowu-okta force-pushed the sw-oidc-middleware-remove-connect-ensure-login-dependency-OKTA-255316 branch from 855c7fa to 61a7174 Compare April 6, 2020 23:24
@shuowu-okta shuowu-okta merged commit 367e8be into master Apr 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants