Skip to content

Refactor WorkspaceAuthContext to use discriminated union types#17491

Merged
Weiko merged 10 commits intomainfrom
c--refactor-workspace-auth-context
Jan 28, 2026
Merged

Refactor WorkspaceAuthContext to use discriminated union types#17491
Weiko merged 10 commits intomainfrom
c--refactor-workspace-auth-context

Conversation

@Weiko
Copy link
Copy Markdown
Member

@Weiko Weiko commented Jan 27, 2026

Context

The previous WorkspaceAuthContext was a single interface with many optional fields, making it unclear which fields are available in different authentication scenarios. This made the code harder to reason about and required runtime checks scattered throughout the codebase.

Changes

  • Introduced a discriminated union type for WorkspaceAuthContext with four specific variants:
    -> UserWorkspaceAuthContext - for authenticated users
    -> ApiKeyWorkspaceAuthContext - for API key authentication
    -> ApplicationWorkspaceAuthContext - for application-based auth
    -> SystemWorkspaceAuthContext - for system/internal operations
    -> PendingActivationUserWorkspaceAuthContext - for pending workspace creation, similar to UserWorkspaceAuthContext but without workspaceMember in it
  • Added type guard functions (isUserAuthContext, isApiKeyAuthContext, etc.) for safe type narrowing
  • Added builder utilities (buildUserAuthContext, buildApiKeyAuthContext, etc.) to construct each context variant with proper type safety
  • Refactored WorkspaceAuthContextMiddleware to use the new builders instead of constructing a loosely-typed object
  • Moved the type definition from twenty-orm/interfaces/ to core-modules/auth/types/ for better organization
  • Updated all consumers across query runners, tool providers, and modules to use the new type location

Notes

  • I had to query User and WorkspaceMember in some parts of tool module that were expecting userWorkspaceId but not the rest of UserWorkspaceAuthContext (that should be required with the new proper type otherwise it would break a lot of logic and mostly permissions with the newly added RLS -> This is what we expect from UserWorkspaceAuthContext and how it's done in the "normal" path in HTTP middleware)
  • WorkspaceMember is in the cache already but ideally we should move User (And Workspace?) in the cache as well to avoid querying the DB after each request (this is also valid for HTTP middleware when we hydrate the Request object btw)

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Jan 27, 2026

Greptile Overview

Greptile Summary

This PR refactors WorkspaceAuthContext from a single interface with many optional fields into a discriminated union with four specific variants: UserWorkspaceAuthContext, ApiKeyWorkspaceAuthContext, ApplicationWorkspaceAuthContext, and SystemWorkspaceAuthContext.

Key changes:

  • Moved type definition from engine/api/common/interfaces/ to engine/core-modules/auth/types/ for better organization
  • Added builder utilities (buildUserAuthContext, buildApiKeyAuthContext, etc.) to construct each context variant with type safety
  • Added type guard functions (isUserAuthContext, isApiKeyAuthContext, etc.) for runtime type narrowing
  • Refactored WorkspaceAuthContextMiddleware to use builders and throw errors for invalid auth states instead of creating loosely-typed objects
  • Updated 59 files across query runners, tool providers, and modules to use the new type location and builders
  • Enhanced database-tool.provider.ts and mcp-protocol.service.ts to properly fetch user and workspace member data when building user contexts

The refactoring improves type safety by making it explicit which fields are available in different authentication scenarios, eliminating runtime checks scattered throughout the codebase.

Confidence Score: 4/5

  • This PR is safe to merge with minimal risk - it's a well-structured type refactoring that improves code clarity
  • The refactoring follows TypeScript best practices and systematically updates all consumers. Type guards lack discriminator fields which could be improved, and there's a potential naming confusion with userWorkspaceId being used as a user ID in one location, but these are non-critical style issues.
  • Pay close attention to mcp-protocol.service.ts for the potential userWorkspaceId naming confusion

Important Files Changed

Filename Overview
packages/twenty-server/src/engine/core-modules/auth/types/workspace-auth-context.type.ts New discriminated union type definition with four auth context variants (User, ApiKey, Application, System)
packages/twenty-server/src/engine/core-modules/auth/middlewares/workspace-auth-context.middleware.ts Refactored middleware to use builder functions for type-safe auth context construction
packages/twenty-server/src/engine/core-modules/auth/guards/is-system-auth-context.guard.ts Type guard for system context using negative checks on apiKey, userWorkspaceId, and application
packages/twenty-server/src/engine/core-modules/tool-provider/providers/database-tool.provider.ts Updated to fetch user and workspace member data to build proper UserWorkspaceAuthContext
packages/twenty-server/src/engine/api/mcp/services/mcp-protocol.service.ts Refactored to build proper auth context variants (user, API key, or system) with user/member fetching

Sequence Diagram

sequenceDiagram
    participant Client
    participant Middleware as WorkspaceAuthContextMiddleware
    participant Builder as Builder Utils
    participant Storage as WorkspaceAuthContextStorage
    participant Consumer as Query Runners/Services

    Client->>Middleware: HTTP Request with auth info
    
    alt API Key Auth
        Middleware->>Builder: buildApiKeyAuthContext(workspace, apiKey)
        Builder-->>Middleware: ApiKeyWorkspaceAuthContext
    else User Auth
        Middleware->>Builder: buildUserAuthContext(workspace, user, ...)
        Builder-->>Middleware: UserWorkspaceAuthContext
    else Application Auth
        Middleware->>Builder: buildApplicationAuthContext(workspace, app)
        Builder-->>Middleware: ApplicationWorkspaceAuthContext
    else No specific auth
        Middleware->>Middleware: throw AuthException
    end

    Middleware->>Storage: withWorkspaceAuthContext(context, callback)
    Storage->>Consumer: Execute with typed context
    
    Consumer->>Consumer: Use type guards<br/>(isUserAuthContext, etc.)
    Consumer->>Consumer: Access type-specific fields<br/>(user, apiKey, application)
    
    Consumer-->>Storage: Return result
    Storage-->>Middleware: Complete
    Middleware-->>Client: Response
Loading

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

}

const authContext: WorkspaceAuthContext = buildUserAuthContext({
workspace: { id: context.workspaceId } as WorkspaceEntity,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm reusing the existing code but we should query the whole workspace entity from the cache here as well instead of type assertion

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 59 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="packages/twenty-server/src/engine/api/mcp/services/mcp-protocol.service.ts">

<violation number="1" location="packages/twenty-server/src/engine/api/mcp/services/mcp-protocol.service.ts:109">
P2: User lookup uses userWorkspaceId as the UserEntity id, but userWorkspaceId is a separate identifier. This will fail to resolve users and reject valid requests when ids differ.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Copy link
Copy Markdown
Contributor

@martmull martmull left a comment

Choose a reason for hiding this comment

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

lgtm

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 28, 2026

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:56300

This environment will automatically shut down when the PR is closed or after 5 hours.

@Weiko Weiko enabled auto-merge January 28, 2026 14:18
@Weiko Weiko added this pull request to the merge queue Jan 28, 2026
Merged via the queue into main with commit fc908e9 Jan 28, 2026
60 checks passed
@Weiko Weiko deleted the c--refactor-workspace-auth-context branch January 28, 2026 18:02
@twenty-eng-sync
Copy link
Copy Markdown

Hey @Weiko! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

2 similar comments
@twenty-eng-sync
Copy link
Copy Markdown

Hey @Weiko! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

@twenty-eng-sync
Copy link
Copy Markdown

Hey @Weiko! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants