-
Notifications
You must be signed in to change notification settings - Fork 4.3k
refactor(api-service): polish inbox channel endpoints #10781
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
Changes from all commits
92f32b0
c483263
a89c3ee
9c718c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| import { ApiProperty } from '@nestjs/swagger'; | ||
|
|
||
| export class InboxChannelConnectionResponseDto { | ||
| @ApiProperty({ | ||
| description: 'The unique identifier of the channel connection.', | ||
| type: String, | ||
| }) | ||
| identifier: string; | ||
| } | ||
|
|
||
| export class InboxListChannelConnectionsResponseDto { | ||
| @ApiProperty({ type: [InboxChannelConnectionResponseDto] }) | ||
| data: InboxChannelConnectionResponseDto[]; | ||
|
|
||
| @ApiProperty({ type: String, nullable: true }) | ||
| next: string | null; | ||
|
|
||
| @ApiProperty({ type: String, nullable: true }) | ||
| previous: string | null; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| import { ApiProperty } from '@nestjs/swagger'; | ||
| import { ChannelEndpointType, ENDPOINT_TYPES } from '@novu/shared'; | ||
|
|
||
| export class InboxChannelEndpointResponseDto { | ||
| @ApiProperty({ | ||
| description: 'The unique identifier of the channel endpoint.', | ||
| type: String, | ||
| }) | ||
| identifier: string; | ||
|
|
||
| @ApiProperty({ | ||
| description: 'Type of channel endpoint', | ||
| enum: Object.values(ENDPOINT_TYPES), | ||
| example: ENDPOINT_TYPES.SLACK_CHANNEL, | ||
| }) | ||
| type: ChannelEndpointType; | ||
| } | ||
|
|
||
| export class InboxListChannelEndpointsResponseDto { | ||
| @ApiProperty({ type: [InboxChannelEndpointResponseDto] }) | ||
| data: InboxChannelEndpointResponseDto[]; | ||
|
|
||
| @ApiProperty({ type: String, nullable: true }) | ||
| next: string | null; | ||
|
|
||
| @ApiProperty({ type: String, nullable: true }) | ||
| previous: string | null; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| import { ChannelConnectionEntity, ChannelEndpointEntity } from '@novu/dal'; | ||
| import { InboxChannelConnectionResponseDto } from './inbox-channel-connection-response.dto'; | ||
| import { InboxChannelEndpointResponseDto } from './inbox-channel-endpoint-response.dto'; | ||
|
|
||
| export function mapChannelConnectionToInboxDto(entity: ChannelConnectionEntity): InboxChannelConnectionResponseDto { | ||
| return { | ||
| identifier: entity.identifier, | ||
| }; | ||
| } | ||
|
|
||
| export function mapChannelEndpointToInboxDto(entity: ChannelEndpointEntity): InboxChannelEndpointResponseDto { | ||
| return { | ||
| identifier: entity.identifier, | ||
| type: entity.type, | ||
| }; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,18 +62,16 @@ export class GenerateSlackOauthUrl { | |
| await this.assertResourceExists(command); | ||
|
|
||
| const { clientId } = await this.getIntegrationCredentials(command.integration); | ||
| const subscriberId = command.connectionMode === 'shared' ? undefined : command.subscriberId; | ||
| const secureState = await this.createSecureState( | ||
| command.integration, | ||
| subscriberId, | ||
| command.subscriberId, | ||
| command.context, | ||
| command.connectionIdentifier, | ||
| command.mode, | ||
| command.connectionMode | ||
| ); | ||
|
Comment on lines
65
to
72
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Description: Inspect subscriberId constraints and OAuth state delimiter parsing.
rg -n -C4 --type=ts '\bsubscriberId\b|decoded\.split\('\''\.'\''\)'Repository: novuhq/novu Length of output: 50369 🏁 Script executed: #!/bin/bash
# Get the exact file and lines mentioned in the review
file_path="apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-slack-oath-url/generate-slack-oauth-url.usecase.ts"
# Check file exists and show relevant lines
if [ -f "$file_path" ]; then
echo "=== File context around lines 65-72 ==="
sed -n '60,75p' "$file_path"
echo ""
echo "=== File context around lines 197-202 ==="
sed -n '190,210p' "$file_path"
else
echo "File not found: $file_path"
# Try to find the file with a flexible search
find . -name "*generate-slack-oauth-url.usecase.ts" -type f
fiRepository: novuhq/novu Length of output: 1492 🏁 Script executed: #!/bin/bash
# Search for the split('.') pattern in the usecase file and related state handling
rg -n "split\('\.'\)|createSecureState|validateAndDecodeState" apps/api/src/app/integrations --type=ts -A 5 -B 2 | head -150Repository: novuhq/novu Length of output: 14838 🏁 Script executed: #!/bin/bash
# Check if there are any validations on subscriberId format in the codebase
rg -n "subscriberId.*pattern|subscriberId.*validate|subscriberId.*format" apps/api/src --type=ts | head -50Repository: novuhq/novu Length of output: 172 🏁 Script executed: #!/bin/bash
sed -n '159,195p' apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-slack-oath-url/generate-slack-oauth-url.usecase.tsRepository: novuhq/novu Length of output: 1234 🏁 Script executed: #!/bin/bash
# Check if there's any validation logic for subscriberId format
rg -n "subscriberId" apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-slack-oath-url/generate-slack-oauth-url.usecase.ts -B 2 -A 2 | head -80Repository: novuhq/novu Length of output: 1727 Fix OAuth state parsing to handle subscriber IDs and context with dot delimiters. Line 200 uses This issue also affects:
🔒 Proposed fix (use last dot as separator)- const [payload, signature] = decoded.split('.');
+ const separatorIndex = decoded.lastIndexOf('.');
+ if (separatorIndex === -1) {
+ throw new Error('Invalid state format');
+ }
+
+ const payload = decoded.slice(0, separatorIndex);
+ const signature = decoded.slice(separatorIndex + 1);
+
+ if (!payload || !signature) {
+ throw new Error('Invalid state format');
+ }
const expectedSignature = createHash(environmentApiKey, payload);🤖 Prompt for AI Agents |
||
|
|
||
| const resolvedScope = | ||
| command.mode === 'link_user' ? undefined : await this.resolveBotScopes(command); | ||
| const resolvedScope = command.mode === 'link_user' ? undefined : await this.resolveBotScopes(command); | ||
|
|
||
| return this.getOAuthUrl(clientId!, secureState, resolvedScope, command.userScope, command.mode); | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.