[dashboard] Disallow team names that might conflict with dashboard URLs #5131

jankeromnes · 2021-08-10T08:59:39Z

Fixes #4469

How to test

Try to create a team called:
- "workspaces" (or any other dashboard page)
- "root" (or any other suspicious name)
- the same as your user

gtsiolis · 2021-08-10T09:11:53Z

Looking at this now! 👀

gtsiolis

Thanks for adding this @jankeromnes! 🔝

Works as expected! ✔️

Left some minor comments below.

components/gitpod-db/src/typeorm/team-db-impl.ts

gtsiolis · 2021-08-10T09:21:54Z

components/gitpod-db/src/typeorm/team-db-impl.ts

+        const userRepo = await this.getUserRepo();
+        const existingUsers = await userRepo.query('SELECT COUNT(id) AS count FROM d_b_user WHERE fullName LIKE ? OR name LIKE ?', [ name, slug ]);
+        if (existingUsers[0].count > 0) {
+            throw new Error('A team cannot have the same name as an existing user');


question: Do we want to surface this kind of information? It seems this is privacy sensitive information, right? Maybe using a more generic message could be better? What do you think? 💭

Suggested change

throw new Error('A team cannot have the same name as an existing user');

throw new Error('Team path is already in use');

Hmm, not sure about the privacy aspect. If someone wants to create a team called "Jan Keromnes", and it fails, I'd be okay with the error message saying that a user is already called that name. 😋

Also, user names come directly from GitHub or GitLab, where they're already public information anyway.

I still think this something we could consider changing in the future but it should be fine shipping this as is for now! 🙈

What I'd love in the future is public Gitpod profiles, e.g. showcasing some public Gitpod stuff you'd like to share with the world. 🙂 Could give Gitpod more of a "social" vibe.

This is one of the reasons I'd vote for making this more privacy-aware but should be fine to skip this for now.

gtsiolis · 2021-08-10T09:43:18Z

components/gitpod-db/src/typeorm/team-db-impl.ts

@@ -15,6 +16,34 @@ import { DBTeamMembership } from "./entity/db-team-membership";
 import { DBUser } from "./entity/db-user";
 import { DBTeamMembershipInvite } from "./entity/db-team-membership-invite";

+const FORBIDDEN_SLUGS = [


praise: Thanks for extending the forbidden slugs here! 🌟

How do we ensure this list does not drift w.r.t. the dashboard URLs?

I don't think a perfect match is required at all times. The imported blocklist already contains a lot of teams names we wouldn't want to ever allow, and this complementary list ensures that current and potential near future dashboard paths aren't immediately taken by teams.

While a collision could still potentially happen in the more distant future, it's highly unlikely, and would be a sufficiently small problem that we can deal with it ad-hoc then.

gtsiolis · 2021-08-10T09:46:14Z

components/gitpod-db/src/typeorm/team-db-impl.ts

+    'access-control',
+    'account',
+    'admin',
+    'blocked',
+    'from-referrer',
+    'install-github-app',
+    'integrations',
+    'login',
+    'new',
+    'notifications',
+    'oauth-approval',
+    'plans',
+    'preferences',
+    'projects',
+    'settings',
+    'setup',
+    'sorry',
+    'start',
+    'subscription',
+    'teams',
+    'upgrade-subscription',
+    'usage',
+    'variables',
+    'workspaces',
+    ...(blocklist),


suggestion: What do you think of adding a few more slugs that could be useful in the near future?

For example, we could include snapshots, prebuilds, as well as branches, issues, pull-requests, and merge-requests. Your call! 🏓

gtsiolis · 2021-08-10T09:46:24Z

components/gitpod-db/src/typeorm/team-db-impl.ts

@@ -4,6 +4,7 @@
 * See License-AGPL.txt in the project root for license information.
 */

+import { list as blocklist } from "the-big-username-blacklist";


praise: Nice one! Thanks @corneliusludmann for also suggesting this in #4469 (comment). 🏀

jankeromnes · 2021-08-10T10:22:40Z

Cool, thanks for the review! Will address your excellent suggestions. 🔝

/hold

roboquat · 2021-08-10T12:31:19Z

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

Associated issue: #4469

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jankeromnes · 2021-08-10T13:30:55Z

/werft run

👍 started the job as gitpod-build-jx-disallow-bad-team-names.5

jankeromnes · 2021-08-10T13:52:45Z

I believe all nits are addressed 😁

@gtsiolis please take another look when you have time 👀

/unhold

gtsiolis · 2021-08-10T14:02:17Z

@jankeromnes UX works like a charm. 🔮

Feel free to forward this to someone on the crew to take a deeper look in the code changes if needed before merging. 🏓
We may need an approval from a dashboard owner as described in [dashboard] Disallow team names that might conflict with dashboard URLs #5131 (comment) but not quite sure if this is the case after manual approval. 🤷

jankeromnes · 2021-08-10T18:02:02Z

Many thanks for the reviews! 🙌

Works as expected! ✔️

UX works like a charm. 🔮

Interpreting as /lgtm 😇

gtsiolis · 2021-08-10T18:07:25Z

Thanks @jankeromnes for merging this and @csweichel for taking a look! 👀

jankeromnes requested a review from gtsiolis August 10, 2021 08:59

roboquat requested review from fntlnz and JanKoehnlein August 10, 2021 08:59

roboquat added the size/M label Aug 10, 2021

jankeromnes removed request for JanKoehnlein and fntlnz August 10, 2021 08:59

jankeromnes added the approved label Aug 10, 2021

gtsiolis reviewed Aug 10, 2021

View reviewed changes

gtsiolis added component: dashboard 🧑‍🚀 crew: teams and projects labels Aug 10, 2021

roboquat added the do-not-merge/hold label Aug 10, 2021

[dashboard] Disallow team names that might conflict with dashboard URLs

558f29b

jankeromnes force-pushed the jx/disallow-bad-team-names branch from b8e485d to 558f29b Compare August 10, 2021 12:31

roboquat removed the do-not-merge/hold label Aug 10, 2021

jankeromnes added the lgtm label Aug 10, 2021

roboquat merged commit 9103431 into main Aug 10, 2021

roboquat deleted the jx/disallow-bad-team-names branch August 10, 2021 18:03

gtsiolis mentioned this pull request Aug 31, 2021

[Dashboard] Make www.gitpod.io redirect work for logged out users #5452

Closed

	throw new Error('A team cannot have the same name as an existing user');
	throw new Error('Team path is already in use');

[dashboard] Disallow team names that might conflict with dashboard URLs #5131

[dashboard] Disallow team names that might conflict with dashboard URLs #5131

Uh oh!

Conversation

jankeromnes commented Aug 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to test

Uh oh!

gtsiolis commented Aug 10, 2021 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gtsiolis left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gtsiolis Aug 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jankeromnes commented Aug 10, 2021 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roboquat commented Aug 10, 2021

Uh oh!

jankeromnes commented Aug 10, 2021 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jankeromnes commented Aug 10, 2021 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gtsiolis commented Aug 10, 2021 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jankeromnes commented Aug 10, 2021 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gtsiolis commented Aug 10, 2021 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jankeromnes commented Aug 10, 2021 •

edited

Loading

gtsiolis commented Aug 10, 2021 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis left a comment •

edited

Loading

gtsiolis Aug 10, 2021 •

edited

Loading

jankeromnes commented Aug 10, 2021 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented Aug 10, 2021 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented Aug 10, 2021 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis commented Aug 10, 2021 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented Aug 10, 2021 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis commented Aug 10, 2021 •

edited by werft-gitpod-dev-com bot

Loading