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

Fix github authentication built-in for electron case #13611

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

tsmaeder
Copy link
Contributor

What it does

Add dummy command to fix github authentication built-int flows. Also includes code to consider sessions which are not created, but restored from storage at registration time.

When deploying Theia as a web-app, one would normally use a web application flow (https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/authorizing-oauth-apps#web-application-flow). However, this will not work, because the VSCode extension explicitly restricts the domains for which that flow can be used. Also, we obviously do not know the "client secret" of the VS Code Github ouath app.

Fixes #13599
Partial fix for #12821

Contributed on behalof of STMicroelectronics

How to test

  1. Install the github-authentication and gitlens plugins
  2. The gitlens plugin will ask for "workspace trust": say "yes"
  3. Click the little manikin icon on the bottom of the left side bar.
  4. Observe if you are logged into github, the menu will display a menu allowing you to "sign out"
  5. If logged in, sign out, for some reason, you'll have to restart Theia now
  6. Open a file that is under git control
  7. Observe: gitlens will display text after the end of the current line. If you hover, you'll get a link "Connect to Github"
  8. Click that link
  9. Follow the instructions to sign into github
  10. Observe: when you now hover over the gitlens text, you'll get a link to the relevant PR for the line, instead of "Connect to Github"

Follow-ups

Make "github" authentication work for the web app case.

Review checklist

Reminder for reviewers

Also includes code to consider sessions which are not created, but
restored from storage at registration time

Fixes eclipse-theia#13599
Partial fix for eclipse-theia#12821

Contributed on behalof of STMicroelectronics

Signed-off-by: Thomas Mäder <[email protected]>
Copy link
Contributor

@martin-fleck-at martin-fleck-at left a comment

Choose a reason for hiding this comment

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

Thank you very much, Thomas! I can confirm the the workflow now works (even though it seems needlessly complicated but that is a different issue entirely ;-) ) with the Electron app and (still) fails with the Browser app. So definitely an improvement.

@tsmaeder tsmaeder merged commit fd988bc into eclipse-theia:master Apr 30, 2024
13 of 14 checks passed
jfaltermeier pushed a commit that referenced this pull request May 2, 2024
Also includes code to consider sessions which are not created, but
restored from storage at registration time

Fixes #13599
Partial fix for #12821

Contributed on behalof of STMicroelectronics

Signed-off-by: Thomas Mäder <[email protected]>
@jfaltermeier jfaltermeier added this to the 1.50.0 milestone Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Authentication UI is not showing Sessions restored from State
3 participants