Skip to content

[server] Introduce RequestContext #19023

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

Merged
merged 3 commits into from
Nov 16, 2023
Merged

[server] Introduce RequestContext #19023

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
84e459e
[server] Introduce RequestContext
geropl Nov 13, 2023
f3a5930
[server] Improve (repeated) Docker build times
geropl Nov 16, 2023
e8e0eee
fix AuthProviderService.createAuthProvider
geropl Nov 16, 2023
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
4 changes: 1 addition & 3 deletions components/gitpod-protocol/src/analytics.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@

export const IAnalyticsWriter = Symbol("IAnalyticsWriter");

type Identity =
| { userId: string | number; anonymousId?: string | number }
| { userId?: string | number; anonymousId: string | number };
type Identity = { userId?: string | number; anonymousId?: string | number; subjectId?: string };

interface Message {
messageId?: string;
Expand Down
3 changes: 3 additions & 0 deletions components/gitpod-protocol/src/messaging/error.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ export const ErrorCodes = {
// 404 Not Found
NOT_FOUND: 404 as const,

// 408 Request Timeout
REQUEST_TIMEOUT: 408 as const,

// 409 Conflict (e.g. already existing)
CONFLICT: 409 as const,

Expand Down
3 changes: 3 additions & 0 deletions components/gitpod-protocol/src/public-api-converter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,9 @@ export class PublicAPIConverter {
if (reason.code === ErrorCodes.INTERNAL_SERVER_ERROR) {
return new ConnectError(reason.message, Code.Internal, metadata, undefined, reason);
}
if (reason.code === ErrorCodes.REQUEST_TIMEOUT) {
return new ConnectError(reason.message, Code.Canceled, metadata, undefined, reason);
}
return new ConnectError(reason.message, Code.InvalidArgument, metadata, undefined, reason);
}
return ConnectError.from(reason, Code.Internal);
Expand Down
1 change: 1 addition & 0 deletions components/gitpod-protocol/src/util/logging.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export interface LogContext {
organizationId?: string;
sessionId?: string;
userId?: string;
subjectId?: string;
workspaceId?: string;
instanceId?: string;
}
Expand Down
3 changes: 2 additions & 1 deletion components/server/leeway.Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@
# See License.AGPL.txt in the project root for license information.

FROM node:18.17.1-slim as builder
COPY components-server--app /installer/

# Install Python, make, gcc and g++ for node-gyp
RUN apt-get update && \
apt-get install -y python3 make gcc g++ && \
apt-get clean && \
rm -rf /var/lib/apt/lists/*

COPY components-server--app /installer/

WORKDIR /app
RUN /installer/install.sh

Expand Down
35 changes: 34 additions & 1 deletion components/server/src/analytics.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@
*/
import { User } from "@gitpod/gitpod-protocol";
import { Request } from "express";
import { IAnalyticsWriter } from "@gitpod/gitpod-protocol/lib/analytics";
import { IAnalyticsWriter, IdentifyMessage, PageMessage, TrackMessage } from "@gitpod/gitpod-protocol/lib/analytics";
import * as crypto from "crypto";
import { clientIp } from "./express-util";
import { ctxTrySubjectId } from "./util/request-context";

export async function trackLogin(user: User, request: Request, authHost: string, analytics: IAnalyticsWriter) {
// make new complete identify call for each login
Expand Down Expand Up @@ -129,3 +130,35 @@ function stripCookie(cookie: string) {
return cookie;
}
}

export class ContextAwareAnalyticsWriter implements IAnalyticsWriter {
constructor(readonly writer: IAnalyticsWriter) {}

identify(msg: IdentifyMessage): void {
this.writer.identify(msg);
}

track(msg: TrackMessage): void {
this.writer.track(msg);
}

page(msg: PageMessage): void {
const traceIds = this.getAnalyticsIds();
this.writer.page({
...msg,
userId: msg.userId || traceIds.userId,
subjectId: msg.subjectId || traceIds.subjectId,
});
}

private getAnalyticsIds(): { userId?: string; subjectId?: string } {
const subjectId = ctxTrySubjectId();
if (!subjectId) {
return {};
}
return {
userId: subjectId.userId(),
subjectId: subjectId.toString(),
};
}
}
63 changes: 33 additions & 30 deletions components/server/src/api/auth-provider-service-api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,44 +23,46 @@ import {
DeleteAuthProviderResponse,
} from "@gitpod/public-api/lib/gitpod/v1/authprovider_pb";
import { AuthProviderService } from "../auth/auth-provider-service";
import { AuthProviderEntry, AuthProviderInfo } from "@gitpod/gitpod-protocol";
import { AuthProviderEntry, AuthProviderInfo, User } from "@gitpod/gitpod-protocol";
import { Unauthenticated } from "./unauthenticated";
import { validate as uuidValidate } from "uuid";
import { selectPage } from "./pagination";
import { ctxUserId } from "../util/request-context";
import { UserService } from "../user/user-service";

@injectable()
export class AuthProviderServiceAPI implements ServiceImpl<typeof AuthProviderServiceInterface> {
constructor(
@inject(PublicAPIConverter) private readonly apiConverter: PublicAPIConverter,
@inject(AuthProviderService) private readonly authProviderService: AuthProviderService,
@inject(UserService) private readonly userService: UserService,
) {}

async createAuthProvider(
request: CreateAuthProviderRequest,
context: HandlerContext,
_: HandlerContext,
): Promise<CreateAuthProviderResponse> {
const ownerId = request.owner.case === "ownerId" ? request.owner.value : "";
const organizationId = request.owner.case === "organizationId" ? request.owner.value : "";

if (!uuidValidate(organizationId) && !uuidValidate(ownerId)) {
throw new ConnectError("organizationId or ownerId is required", Code.InvalidArgument);
}

if (organizationId) {
const result = await this.authProviderService.createOrgAuthProvider(context.user.id, {
if (!uuidValidate(organizationId)) {
throw new ConnectError("organizationId is required", Code.InvalidArgument);
}

const result = await this.authProviderService.createOrgAuthProvider(ctxUserId(), {
organizationId,
host: request.host,
ownerId: context.user.id,
ownerId: ctxUserId(),
type: this.apiConverter.fromAuthProviderType(request.type),
clientId: request.oauth2Config?.clientId,
clientSecret: request.oauth2Config?.clientSecret,
});

return new CreateAuthProviderResponse({ authProvider: this.apiConverter.toAuthProvider(result) });
} else {
const result = await this.authProviderService.createAuthProviderOfUser(context.user.id, {
const result = await this.authProviderService.createAuthProviderOfUser(ctxUserId(), {
host: request.host,
ownerId: context.user.id,
ownerId: ctxUserId(),
type: this.apiConverter.fromAuthProviderType(request.type),
clientId: request.oauth2Config?.clientId,
clientSecret: request.oauth2Config?.clientSecret,
Expand All @@ -69,12 +71,12 @@ export class AuthProviderServiceAPI implements ServiceImpl<typeof AuthProviderSe
return new CreateAuthProviderResponse({ authProvider: this.apiConverter.toAuthProvider(result) });
}
}
async getAuthProvider(request: GetAuthProviderRequest, context: HandlerContext): Promise<GetAuthProviderResponse> {
async getAuthProvider(request: GetAuthProviderRequest, _: HandlerContext): Promise<GetAuthProviderResponse> {
if (!request.authProviderId) {
throw new ConnectError("authProviderId is required", Code.InvalidArgument);
}

const authProvider = await this.authProviderService.getAuthProvider(context.user.id, request.authProviderId);
const authProvider = await this.authProviderService.getAuthProvider(ctxUserId(), request.authProviderId);
if (!authProvider) {
throw new ConnectError("Provider not found.", Code.NotFound);
}
Expand All @@ -84,10 +86,7 @@ export class AuthProviderServiceAPI implements ServiceImpl<typeof AuthProviderSe
});
}

async listAuthProviders(
request: ListAuthProvidersRequest,
context: HandlerContext,
): Promise<ListAuthProvidersResponse> {
async listAuthProviders(request: ListAuthProvidersRequest, _: HandlerContext): Promise<ListAuthProvidersResponse> {
const target = request.id;
const ownerId = target.case === "userId" ? target.value : "";
const organizationId = target.case === "organizationId" ? target.value : "";
Expand All @@ -97,8 +96,8 @@ export class AuthProviderServiceAPI implements ServiceImpl<typeof AuthProviderSe
}

const authProviders = organizationId
? await this.authProviderService.getAuthProvidersOfOrg(context.user.id, organizationId)
: await this.authProviderService.getAuthProvidersOfUser(context.user.id);
? await this.authProviderService.getAuthProvidersOfOrg(ctxUserId(), organizationId)
: await this.authProviderService.getAuthProvidersOfUser(ctxUserId());

const selectedProviders = selectPage(authProviders, request.pagination);
const redacted = selectedProviders.map(AuthProviderEntry.redact.bind(AuthProviderEntry));
Expand All @@ -118,9 +117,13 @@ export class AuthProviderServiceAPI implements ServiceImpl<typeof AuthProviderSe
@Unauthenticated()
async listAuthProviderDescriptions(
request: ListAuthProviderDescriptionsRequest,
context: HandlerContext,
_: HandlerContext,
): Promise<ListAuthProviderDescriptionsResponse> {
const user = context.user;
const userId = ctxUserId();
Copy link
Member

Choose a reason for hiding this comment

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

@geropl, just realized, this actually throws if unauthenticated.

Need to address it, as it breaks Login page with gRPC enabled.

Copy link
Member

Choose a reason for hiding this comment

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

#19094

Copy link
Member Author

Choose a reason for hiding this comment

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

@AlexTugarev Yes, this one throws. If we need one that does not throw, we should name it ctxTryUserId, following the pattern in request-context.ts 👍

let user: User | undefined = undefined;
if (userId) {
user = await this.userService.findUserById(userId, userId);
}
const aps = user
? await this.authProviderService.getAuthProviderDescriptions(user)
: await this.authProviderService.getAuthProviderDescriptionsUnauthenticated();
Expand All @@ -135,7 +138,7 @@ export class AuthProviderServiceAPI implements ServiceImpl<typeof AuthProviderSe

async updateAuthProvider(
request: UpdateAuthProviderRequest,
context: HandlerContext,
_: HandlerContext,
): Promise<UpdateAuthProviderResponse> {
if (!request.authProviderId) {
throw new ConnectError("authProviderId is required", Code.InvalidArgument);
Expand All @@ -146,23 +149,23 @@ export class AuthProviderServiceAPI implements ServiceImpl<typeof AuthProviderSe
throw new ConnectError("clientId or clientSecret are required", Code.InvalidArgument);
}

const authProvider = await this.authProviderService.getAuthProvider(context.user.id, request.authProviderId);
const authProvider = await this.authProviderService.getAuthProvider(ctxUserId(), request.authProviderId);
if (!authProvider) {
throw new ConnectError("Provider not found.", Code.NotFound);
}

let entry: AuthProviderEntry;
if (authProvider.organizationId) {
entry = await this.authProviderService.updateOrgAuthProvider(context.user.id, {
entry = await this.authProviderService.updateOrgAuthProvider(ctxUserId(), {
id: request.authProviderId,
organizationId: authProvider.organizationId,
clientId: clientId,
clientSecret: clientSecret,
});
} else {
entry = await this.authProviderService.updateAuthProviderOfUser(context.user.id, {
entry = await this.authProviderService.updateAuthProviderOfUser(ctxUserId(), {
id: request.authProviderId,
ownerId: context.user.id,
ownerId: ctxUserId(),
clientId: clientId,
clientSecret: clientSecret,
});
Expand All @@ -175,25 +178,25 @@ export class AuthProviderServiceAPI implements ServiceImpl<typeof AuthProviderSe

async deleteAuthProvider(
request: DeleteAuthProviderRequest,
context: HandlerContext,
_: HandlerContext,
): Promise<DeleteAuthProviderResponse> {
if (!request.authProviderId) {
throw new ConnectError("authProviderId is required", Code.InvalidArgument);
}

const authProvider = await this.authProviderService.getAuthProvider(context.user.id, request.authProviderId);
const authProvider = await this.authProviderService.getAuthProvider(ctxUserId(), request.authProviderId);
if (!authProvider) {
throw new ConnectError("Provider not found.", Code.NotFound);
}

if (authProvider.organizationId) {
await this.authProviderService.deleteAuthProviderOfOrg(
context.user.id,
ctxUserId(),
authProvider.organizationId,
request.authProviderId,
);
} else {
await this.authProviderService.deleteAuthProviderOfUser(context.user.id, request.authProviderId);
await this.authProviderService.deleteAuthProviderOfUser(ctxUserId(), request.authProviderId);
}

return new DeleteAuthProviderResponse();
Expand Down
25 changes: 17 additions & 8 deletions components/server/src/api/configuration-service-api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import {
} from "@gitpod/public-api/lib/gitpod/v1/configuration_pb";
import { PaginationResponse } from "@gitpod/public-api/lib/gitpod/v1/pagination_pb";
import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
import { ctxUserId } from "../util/request-context";
import { UserService } from "../user/user-service";

@injectable()
export class ConfigurationServiceAPI implements ServiceImpl<typeof ConfigurationServiceInterface> {
Expand All @@ -28,11 +30,13 @@ export class ConfigurationServiceAPI implements ServiceImpl<typeof Configuration
private readonly projectService: ProjectsService,
@inject(PublicAPIConverter)
private readonly apiConverter: PublicAPIConverter,
@inject(UserService)
private readonly userService: UserService,
) {}

async createConfiguration(
req: CreateConfigurationRequest,
context: HandlerContext,
_: HandlerContext,
): Promise<CreateConfigurationResponse> {
if (!req.organizationId) {
throw new ApplicationError(ErrorCodes.BAD_REQUEST, "organization_id is required");
Expand All @@ -41,6 +45,11 @@ export class ConfigurationServiceAPI implements ServiceImpl<typeof Configuration
throw new ApplicationError(ErrorCodes.BAD_REQUEST, "clone_url is required");
}

const installer = await this.userService.findUserById(ctxUserId(), ctxUserId());
if (!installer) {
throw new ApplicationError(ErrorCodes.NOT_FOUND, "user not found");
}

const project = await this.projectService.createProject(
{
teamId: req.organizationId,
Expand All @@ -49,27 +58,27 @@ export class ConfigurationServiceAPI implements ServiceImpl<typeof Configuration
appInstallationId: "",
slug: "",
},
context.user,
installer,
);

return new CreateConfigurationResponse({
configuration: this.apiConverter.toConfiguration(project),
});
}

async getConfiguration(req: GetConfigurationRequest, context: HandlerContext) {
async getConfiguration(req: GetConfigurationRequest, _: HandlerContext) {
if (!req.configurationId) {
throw new ApplicationError(ErrorCodes.BAD_REQUEST, "configuration_id is required");
}

const project = await this.projectService.getProject(context.user.id, req.configurationId);
const project = await this.projectService.getProject(ctxUserId(), req.configurationId);

return {
configuration: this.apiConverter.toConfiguration(project),
};
}

async listConfigurations(req: ListConfigurationsRequest, context: HandlerContext) {
async listConfigurations(req: ListConfigurationsRequest, _: HandlerContext) {
if (!req.organizationId) {
throw new ApplicationError(ErrorCodes.BAD_REQUEST, "organization_id is required");
}
Expand All @@ -78,7 +87,7 @@ export class ConfigurationServiceAPI implements ServiceImpl<typeof Configuration
const currentPage = req.pagination?.page ?? 1;
const offset = currentPage > 1 ? (currentPage - 1) * limit : 0;

const { rows, total } = await this.projectService.findProjects(context.user.id, {
const { rows, total } = await this.projectService.findProjects(ctxUserId(), {
organizationId: req.organizationId,
searchTerm: req.searchTerm,
orderBy: "name",
Expand All @@ -96,12 +105,12 @@ export class ConfigurationServiceAPI implements ServiceImpl<typeof Configuration
});
}

async deleteConfiguration(req: DeleteConfigurationRequest, handler: HandlerContext) {
async deleteConfiguration(req: DeleteConfigurationRequest, _: HandlerContext) {
if (!req.configurationId) {
throw new ApplicationError(ErrorCodes.BAD_REQUEST, "configuration_id is required");
}

await this.projectService.deleteProject(handler.user.id, req.configurationId);
await this.projectService.deleteProject(ctxUserId(), req.configurationId);

return new DeleteConfigurationResponse();
}
Expand Down
13 changes: 0 additions & 13 deletions components/server/src/api/handler-context-augmentation.d.ts
View file
Open in desktop

This file was deleted.

Loading