Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI: Dex integration #2042

Merged
merged 11 commits into from
Dec 6, 2019
Merged

UI: Dex integration #2042

merged 11 commits into from
Dec 6, 2019

Conversation

carlito-scality
Copy link
Contributor

@carlito-scality carlito-scality commented Nov 13, 2019

Component: UI

Context:

  • Integrate Dex login using redux-oidc library

Summary:

Acceptance criteria:

  • The MetalK8s UI should directly use dex to authenticate a user using OAuth2 flow.
  • Once logged in, the reste should works like before without regressions

Closes: #2015

@carlito-scality carlito-scality added moonshot topic:ui UI-related issues complexity:hard Something that may require up to a week to fix labels Nov 13, 2019
@bert-e
Copy link
Contributor

bert-e commented Nov 13, 2019

Hello carlito-scality,

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
Copy link
Contributor

bert-e commented Nov 13, 2019

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:

@carlito-scality carlito-scality force-pushed the feature/dex-integration-in-ui branch 2 times, most recently from e72662b to 0adb9da Compare November 18, 2019 10:55
@bert-e
Copy link
Contributor

bert-e commented Nov 18, 2019

Conflict

There is a conflict between your branch feature/dex-integration-in-ui and the
destination branch development/2.4.

Please resolve the conflict on the feature branch (feature/dex-integration-in-ui).

 $ git fetch
 $ git checkout origin/feature/dex-integration-in-ui
 $ git merge origin/development/2.4
 $ # <intense conflict resolution>
 $ git commit
 $ git push origin HEAD:feature/dex-integration-in-ui

@carlito-scality carlito-scality force-pushed the feature/dex-integration-in-ui branch 4 times, most recently from 5d50b5a to 2fbcb2b Compare November 20, 2019 08:22
@bert-e
Copy link
Contributor

bert-e commented Nov 20, 2019

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:

@carlito-scality carlito-scality force-pushed the feature/dex-integration-in-ui branch 2 times, most recently from 2e37316 to 1b61997 Compare November 21, 2019 15:23
@gdemonet gdemonet changed the base branch from development/2.4 to development/2.5 November 21, 2019 22:49
@gdemonet
Copy link
Contributor

/after_pull_request=2063

@bert-e
Copy link
Contributor

bert-e commented Nov 21, 2019

Waiting for other pull request(s)

The current pull request is locked by the after_pull_request option.

In order for me to merge this pull request, run the following actions first:

➡️ Merge the OPEN pull request:

Alternatively, delete all the after_pull_request comments from this pull request.

The following options are set: after_pull_request

@gdemonet
Copy link
Contributor

@carlito-scality you'll need to rebase on development/2.5 when #2063 gets merged.

@gdemonet
Copy link
Contributor

/after_pull_request=2025

@bert-e
Copy link
Contributor

bert-e commented Nov 21, 2019

Waiting for other pull request(s)

The current pull request is locked by the after_pull_request option.

In order for me to merge this pull request, run the following actions first:

➡️ Merge the OPEN pull requests:

Alternatively, delete all the after_pull_request comments from this pull request.

The following options are set: after_pull_request

@bert-e
Copy link
Contributor

bert-e commented Nov 25, 2019

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 options are set: after_pull_request

@carlito-scality carlito-scality marked this pull request as ready for review November 25, 2019 17:02
@carlito-scality carlito-scality requested a review from a team as a code owner November 25, 2019 17:02
@ChengYanJin ChengYanJin force-pushed the feature/dex-integration-in-ui branch 2 times, most recently from a9e7b6f to 3c0c96e Compare November 26, 2019 15:12
ChengYanJin
ChengYanJin previously approved these changes Nov 27, 2019
Copy link
Contributor

@ChengYanJin ChengYanJin left a comment

Choose a reason for hiding this comment

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

LGTM! Great job!!!
We will need to handle share user data between tabs in order to eliminate unnecessary authentication.

ChengYanJin
ChengYanJin previously approved these changes Nov 29, 2019
@carlito-scality carlito-scality force-pushed the feature/dex-integration-in-ui branch 2 times, most recently from d288e24 to 81ef5b9 Compare December 3, 2019 10:19
@TeddyAndrieux TeddyAndrieux force-pushed the feature/dex-integration-in-ui branch 2 times, most recently from e43f24b to f250e1e Compare December 4, 2019 21:40
carlito-scality and others added 11 commits December 5, 2019 17:48
- re-implement the login workflow using redux-oidc library
- update the way to authenticate to Salt
- remove old Login page and image
- catch the redux-oidc USER_FOUND action, to update ApiK8s services and authenticate to salt
- add isUserLoaded state to check if the user is loaded
…irect urls

- url_oidc_provider allow to fetch /oidc/.well-known/openid-configuration file
- url_redirect should contain full URL since it should be regirstered as it is in the DEX config and it is used for redirection once the login in DEX login form is successful.
Ref: #2015
- After the login in Dex Form is successful, we go back to the CallbackPage containing a Loader with the text
Ref: #2015
- update cypress.json with new default login
- add return null to PrivateRoute since it should always return something. Otherswise, we got an JS error.
Ref: #2015
- Display only username instead of username + email
- update cypress tests
Ref: #2015
- it is introduced during last rebasing
-We add redux-oidc for dex login which increase the bundle size.
When we try to retrieve informations from the mine from salt-master we
cannot use module `mine.get` we need to use `saltutil.runner` with
`mine.get`, this approach works for salt-master but not for
salt-minion not on the same machine as the salt-master
Copy link
Collaborator

@TeddyAndrieux TeddyAndrieux left a comment

Choose a reason for hiding this comment

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

backend part LGTM

@carlito-scality
Copy link
Contributor Author

/approve

@bert-e
Copy link
Contributor

bert-e commented Dec 6, 2019

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.5

The following branches will NOT be impacted:

  • development/1.0
  • development/1.1
  • development/1.2
  • development/1.3
  • development/2.0
  • development/2.1
  • development/2.2
  • development/2.3
  • development/2.4

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, after_pull_request

@bert-e
Copy link
Contributor

bert-e commented Dec 6, 2019

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

  • ✔️ development/2.5

The following branches have NOT changed:

  • development/1.0
  • development/1.1
  • development/1.2
  • development/1.3
  • development/2.0
  • development/2.1
  • development/2.2
  • development/2.3
  • development/2.4

Please check the status of the associated issue None.

Goodbye carlito-scality.

@bert-e bert-e merged commit 1364f5f into development/2.5 Dec 6, 2019
@bert-e bert-e deleted the feature/dex-integration-in-ui branch December 6, 2019 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity:hard Something that may require up to a week to fix topic:ui UI-related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants