1487 extensibility look into public domains to identify workspace #14456
Conversation
martmull
commented
Sep 12, 2025
martmull
commented
- identify workspace based on public domains
- add cron job to validate public domains
- add endpoint to validate a public domain
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This pull request implements extensibility feature 1487 which adds workspace identification capabilities based on public domains. The changes introduce three main functionalities:
Workspace Identification Enhancement: The DomainManagerService has been expanded to support public domain lookup as a third identification strategy, after custom domains and subdomains. The service now includes a new method getWorkspaceByOriginOrDefaultWorkspace that can resolve workspaces through public domains registered in the PublicDomain entity. This allows users to access workspaces through publicly registered domains that are linked to specific workspaces.
Automated Domain Validation: A new cron job system validates public domain DNS records hourly. The CheckPublicDomainsValidRecordsCronCommand schedules the job, while CheckPublicDomainsValidRecordsCronJob performs the actual validation work. This ensures that public domains remain properly configured and accessible over time.
Manual Domain Validation API: A new GraphQL mutation checkPublicDomainValidRecords has been added to the PublicDomainResolver, allowing clients to manually trigger DNS validation for specific public domains within their workspace context.
The implementation follows established patterns in the codebase, with proper module registration, dependency injection, and comprehensive test coverage. The PublicDomain entity has been integrated into the DomainManagerModule and DatabaseCommandModule to enable the new functionality. The changes maintain backward compatibility while extending the domain resolution capabilities of the system.
Confidence score: 2/5
- This PR has critical logic issues that could cause production problems
- Score lowered due to incorrect SQL filtering logic in the cron job that filters by creation hour instead of validation timestamps
- Pay close attention to
check-public-domains-valid-records.cron.job.tsandguard-redirect.service.ts
9 files reviewed, 6 comments
| subdomain: subdomainAndCustomDomainFromReferer.subdomain, | ||
| customDomain: subdomainAndCustomDomainFromReferer.customDomain, | ||
| subdomain: subdomainAndDomainFromReferer.subdomain, | ||
| customDomain: subdomainAndDomainFromReferer.domain, |
There was a problem hiding this comment.
logic: Mapping generic 'domain' to 'customDomain' may be misleading - consider validating if the domain is actually custom vs public
| useClass: Repository, | ||
| useValue: { | ||
| find: jest.fn(), | ||
| findOne: jest.fn(), | ||
| }, | ||
| }, | ||
| { | ||
| provide: getRepositoryToken(PublicDomain), | ||
| useClass: Repository, | ||
| useValue: { | ||
| findOne: jest.fn(), | ||
| }, | ||
| }, |
There was a problem hiding this comment.
logic: Mock configuration has both useClass: Repository and useValue - this creates conflicting provider definitions. Remove useClass: Repository as useValue takes precedence
| useClass: Repository, | |
| useValue: { | |
| find: jest.fn(), | |
| findOne: jest.fn(), | |
| }, | |
| }, | |
| { | |
| provide: getRepositoryToken(PublicDomain), | |
| useClass: Repository, | |
| useValue: { | |
| findOne: jest.fn(), | |
| }, | |
| }, | |
| useValue: { | |
| find: jest.fn(), | |
| findOne: jest.fn(), | |
| }, | |
| }, | |
| { | |
| provide: getRepositoryToken(PublicDomain), | |
| useValue: { | |
| findOne: jest.fn(), | |
| }, |
| jest.spyOn(workspaceRepository, 'find').mockResolvedValueOnce([ | ||
| { | ||
| id: 'workspace-id', | ||
| }, | ||
| ] as unknown as Promise<Workspace[]>); |
There was a problem hiding this comment.
syntax: Mock return value has incorrect type casting - mockResolvedValueOnce expects the resolved value directly, not wrapped in Promise type
| expect(result?.id).toEqual('workspace-id1'); | ||
| }); | ||
|
|
||
| it('should return undefined is nothing found', async () => { |
There was a problem hiding this comment.
syntax: Typo in test description: 'undefined is nothing found' should be 'undefined if nothing found'
| it('should return undefined is nothing found', async () => { | |
| it('should return undefined if nothing found', async () => { |
| @@ -44,4 +53,21 @@ export class PublicDomainResolver { | |||
|
|
|||
| return true; | |||
| } | |||
|
|
|||
| @Mutation(() => DomainValidRecords, { nullable: true }) | |||
| @UseGuards(WorkspaceAuthGuard) | |||
There was a problem hiding this comment.
style: redundant @UseGuards(WorkspaceAuthGuard) since it's already applied at class level
| @UseGuards(WorkspaceAuthGuard) | |
| async checkPublicDomainValidRecords( |
| createdAt: Raw( | ||
| (alias) => `EXTRACT(HOUR FROM ${alias}) = EXTRACT(HOUR FROM NOW())`, | ||
| ), |
There was a problem hiding this comment.
logic: Filtering by creation hour seems incorrect for domain validation. Consider using a timestamp field that tracks when domains were last validated, or remove this filter to check all domains.
| activationStatus: WorkspaceActivationStatus.ACTIVE, | ||
| customDomain: Not(IsNull()), | ||
| createdAt: Raw( | ||
| (alias) => `EXTRACT(HOUR FROM ${alias}) = EXTRACT(HOUR FROM NOW())`, |
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:42790 This environment will automatically shut down when the PR is closed or after 5 hours. |
