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.

Sign up for GitHub

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

Jump to bottom

[desktop] Add Gitpod to Settings sync #139

Closed
wants to merge 43 commits into from
Closed

[desktop] Add Gitpod to Settings sync #139

wants to merge 43 commits into from

Conversation

filiptronicek
Copy link
Member

@filiptronicek filiptronicek commented Oct 5, 2021 •
edited
Loading

Add Gitpod as an authentication and Settings Sync provider via the gitpod.gitpod-desktop extension.

  • invalidate bad token(s) on startup (27f4c9c)
  • fix the openExternal issue
  • implement code exchange
  • Fix 401 WS Error
  • contribute Gitpod endpoint config to the manifest

Blocked by gitpod-io/gitpod#6272

@filiptronicek filiptronicek added the enhancement New feature or request label Oct 5, 2021
@filiptronicek filiptronicek self-assigned this Oct 5, 2021
@akosyakov akosyakov mentioned this pull request Oct 6, 2021
Closed
8 tasks
@akosyakov
Copy link
Member

@filiptronicek Could you squash in one commit and rebase on gp-code/main? I tried to but did not figure out what is the first commit and which changes should be applied during rebase.

akosyakov
akosyakov reviewed Oct 6, 2021
View reviewed changes
Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

It looks promising, but the code structure needs more love. Also I could not find how we invalidate existing tokens. It would be nice to align in these regards with GH auth extension.

extensions/gitpod/src/registerAuth.ts Outdated Show resolved Hide resolved
extensions/gitpod/src/registerAuth.ts Outdated Show resolved Hide resolved
extensions/gitpod/src/registerAuth.ts Outdated Show resolved Hide resolved
extensions/gitpod/src/registerAuth.ts Outdated Show resolved Hide resolved
extensions/gitpod/src/registerAuth.ts Outdated Show resolved Hide resolved
extensions/gitpod/src/registerAuth.ts Outdated Show resolved Hide resolved
extensions/gitpod/src/registerAuth.ts Outdated Show resolved Hide resolved
extensions/gitpod/src/registerAuth.ts Outdated Show resolved Hide resolved
extensions/gitpod/src/registerAuth.ts Outdated Show resolved Hide resolved
extensions/gitpod/src/registerAuth.ts Outdated Show resolved Hide resolved
akosyakov
akosyakov reviewed Oct 12, 2021
View reviewed changes
Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

oh, forgot to publish comments several days ago (

extensions/gitpod/src/auth.ts Outdated Show resolved Hide resolved
extensions/gitpod/src/auth.ts Outdated Show resolved Hide resolved
extensions/gitpod/src/auth.ts Outdated Show resolved Hide resolved
extensions/gitpod/src/auth.ts Outdated Show resolved Hide resolved
extensions/gitpod/src/auth.ts
if (!enabled) {
try {
await config.update('configurationSync.store', undefined, true);
updateSyncContext();
Copy link
Member

Choose a reason for hiding this comment

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

we should not do it here, but listern to configuration changes and react it.

setSettingsSync should only update config nothing else, side effects should be done via listeners

extensions/gitpod/src/auth.ts Outdated Show resolved Hide resolved
extensions/gitpod/src/auth.ts Outdated

// Wait until a session is added to the context's secret store
const authPromise = promiseFromEvent(context.secrets.onDidChange, (changeEvent: vscode.SecretStorageChangeEvent, resolve, reject): void => {
if (changeEvent.key !== 'gitpod.authSessions') {
Copy link
Member

Choose a reason for hiding this comment

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

please introduce a class GitpodAuthSession which provides methods to set/get/delete and event to indicate when it is changed. gitpod.authSessions should be a constant within such class and not used by the rest of code

extensions/gitpod/src/auth.ts Show resolved Hide resolved
bigint
bigint suggested changes Oct 13, 2021
View reviewed changes
Copy link

@bigint bigint left a comment

Choose a reason for hiding this comment

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

Some lil typo fixes 😄

extensions/gitpod/src/auth.ts Outdated Show resolved Hide resolved
extensions/gitpod/src/auth.ts Outdated Show resolved Hide resolved
extensions/gitpod/src/auth.ts Outdated Show resolved Hide resolved
extensions/gitpod/src/auth.ts Outdated Show resolved Hide resolved
extensions/gitpod/src/auth.ts Outdated Show resolved Hide resolved
extensions/gitpod/src/auth.ts Outdated Show resolved Hide resolved
extensions/gitpod/src/auth.ts Outdated Show resolved Hide resolved
@akosyakov akosyakov force-pushed the gp-auth-via-gp-ext branch 2 times, most recently from fece1dd to a0ddbc9 Compare October 21, 2021 13:34
@jeanp413 jeanp413 force-pushed the gp-code/main branch 2 times, most recently from d6a1f58 to 0800cc4 Compare October 29, 2021 15:32
@jeanp413 jeanp413 force-pushed the gp-code/main branch 2 times, most recently from c23521a to 7a7d2cc Compare November 5, 2021 18:50
geropl and others added 8 commits March 30, 2022 19:33
@geropl @jeanp413
[gitpod] Add 'User-Agent' to API connection headers
b1acac6
@jeanp413
Always enable proposed API
9de989f
@mustard-mh @jeanp413
support timeout of local-app
d677a73
@jeanp413
Add notification telemetry
0cc83b1
@jeanp413
Add telemetry for user config and extensions
ef19a61
@jeanp413
Add telemetry for getting started
a728832
@Pothulapati @jeanp413
gitpod: bump node-fetch version in extensions
4808fba 
Related to gitpod-io/gitpod#7360

Currently, When the server has a root cert cross-signed by the
expired `DST ROOT CA X3`. There's a possiblity of the client having
an older version of `electron` in their code that causes the cert
expiry failure.

While, asking users to update their vs code should be our first
suggestion, In some cases using the latest version of `node-fetch`
also fixed the issue (not exactly sure why). This PR bumps the
`node-fetch` dependency to the one that the `code-server` uses.

Signed-off-by: Tarun Pothulapati <tarun@gitpod.io>
@mustard-mh @jeanp413
Remove vscode_session phase running end
3125d4c
jeanp413 and others added 13 commits April 4, 2022 09:26
@jeanp413
Add export logs contribution
76e0439
@filiptronicek @jeanp413
Add Gitpod to Settings sync
754ebb2 
refactor `createOauth2URL`

Add lock changes

Rewrite `getSessions` and `removeSession` functions

Don't bore the user with expired error

Docs

Add todo

Add get and set functions for stored auth sessions

Change oauth2 creation function and update clientID to be up to date

Have popup preference stored in the global context

Handle ws errors

Max WS retries
@filiptronicek @jeanp413
Remove strict null checking
7597be9 
Co-authored-by: bigint <b1gint@protonmail.com>
@filiptronicek @jeanp413
Grammar
f8ee95d 
Co-authored-by: bigint <b1gint@protonmail.com>
@filiptronicek @jeanp413
Typos, grammar, all that kind of awesome stuff!
5fd96f8 
Co-authored-by: bigint <b1gint@protonmail.com>
@filiptronicek @jeanp413
Don't cache base URL
99aff73
@filiptronicek @jeanp413
Rewrite session logic into a class
5370b06
@filiptronicek @jeanp413
Add code exchange code
409b82a
@filiptronicek @jeanp413
Fix that damn bug
1b9043b
@filiptronicek @jeanp413
Fix code exchange bad request
1d88940
@filiptronicek @jeanp413
Fix WS endpoint URL and some other things
94cc6fc
@akosyakov @jeanp413
fix JWT token parsing
914c571
@akosyakov @jeanp413
invalidate sessions
17814a3
@filiptronicek filiptronicek mentioned this pull request Apr 5, 2022
Closed
@jeanp413 jeanp413 closed this Apr 13, 2022
@jeanp413
Copy link
Member

continuation in #337

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akosyakov akosyakov akosyakov left review comments

@bigint bigint bigint requested changes

Assignees

@filiptronicek filiptronicek

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.