feat: Implement login manager and login/logout commands #910
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
mmorhun
requested review from
benoitf,
l0rd,
tolusha,
sleshchenko,
AndrienkoAleksandr and
akurinnoy
|
Skipping CI for Draft Pull Request. |
|
Only |
50 tasks
|
It is better to split login/logout functionality into the following commands: |
tolusha
reviewed
Oct 12, 2020
tolusha
reviewed
Oct 12, 2020
tolusha
reviewed
Oct 12, 2020
tolusha
reviewed
Oct 12, 2020
tolusha
reviewed
Oct 12, 2020
tolusha
reviewed
Oct 12, 2020
tolusha
reviewed
Oct 12, 2020
tolusha
reviewed
Oct 12, 2020
tolusha
reviewed
Oct 12, 2020
tolusha
reviewed
Oct 12, 2020
tolusha
reviewed
Oct 12, 2020
mmorhun
force-pushed
the
che-16415
branch
2 times, most recently
from
d21dd75
to
4f6ed85
Compare
Signed-off-by: Mykola Morhun <mmorhun@redhat.com>
…atly. Signed-off-by: Mykola Morhun <mmorhun@redhat.com>
|
Rebased |
tolusha
approved these changes
Oct 21, 2020
AndrienkoAleksandr
approved these changes
Oct 21, 2020
yhontyk
reviewed
Oct 21, 2020
src/commands/auth/use.ts
Outdated
| [USERNAME_KEY]: username, | ||
| interactive: flags.boolean({ | ||
| char: 'i', | ||
| description: 'Select active login session in interactive mode', |
There was a problem hiding this comment.
Suggested change
| description: 'Select active login session in interactive mode', | |
| description: 'Select an active login session in interactive mode', |
src/commands/auth/use.ts
Outdated
| 'chectl auth:use -u another-user-on-this-server', | ||
| '\n\n# Switch to the only user on the given cluster:\n' + | ||
| 'chectl auth:use my.cluster.net', | ||
| '\n\n# Select active login session in interactive mode:\n' + |
There was a problem hiding this comment.
Suggested change
| '\n\n# Select active login session in interactive mode:\n' + | |
| '\n\n# Select an active login session in interactive mode:\n' + |
src/api/che-login-manager.ts
Outdated
| if (error && error.response && error.response.data && error.response.data.error_description) { | ||
| message = error.response.data.error_description | ||
| } | ||
| throw new Error(`Failed to get access token from ${keycloakTokenUrl}. Cause: ${message}`) |
There was a problem hiding this comment.
Suggested change
| throw new Error(`Failed to get access token from ${keycloakTokenUrl}. Cause: ${message}`) | |
| throw new Error(`Failed to get the access token from ${keycloakTokenUrl}. Cause: ${message}`) |
test/e2e/openshift.test.ts
Outdated
| @@ -44,6 +47,37 @@ describe('Eclipse Che deploy test suite', () => { | |||
| }) | |||
| }) | |||
|
|
|||
| describe('Che server authentication', () => { | |||
| it('Should login into Che server with username and password', async () => { | |||
There was a problem hiding this comment.
Suggested change
| it('Should login into Che server with username and password', async () => { | |
| it('Should log in to Che server with username and password', async () => { |
Signed-off-by: Mykola Morhun <mmorhun@redhat.com>
|
New changes are detected. LGTM label has been removed. |
rhopp
added a commit
to eclipse-che/che
that referenced
this pull request
Oct 22, 2020
This is needed because of che-incubator/chectl#910 Signed-off-by: Radim Hopp <rhopp@redhat.com>
rhopp
added a commit
to eclipse-che/che
that referenced
this pull request
Oct 22, 2020
This is needed because of che-incubator/chectl#910 Signed-off-by: Radim Hopp <rhopp@redhat.com>
rhopp
added a commit
to eclipse-che/che
that referenced
this pull request
Oct 22, 2020
This is needed because of che-incubator/chectl#910 Signed-off-by: Radim Hopp <rhopp@redhat.com>
rhopp
added a commit
to eclipse-che/che
that referenced
this pull request
Oct 22, 2020
This is needed because of che-incubator/chectl#910 Signed-off-by: Radim Hopp <rhopp@redhat.com>
rhopp
added a commit
to rhopp/che-tests-playground
that referenced
this pull request
Apr 16, 2021
This is needed because of che-incubator/chectl#910 Signed-off-by: Radim Hopp <rhopp@redhat.com>
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.
Signed-off-by: Mykola Morhun mmorhun@redhat.com
What does this PR do?
Before this change users had to provide Che server API URL and access token for each run of all commands that require authentication. This is not a user-friendly way to go. In this PR login manager is implemented and new commands introduced:
auth:loginlogins user into a Che server. So, after login user can just run commands without Che API URL and access token parameters.auth:logoutlogs out user from current Che serverauth:get- show current login infoauth:list- shows all available loginsauth:use- switches between login sessionsauth:delete- deletes specified login session or all logins for the given Che serverAlso, comfortable interface for using in commands is created.
The implementation supports 3 login methods via:
Note, login manager does not (and should not) depend on
kubectloroclogin information. It might use it, thought, to simplify login process.Under the hood, login manager saves users refresh tokens. Also it is capable to automatically remove outdated ones.
What issues does this PR fix or reference?
eclipse-che/che#16415
How to test this PR?
At least two running instances of Eclispe Che is needed (
s1,s2) with two users (u1(admin user),u2) in each. One instance should have OAuth enabled on Openshift, where the second one should use password login on Kubernetes. For testing purposes I usedworkspace:listcommand, so it is helpful when each account has different workspaces (ideally different number of them).User scenario:
chectl workspace:listandchectl context:listcommandss1u2:chectl auth:login <s1> -t <refresh-token>.s2u1:chectl auth:login <s2> -u <username> -p <password>s2u2:chectl auth:login <s2> -u <username>(password should be asked)ocintos1u1. Runchectl auth:login(if u1 is not admin user, provide<s1>argument)chectl auth:use <s2> -u <u2>chectl auth:delete <s1> -u <u1>chectl auth:logouts3which is a single user Che installationchectl workspace:list --che-api-endpoint <s3>for single user instance.See docs how to get refresh token manually and feel the difference =)
PR Checklist
As the author of this Pull Request I made sure that:
What issues does this PR fix or referenceandHow to test this PRcompletedReviewers
Reviewers, please comment how you tested the PR when approving it.