Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,15 @@ export class DefaultPushNotificationSender implements PushNotificationSender {
headers[this.options.tokenHeaderName] = pushConfig.token;
}

if (pushConfig.authentication && pushConfig.authentication.credentials) {
const { schemes, credentials } = pushConfig.authentication;
if (schemes.includes('Bearer')) {
headers['Authorization'] = `Bearer ${credentials}`;
} else if (schemes.includes('Basic')) {
headers['Authorization'] = `Basic ${credentials}`;
}
Comment on lines +87 to +91

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The current implementation for handling authentication schemes has a flaw. If both Bearer and Basic schemes are present in the schemes array, the Authorization header will always be set to Basic because its check comes after Bearer, overwriting any previously set value. This behavior is non-obvious and likely not what's intended, as the order in the schemes array is ignored.

To make the behavior explicit and prefer one scheme over the other (e.g., Bearer is generally preferred over Basic), you should use an if-else if structure. This ensures only one authentication header is set, with a clear precedence.

        if (schemes.includes('Bearer')) {
          headers['Authorization'] = `Bearer ${credentials}`;
        } else if (schemes.includes('Basic')) {
          headers['Authorization'] = `Basic ${credentials}`;
        }

}

const response = await fetch(url, {
method: 'POST',
headers,
Expand Down
145 changes: 145 additions & 0 deletions test/server/push_notification_integration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -700,5 +700,150 @@
);
});
});
it('should send correct Authorization header when authentication config is provided', async () => {
const pushConfig: PushNotificationConfig = {
id: 'auth-test-config',
url: `${testServerUrl}/notify`,
authentication: {
schemes: ['Bearer', 'Basic'],
credentials: 'test-credentials',
},
};

const params: MessageSendParams = {
message: createTestMessage('Test with authentication'),
configuration: {
pushNotificationConfig: pushConfig,
},
};

// Mock the agent executor to publish completion
mockAgentExecutor.execute.callsFake(async (ctx, bus) => {

Check failure on line 721 in test/server/push_notification_integration.spec.ts

View workflow job for this annotation

GitHub Actions / coverage

test/server/push_notification_integration.spec.ts > Push Notification Integration Tests > Push Notification Header Configuration Tests > should send correct Authorization header when authentication config is provided

TypeError: mockAgentExecutor.execute.callsFake is not a function ❯ test/server/push_notification_integration.spec.ts:721:33

Check failure on line 721 in test/server/push_notification_integration.spec.ts

View workflow job for this annotation

GitHub Actions / eslint

Parameter 'bus' implicitly has an 'any' type.

Check failure on line 721 in test/server/push_notification_integration.spec.ts

View workflow job for this annotation

GitHub Actions / eslint

Parameter 'ctx' implicitly has an 'any' type.

Check failure on line 721 in test/server/push_notification_integration.spec.ts

View workflow job for this annotation

GitHub Actions / eslint

Property 'callsFake' does not exist on type 'Mock<(requestContext: RequestContext, eventBus: ExecutionEventBus) => Promise<void>>'.

Check failure on line 721 in test/server/push_notification_integration.spec.ts

View workflow job for this annotation

GitHub Actions / test (22)

test/server/push_notification_integration.spec.ts > Push Notification Integration Tests > Push Notification Header Configuration Tests > should send correct Authorization header when authentication config is provided

TypeError: mockAgentExecutor.execute.callsFake is not a function ❯ test/server/push_notification_integration.spec.ts:721:33

Check failure on line 721 in test/server/push_notification_integration.spec.ts

View workflow job for this annotation

GitHub Actions / test (24)

test/server/push_notification_integration.spec.ts > Push Notification Integration Tests > Push Notification Header Configuration Tests > should send correct Authorization header when authentication config is provided

TypeError: mockAgentExecutor.execute.callsFake is not a function ❯ test/server/push_notification_integration.spec.ts:721:33

Check failure on line 721 in test/server/push_notification_integration.spec.ts

View workflow job for this annotation

GitHub Actions / test (20)

test/server/push_notification_integration.spec.ts > Push Notification Integration Tests > Push Notification Header Configuration Tests > should send correct Authorization header when authentication config is provided

TypeError: mockAgentExecutor.execute.callsFake is not a function ❯ test/server/push_notification_integration.spec.ts:721:33
const taskId = ctx.taskId;
const contextId = ctx.contextId;

bus.publish({
id: taskId,
contextId,
status: { state: 'working' },
kind: 'task',
});

bus.publish({
taskId,
contextId,
kind: 'status-update',
status: { state: 'completed' },
final: true,
});

bus.finished();
});

await handler.sendMessage(params);

// Wait for async push notifications to be sent
await new Promise((resolve) => setTimeout(resolve, 200));

// Verify notification received
assert.lengthOf(
receivedNotifications,
2,
'Should send notifications for working and completed states'
);

// Check the last notification (completed)
const notification = receivedNotifications[1];

// Verify Authorization header
// Bearer should be prioritized over Basic
assert.equal(
notification.headers['authorization'],
'Bearer test-credentials',
'Should prioritize Bearer scheme'
);
});

it('should send Bearer Authorization header', async () => {
const pushConfig: PushNotificationConfig = {
id: 'bearer-auth-test',
url: `${testServerUrl}/notify`,
authentication: {
schemes: ['Bearer'],
credentials: 'bearer-token',
},
};

const params: MessageSendParams = {
message: createTestMessage('Test with Bearer auth'),
configuration: {
pushNotificationConfig: pushConfig,
},
};

mockAgentExecutor.execute.callsFake(async (ctx, bus) => {

Check failure on line 784 in test/server/push_notification_integration.spec.ts

View workflow job for this annotation

GitHub Actions / coverage

test/server/push_notification_integration.spec.ts > Push Notification Integration Tests > Push Notification Header Configuration Tests > should send Bearer Authorization header

TypeError: mockAgentExecutor.execute.callsFake is not a function ❯ test/server/push_notification_integration.spec.ts:784:33

Check failure on line 784 in test/server/push_notification_integration.spec.ts

View workflow job for this annotation

GitHub Actions / eslint

Parameter 'bus' implicitly has an 'any' type.

Check failure on line 784 in test/server/push_notification_integration.spec.ts

View workflow job for this annotation

GitHub Actions / eslint

Parameter 'ctx' implicitly has an 'any' type.

Check failure on line 784 in test/server/push_notification_integration.spec.ts

View workflow job for this annotation

GitHub Actions / eslint

Property 'callsFake' does not exist on type 'Mock<(requestContext: RequestContext, eventBus: ExecutionEventBus) => Promise<void>>'.

Check failure on line 784 in test/server/push_notification_integration.spec.ts

View workflow job for this annotation

GitHub Actions / test (22)

test/server/push_notification_integration.spec.ts > Push Notification Integration Tests > Push Notification Header Configuration Tests > should send Bearer Authorization header

TypeError: mockAgentExecutor.execute.callsFake is not a function ❯ test/server/push_notification_integration.spec.ts:784:33

Check failure on line 784 in test/server/push_notification_integration.spec.ts

View workflow job for this annotation

GitHub Actions / test (24)

test/server/push_notification_integration.spec.ts > Push Notification Integration Tests > Push Notification Header Configuration Tests > should send Bearer Authorization header

TypeError: mockAgentExecutor.execute.callsFake is not a function ❯ test/server/push_notification_integration.spec.ts:784:33

Check failure on line 784 in test/server/push_notification_integration.spec.ts

View workflow job for this annotation

GitHub Actions / test (20)

test/server/push_notification_integration.spec.ts > Push Notification Integration Tests > Push Notification Header Configuration Tests > should send Bearer Authorization header

TypeError: mockAgentExecutor.execute.callsFake is not a function ❯ test/server/push_notification_integration.spec.ts:784:33
bus.publish({
id: ctx.taskId,
contextId: ctx.contextId,
status: { state: 'working' },
kind: 'task',
});
bus.publish({
taskId: ctx.taskId,
contextId: ctx.contextId,
kind: 'status-update',
status: { state: 'completed' },
final: true,
});
bus.finished();
});

await handler.sendMessage(params);
await new Promise((resolve) => setTimeout(resolve, 200));

assert.lengthOf(receivedNotifications, 2);
assert.equal(receivedNotifications[1].headers['authorization'], 'Bearer bearer-token');
});

it('should send Basic Authorization header', async () => {
const pushConfig: PushNotificationConfig = {
id: 'basic-auth-test',
url: `${testServerUrl}/notify`,
authentication: {
schemes: ['Basic'],
credentials: 'basic-creds',
},
};

const params: MessageSendParams = {
message: createTestMessage('Test with Basic auth'),
configuration: {
pushNotificationConfig: pushConfig,
},
};

mockAgentExecutor.execute.callsFake(async (ctx, bus) => {

Check failure on line 825 in test/server/push_notification_integration.spec.ts

View workflow job for this annotation

GitHub Actions / coverage

test/server/push_notification_integration.spec.ts > Push Notification Integration Tests > Push Notification Header Configuration Tests > should send Basic Authorization header

TypeError: mockAgentExecutor.execute.callsFake is not a function ❯ test/server/push_notification_integration.spec.ts:825:33

Check failure on line 825 in test/server/push_notification_integration.spec.ts

View workflow job for this annotation

GitHub Actions / eslint

Parameter 'bus' implicitly has an 'any' type.

Check failure on line 825 in test/server/push_notification_integration.spec.ts

View workflow job for this annotation

GitHub Actions / eslint

Parameter 'ctx' implicitly has an 'any' type.

Check failure on line 825 in test/server/push_notification_integration.spec.ts

View workflow job for this annotation

GitHub Actions / eslint

Property 'callsFake' does not exist on type 'Mock<(requestContext: RequestContext, eventBus: ExecutionEventBus) => Promise<void>>'.

Check failure on line 825 in test/server/push_notification_integration.spec.ts

View workflow job for this annotation

GitHub Actions / test (22)

test/server/push_notification_integration.spec.ts > Push Notification Integration Tests > Push Notification Header Configuration Tests > should send Basic Authorization header

TypeError: mockAgentExecutor.execute.callsFake is not a function ❯ test/server/push_notification_integration.spec.ts:825:33

Check failure on line 825 in test/server/push_notification_integration.spec.ts

View workflow job for this annotation

GitHub Actions / test (24)

test/server/push_notification_integration.spec.ts > Push Notification Integration Tests > Push Notification Header Configuration Tests > should send Basic Authorization header

TypeError: mockAgentExecutor.execute.callsFake is not a function ❯ test/server/push_notification_integration.spec.ts:825:33

Check failure on line 825 in test/server/push_notification_integration.spec.ts

View workflow job for this annotation

GitHub Actions / test (20)

test/server/push_notification_integration.spec.ts > Push Notification Integration Tests > Push Notification Header Configuration Tests > should send Basic Authorization header

TypeError: mockAgentExecutor.execute.callsFake is not a function ❯ test/server/push_notification_integration.spec.ts:825:33
bus.publish({
id: ctx.taskId,
contextId: ctx.contextId,
status: { state: 'working' },
kind: 'task',
});
bus.publish({
taskId: ctx.taskId,
contextId: ctx.contextId,
kind: 'status-update',
status: { state: 'completed' },
final: true,
});
bus.finished();
});

await handler.sendMessage(params);
await new Promise((resolve) => setTimeout(resolve, 200));

assert.lengthOf(receivedNotifications, 2);
assert.equal(receivedNotifications[1].headers['authorization'], 'Basic basic-creds');
});
});
});
Loading