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

Revert commit that change the DB schema tokens → additionalEmails for now #4797

Merged
merged 3 commits into from
Jul 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ All notable changes to this project will be documented in this file.
## July 2021

- Fix `image.context` in `.gitpod.yml` ([#4715](https://github.com/gitpod-io/gitpod/pull/4715))
- Support custom commit email address for GitHub and GitLab (keep email private). Thanks to [@philschatz](https://github.com/philschatz) for the contribution! ([#4115](https://github.com/gitpod-io/gitpod/pull/4115))

## June 2021

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ const end = new Date(Date.UTC(2000, 2, 1)).toISOString();
authProviderId: 'github.com',
authId: 'Sven',
authName: 'Sven',
tokens: []
}]
});
await this.workspaceDb.store({
Expand Down
28 changes: 12 additions & 16 deletions components/gitpod-db/src/typeorm/entity/db-identity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import { Entity, Column, PrimaryColumn, ManyToOne, Index } from "typeorm";

import { Identity, Email } from "@gitpod/gitpod-protocol";
import { Identity, Token } from "@gitpod/gitpod-protocol";
import { DBUser } from "./db-user";
import { Transformer } from "../transformer";

Expand All @@ -33,24 +33,20 @@ export class DBIdentity implements Identity {
})
primaryEmail?: string;

/** @deprecated */
@Column({
type: 'simple-json',
transformer: Transformer.compose(
{
to(value: any): any {
return value;
},
from(value: any): Email[] {
// We reused the 'tokens' database field. Ignore old values
// in the DB and replace them with an empty array.
return Email.isEmailArray(value) ? value : [];
}
type: "simple-json",
// We want to deprecate the field without changing the schema just yet so we silence all writes and reads
transformer: {
to(value: any): any {
return [];
},
Transformer.SIMPLE_JSON([])
),
nullable: false,
from(value: any): any {
return [];
}
}
})
additionalEmails?: Email[];
tokens: Token[];

@Column()
deleted?: boolean;
Expand Down

This file was deleted.

8 changes: 1 addition & 7 deletions components/gitpod-db/src/typeorm/transformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

import { ValueTransformer } from "typeorm/decorator/options/ValueTransformer";
import { EncryptionService } from "@gitpod/gitpod-protocol/lib/encryption/encryption-service";
import { log } from "@gitpod/gitpod-protocol/lib/util/logging";


export namespace Transformer {
Expand Down Expand Up @@ -46,12 +45,7 @@ export namespace Transformer {
return JSON.stringify(value || defaultValue);
},
from(value: any): any {
try {
return typeof value === 'object' ? value : JSON.parse(value);
} catch (e) {
log.error(`Cannot parse JSON during TypeORM transformation. Returning default value '${JSON.stringify(defaultValue)}' instead. Value: ${JSON.stringify(value)}`, e);
return defaultValue;
}
return JSON.parse(value);
}
};
}
Expand Down
3 changes: 2 additions & 1 deletion components/gitpod-db/src/typeorm/user-db-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,8 @@ export class TypeORMUserDBImpl implements UserDB {
const dbUser = user as DBUser;
// Here we need to fill the pseudo column 'user' in DBIdentity (see there for details)
dbUser.identities.forEach(id => id.user = dbUser);
dbUser.identities.forEach(id => id.additionalEmails = id.additionalEmails || []);
// TODO deprecated: Remove once we delete that column
dbUser.identities.forEach(id => id.tokens = []);
return dbUser;
}

Expand Down
25 changes: 4 additions & 21 deletions components/gitpod-protocol/src/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ export namespace User {
const res = { ...user };
delete (res.additionalData);
res.identities = res.identities.map(i => {
delete (i.tokens);

// The user field is not in the Identity shape, but actually exists on DBIdentity.
// Trying to push this object out via JSON RPC will fail because of the cyclic nature
// of this field.
Expand Down Expand Up @@ -301,7 +303,8 @@ export interface Identity {
authId: string;
authName: string;
primaryEmail?: string;
additionalEmails?: Email[];
/** @deprecated */
tokens?: Token[];
/** This is a flag that triggers the HARD DELETION of this entity */
deleted?: boolean;
// readonly identities cannot be modified by the user
Expand All @@ -322,26 +325,6 @@ export namespace Identity {
}
}

export enum EmailType {
COMMIT = 'commit',
OTHER = 'other'
}

export interface Email {
address: string;
type: EmailType;
}

export namespace Email {
export function is(data: any): data is Email {
return data.hasOwnProperty('address')
&& data.hasOwnProperty('type');
}
export function isEmailArray(data: any): data is Email[] {
return Array.isArray(data) && data.every(x => Email.is(x));
}
}

export interface Token {
value: string;
scopes: string[];
Expand Down
3 changes: 1 addition & 2 deletions components/server/src/auth/auth-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@


import * as express from 'express';
import { AuthProviderInfo, User, OAuth2Config, AuthProviderEntry, Email } from "@gitpod/gitpod-protocol";
import { AuthProviderInfo, User, OAuth2Config, AuthProviderEntry } from "@gitpod/gitpod-protocol";
import { saveSession } from '../express-util';

import { UserEnvVarValue } from "@gitpod/gitpod-protocol";
Expand Down Expand Up @@ -65,7 +65,6 @@ export interface AuthUser {
readonly authId: string;
readonly authName: string;
readonly primaryEmail: string;
readonly additionalEmails?: Email[];
readonly name?: string;
readonly avatarUrl?: string;
}
Expand Down
24 changes: 14 additions & 10 deletions components/server/src/github/github-auth-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import { injectable } from 'inversify';
import * as express from "express"
import { AuthProviderInfo, EmailType } from '@gitpod/gitpod-protocol';
import { AuthProviderInfo } from '@gitpod/gitpod-protocol';
import { log } from '@gitpod/gitpod-protocol/lib/util/logging';
import { GitHubScope } from "./scopes";
import { AuthUserSetup } from "../auth/auth-provider";
Expand Down Expand Up @@ -98,22 +98,26 @@ export class GitHubAuthProvider extends GenericAuthProvider {
.map((s: string) => s.trim())
);

const primary = userEmails.filter(e => e.primary)[0]!;
const proxy = userEmails.find(e => e.email.endsWith(`@users.noreply.${this.config.host}`));

const additionalEmails = userEmails.filter(e => !e.primary && e.verified && e.visibility !== 'private').map(e => ({address: e.email, type: EmailType.OTHER}))
if (primary.visibility === 'private' && proxy) {
additionalEmails.push({ address: proxy.email, type: EmailType.COMMIT });
}
const filterPrimaryEmail = (emails: typeof userEmails) => {
if (this.env.blockNewUsers) {
// if there is any verified email with a domain that is in the blockNewUsersPassList then use this email as primary email
const emailDomainInPasslist = (mail: string) => this.env.blockNewUsersPassList.some(e => mail.endsWith(`@${e}`));
const result = emails.filter(e => e.verified).filter(e => emailDomainInPasslist(e.email))
if (result.length > 0) {
return result[0].email;
}
}
// otherwise use GitHub's primary email as Gitpod's primary email
return emails.filter(e => e.primary)[0].email;
};

return <AuthUserSetup>{
authUser: {
authId: String(id),
authName: login,
avatarUrl: avatar_url,
name,
primaryEmail: primary.email,
additionalEmails,
primaryEmail: filterPrimaryEmail(userEmails)
},
currentScopes
}
Expand Down
1 change: 0 additions & 1 deletion components/server/src/gitlab/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ export namespace GitLab {
// https://docs.gitlab.com/ee/api/users.html#list-current-user-for-normal-users
export interface User extends UserSchemaDefault {
email: string;
commit_email?: string;
state: "active" | string;
confirmed_at: string | undefined,
private_profile: boolean;
Expand Down
7 changes: 3 additions & 4 deletions components/server/src/gitlab/gitlab-auth-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import * as express from "express";
import { injectable } from 'inversify';
import { log } from '@gitpod/gitpod-protocol/lib/util/logging';
import { AuthProviderInfo, EmailType } from '@gitpod/gitpod-protocol';
import { AuthProviderInfo } from '@gitpod/gitpod-protocol';
import { GitLabScope } from "./scopes";
import { UnconfirmedUserException } from "../auth/errors";
import { GitLab } from "./api";
Expand Down Expand Up @@ -73,16 +73,15 @@ export class GitLabAuthProvider extends GenericAuthProvider {
throw UnconfirmedUserException.create(unconfirmedUserMessage, result);
}
}
const { id, username, avatar_url, name, email, commit_email } = result;
const { id, username, avatar_url, name, email } = result;

return <AuthUserSetup>{
authUser: {
authId: String(id),
authName: username,
avatarUrl: avatar_url || undefined,
name,
primaryEmail: email,
additionalEmails: commit_email ? [{ "address": commit_email, "type": EmailType.COMMIT }] : [],
primaryEmail: email
},
currentScopes: this.readScopesFromVerifyParams(tokenResponse)
}
Expand Down
3 changes: 1 addition & 2 deletions components/server/src/user/user-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,7 @@ export class UserService {
protected handleNewUser(newUser: User, isFirstUser: boolean) {
if (this.env.blockNewUsers) {
const emailDomainInPasslist = (mail: string) => this.env.blockNewUsersPassList.some(e => mail.endsWith(`@${e}`));
const canPass = newUser.identities.some(i => (!!i.primaryEmail && emailDomainInPasslist(i.primaryEmail)) ||
i.additionalEmails?.some(e => emailDomainInPasslist(e.address)));
const canPass = newUser.identities.some(i => !!i.primaryEmail && emailDomainInPasslist(i.primaryEmail));

newUser.blocked = !canPass;
}
Expand Down
2 changes: 2 additions & 0 deletions components/server/src/workspace/gitpod-server-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1660,6 +1660,8 @@ export class GitpodServerImpl<Client extends GitpodClient, Server extends Gitpod
const res = { ...user };
delete (res.additionalData);
res.identities = res.identities.map(i => {
delete (i.tokens);

// The user field is not in the Identity shape, but actually exists on DBIdentity.
// Trying to push this object out via JSON RPC will fail because of the cyclic nature
// of this field.
Expand Down
4 changes: 2 additions & 2 deletions components/server/src/workspace/workspace-starter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import { CloneTargetMode, FileDownloadInitializer, GitAuthMethod, GitConfig, GitInitializer, PrebuildInitializer, SnapshotInitializer, WorkspaceInitializer } from "@gitpod/content-service/lib";
import { CompositeInitializer, FromBackupInitializer } from "@gitpod/content-service/lib/initializer_pb";
import { DBUser, DBWithTracing, TracedUserDB, TracedWorkspaceDB, UserDB, WorkspaceDB } from '@gitpod/gitpod-db/lib';
import { CommitContext, Disposable, GitpodToken, GitpodTokenType, IssueContext, NamedWorkspaceFeatureFlag, PullRequestContext, RefType, SnapshotContext, StartWorkspaceResult, User, UserEnvVar, UserEnvVarValue, WithEnvvarsContext, WithPrebuild, Workspace, WorkspaceContext, WorkspaceImageSource, WorkspaceImageSourceDocker, WorkspaceImageSourceReference, WorkspaceInstance, WorkspaceInstanceConfiguration, WorkspaceInstanceStatus, WorkspaceProbeContext, Permission, HeadlessLogEvent, HeadlessWorkspaceEventType, DisposableCollection, AdditionalContentContext, ImageConfigFile, EmailType } from "@gitpod/gitpod-protocol";
import { CommitContext, Disposable, GitpodToken, GitpodTokenType, IssueContext, NamedWorkspaceFeatureFlag, PullRequestContext, RefType, SnapshotContext, StartWorkspaceResult, User, UserEnvVar, UserEnvVarValue, WithEnvvarsContext, WithPrebuild, Workspace, WorkspaceContext, WorkspaceImageSource, WorkspaceImageSourceDocker, WorkspaceImageSourceReference, WorkspaceInstance, WorkspaceInstanceConfiguration, WorkspaceInstanceStatus, WorkspaceProbeContext, Permission, HeadlessLogEvent, HeadlessWorkspaceEventType, DisposableCollection, AdditionalContentContext, ImageConfigFile } from "@gitpod/gitpod-protocol";
import { IAnalyticsWriter } from '@gitpod/gitpod-protocol/lib/analytics';
import { log } from '@gitpod/gitpod-protocol/lib/util/logging';
import { TraceContext } from "@gitpod/gitpod-protocol/lib/util/tracing";
Expand Down Expand Up @@ -728,7 +728,7 @@ export class WorkspaceStarter {

const gitSpec = new GitSpec();
gitSpec.setUsername(user.fullName || identity.authName);
gitSpec.setEmail(identity.additionalEmails?.find(e => e.type === EmailType.COMMIT)?.address || identity.primaryEmail!);
gitSpec.setEmail(identity.primaryEmail!);
return gitSpec;
}

Expand Down