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

refactor(server): Add ApiUrl + ServerUrl env + allow usage of https #8579

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
4 changes: 4 additions & 0 deletions packages/twenty-server/.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,7 @@ ACCESS_TOKEN_SECRET=replace_me_with_a_random_string_access
# PG_SSL_ALLOW_SELF_SIGNED=true
# SESSION_STORE_SECRET=replace_me_with_a_random_string_session
# ENTERPRISE_KEY=replace_me_with_a_valid_enterprise_key
###### --------------> !!! FOR CHARLES AND FELIX !!! we can create a gist in twenty if you want <---------------------------
AMoreaux marked this conversation as resolved.
Show resolved Hide resolved
###### To configure a local certificate you can follow these step https://gist.github.com/AMoreaux/635ca9c38924d42a4d914dabe4376f72
AMoreaux marked this conversation as resolved.
Show resolved Hide resolved
# SSL_KEY_PATH="./certs/your-cert.key"
# SSL_CERT_PATH="./certs/your-cert.crt"
12 changes: 3 additions & 9 deletions packages/twenty-server/src/engine/api/rest/rest-api.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import { AxiosResponse } from 'axios';

import { Query } from 'src/engine/api/rest/core/types/query.type';
import { getServerUrl } from 'src/utils/get-server-url';
Copy link
Member

Choose a reason for hiding this comment

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

add TODO + Deprecate flag

import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';
import { RestApiException } from 'src/engine/api/rest/errors/RestApiException';
import { ApiUrl } from 'src/engine/utils/server-and-api-urls';

export enum GraphqlApiType {
CORE = 'core',
Expand All @@ -16,16 +16,10 @@ export enum GraphqlApiType {

@Injectable()
export class RestApiService {
constructor(
private readonly environmentService: EnvironmentService,
private readonly httpService: HttpService,
) {}
constructor(private readonly httpService: HttpService) {}

async call(graphqlApiType: GraphqlApiType, request: Request, data: Query) {
const baseUrl = getServerUrl(
request,
this.environmentService.get('SERVER_URL'),
);
const baseUrl = getServerUrl(request, ApiUrl.get());
Copy link
Member

Choose a reason for hiding this comment

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

pass API_URL from environmentService ==> actually use the existing SERVER_URL

let response: AxiosResponse;
const url = `${baseUrl}/${
graphqlApiType === GraphqlApiType.CORE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,16 @@ export class EnvironmentVariables {
FRONT_BASE_URL: string;

// Server URL
// URL of the nodejs server
// use an SSL certificate to be compliant with security certifications
@IsUrl({ require_tld: false })
@IsOptional()
SERVER_URL: string;
SERVER_URL = 'http://localhost';

// URL of the API, differ from SERVER_URL if you use a proxy like a load balancer
@IsOptional()
@IsUrl({ require_tld: false })
API_URL: string;
AMoreaux marked this conversation as resolved.
Show resolved Hide resolved

@IsString()
APP_SECRET: string;
Expand Down Expand Up @@ -475,6 +482,15 @@ export class EnvironmentVariables {
// milliseconds
@CastToPositiveNumber()
SERVERLESS_FUNCTION_EXEC_THROTTLE_TTL = 1000;

// SSL
@IsString()
@ValidateIf((env) => env.SERVER_URL.startsWith('https'))
SSL_KEY_PATH: string;

@IsString()
@ValidateIf((env) => env.SERVER_URL.startsWith('https'))
SSL_CERT_PATH: string;
AMoreaux marked this conversation as resolved.
Show resolved Hide resolved
}

export const validate = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { Request } from 'express';
import { OpenAPIV3_1 } from 'openapi-types';

import { AccessTokenService } from 'src/engine/core-modules/auth/token/services/access-token.service';
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';
import { baseSchema } from 'src/engine/core-modules/open-api/utils/base-schema.utils';
import {
computeMetadataSchemaComponents,
Expand Down Expand Up @@ -38,20 +37,17 @@ import { ObjectMetadataService } from 'src/engine/metadata-modules/object-metada
import { capitalize } from 'src/utils/capitalize';
import { getServerUrl } from 'src/utils/get-server-url';
import { DatabaseEventAction } from 'src/engine/api/graphql/graphql-query-runner/enums/database-event-action';
import { ApiUrl } from 'src/engine/utils/server-and-api-urls';

@Injectable()
export class OpenApiService {
constructor(
private readonly accessTokenService: AccessTokenService,
private readonly environmentService: EnvironmentService,
private readonly objectMetadataService: ObjectMetadataService,
) {}

async generateCoreSchema(request: Request): Promise<OpenAPIV3_1.Document> {
const baseUrl = getServerUrl(
request,
this.environmentService.get('SERVER_URL'),
);
const baseUrl = getServerUrl(request, ApiUrl.get());
Copy link
Member

Choose a reason for hiding this comment

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

pass API_URL from environmentService


const schema = baseSchema('core', baseUrl);

Expand Down Expand Up @@ -121,10 +117,7 @@ export class OpenApiService {
async generateMetaDataSchema(
request: Request,
): Promise<OpenAPIV3_1.Document> {
const baseUrl = getServerUrl(
request,
this.environmentService.get('SERVER_URL'),
);
const baseUrl = getServerUrl(request, ApiUrl.get());

const schema = baseSchema('metadata', baseUrl);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@ import { InjectRepository } from '@nestjs/typeorm';
import { Issuer } from 'openid-client';
import { Repository } from 'typeorm';

import { InjectCacheStorage } from 'src/engine/core-modules/cache-storage/decorators/cache-storage.decorator';
import { CacheStorageService } from 'src/engine/core-modules/cache-storage/services/cache-storage.service';
import { CacheStorageNamespace } from 'src/engine/core-modules/cache-storage/types/cache-storage-namespace.enum';
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';
import { FeatureFlagKey } from 'src/engine/core-modules/feature-flag/enums/feature-flag-key.enum';
import { FeatureFlagEntity } from 'src/engine/core-modules/feature-flag/feature-flag.entity';
import { FindAvailableSSOIDPOutput } from 'src/engine/core-modules/sso/dtos/find-available-SSO-IDP.output';
Expand All @@ -28,6 +24,7 @@ import {
WorkspaceSSOIdentityProvider,
} from 'src/engine/core-modules/sso/workspace-sso-identity-provider.entity';
import { User } from 'src/engine/core-modules/user/user.entity';
import { ApiUrl } from 'src/engine/utils/server-and-api-urls';

@Injectable()
// eslint-disable-next-line @nx/workspace-inject-workspace-repository
Expand All @@ -39,9 +36,6 @@ export class SSOService {
private readonly workspaceSSOIdentityProviderRepository: Repository<WorkspaceSSOIdentityProvider>,
@InjectRepository(User, 'core')
private readonly userRepository: Repository<User>,
private readonly environmentService: EnvironmentService,
@InjectCacheStorage(CacheStorageNamespace.EngineWorkspace)
private readonly cacheStorageService: CacheStorageService,
) {}

private async isSSOEnabled(workspaceId: string) {
Expand Down Expand Up @@ -189,7 +183,7 @@ export class SSOService {
buildCallbackUrl(
identityProvider: Pick<WorkspaceSSOIdentityProvider, 'type'>,
) {
const callbackURL = new URL(this.environmentService.get('SERVER_URL'));
const callbackURL = new URL(ApiUrl.get());

callbackURL.pathname = `/auth/${identityProvider.type.toLowerCase()}/callback`;

Expand All @@ -199,7 +193,11 @@ export class SSOService {
buildIssuerURL(
identityProvider: Pick<WorkspaceSSOIdentityProvider, 'id' | 'type'>,
) {
return `${this.environmentService.get('SERVER_URL')}/auth/${identityProvider.type.toLowerCase()}/login/${identityProvider.id}`;
const authorizationUrl = new URL(ApiUrl.get());
Copy link
Member

Choose a reason for hiding this comment

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

pass API_URL from environmentService


authorizationUrl.pathname = `/auth/${identityProvider.type.toLowerCase()}/login/${identityProvider.id}`;

return authorizationUrl.toString();
}

private isOIDCIdentityProvider(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { UserWorkspace } from 'src/engine/core-modules/user-workspace/user-works
import { User } from 'src/engine/core-modules/user/user.entity';
import { WorkspaceInvitationException } from 'src/engine/core-modules/workspace-invitation/workspace-invitation.exception';
import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
import { ApiUrl } from 'src/engine/utils/server-and-api-urls';

import { WorkspaceInvitationService } from './workspace-invitation.service';

Expand Down Expand Up @@ -70,6 +71,7 @@ describe('WorkspaceInvitationService', () => {
environmentService = module.get<EnvironmentService>(EnvironmentService);
emailService = module.get<EmailService>(EmailService);
onboardingService = module.get<OnboardingService>(OnboardingService);
ApiUrl.set('http://localhost:3000');
Copy link
Member

Choose a reason for hiding this comment

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

not need anymore

});

it('should be defined', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
WorkspaceInvitationExceptionCode,
} from 'src/engine/core-modules/workspace-invitation/workspace-invitation.exception';
import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
import { ApiUrl } from 'src/engine/utils/server-and-api-urls';

@Injectable()
// eslint-disable-next-line @nx/workspace-inject-workspace-repository
Expand Down Expand Up @@ -234,7 +235,7 @@ export class WorkspaceInvitationService {
link: link.toString(),
workspace: { name: workspace.displayName, logo: workspace.logo },
sender: { email: sender.email, firstName: sender.firstName },
serverUrl: this.environmentService.get('SERVER_URL'),
serverUrl: ApiUrl.get(),
AMoreaux marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

pass API_URL from environmentService

};

const emailTemplate = SendInviteLinkEmail(emailData);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { ServerUrl, ApiUrl } from 'src/engine/utils/server-and-api-urls';

describe('ServerUrl', () => {
afterEach(() => {
// Reset the serverUrl after each test
ServerUrl.set('');
});

test('should throw error when getting uninitialized ServerUrl', () => {
expect(() => ServerUrl.get()).toThrow(
'ServerUrl is not initialized. Call set() first.',
);
});

test('should set and get ServerUrl correctly', () => {
const url = 'http://localhost:3000';

ServerUrl.set(url);
expect(ServerUrl.get()).toBe(url);
});
});

describe('ApiUrl', () => {
beforeEach(() => {
// Reset the ServerUrl and apiUrl before each test
ServerUrl.set('');
ApiUrl.set('');
});

test('should throw error when getting uninitialized ApiUrl', () => {
expect(() => ApiUrl.get()).toThrow(
'apiUrl is not initialized. Call set() first.',
);
});

test('should throw error when setting ApiUrl without initializing ServerUrl', () => {
expect(() => ApiUrl.set()).toThrow(
'ServerUrl is not initialized. Call set() first.',
);
});

test('should set and get ApiUrl correctly', () => {
const apiUrl = 'http://api.example.com';

ApiUrl.set(apiUrl);
expect(ApiUrl.get()).toBe(apiUrl);
});

test('should set ApiUrl to ServerUrl value if no argument is passed', () => {
const serverUrl = 'http://localhost:3000';

ServerUrl.set(serverUrl);
ApiUrl.set(); // Set without argument, it should use ServerUrl.get()
expect(ApiUrl.get()).toBe(serverUrl);
});
});
37 changes: 37 additions & 0 deletions packages/twenty-server/src/engine/utils/server-and-api-urls.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// The url of the Server, should be exposed in a private network
const ServerUrl = (() => {
Copy link
Member

Choose a reason for hiding this comment

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

remove these classes

let serverUrl = '';

return {
get: () => {
if (serverUrl === '') {
throw new Error('ServerUrl is not initialized. Call set() first.');
}

return serverUrl;
},
set: (url: string) => {
serverUrl = url;
},
AMoreaux marked this conversation as resolved.
Show resolved Hide resolved
};
})();

// The url of the API callable from the public network
const ApiUrl = (() => {
let apiUrl = '';

return {
get: () => {
if (apiUrl === '') {
throw new Error('apiUrl is not initialized. Call set() first.');
}

return apiUrl;
},
set: (url: string = ServerUrl.get()) => {
apiUrl = url;
},
AMoreaux marked this conversation as resolved.
Show resolved Hide resolved
};
})();

export { ServerUrl, ApiUrl };
34 changes: 33 additions & 1 deletion packages/twenty-server/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { ValidationPipe } from '@nestjs/common';
import { NestFactory } from '@nestjs/core';
import { NestExpressApplication } from '@nestjs/platform-express';

import fs from 'fs';

import session from 'express-session';
import bytes from 'bytes';
import { useContainer } from 'class-validator';
Expand All @@ -17,17 +19,37 @@ import './instrument';

import { settings } from './engine/constants/settings';
import { generateFrontConfig } from './utils/generate-front-config';
import { ServerUrl, ApiUrl } from './engine/utils/server-and-api-urls';

const bootstrap = async () => {
const app = await NestFactory.create<NestExpressApplication>(AppModule, {
cors: true,
bufferLogs: process.env.LOGGER_IS_BUFFER_ENABLED === 'true',
rawBody: true,
snapshot: process.env.DEBUG_MODE === 'true',
...(process.env.SERVER_URL &&
process.env.SERVER_URL.startsWith('https') &&
process.env.SSL_KEY_PATH &&
Copy link
Member

Choose a reason for hiding this comment

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

I would only make the check SSL_KEY_PATH and SSL_CERT_PATH

process.env.SSL_CERT_PATH
? {
httpsOptions: {
key: fs.readFileSync(process.env.SSL_KEY_PATH),
cert: fs.readFileSync(process.env.SSL_CERT_PATH),
},
}
: {}),
AMoreaux marked this conversation as resolved.
Show resolved Hide resolved
});
const logger = app.get(LoggerService);
const environmentService = app.get(EnvironmentService);

const serverUrl = new URL(
environmentService.get('SERVER_URL').startsWith('http')
? environmentService.get('SERVER_URL')
: `http://${environmentService.get('SERVER_URL')}`,
Weiko marked this conversation as resolved.
Show resolved Hide resolved
);
AMoreaux marked this conversation as resolved.
Show resolved Hide resolved

serverUrl.port = environmentService.get('PORT').toString();

// TODO: Double check this as it's not working for now, it's going to be heplful for durable trees in twenty "orm"
// // Apply context id strategy for durable trees
// ContextIdFactory.apply(new AggregateByWorkspaceContextIdStrategy());
Expand Down Expand Up @@ -68,7 +90,17 @@ const bootstrap = async () => {
app.use(session(getSessionStorageOptions(environmentService)));
}

await app.listen(process.env.PORT ?? 3000);
await app.listen(serverUrl.port, serverUrl.hostname);
Copy link
Member

Choose a reason for hiding this comment

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

this won't work for self-hosting configuration where both server and front are server by server + this will prevent internal curls


const url = new URL(await app.getUrl());

// prevent ipv6 issue for redirectUri builder
url.hostname = url.hostname === '[::1]' ? 'localhost' : url.hostname;

ServerUrl.set(url.toString());
Copy link
Member

Choose a reason for hiding this comment

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

Make API_URL mandatory (maybe with default in environment.variable or in .env.example)

ApiUrl.set(environmentService.get('API_URL'));
AMoreaux marked this conversation as resolved.
Show resolved Hide resolved

logger.log(`Application is running on: ${url.toString()}`, 'Server Info');
Copy link
Member

Choose a reason for hiding this comment

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

API is running

Copy link
Member

Choose a reason for hiding this comment

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

remove

};

bootstrap();
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ yarn command:prod cron:calendar:calendar-event-list-fetch
['REDIS_URL', 'redis://localhost:6379', 'Redis connection url'],
['FRONT_BASE_URL', 'http://localhost:3001', 'Url to the hosted frontend'],
Copy link
Member

@charlesBochet charlesBochet Nov 22, 2024

Choose a reason for hiding this comment

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

this will be renamed FRONT_DOMAIN + default subDomain + protocol

Copy link
Member

Choose a reason for hiding this comment

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

redirect (workspace.subDomain . FRONT_DOMAIN)

Copy link
Member

Choose a reason for hiding this comment

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

Let's keep: FRONT_BASE_URL and extract defaultSubDomain + protocol + domain from there when we need it

Copy link
Member

Choose a reason for hiding this comment

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

in case we are not in multidomain, we should expect defaultSubDomain to be extratable from FRONT_BASE_URL

['SERVER_URL', 'http://localhost:3000', 'Url to the hosted server'],
['API_URL', 'http://my-load-balancer', 'Url to the public endpoint'],
Copy link
Member

Choose a reason for hiding this comment

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

URL to the API

['PORT', '3000', 'Port'],
['CACHE_STORAGE_TYPE', 'redis', 'Cache type (memory, redis...)'],
['CACHE_STORAGE_TTL', '3600 * 24 * 7', 'Cache TTL in seconds']
Expand Down
Loading