This repository has been archived by the owner on Jan 26, 2025. It is now read-only.
Middleware okta logout #379
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
packages/oidc-middleware/README.md
Outdated
|
|
||
| Optional config: | ||
|
|
||
| * **loginRedirectUri** - The URI for your app that Okta will redirect users to after sign in to create the local session. Locally, this is usually `http://localhost:3000/authorization-code/callback`. When deployed, this should be `https://{yourProductionDomain}/authorization-code/callback`. This will default to `{baseUrl}{routes.loginCallback.path}` if `appBaseUrl` is provided, or the (deprecated) `redirect_uri` if appBaseUrl is not provided. Unless your redirect is to a different application, it is recommended to NOT set this parameter and instead set `appBaseUrl` and (if different than the default of `/authorization-code/callback`) `routes.loginCallback.path`. | ||
| * **loginRedirectUri** - The URI for your app that Okta will redirect users to after sign in to create the local session. Locally, this is usually `http://localhost:3000/authorization-code/callback`. When deployed, this should be `https://{yourProductionDomain}/authorization-code/callback`. This will default to `{appBaseUrl}{routes.loginCallback.path}` if `appBaseUrl` is provided, or the (deprecated) `redirect_uri` if appBaseUrl is not provided. Unless your redirect is to a different application, it is recommended to NOT set this parameter and instead set `appBaseUrl` and (if different than the default of `/authorization-code/callback`) `routes.loginCallback.path`. |
There was a problem hiding this comment.
nit: *appBaseUrl is not provided. (missing `` around appBaseUrl)
packages/oidc-middleware/README.md
Outdated
| @@ -291,6 +318,9 @@ const oidc = new ExpressOIDC({ | |||
| * **`loginCallback.handler`** - A function that is called after a successful authentication callback, but before the final redirect within your application. Useful for requirements such as conditional post-authentication redirects, or sending data to logging systems. | |||
| * **`loginCallback.path`** - The URI that this library will host the login callback handler on. Defaults to `/authorization-code/callback`. Must match a value from the Login Redirect Uri list from the Okta console for this application. | |||
| * **`login.path`** - The URI that redirects the user to the Okta authorize endpoint. Defaults to `/login`. | |||
| * **`logout.path`** - The RUI that redirects the user to the Okta logout endpoint. Defaults to `/logout`. | |||
| connectUtil.createLogoutHandler = context => logout.forceLogoutAndRevoke(context); | ||
|
|
||
| connectUtil.createLogoutCallbackHandler = context => { | ||
| return (req, res, next) => { |
There was a problem hiding this comment.
What is next used for?
There was a problem hiding this comment.
Nothing here - removed
| @@ -233,7 +234,7 @@ describe('new ExpressOIDC()', () => { | |||
|
|
|||
|
|
|||
| it('should set the HTTP timeout to 10 seconds', () => { | |||
| new ExpressOIDC({ | |||
| new ExpressOIDC({ | |||
There was a problem hiding this comment.
nit: no need for trailing space
| async performLogout() { | ||
| const logoutButton = $('#logout'); | ||
| await logoutButton.click(); | ||
| await browser.wait(EC.not(EC.presenceOf(logoutButton)), 10000); |
There was a problem hiding this comment.
Wondering, how was 10000 decided? (function above use 50000)
There was a problem hiding this comment.
Those are Vijet questions - the 50000 is notably large because that's the initial page load and bacon had issues with it, and most of the rest are 5000. This got bumped when we were trying to figure out a core build issue, but I believe that ended up being something else, so I'll return this to 5000.
There was a problem hiding this comment.
BREAKING CHANGE: Configurations now require appBaseUrl. redirect_uri is deprecated
BREAKING CHANGE: adds POST-based /logout and /logout/callback routes that will override existing matching routes defined by the user (if any). See `Upgrading` in the README for more info.
swiftone
force-pushed
the
middleware-okta-logout
branch
from
dd889ec to
37e5dbd
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
/logout is GET-based and only destroys the local session
Issue Number: #162
What is the new behavior?
/logout is POST-based and performs an Okta logout and revokes related tokens.
Does this PR introduce a breaking change?
See 'Upgrading' in README
Other information
Reviewers
@manueltanzi-okta @vijetmahabaleshwar-okta