Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
6d24e66
Adding new project? and oauth? params to CreateServerClientOpts
dylburger Sep 26, 2024
9b7dcf1
Small refactor
dylburger Sep 26, 2024
975d669
Simplifying
dylburger Sep 26, 2024
651d41b
Fixing JSDoc
dylburger Sep 26, 2024
71dd92f
CodeRabbit fix
dylburger Sep 26, 2024
ad1804d
Merge remote-tracking branch 'origin/master' into sdk/connect-workflo…
dylburger Oct 7, 2024
4385093
AppId -> AppNameSlug
dylburger Oct 7, 2024
87d8d86
Forcing OAuth for invoking workflows for external users
dylburger Oct 7, 2024
ce22436
Mocking client and mock OAuth client once
dylburger Oct 8, 2024
d4a8394
Improving tests
dylburger Oct 8, 2024
1b02d65
Small comment
dylburger Oct 8, 2024
0bce50e
Adding support for baseWorkflowDomain, ID vs URL
dylburger Oct 8, 2024
075e400
Adding authType to invokeWorkflow example
dylburger Oct 8, 2024
7c7b4ac
Merge remote-tracking branch 'origin/master' into sdk/connect-workflo…
dylburger Oct 28, 2024
ccf848e
Biz PR feedback
dylburger Oct 28, 2024
9a4af88
Merge remote-tracking branch 'origin/master' into sdk/connect-workflo…
dylburger Oct 28, 2024
faf0f8d
ServerClient -> BackendClient
dylburger Oct 29, 2024
5e85ef5
Updating all names to noun-verb form.
dylburger Oct 29, 2024
85748a4
Merge remote-tracking branch 'origin/master' into sdk/connect-workflo…
dylburger Oct 29, 2024
06a17b5
1.0.0
dylburger Oct 29, 2024
e63a89f
Fixing browser / server exports
dylburger Oct 30, 2024
bfbc249
createClient -> createBackendClient
dylburger Oct 30, 2024
314b644
frontend: createClient -> createFrontendClient
dylburger Oct 30, 2024
1a1faf0
Adding `npm link` docs
dylburger Oct 30, 2024
0e9866f
Adding cd
dylburger Oct 30, 2024
cf31cda
Fixing one command
dylburger Oct 30, 2024
fe7a66d
Localize the Node.js version change
jverce Oct 30, 2024
0c20e6c
Adding npm watch
dylburger Oct 30, 2024
6d23fb9
New lock file
dylburger Oct 30, 2024
2db68e3
Merge branch 'sdk/connect-workflow-invocation' of github.com:Pipedrea…
dylburger Oct 30, 2024
aa7fb1a
Move contributor guide to its own file
jverce Oct 30, 2024
9999664
Apply suggestion from CodeRabbit
jverce Oct 30, 2024
89ff7c1
Cosmetic changes in the README
jverce Oct 30, 2024
7964dde
Revert to old naming convention
jverce Oct 31, 2024
3429a92
A bit of test cleanup
jverce Oct 31, 2024
1886bd9
Final touches
jverce Oct 31, 2024
d27a9b1
Fix typo
jverce Oct 31, 2024
e775d04
Lint markdown files in the SDK
jverce Oct 31, 2024
60ac2da
Fix indentation in docstring
jverce Oct 31, 2024
4695be5
Verify that project ID is present
jverce Oct 31, 2024
a97736d
Only lint SDK files
jverce Oct 31, 2024
003aea7
Name Github workflow accordingly
jverce Oct 31, 2024
9525871
Fix pnpm case
jverce Oct 31, 2024
0e0b7b3
TS suggestions from CodeRabbit
jverce Oct 31, 2024
0f4f1a3
Dry the tests as per CodeRabbit
jverce Nov 1, 2024
4d89d5c
Use latest checkout action
jverce Nov 1, 2024
866efdd
Limit concurrency of the SDK CI/CD workflows
jverce Nov 1, 2024
60e9e81
Merge branch 'master' into sdk/connect-workflow-invocation
jverce Nov 1, 2024
3025cf5
Validate workflow URLs better
jverce Nov 1, 2024
7ec1156
Disable linting on ts-ignore lines
jverce Nov 1, 2024
a33b75f
More URL sanitizing
jverce Nov 1, 2024
6596349
Merge branch 'master' into sdk/connect-workflow-invocation
jverce Nov 1, 2024
7994412
Merge branch 'master' into sdk/connect-workflow-invocation
jverce Nov 4, 2024
0c7ca6f
Turn enums into const
jverce Nov 4, 2024
29ab565
Enhance param validation for workflow invocation
jverce Nov 4, 2024
401522c
YOLO version
jverce Nov 4, 2024
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
136 changes: 127 additions & 9 deletions packages/sdk/src/server/__tests__/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,18 @@ describe("ServerClient", () => {
const client = createClient(params);
expect(client).toBeInstanceOf(ServerClient);
});

it("should mock the createClient method with a project object and return a ServerClient instance", () => {
const params = {
project: {
publicKey: "test-public-key",
secretKey: "test-secret",
},
};

const client = createClient(params);
expect(client).toBeInstanceOf(ServerClient);
});
});

describe("makeRequest", () => {
Expand Down Expand Up @@ -534,12 +546,16 @@ describe("ServerClient", () => {
} as unknown as ClientCredentials;

// Inject the mock oauthClient into the ServerClient instance
client = new ServerClient(
const client = new ServerClient(
{
publicKey: "test-public-key",
secretKey: "test-secret-key",
oauthClientId: "test-client-id",
oauthClientSecret: "test-client-secret",
project: {
publicKey: "test-public-key",
secretKey: "test",
},
oauth: {
clientId: "test-client-id",
clientSecret: "test-client-secret",
},
},
oauthClientMock,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider refactoring repeated ServerClient initialization

The initialization of ServerClient with mocked OAuth tokens in this test is repeated in other tests (e.g., lines 626-638 and 673-707). To improve maintainability and reduce code duplication, consider refactoring this setup into a shared helper function or utilizing a common beforeEach hook where appropriate.

Expand Down Expand Up @@ -607,10 +623,14 @@ describe("ServerClient", () => {

const client = new ServerClient(
{
publicKey: "test-public-key",
secretKey: "test-secret-key",
oauthClientId: "test-client-id",
oauthClientSecret: "test-client-secret",
project: {
publicKey: "test-public-key",
secretKey: "test",
},
oauth: {
clientId: "test-client-id",
clientSecret: "test-client-secret",
},
},
oauthClientMock,
);
Expand Down Expand Up @@ -643,4 +663,102 @@ describe("ServerClient", () => {
);
});
});

describe("invokeWorkflowForExternalUser", () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New tests for invokeWorkflowForExternalUser

let client: ServerClient;

beforeEach(() => {
fetchMock.resetMocks();
// Create mock AccessToken objects
const expiredTokenMock = {
token: {
access_token: "expired-oauth-token",
},
expired: jest.fn().mockReturnValue(true),
};

const newTokenMock = {
token: {
access_token: "new-oauth-token",
},
expired: jest.fn().mockReturnValue(false),
};

const getTokenMock = jest
.fn()
.mockResolvedValueOnce(expiredTokenMock)
.mockResolvedValueOnce(newTokenMock);

const oauthClientMock = {
getToken: getTokenMock,
} as unknown as ClientCredentials;
client = new ServerClient(
{
project: {
publicKey: "test-public-key",
secretKey: "test",
},
oauth: {
clientId: "test-client-id",
clientSecret: "test-client-secret",
},
},
oauthClientMock,
);
});

it("should include externalUserId and publicKey headers", async () => {
fetchMock.mockResponseOnce(
JSON.stringify({
result: "workflow-response",
}),
{
headers: {
"Content-Type": "application/json",
},
},
);

const result = await client.invokeWorkflowForExternalUser("https://example.com/workflow", "external-user-id", {
body: {
foo: "bar",
},
});

expect(result).toEqual({
result: "workflow-response",
});

expect(fetchMock).toHaveBeenCalledWith(
"https://example.com/workflow",
expect.objectContaining({
headers: expect.objectContaining({
"X-PD-External-User-ID": "external-user-id",
"X-PD-Project-Public-Key": "test-public-key",
}),
}),
);
});

it("should throw error when externalUserId is missing", async () => {
await expect(client.invokeWorkflowForExternalUser("https://example.com/workflow", "", {
body: {
foo: "bar",
},
})).rejects.toThrow("External user ID is required");
});

it("should throw error when publicKey is missing", async () => {
const clientWithoutPublicKey = new ServerClient({
secretKey: "test-secret-key",
});

await expect(clientWithoutPublicKey.invokeWorkflowForExternalUser("https://example.com/workflow", "external-user-id", {
body: {
foo: "bar",
},
})).rejects.toThrow("Project public key is required to map the external user ID to the correct project");
});
});

});
104 changes: 98 additions & 6 deletions packages/sdk/src/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,31 +7,55 @@ import {
ClientCredentials,
} from "simple-oauth2";

export type ProjectKeys = {
publicKey: string;
secretKey: string;
};

export type PipedreamOAuthClient = {
clientId: string;
clientSecret: string;
};

/**
* Options for creating a server-side client.
* This is used to configure the ServerClient instance.
*/
export type CreateServerClientOpts = {
/**
* @deprecated Use the `project` object instead.
* The public API key for accessing the service.
*/
publicKey?: string;

/**
* @deprecated Use the `project` object instead.
* The secret API key for accessing the service.
*/
secretKey?: string;

/**
* @deprecated Use the `oauth` object instead.
* The client ID of your workspace's OAuth application.
*/
oauthClientId?: string;

/**
* @deprecated Use the `oauth` object instead.
* The client secret of your workspace's OAuth application.
*/
oauthClientSecret?: string;

/**
* The project object, containing publicKey and secretKey.
*/
project?: ProjectKeys;

/**
* The OAuth object, containing client ID and client secret.
*/
oauth?: PipedreamOAuthClient;

/**
* The API host URL. Used by Pipedream employees. Defaults to "api.pipedream.com" if not provided.
*/
Expand Down Expand Up @@ -310,6 +334,10 @@ export class ServerClient {
) {
this.secretKey = opts.secretKey;
this.publicKey = opts.publicKey;
if (opts.project) {
this.publicKey = opts.project.publicKey;
this.secretKey = opts.project.secretKey;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Consider clarifying precedence of new vs. deprecated properties

The current implementation overwrites the deprecated properties with the new ones from the project object. This behavior might lead to unexpected results if both sets of properties are provided.

Consider the following improvements:

  1. Add a warning log when both sets of properties are provided.
  2. Implement a clear precedence rule, e.g., prefer the new project object over deprecated properties.
  3. Document this behavior in the class or method documentation.

Example implementation:

if (opts.project) {
  if (this.publicKey || this.secretKey) {
    console.warn('Both deprecated and new project properties provided. Using new project properties.');
  }
  this.publicKey = opts.project.publicKey;
  this.secretKey = opts.project.secretKey;
}


const { apiHost = "api.pipedream.com" } = opts;
this.baseURL = `https://${apiHost}/v1`;
Expand All @@ -327,18 +355,21 @@ export class ServerClient {
{
oauthClientId: id,
oauthClientSecret: secret,
oauth,
}: CreateServerClientOpts,
tokenHost: string,
) {
if (!id || !secret) {
if (!oauth || !id || !secret) {
return;
}

const client = {
id: id ?? oauth.clientId,
secret: secret ?? oauth.clientSecret,
};

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix logical error in OAuth client configuration

The current implementation has a logical error that prevents the OAuth client from being configured when only the new oauth object is provided. To address this issue and ensure proper configuration, please apply the following changes:

-if (!oauth || !id || !secret) {
+if (!(oauth || (id && secret))) {
  return;
}

const client = {
-  id: id ?? oauth.clientId,
-  secret: secret ?? oauth.clientSecret,
+  id: oauth?.clientId ?? id,
+  secret: oauth?.clientSecret ?? secret,
};

This change ensures that the OAuth client is configured correctly when either the new oauth object or the deprecated properties are provided.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!oauth || !id || !secret) {
return;
}
const client = {
id: id ?? oauth.clientId,
secret: secret ?? oauth.clientSecret,
};
if (!(oauth || (id && secret))) {
return;
}
const client = {
id: oauth?.clientId ?? id,
secret: oauth?.clientSecret ?? secret,
};

this.oauthClient = new ClientCredentials({
client: {
id,
secret,
},
client,
auth: {
tokenHost,
tokenPath: "/v1/oauth/token",
Expand Down Expand Up @@ -716,7 +747,6 @@ export class ServerClient {

/**
* Invokes a workflow using the URL of its HTTP interface(s), by sending an
* HTTP POST request with the provided body.
*
* @param url - The URL of the workflow's HTTP interface.
* @param opts - The options for the request.
Expand Down Expand Up @@ -764,4 +794,66 @@ export class ServerClient {
body,
});
}

/**
* Invokes a workflow for a Pipedream Connect user in a project
*
* @param url - The URL of the workflow's HTTP interface.
* @param externalUserId — Your end user ID, for whom you're invoking the workflow.
* @param opts - The options for the request.
* @param opts.body - The body of the request. It must be a JSON-serializable
* value (e.g. an object, null, a string, etc.).
* @param opts.headers - The headers to include in the request. Note that the
* Authorization header will always be set with an OAuth access token
* retrieved by the client.
*
* @returns A promise resolving to the response from the workflow.
*
* @example
*
* ```typescript
* const response = await client.invokeWorkflow(
* "https://your-workflow-url.m.pipedream.net",
* "your-external-user-id",
* {
* body: {
* foo: 123,
* bar: "abc",
* baz: null,
* },
* headers: {
* "Accept": "application/json",
* },
* },
* );
* console.log(response);
* ```
*/
public async invokeWorkflowForExternalUser(url: string, externalUserId: string, opts: RequestOptions = {}): Promise<unknown> {
const {
body,
headers = {},
} = opts;

if (!externalUserId) {
throw new Error("External user ID is required");
}

if (!this.publicKey) {
throw new Error("Project public key is required to map the external user ID to the correct project");
}

return this.makeRequest("", {
...opts,
baseURL: url,
method: opts.method || "POST", // Default to POST if not specified
headers: {
...headers,
"Authorization": await this.oauthAuthorizationHeader(),
"X-PD-External-User-ID": externalUserId,
"X-PD-Project-Public-Key": this.publicKey,
},
body,
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add input validation for the url parameter

In the invokeWorkflowForExternalUser method, the url parameter is used without validation. To prevent runtime errors and enhance security, consider validating the url to ensure it is a well-formed URL.

Add URL validation:

 public async invokeWorkflowForExternalUser(
   url: string,
   externalUserId: string,
   opts: RequestOptions = {},
 ): Promise<unknown> {
+  try {
+    new URL(url);
+  } catch {
+    throw new Error(`Invalid URL provided: ${url}`);
+  }

   const {
     body,
     headers = {},
   } = opts;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public async invokeWorkflowForExternalUser(url: string, externalUserId: string, opts: RequestOptions = {}): Promise<unknown> {
const {
body,
headers = {},
} = opts;
if (!externalUserId) {
throw new Error("External user ID is required");
}
if (!this.publicKey) {
throw new Error("Project public key is required to map the external user ID to the correct project");
}
return this.makeRequest("", {
...opts,
baseURL: url,
method: opts.method || "POST", // Default to POST if not specified
headers: {
...headers,
"Authorization": await this.oauthAuthorizationHeader(),
"X-PD-External-User-ID": externalUserId,
"X-PD-Project-Public-Key": this.publicKey,
},
body,
});
}
public async invokeWorkflowForExternalUser(url: string, externalUserId: string, opts: RequestOptions = {}): Promise<unknown> {
try {
new URL(url);
} catch {
throw new Error(`Invalid URL provided: ${url}`);
}
const {
body,
headers = {},
} = opts;
if (!externalUserId) {
throw new Error("External user ID is required");
}
if (!this.publicKey) {
throw new Error("Project public key is required to map the external user ID to the correct project");
}
return this.makeRequest("", {
...opts,
baseURL: url,
method: opts.method || "POST", // Default to POST if not specified
headers: {
...headers,
"Authorization": await this.oauthAuthorizationHeader(),
"X-PD-External-User-ID": externalUserId,
"X-PD-Project-Public-Key": this.publicKey,
},
body,
});
}

}
Loading