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

Use AuthProviderService API in Dashboard – EXP-847 #19057

Merged
merged 18 commits into from
Nov 16, 2023
Merged

Use AuthProviderService API in Dashboard – EXP-847 #19057

merged 18 commits into from
Nov 16, 2023

Conversation

AlexTugarev
Copy link
Member

@AlexTugarev AlexTugarev commented Nov 10, 2023 •
edited
Loading

Description

This PR make use of the gRPC based AuthProviderService in Dashboard. Most prominent components affected are: Login, Git Integrations for Orgs as well as in user space.

Summary generated by Copilot

🤖 Generated by Copilot at 0cdc898

This pull request updates the dashboard to use the public API for fetching and displaying auth provider descriptions. It also refactors some code to use the AuthProviderType enum and the new AuthProviderDescription data type.

Related Issue(s)

Fixes EXP-847

How to test

Documentation

Preview status

Gitpod was successfully deployed to your preview environment.

Build Options

Build
  • /werft with-werft
    Run the build with werft instead of GHA
  • leeway-no-cache
  • /werft no-test
    Run Leeway with --dont-test
Publish
  • /werft publish-to-npm
  • /werft publish-to-jb-marketplace
Installer
  • analytics=segment
  • with-dedicated-emulation
  • workspace-feature-flags
    Add desired feature flags to the end of the line above, space separated
Preview Environment / Integration Tests
  • /werft with-local-preview
    If enabled this will build install/preview
  • /werft with-preview
  • /werft with-large-vm
  • /werft with-gce-vm
    If enabled this will create the environment on GCE infra
  • with-integration-tests=all
    Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh. If enabled, with-preview and with-large-vm will be enabled.
  • with-monitoring

/hold

@AlexTugarev AlexTugarev force-pushed the at/authprovider-api-dashboard branch from 0cdc898 to b91164a Compare November 10, 2023 12:31
@AlexTugarev
Copy link
Member Author

Screenshot 2023-11-10 at 14 04 58

First part, the Login page was easy :-)

@roboquat roboquat added size/L and removed size/M labels Nov 10, 2023
@AlexTugarev AlexTugarev force-pushed the at/authprovider-api-dashboard branch from fc3be82 to a8f060e Compare November 13, 2023 10:05
Base automatically changed from at/authprovider_api to main November 13, 2023 13:38
@roboquat roboquat added size/XXL and removed size/XL labels Nov 13, 2023
@AlexTugarev AlexTugarev force-pushed the at/authprovider-api-dashboard branch from cad078c to fe333d2 Compare November 13, 2023 13:44
@roboquat roboquat added size/XL and removed size/XXL labels Nov 13, 2023
@AlexTugarev AlexTugarev force-pushed the at/authprovider-api-dashboard branch from fe333d2 to b1e7bbb Compare November 13, 2023 14:51
AlexTugarev
AlexTugarev commented Nov 13, 2023
View reviewed changes
components/gitpod-protocol/src/auth-providers.ts
};
}

export function getScopesForAuthProviderType(type: AuthProviderType | string) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is all static information which can be derived from type. Pulling this into gitpod-protocol enables to make use of it equally in dashboard and in server.

@roboquat roboquat added size/XXL and removed size/XL labels Nov 13, 2023
@AlexTugarev AlexTugarev marked this pull request as ready for review November 14, 2023 09:41
@AlexTugarev AlexTugarev requested a review from a team as a code owner November 14, 2023 09:41
@AlexTugarev
Copy link
Member Author

AlexTugarev commented Nov 14, 2023 •
edited
Loading

Screenshot 2023-11-14 at 10 59 47
  • Label isn't mapped.

@mustard-mh mustard-mh self-requested a review November 14, 2023 10:35
@AlexTugarev AlexTugarev changed the title Use AuthProviderService API in Dashboard Use AuthProviderService API in Dashboard – EXP-847 Nov 14, 2023
mustard-mh
mustard-mh reviewed Nov 14, 2023
View reviewed changes
Copy link
Contributor

@mustard-mh mustard-mh left a comment

Choose a reason for hiding this comment

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

We also need to upgrade setup.tsx CACHE_KEY

components/dashboard/src/workspaces/CreateWorkspacePage.tsx
? "repo"
: authProvider.type === AuthProviderType.GITLAB
? "api"
: "";
Copy link
Contributor

Choose a reason for hiding this comment

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

the logic changed here, could you give more context about it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch!
I tried to fix the previous state, but didn't finish it apparently.

Old logic reads like this: If type is GitHub use GH scope, otherwise use GitLab scope. This is obviously wrong for BB and BBS.

The update reads like: this: if type is GitHub use GH scope, otherwise if type is GitLab use GL scope, otherwise use whatever is default for the provider type, which is known on server side.

I think that state is already a small fix. It would be great to verify though.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that state is already a small fix. It would be great to verify though.

Do we have any account (like in 1Password) that could test those providers?

Copy link
Member Author

Choose a reason for hiding this comment

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

BBS account in 1P, otherwise we can patch the deployment's secret in K8s to remove GitLab/Bitbucket from the default list. This gives us the change to create them in-App.

components/dashboard/src/data/auth-providers/update-org-auth-provider-mutation.ts Show resolved Hide resolved
@AlexTugarev
Copy link
Member Author

Need to investigate the refresh page issue from #19057 (comment)

@AlexTugarev AlexTugarev force-pushed the at/authprovider-api-dashboard branch from 979140a to 133ccc2 Compare November 16, 2023 14:05
@AlexTugarev
Copy link
Member Author

#19057 (comment) was caused by AuthProviderClasses being missing in supported messages. It works fine now.

@AlexTugarev
Copy link
Member Author

#19057 (comment) is fixed as well. This was a bug in the shim.

mustard-mh
mustard-mh approved these changes Nov 16, 2023
View reviewed changes
Copy link
Contributor

@mustard-mh mustard-mh left a comment

Choose a reason for hiding this comment

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

Reported bugs fixed 🎉

@mustard-mh
Copy link
Contributor

mustard-mh commented Nov 16, 2023 •
edited
Loading

Did you changed CACHE_VERSION? Yes

@AlexTugarev
Copy link
Member Author

/unhold

@roboquat roboquat merged commit 21bb3c1 into main Nov 16, 2023
15 of 16 checks passed
@roboquat roboquat deleted the at/authprovider-api-dashboard branch November 16, 2023 15:53
@AlexTugarev AlexTugarev mentioned this pull request Nov 16, 2023
Merged
14 tasks
@Siddhant-K-code Siddhant-K-code mentioned this pull request Jan 16, 2024
Merged
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mustard-mh mustard-mh mustard-mh approved these changes

Assignees
No one assigned
Labels
size/XXL team: team-experience
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AlexTugarev @mustard-mh @roboquat