shell: Add defaultDexConnector option to automatically trigger login on a predefined Dex connector #3443

JBWatenbergScality · 2021-07-09T15:18:17Z

Component:

shell

Context:

Dex is providing the possibility to use several connectors to federate OIDC providers. However under some conditions we may need to force the usage of a specific connector.

Summary:

This PR add a defaultDexConnector option to enable this.

This option can be added to the auth section of micro application runtime-app-configuration.
It intercept the authorization_endpoint of the oidc web finger and append it the connector_id query parameter.

The connector choice can be forced by adding the displayLoginChoice query parameter to the current URL.
This enable to maintain UI login possible when the default connector is unavailable.

Additionaly as Dex is not supporting end_session_endpoint the logout is reviewed to clean the cookies of the
domain and clearing the localStorage entries of oidc client when providerLogout is set to true.

Acceptance criteria:

We shall be able to login directly using the default dex connector when the property is defined

bert-e · 2021-07-09T15:18:19Z

Hello jbwatenbergscality,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Status report is not available.

bert-e · 2021-07-09T15:18:23Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

bert-e · 2021-07-19T10:08:27Z

Integration data created

I have created the integration data for the additional destination branches.

this pull request will merge improvement/add-dex-default-connector-option into
development/2.10
w/2.11/improvement/add-dex-default-connector-option will be merged into development/2.11

The following branches will NOT be impacted:

development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7
development/2.8
development/2.9

You can set option create_pull_requests if you need me to create
integration pull requests in addition to integration branches, with:

@bert-e create_pull_requests

bert-e · 2021-07-19T10:08:28Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

ChengYanJin · 2021-07-20T06:55:31Z

shell-ui/src/auth/AuthProvider.js

@@ -39,6 +57,9 @@ function OAuth2AuthProvider({ children }: { children: Node }) {
    loadUserInfo: true,
    automaticSilentRenew: true,
    monitorSession: false,
+    MetadataServiceCtor: authConfig.defaultDexConnector


I am wondering should we add this defaultDexConnector prop to type OIDCConfig?

Done in db1812c

ChengYanJin · 2021-07-20T07:05:45Z

shell-ui/src/auth/AuthProvider.js

@@ -39,6 +57,9 @@ function OAuth2AuthProvider({ children }: { children: Node }) {
    loadUserInfo: true,
    automaticSilentRenew: true,
    monitorSession: false,
+    MetadataServiceCtor: authConfig.defaultDexConnector
+      ? defaultDexConnectorMetadataService(authConfig.defaultDexConnector)


It's more a question:
If we want to use Dex to authenticate the users against a connector-based interface, how should we set the defaultDexConnector in runtime-app-configuration?

Didn't we start documenting all this rework of shell-ui? Would be useful IMO, both now for reviewers catching up with the progress, and in the future for maintenance / onboarding.

defaultDexConnector should take the connector id as defined in dex configuration.
For example given the following Dex connectors configuration :

connectors: - type: oidc id: keycloak name: Keycloak config: //....

Providing keycloak as defaultDexConnector in runtime-app-configuration will automatically redirect the user to this connector.

I've opened #3476 regarding the documentation and will address it as soon as possible

gdemonet

No need to tackle documentation as part of this PR 😉

shell-ui/src/auth/AuthProvider.js

gdemonet · 2021-07-20T08:39:03Z

shell-ui/src/auth/AuthProvider.js

@@ -39,6 +57,9 @@ function OAuth2AuthProvider({ children }: { children: Node }) {
    loadUserInfo: true,
    automaticSilentRenew: true,
    monitorSession: false,
+    MetadataServiceCtor: authConfig.defaultDexConnector
+      ? defaultDexConnectorMetadataService(authConfig.defaultDexConnector)


Didn't we start documenting all this rework of shell-ui? Would be useful IMO, both now for reviewers catching up with the progress, and in the future for maintenance / onboarding.

bert-e · 2021-07-20T08:39:51Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

The following reviewers are expecting changes from the author, or must review again:

@gdemonet

bert-e · 2021-08-04T07:59:09Z

History mismatch

Merge commit #72d160dc1c6470be75c092cfd23c7ed969af5b1f on the integration branch
w/2.11/improvement/add-dex-default-connector-option is merging a branch which is neither the current
branch improvement/add-dex-default-connector-option nor the development branch
development/2.11.

It is likely due to a rebase of the branch improvement/add-dex-default-connector-option and the
merge is not possible until all related w/* branches are deleted or updated.

Please use the reset command to have me reinitialize these branches.

JBWatenbergScality · 2021-08-04T08:14:49Z

/reset

bert-e · 2021-08-04T08:14:54Z

Reset complete

I have successfully deleted this pull request's integration branches.

bert-e · 2021-08-04T08:15:00Z

Integration data created

I have created the integration data for the additional destination branches.

this pull request will merge improvement/add-dex-default-connector-option into
development/2.10
w/2.11/improvement/add-dex-default-connector-option will be merged into development/2.11

The following branches will NOT be impacted:

development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7
development/2.8
development/2.9

You can set option create_pull_requests if you need me to create
integration pull requests in addition to integration branches, with:

@bert-e create_pull_requests

bert-e · 2021-08-04T08:15:01Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

The following reviewers are expecting changes from the author, or must review again:

@gdemonet

bert-e · 2021-09-06T08:56:07Z

Branches have diverged

This pull request's source branch improvement/add-dex-default-connector-option has diverged from
development/2.11 by more than 50 commits.

To avoid any integration risks, please re-synchronize them using one of the
following solutions:

Merge origin/development/2.11 into improvement/add-dex-default-connector-option
Rebase improvement/add-dex-default-connector-option onto origin/development/2.11

Note: If you choose to rebase, you may have to ask me to rebuild
integration branches using the reset command.

JBWatenbergScality · 2021-09-06T08:57:11Z

/reset

bert-e · 2021-09-06T08:57:16Z

Reset complete

I have successfully deleted this pull request's integration branches.

bert-e · 2021-09-06T08:57:19Z

Branches have diverged

This pull request's source branch improvement/add-dex-default-connector-option has diverged from
development/2.11 by more than 50 commits.

To avoid any integration risks, please re-synchronize them using one of the
following solutions:

Merge origin/development/2.11 into improvement/add-dex-default-connector-option
Rebase improvement/add-dex-default-connector-option onto origin/development/2.11

Note: If you choose to rebase, you may have to ask me to rebuild
integration branches using the reset command.

…on a predefined Dex connector. This option can be added to the auth section of micro application `runtime-app-configuration`. It intercept the authorization_endpoint of the oidc web finger and append it the `connector_id` query parameter. The connector choice can be forced by adding the `displayLoginChoice` query parameter to the current URL. This enable to maintain UI login possible when the default connector is unavailable.

…session_endpoint is not supported Dex is not supporting `end_session_endpoint` the logout is reviewed to clean the cookies of the domain and clearing the localStorage entries of oidc client when `providerLogout` is set to true. Ref: #2831

bert-e · 2021-09-06T09:13:22Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

The following reviewers are expecting changes from the author, or must review again:

@gdemonet

bert-e · 2021-09-14T07:35:07Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

JBWatenbergScality · 2021-09-14T08:44:39Z

/approve

bert-e · 2021-09-14T08:44:46Z

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

✔️ development/2.11

The following branches will NOT be impacted:

development/2.0
development/2.1
development/2.10
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7
development/2.8
development/2.9

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

Any commit you add on the source branch will trigger a new cycle after the
current queue is merged.
Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

bert-e · 2021-09-14T10:14:05Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/2.11

The following branches have NOT changed:

development/2.0
development/2.1
development/2.10
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7
development/2.8
development/2.9

Please check the status of the associated issue None.

Goodbye jbwatenbergscality.

JBWatenbergScality added the topic:ui UI-related issues label Jul 9, 2021

JBWatenbergScality marked this pull request as ready for review July 19, 2021 10:08

JBWatenbergScality requested a review from a team as a code owner July 19, 2021 10:08

ChengYanJin reviewed Jul 20, 2021

View reviewed changes

gdemonet suggested changes Jul 20, 2021

View reviewed changes

JBWatenbergScality force-pushed the improvement/add-dex-default-connector-option branch from 72d160d to e656680 Compare August 4, 2021 07:59

JBWatenbergScality changed the base branch from development/2.10 to development/2.11 September 6, 2021 08:56

JBWatenbergScality added 2 commits September 6, 2021 11:12

JBWatenbergScality force-pushed the improvement/add-dex-default-connector-option branch from e656680 to a06e266 Compare September 6, 2021 09:13

gdemonet approved these changes Sep 14, 2021

View reviewed changes

bert-e merged commit 68d5c36 into development/2.11 Sep 14, 2021

bert-e deleted the improvement/add-dex-default-connector-option branch September 14, 2021 10:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

shell: Add defaultDexConnector option to automatically trigger login on a predefined Dex connector #3443

shell: Add defaultDexConnector option to automatically trigger login on a predefined Dex connector #3443

JBWatenbergScality commented Jul 9, 2021

bert-e commented Jul 9, 2021

bert-e commented Jul 9, 2021

bert-e commented Jul 19, 2021

bert-e commented Jul 19, 2021

ChengYanJin Jul 20, 2021

JBWatenbergScality Aug 4, 2021

ChengYanJin Jul 20, 2021

gdemonet Jul 20, 2021

JBWatenbergScality Aug 4, 2021

JBWatenbergScality Aug 4, 2021

gdemonet left a comment

gdemonet Jul 20, 2021

bert-e commented Jul 20, 2021

bert-e commented Aug 4, 2021

JBWatenbergScality commented Aug 4, 2021

bert-e commented Aug 4, 2021

bert-e commented Aug 4, 2021

bert-e commented Aug 4, 2021

bert-e commented Sep 6, 2021

JBWatenbergScality commented Sep 6, 2021

bert-e commented Sep 6, 2021

bert-e commented Sep 6, 2021

bert-e commented Sep 6, 2021

bert-e commented Sep 14, 2021

JBWatenbergScality commented Sep 14, 2021

bert-e commented Sep 14, 2021

bert-e commented Sep 14, 2021

shell: Add defaultDexConnector option to automatically trigger login on a predefined Dex connector #3443

shell: Add defaultDexConnector option to automatically trigger login on a predefined Dex connector #3443

Conversation

JBWatenbergScality commented Jul 9, 2021

bert-e commented Jul 9, 2021

Hello jbwatenbergscality,

bert-e commented Jul 9, 2021

Waiting for approval

bert-e commented Jul 19, 2021

Integration data created

bert-e commented Jul 19, 2021

Waiting for approval

ChengYanJin Jul 20, 2021

Choose a reason for hiding this comment

JBWatenbergScality Aug 4, 2021

Choose a reason for hiding this comment

ChengYanJin Jul 20, 2021

Choose a reason for hiding this comment

gdemonet Jul 20, 2021

Choose a reason for hiding this comment

JBWatenbergScality Aug 4, 2021

Choose a reason for hiding this comment

JBWatenbergScality Aug 4, 2021

Choose a reason for hiding this comment

gdemonet left a comment

Choose a reason for hiding this comment

gdemonet Jul 20, 2021

Choose a reason for hiding this comment

bert-e commented Jul 20, 2021

Waiting for approval

bert-e commented Aug 4, 2021

History mismatch

JBWatenbergScality commented Aug 4, 2021

bert-e commented Aug 4, 2021

Reset complete

bert-e commented Aug 4, 2021

Integration data created

bert-e commented Aug 4, 2021

Waiting for approval

bert-e commented Sep 6, 2021

Branches have diverged

JBWatenbergScality commented Sep 6, 2021

bert-e commented Sep 6, 2021

Reset complete

bert-e commented Sep 6, 2021

Branches have diverged

bert-e commented Sep 6, 2021

Waiting for approval

bert-e commented Sep 14, 2021

Waiting for approval

JBWatenbergScality commented Sep 14, 2021

bert-e commented Sep 14, 2021

In the queue

bert-e commented Sep 14, 2021