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

Improve login procedure for GitHub PullRequest Plugin #14217

Closed
vparfonov opened this issue Aug 13, 2019 · 18 comments
Closed

Improve login procedure for GitHub PullRequest Plugin #14217

vparfonov opened this issue Aug 13, 2019 · 18 comments
Assignees
Labels
area/plugin-port Issues related to the port plugin kind/enhancement A feature request - must adhere to the feature request template. severity/P1 Has a major impact to usage or development of the system. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community.
Milestone

Comments

@vparfonov
Copy link
Contributor

Is your enhancement related to a problem? Please describe.

At the moment we have ability to use GitHub Pull Requests for Visual Studio Code, in side Che Theia. In case you want to provide new PullRequest, plugin may request you to login with the OAuth.
Problem is in OAuth server hardcoded on plugin side and it point to VS Code server.
As workaround we provide document which allow setup OAuth token manually:

  1. Authenticate: Run F1 => GitHub Pull Requests: Manually Provide Authentication Response command and paste the GitHub token.
  2. Select the repository permissions when generating the token.

Describe the solution you'd like

Most reasonable solution will be provide ability for setup OAuth server in plugin settings. For this we need to propose PR for original plugin repository.

Describe alternatives you've considered

Fork plugin and replace server url but I don't want consider it as solution. I just described it here to make solutions visible for other.

Additional context

@vparfonov vparfonov added kind/enhancement A feature request - must adhere to the feature request template. area/plugin-port Issues related to the port plugin labels Aug 13, 2019
@che-bot che-bot added the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Aug 13, 2019
@slemeur slemeur added this to the 7.2.0 milestone Aug 14, 2019
@slemeur slemeur removed the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Aug 14, 2019
@vinokurig vinokurig self-assigned this Sep 11, 2019
@sunix
Copy link
Contributor

sunix commented Sep 11, 2019

could we identify where/how to set the token (from another plugin/extension/or anything else) and by pass the github extension ? If the token is already set, the github extension would work right away.

@vparfonov vparfonov modified the milestones: 7.2.0, Backlog - IDE 1 Sep 23, 2019
@vparfonov vparfonov added the severity/P1 Has a major impact to usage or development of the system. label Sep 23, 2019
@vinokurig vinokurig added the status/in-progress This issue has been taken by an engineer and is under active development. label Oct 1, 2019
@vparfonov vparfonov modified the milestones: Backlog - IDE 1, 7.3.0 Oct 2, 2019
@vparfonov vparfonov modified the milestones: 7.3.0, Backlog - IDE 1 Oct 11, 2019
@vparfonov vparfonov modified the milestones: Backlog - IDE 1, 7.4.0 Oct 23, 2019
@vparfonov vparfonov added status/blocked Issue that can’t be moved forward. Must include a comment on the reason for the blockage. and removed status/in-progress This issue has been taken by an engineer and is under active development. labels Nov 4, 2019
@vparfonov vparfonov modified the milestones: 7.4.0, Backlog - IDE 1 Nov 4, 2019
@vparfonov
Copy link
Contributor Author

We blocked at the moment with no ability use oAuth service in Che multi-user

@ericwill ericwill mentioned this issue Dec 17, 2019
15 tasks
@vinokurig
Copy link
Contributor

vinokurig commented Jan 8, 2020

It is blocked by #15621 and #15631

@vinokurig vinokurig added status/blocked Issue that can’t be moved forward. Must include a comment on the reason for the blockage. and removed status/blocked Issue that can’t be moved forward. Must include a comment on the reason for the blockage. labels Jan 8, 2020
@ericwill ericwill mentioned this issue Jan 28, 2020
12 tasks
@ericwill ericwill mentioned this issue Feb 19, 2020
21 tasks
@ericwill ericwill mentioned this issue Mar 12, 2020
50 tasks
@ericwill ericwill mentioned this issue Apr 1, 2020
47 tasks
@ericwill ericwill mentioned this issue Apr 22, 2020
52 tasks
@sunix
Copy link
Contributor

sunix commented Jun 5, 2020

hello @vinokurig any progress on this?

@vinokurig
Copy link
Contributor

vinokurig commented Jun 9, 2020

@sunix, not yet, it depends on eclipse-theia/theia#7661

@sunix
Copy link
Contributor

sunix commented Jun 26, 2020

@vinokurig can't we move forward without that?
Any other options?
Can you details all the steps to have this done and where we are?
Can't we set the token programmatically either on the file system or using the command (that we use manually ATM)?

@sunix
Copy link
Contributor

sunix commented Jul 15, 2020

@ericwill @vinokurig any progress on that one?

@ericwill
Copy link
Contributor

@ericwill @vinokurig any progress on that one?

We are currently working on other items, this one will have to wait a sprint or two.

@sunix
Copy link
Contributor

sunix commented Jul 23, 2020

@vinokurig @ericwill
What I suggest is that we patch /home/theia/.theia/plugin-storage/global-storage.json adding

"keychain: github.com": "the-github-token-retrieved-somewhere"

at startup of the plugin or in an entrypoint.

@vinokurig
Copy link
Contributor

This may help to save the login session between workspace restarts, but the initial issue is to make the first login procedure as smooth as possible right? For that we need to have the vscode-git plugin been updated to the latest version: #16041 and then with the help of the authentication plugin API: #14217 we can register the Che GitHub oauth provider for the github PR plugin.

@sunix
Copy link
Contributor

sunix commented Jul 31, 2020

But if we already have the github token https://www.eclipse.org/che/docs/che-7/configuring-github-oauth/ ... can't we reuse it ? rather than doing this from vscode.

@vinokurig
Copy link
Contributor

We already using it in the github-oauth plugin, but it works by calling it's own command from the command pallet. The native command which is registered by the github PR plugin and appears in the notification uses the authentication API.

@sunix
Copy link
Contributor

sunix commented Jul 31, 2020

so we can just call that code or command from github-oauth in Che and add the token to /home/theia/.theia/plugin-storage/global-storage.json ... no? why is it mandatory to override/register the authentication API if we know that it will take more than 6 months. A quick fix/workaround would be nice ... it's been a year this is here and new comers are trying to use the GHplugin but can't sign it ...

@vinokurig
Copy link
Contributor

@sunix

so we can just call that code or command from github-oauth in Che and add the token to /home/theia/.theia/plugin-storage/global-storage.json

We already do the similar approach. The problem is that the github-oauth opens a pop-up to login to GitHub and we don't call this command on the plugin start to not to show the pup-up on workspace start. We have a special command for that. At the same time we see a notification from the github PR plugin which wants us to use the vscode-oauth.

why is it mandatory to override/register the authentication API

The latest version of the plugin uses the authentication API so anyway we need it, if we want to update the plugin.

it's been a year this is here and new comers are trying to use the GHplugin but can't sign it ...

The workaround is documented.

@sunix
Copy link
Contributor

sunix commented Aug 3, 2020

The workaround is not acceptable if the user is getting into signin with the GH pr plugin that doesn't work with Che. How will the user magically find that documentation?
but why not just call github-oauth once, the first time at workspace start ?
My suggestion: if waiting for eclipse-theia/theia#7661 would takes more than 1 month we should find a way to help the user to set the token, to me github-oauth is a good workaround.
Definitely the workaround in the doc is not a good one as the user will never find that doc. Also token is lost after each restart of the workspace.

@sunix
Copy link
Contributor

sunix commented Aug 3, 2020

Let's take it differently ... if you look at https://twitter.com/eclipse_che/status/1290225898998755330 how many new users do you think would be able to get signed in and create a PR out of it?
My guess: I would say 0%. 2% would create an issue, 98% would give up, 0% would find the "workaround" doc.

@vinokurig
Copy link
Contributor

Is fixed by eclipse-che/che-theia#841

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/plugin-port Issues related to the port plugin kind/enhancement A feature request - must adhere to the feature request template. severity/P1 Has a major impact to usage or development of the system. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community.
Projects
None yet
Development

No branches or pull requests

6 participants