Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Apply GitHub authentication plugin #604

Merged
merged 6 commits into from
Jan 29, 2020
Merged

Apply GitHub authentication plugin #604

merged 6 commits into from
Jan 29, 2020

Conversation

vinokurig
Copy link
Contributor

@vinokurig vinokurig commented Jan 17, 2020 •
edited
Loading

What does this PR do?

Add a plugin that authenticates vscode GitHub PR plugin launched in Che-Theia
ezgif-2-e8952916c88e

The plugin calls Che oAuth API service to get the GitHub token. Then it injects it to the user preferences. When the browser page is refreshed the vscode GitHub PR plugin fetches the token and cleans the token from the preferences file.

This works only in single user Che. When eclipse-che/che#15672 is applied the token could be fetched in the multi-user.

What can be improved:
The login actions that provided by the native vscode GitHub plugin are still redirect to the vscode authentication service. To override them we need:

  1. Apply Urihandler plugin API method: [Plugin-Api] Apply window.registerUriHandler() function eclipse-theia/theia#5119. It is used by the vscode GitHub PR plugin to handle the callback request from authentication server.
  2. Authentication server url is hard-coded in the vscode GitHub PR plugin: https://github.com/microsoft/vscode-pull-request-github/blob/ddaa2bcd06ebc931e075cc9cab5a83f683e93e83/src/authentication/githubServer.ts#L16-L17
    https://github.com/microsoft/vscode-pull-request-github/blob/ddaa2bcd06ebc931e075cc9cab5a83f683e93e83/src/authentication/githubServer.ts#L177-L179
    so we need to propose a PR to the vscode GitHub PR plugin repository that allows to override the authentication urls.

When the steps above are done we can setup the vscode GitHub PR plugin to use Che oAuth service for authentication and get rid of this authentication plugin.

What issues does this PR fix or reference?

eclipse-che/che#14217

Release Notes

Docs PR

eclipse-che/che-docs#1055

@vinokurig
Apply Github authentication plugin
5409ef7 
Signed-off-by: Igor Vinokur <ivinokur@redhat.com>
@vinokurig
fixup! Apply Github authentication plugin
36b06f8 
Signed-off-by: Igor Vinokur <ivinokur@redhat.com>
@benoitf
Copy link
Contributor

benoitf commented Jan 17, 2020

hello, why do we need to refresh the browser ? (on UX it may not be so nice to do that)

@vinokurig
fixup! Apply Github authentication plugin
a911caa 
Signed-off-by: Igor Vinokur <ivinokur@redhat.com>
@vinokurig
Copy link
Contributor Author

@benoitf I agree that it is not a good UX, but the vscode GitHub PR plugin loads the token from configuration on it's initialisation https://github.com/microsoft/vscode-pull-request-github/blob/6b019aa02b90cc5729f14ba8b98f653b34fa64a6/src/extension.ts#L37

@che-bot
Copy link
Contributor

che-bot commented Jan 17, 2020

✅ E2E Happy path tests succeed 🎉

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:604
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:604

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

@che-bot
Copy link
Contributor

che-bot commented Jan 17, 2020

✅ E2E Happy path tests succeed 🎉

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:604
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:604

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

@che-bot
Copy link
Contributor

che-bot commented Jan 17, 2020

✅ E2E Happy path tests succeed 🎉

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:604
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:604

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

benoitf
benoitf approved these changes Jan 24, 2020
View reviewed changes
Copy link
Contributor

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

👍

@che-bot
Copy link
Contributor

che-bot commented Jan 24, 2020

✅ E2E Happy path tests succeed 🎉

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:604
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:604

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

@che-bot
Copy link
Contributor

che-bot commented Jan 27, 2020

✅ E2E Happy path tests succeed 🎉

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:604
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:604

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

@che-bot
Copy link
Contributor

che-bot commented Jan 27, 2020

❌ E2E Happy path tests failed ❗

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:604
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:604

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

⚠️ https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

ℹ️ Use comment "crw-ci-test" to rerun happy path E2E test.

@vinokurig vinokurig mentioned this pull request Jan 27, 2020
Merged
@vinokurig
Copy link
Contributor Author

crw-ci-test

@che-bot
Copy link
Contributor

che-bot commented Jan 27, 2020

✅ E2E Happy path tests succeed 🎉

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:604
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:604

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

@vinokurig vinokurig merged commit 83efca5 into master Jan 29, 2020
@sleshchenko sleshchenko deleted the che-14217 branch February 26, 2020 10:56
@sleshchenko sleshchenko restored the che-14217 branch February 26, 2020 10:56
@vinokurig vinokurig deleted the che-14217 branch February 26, 2020 15:44
vinokurig pushed a commit that referenced this pull request Apr 6, 2021
@che-bot @mkuznyetsov
[release] Add 7.18.1 plugins in master (#604)
432eecf 
Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

Co-authored-by: Mykhailo Kuznietsov <mkuznets@redhat.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@benoitf benoitf benoitf approved these changes

@RomanNikitenko RomanNikitenko RomanNikitenko approved these changes

@l0rd l0rd Awaiting requested review from l0rd

@slemeur slemeur Awaiting requested review from slemeur

@vitaliy-guliy vitaliy-guliy Awaiting requested review from vitaliy-guliy

@azatsarynnyy azatsarynnyy Awaiting requested review from azatsarynnyy

@vzhukovs vzhukovs Awaiting requested review from vzhukovs

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@vinokurig @benoitf @che-bot @RomanNikitenko