Improvements for dex auth #251
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
…ss_token or id_token exist in the url, small contextswitcher bug, refactored backdrop since inline if-elses are breaking in svelte oncreate,
…nts-for-dex-auth # Conflicts: # core/src/navigation/services/navigation.js
maxmarkus
requested review from
antiheld,
dariadomagala-sap,
hardl,
jesusreal,
kwiatekus,
parostatkiem-zz,
pekura and
y-kkamil
as code owners
pekura
suggested changes
Nov 27, 2018
core/src/services/routing.js
Outdated
| @@ -288,6 +288,12 @@ const buildRoute = (node, path, params) => | |||
| : buildRoute(node.parent, `/${node.parent.pathSegment}${path}`, params); | |||
|
|
|||
| export const handleRouteChange = async (path, component, node, config) => { | |||
| if ( | |||
| window.location.href.indexOf('access_token') !== -1 || | |||
There was a problem hiding this comment.
- Can we do this check a bit more specific? At least indexOf('access_token=').
- Can we make it configurable? Like list of regex expressions for paths to be ignored by Luigi routing? That way the Luigi app owner would have full control over it.
There was a problem hiding this comment.
Yes, thats a good idea :-)
parostatkiem-zz
previously requested changes
Nov 28, 2018
| methods: { | ||
| setBackdropClass() { | ||
| const baseClasses = 'fd-ui__overlay fd-overlay fd-overlay--modal '; | ||
| if (!this.get().backdropActive) { |
There was a problem hiding this comment.
I highly recommend https://www.npmjs.com/package/classnames for this. It can do exactly the same what this function does but with less code :)
There was a problem hiding this comment.
Less code, but more dependencies. Not a fan of this. I guess if we want to have it, we can write it by ourself with a small helper function. similar to ['class1', 'class2'].join(' ').
| @@ -51,6 +53,7 @@ | |||
| import LogoTitle from './LogoTitle.html'; | |||
| import Authorization from '../Authorization.html'; | |||
| import { handleRouteClick } from '../services/routing.js'; | |||
| import * as Navigation from './services/navigation.js'; | |||
There was a problem hiding this comment.
How about {isLoggedIn}?
There was a problem hiding this comment.
I did this intentionally to make it easier to distinguish between internal and imported functions, but it is a pure matter of taste.
maxmarkus
requested review from
bszwarc,
kazydek,
klaudiagrz and
tomekpapiernik
as code owners
pekura
approved these changes
Nov 28, 2018
Co-Authored-By: kwiatekus <[email protected]>
kwiatekus
approved these changes
Nov 28, 2018
…kus/luigi into feature/improvements-for-dex-auth
This reverts commit e2501ee.
maxmarkus
dismissed
parostatkiem-zz’s stale review
no actions taken based on the comments, verified with kk
stanleychh
pushed a commit
to stanleychh/luigi
that referenced
this pull request
Dec 30, 2021
* added possibility for root redirect_uris by disabling routing if access_token or id_token exist in the url, small contextswitcher bug, refactored backdrop since inline if-elses are breaking in svelte oncreate, * iframe resiliency for logout * added logout.html and changed logoutUrl, fixed small bugs causing errors * added skipRoutingForUrlPatterns configuration setting for skipping route handling * Update docs/navigation-configuration.md
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Improvements which are required for console.