Skip to content

Commit

Permalink
Revert "[server] support separate commit email"
Browse files Browse the repository at this point in the history
This reverts commit e59e14d.
  • Loading branch information
corneliusludmann committed Jul 13, 2021
1 parent 2649821 commit 92c30ad
Show file tree
Hide file tree
Showing 13 changed files with 42 additions and 78 deletions.
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.

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

0 comments on commit 92c30ad

Please sign in to comment.