Secure connexion between TinyBird and webhookResponseGraph #7913

anamarn · 2024-10-21T11:15:53Z

TLDR:
Secure connexion between tinybird and twenty using jwt when accessing datasource from tinybird.

Solves:
https://github.com/twentyhq/private-issues/issues/73

In order to test:

Set ANALYTICS_ENABLED to true
Set TINYBIRD_JWT_TOKEN to the ADMIN token from the workspace twenty_analytics_playground
Set TINYBIRD_JWT_TOKEN to the datasource or your admin token from the workspace twenty_analytics_playground
Create a Webhook in twenty and set wich events it needs to track
Run twenty-worker in order to make the webhooks work.
Do your tasks in order to populate the data
Enter to settings> webhook>your webhook and the statistics section should be displayed.

greptile-apps

PR Summary

This pull request introduces JWT support for secure connections between TinyBird and the webhook response graph. Key changes include:

Added analyticsTinybirdJwt field to User type in GraphQL schema
Created new useAnalyticsTinybirdJwt hook for retrieving TinyBird JWT
Modified fetchGraphDataOrThrow to use JWT for TinyBird API authorization
Updated AnalyticsService to generate and use JWT for TinyBird authentication
Added new environment variables for TinyBird JWT configuration
Integrated AnalyticsModule into UserModule and UserResolver

These changes enhance security by replacing hardcoded tokens with dynamic JWTs for TinyBird data access.

_{11 file(s) reviewed, 18 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2024-10-21T11:17:07Z

packages/twenty-front/src/modules/auth/states/currentUserState.ts

@@ -7,6 +7,7 @@ export type CurrentUser = Pick<
  | 'id'
  | 'email'
  | 'supportUserHash'
+  | 'analyticsTinybirdJwt'


style: Consider adding a comment explaining the purpose of 'analyticsTinybirdJwt' for future maintainers

greptile-apps · 2024-10-21T11:17:29Z

packages/twenty-front/src/modules/settings/developers/webhook/hooks/useAnalyticsTinybirdJwt.ts

@@ -0,0 +1,8 @@
+import { useRecoilValue } from 'recoil';


style: consider using more specific imports from recoil to reduce bundle size

greptile-apps · 2024-10-21T11:17:29Z

packages/twenty-front/src/modules/settings/developers/webhook/hooks/useAnalyticsTinybirdJwt.ts

+
+export const useAnalyticsTinybirdJwt = (): string | null | undefined => {
+  const currentUser = useRecoilValue(currentUserState);
+  return currentUser?.analyticsTinybirdJwt;


style: consider adding a null check before accessing analyticsTinybirdJwt

greptile-apps · 2024-10-21T11:17:49Z

packages/twenty-front/src/modules/settings/developers/webhook/hooks/useGraphData.tsx

 import { fetchGraphDataOrThrow } from '@/settings/developers/webhook/utils/fetchGraphDataOrThrow';
 import { SnackBarVariant } from '@/ui/feedback/snack-bar-manager/components/SnackBar';
 import { useSnackBar } from '@/ui/feedback/snack-bar-manager/hooks/useSnackBar';

 export const useGraphData = (webhookId: string) => {
  const { enqueueSnackBar } = useSnackBar();
+  const supportTinybirdJwt = useAnalyticsTinybirdJwt();


style: Consider adding a comment explaining the purpose of supportTinybirdJwt

greptile-apps · 2024-10-21T11:17:50Z

packages/twenty-front/src/modules/settings/developers/webhook/hooks/useGraphData.tsx

  const fetchGraphData = async (
    windowLengthGraphOption: '7D' | '1D' | '12H' | '4H',
  ) => {
    try {
      return await fetchGraphDataOrThrow({
        webhookId,
        windowLength: windowLengthGraphOption,
+        tinybirdJwt: supportTinybirdJwt ?? undefined,


style: Use optional chaining instead of nullish coalescing

greptile-apps · 2024-10-21T11:19:27Z

packages/twenty-server/src/engine/core-modules/analytics/analytics.service.ts

+      ],
+    };
+
+    return this.jwtService.sign(payload);


logic: JWT signing options (like expiration) are not specified. Consider adding these for better security.

greptile-apps · 2024-10-21T11:19:50Z

packages/twenty-server/src/engine/core-modules/environment/environment-variables.ts

+
+  @IsString()
+  @ValidateIf((env) => env.ANALYTICS_ENABLED)
+  TINYBIRD_TOKEN_EXPIRES_IN: string;


style: Consider adding a validator to ensure TINYBIRD_TOKEN_EXPIRES_IN is a valid duration string

greptile-apps · 2024-10-21T11:20:40Z

packages/twenty-server/src/engine/core-modules/user/user.resolver.ts

@@ -55,6 +56,7 @@ export class UserResolver {
    private readonly onboardingService: OnboardingService,
    private readonly userVarService: UserVarsService,
    private readonly fileService: FileService,
+    private readonly analyticsservice: AnalyticsService,


syntax: Typo in service name: 'analyticsservice' should be 'analyticsService'

greptile-apps · 2024-10-21T11:20:41Z

packages/twenty-server/src/engine/core-modules/user/user.resolver.ts

+  async analyticsTinybirdJwt(
+    @AuthWorkspace() workspace: Workspace | undefined,
+  ): Promise<string | null> {
+    return await this.analyticsservice.generateJWT(workspace?.id);
+  }


logic: Consider adding error handling for cases where JWT generation fails

greptile-apps · 2024-10-21T11:20:41Z

packages/twenty-server/src/engine/core-modules/user/user.resolver.ts

+  @ResolveField(() => String, {
+    nullable: true,
+  })
+  async analyticsTinybirdJwt(
+    @AuthWorkspace() workspace: Workspace | undefined,
+  ): Promise<string | null> {
+    return await this.analyticsservice.generateJWT(workspace?.id);
+  }


style: Add documentation for the new analyticsTinybirdJwt field

charlesBochet · 2024-10-21T11:22:06Z

packages/twenty-front/src/modules/settings/developers/webhook/hooks/useAnalyticsTinybirdJwt.ts

+
+import { currentUserState } from '@/auth/states/currentUserState';
+
+export const useAnalyticsTinybirdJwt = (): string | null | undefined => {


can it be null

yes it can be null when ANALYTICS is not enabled in the server

charlesBochet · 2024-10-21T11:22:38Z

packages/twenty-front/src/modules/settings/developers/webhook/hooks/useGraphData.tsx

  const fetchGraphData = async (
    windowLengthGraphOption: '7D' | '1D' | '12H' | '4H',
  ) => {
    try {
      return await fetchGraphDataOrThrow({
        webhookId,
        windowLength: windowLengthGraphOption,
+        tinybirdJwt: supportTinybirdJwt ?? undefined,


?? undefined is not useful, we should throw before if the jwt is undefined

charlesBochet · 2024-10-21T11:22:47Z

packages/twenty-front/src/modules/settings/developers/webhook/utils/fetchGraphDataOrThrow.ts

@@ -4,22 +4,24 @@ import { WEBHOOK_GRAPH_API_OPTIONS_MAP } from '@/settings/developers/webhook/con
 type fetchGraphDataOrThrowProps = {
  webhookId: string;
  windowLength: '7D' | '1D' | '12H' | '4H';
+  tinybirdJwt?: string;


should be required

charlesBochet · 2024-10-21T11:24:05Z

packages/twenty-server/src/engine/core-modules/analytics/analytics.module.ts

@@ -1,17 +1,34 @@
+/* eslint-disable no-restricted-imports */


charlesBochet · 2024-10-21T11:29:35Z

packages/twenty-server/src/engine/core-modules/analytics/analytics.service.ts

@@ -58,7 +60,7 @@ export class AnalyticsService {
    const config: AxiosRequestConfig = {
      headers: {
        Authorization:
-          'Bearer ' + this.environmentService.get('TINYBIRD_TOKEN'),
+          'Bearer ' + this.environmentService.get('TINYBIRD_DATASOURCE_TOKEN'),


TINYBIRD_INGEST_TOKEN
(vs TINYBIRD_GENERATE_JWT_TOKEN)

charlesBochet · 2024-10-21T11:29:48Z

packages/twenty-server/src/engine/core-modules/analytics/analytics.service.ts

@@ -86,4 +88,22 @@ export class AnalyticsService {

    return { success: true };
  }
+
+  async generateJWT(workspaceId: string | null | undefined) {


generateWorkspaceJwt

charlesBochet · 2024-10-21T11:30:15Z

packages/twenty-server/src/engine/core-modules/user/user.resolver.ts

@@ -55,6 +56,7 @@ export class UserResolver {
    private readonly onboardingService: OnboardingService,
    private readonly userVarService: UserVarsService,
    private readonly fileService: FileService,
+    private readonly analyticsservice: AnalyticsService,


analyticsService

charlesBochet · 2024-10-21T11:30:32Z

packages/twenty-server/src/engine/core-modules/environment/environment-variables.ts


  @IsString()
  @ValidateIf((env) => env.ANALYTICS_ENABLED)
-  TINYBIRD_TOKEN: string;
+  TINYBIRD_DATASOURCE_TOKEN: string;


add these to doc

add jwt support for webhooks graphs

7663067

anamarn requested a review from charlesBochet October 21, 2024 11:18

charlesBochet force-pushed the main branch from 1c484d5 to 2f72e22 Compare October 21, 2024 11:19

greptile-apps bot reviewed Oct 21, 2024

View reviewed changes

charlesBochet reviewed Oct 21, 2024

View reviewed changes

charlesBochet self-assigned this Oct 21, 2024

charlesBochet added the -PR: awaiting author label Oct 21, 2024

anamarn and others added 3 commits October 21, 2024 15:08

correct variable names and pr comments

231195d

Merge branch 'main' into feat/tinybird-jwt

c5b2677

Last fixes according to review

571e067

charlesBochet approved these changes Oct 21, 2024

View reviewed changes

charlesBochet added 4 commits October 21, 2024 17:13

Try fix CI

c9cb9fe

Fix CI

6d2e63e

Fix CI

988e02c

Fix tests

05af2d5

charlesBochet merged commit 373926b into main Oct 21, 2024
12 checks passed

charlesBochet deleted the feat/tinybird-jwt branch October 21, 2024 15:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Secure connexion between TinyBird and webhookResponseGraph #7913

Secure connexion between TinyBird and webhookResponseGraph #7913

anamarn commented Oct 21, 2024

greptile-apps bot left a comment

greptile-apps bot Oct 21, 2024

greptile-apps bot Oct 21, 2024

greptile-apps bot Oct 21, 2024

greptile-apps bot Oct 21, 2024

greptile-apps bot Oct 21, 2024

greptile-apps bot Oct 21, 2024

greptile-apps bot Oct 21, 2024

greptile-apps bot Oct 21, 2024

greptile-apps bot Oct 21, 2024

greptile-apps bot Oct 21, 2024

charlesBochet Oct 21, 2024

anamarn Oct 21, 2024

charlesBochet Oct 21, 2024

charlesBochet Oct 21, 2024

charlesBochet Oct 21, 2024

charlesBochet Oct 21, 2024

charlesBochet Oct 21, 2024

charlesBochet Oct 21, 2024

charlesBochet Oct 21, 2024


		import { currentUserState } from '@/auth/states/currentUserState';

		export const useAnalyticsTinybirdJwt = (): string \| null \| undefined => {

Secure connexion between TinyBird and webhookResponseGraph #7913

Secure connexion between TinyBird and webhookResponseGraph #7913

Conversation

anamarn commented Oct 21, 2024

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Oct 21, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 21, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 21, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 21, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 21, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 21, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 21, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 21, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 21, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment