Skip to content

Commit

Permalink
[server] restrict allowed phone numbers
Browse files Browse the repository at this point in the history
  • Loading branch information
svenefftinge committed Sep 23, 2022
1 parent 689b7f8 commit 6b55247
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/**
* Copyright (c) 2022 Gitpod GmbH. All rights reserved.
* Licensed under the GNU Affero General Public License (AGPL).
* See License-AGPL.txt in the project root for license information.
*/

import { MigrationInterface, QueryRunner } from "typeorm";
import { columnExists } from "./helper/helper";

const D_B_USER = "d_b_user";
const COL_PHONE_NUMBER = "verificationPhoneNumber";

export class IndexPhoneNumber1663784254956 implements MigrationInterface {
public async up(queryRunner: QueryRunner): Promise<void> {
if (!(await columnExists(queryRunner, D_B_USER, COL_PHONE_NUMBER))) {
await queryRunner.query(
`ALTER TABLE ${D_B_USER} ADD INDEX (${COL_PHONE_NUMBER}), ALGORITHM=INPLACE, LOCK=NONE `,
);
}
}

public async down(queryRunner: QueryRunner): Promise<void> {}
}
16 changes: 16 additions & 0 deletions components/gitpod-db/src/typeorm/user-db-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,22 @@ export class TypeORMUserDBImpl implements UserDB {
async getByRefreshToken(refreshTokenToken: string): Promise<OAuthToken> {
throw new Error("Not implemented");
}

async countUsagesOfPhoneNumber(phoneNumber: string): Promise<number> {
return (await this.getUserRepo())
.createQueryBuilder()
.where("verificationPhoneNumber = :phoneNumber", { phoneNumber })
.getCount();
}

async isBlockedPhoneNumber(phoneNumber: string): Promise<boolean> {
const blockedUsers = await (await this.getUserRepo())
.createQueryBuilder()
.where("verificationPhoneNumber = :phoneNumber", { phoneNumber })
.andWhere("blocked = true")
.getCount();
return blockedUsers > 0;
}
}

export class TransactionalUserDBImpl extends TypeORMUserDBImpl {
Expand Down
2 changes: 2 additions & 0 deletions components/gitpod-db/src/user-db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ export interface UserDB extends OAuthUserRepository, OAuthTokenRepository {
storeGitpodToken(token: GitpodToken & { user: DBUser }): Promise<void>;
deleteGitpodToken(tokenHash: string): Promise<void>;
deleteGitpodTokensNamedLike(userId: string, namePattern: string): Promise<void>;
countUsagesOfPhoneNumber(phoneNumber: string): Promise<number>;
isBlockedPhoneNumber(phoneNumber: string): Promise<boolean>;
}
export type PartialUserUpdate = Partial<Omit<User, "identities">> & Pick<User, "id">;

Expand Down
11 changes: 10 additions & 1 deletion components/server/src/auth/verification-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@ import { inject, injectable, postConstruct } from "inversify";
import { Config } from "../config";
import { Twilio } from "twilio";
import { ServiceContext } from "twilio/lib/rest/verify/v2/service";
import { WorkspaceDB } from "@gitpod/gitpod-db/lib";
import { UserDB, WorkspaceDB } from "@gitpod/gitpod-db/lib";
import { ConfigCatClientFactory } from "@gitpod/gitpod-protocol/lib/experiments/configcat-server";

@injectable()
export class VerificationService {
@inject(Config) protected config: Config;
@inject(WorkspaceDB) protected workspaceDB: WorkspaceDB;
@inject(UserDB) protected userDB: UserDB;
@inject(ConfigCatClientFactory) protected readonly configCatClientFactory: ConfigCatClientFactory;

protected verifyService: ServiceContext;
Expand Down Expand Up @@ -59,6 +60,14 @@ export class VerificationService {
if (!this.verifyService) {
throw new Error("No verification service configured.");
}
const isBlockedNumber = this.userDB.isBlockedPhoneNumber(phoneNumber);
const usages = await this.userDB.countUsagesOfPhoneNumber(phoneNumber);
if (usages > 3) {
throw new Error("The given phone number has been used more than three times.");
}
if (await isBlockedNumber) {
throw new Error("The given phone number is blocked due to abuse.");
}
const verification = await this.verifyService.verifications.create({ to: phoneNumber, channel: "sms" });
log.info("Verification code sent", { phoneNumber, status: verification.status });
}
Expand Down
27 changes: 27 additions & 0 deletions components/server/src/user/phone-number.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/**
* Copyright (c) 2020 Gitpod GmbH. All rights reserved.
* Licensed under the GNU Affero General Public License (AGPL).
* See License-AGPL.txt in the project root for license information.
*/

import * as chai from "chai";
import { suite, test } from "mocha-typescript";
import { formatPhoneNumber } from "./phone-numbers";
const expect = chai.expect;

@suite
export class PhoneNumberSpec {
@test public testFormatPhoneNumber() {
const tests = [
["00123234254", "+123234254"],
["+1 232 34 254", "+123234254"],
["001 23234-254", "+123234254"],
["0012swedfkwejfew32sdf3sdvsf sdv fsdv4254", "+123234254"],
];

for (const test of tests) {
expect(formatPhoneNumber(test[0]), "Values : " + JSON.stringify(test)).to.eq(test[1]);
}
}
}
module.exports = new PhoneNumberSpec();
23 changes: 23 additions & 0 deletions components/server/src/user/phone-numbers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/**
* Copyright (c) 2022 Gitpod GmbH. All rights reserved.
* Licensed under the GNU Affero General Public License (AGPL).
* See License-AGPL.txt in the project root for license information.
*/

/**
* a simply cleaning method, that removes all non numbers and repaces a leading '00' with a leading '+' sign.
*
* @param phoneNumber
* @returns formatted phone number
*/
export function formatPhoneNumber(phoneNumber: string): string {
var cleanPhoneNumber = phoneNumber.trim();
if (cleanPhoneNumber.startsWith("+")) {
cleanPhoneNumber = "00" + cleanPhoneNumber.substring(1);
}
cleanPhoneNumber = cleanPhoneNumber.replace(/\D/g, "");
if (cleanPhoneNumber.startsWith("00")) {
cleanPhoneNumber = "+" + cleanPhoneNumber.substring(2);
}
return cleanPhoneNumber;
}
8 changes: 5 additions & 3 deletions components/server/src/workspace/gitpod-server-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ import { VerificationService } from "../auth/verification-service";
import { BillingMode } from "@gitpod/gitpod-protocol/lib/billing-mode";
import { EntitlementService } from "../billing/entitlement-service";
import { WorkspaceClasses } from "./workspace-classes";
import { formatPhoneNumber } from "../user/phone-numbers";

// shortcut
export const traceWI = (ctx: TraceContext, wi: Omit<LogContext, "userId">) => TraceContext.setOWI(ctx, wi); // userId is already taken care of in WebsocketConnectionManager
Expand Down Expand Up @@ -469,16 +470,17 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
return user;
}

public async sendPhoneNumberVerificationToken(ctx: TraceContext, phoneNumber: string): Promise<void> {
public async sendPhoneNumberVerificationToken(ctx: TraceContext, rawPhoneNumber: string): Promise<void> {
this.checkUser("sendPhoneNumberVerificationToken");
return this.verificationService.sendVerificationToken(phoneNumber);
return this.verificationService.sendVerificationToken(formatPhoneNumber(rawPhoneNumber));
}

public async verifyPhoneNumberVerificationToken(
ctx: TraceContext,
phoneNumber: string,
rawPhoneNumber: string,
token: string,
): Promise<boolean> {
const phoneNumber = formatPhoneNumber(rawPhoneNumber);
const user = this.checkUser("verifyPhoneNumberVerificationToken");
const checked = await this.verificationService.verifyVerificationToken(phoneNumber, token);
if (!checked) {
Expand Down

0 comments on commit 6b55247

Please sign in to comment.