Skip to content

603 better oauth datasources - #611

Draft
NaderRNA wants to merge 40 commits into
developfrom
603-better_oauth_datasources
Draft

603 better oauth datasources#611
NaderRNA wants to merge 40 commits into
developfrom
603-better_oauth_datasources

Conversation

@NaderRNA

@NaderRNA NaderRNA commented Oct 15, 2024

Copy link
Copy Markdown
Collaborator

Ability to connect to datasources (airbyte) using OAuth login page

Uses passport.js to authenticate the OAuth, gets the refreshToken (which is the only thing airbyte needs to establish a connection to a source with OAuth).

Aims to keep current datasource flow as unchanged as possible.

We only have client ID and client Secrets for the following which will be the only ones supported:
(crossed out oauth providers are implemented)

  • Hubspot
  • Salesforce (forcedotcom)
  • Airtable
  • Google Ads (not sure if we have an approved clientId and secret)
  • Xero
  • Slack

@NaderRNA
NaderRNA changed the base branch from master to develop October 15, 2024 22:39
@github-actions

github-actions Bot commented Oct 15, 2024

Copy link
Copy Markdown

File Coverage
All files 96%
src/lib/utils/validationutils.ts 95%

Minimum allowed coverage is 80%

Generated by 🐒 cobertura-action against b4f9ea2

@github-advanced-security github-advanced-security AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ESLint found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

…handling of new route to get the redirect url for oauth to be completed
Comment thread webapp/src/router.ts
server.get('/register', unauthedMiddlewareChain, checkSessionWelcome, renderStaticPage(app, '/register'));
server.get(
'/login',
unauthedMiddlewareChain,

Check failure

Code scanning / CodeQL

Missing rate limiting

This route handler performs [authorization](1), but is not rate-limited.
Comment thread webapp/src/router.ts
);
server.get(
'/register',
unauthedMiddlewareChain,

Check failure

Code scanning / CodeQL

Missing rate limiting

This route handler performs [authorization](1), but is not rate-limited.

Copilot Autofix

AI almost 2 years ago

To fix the problem, we need to introduce rate limiting to the routes that use the unauthedMiddlewareChain. The best way to do this is by using the express-rate-limit package, which allows us to easily set up rate limiting middleware. We will configure a rate limiter to allow a maximum of 100 requests per 15 minutes and apply it to the relevant routes.

  1. Install the express-rate-limit package.
  2. Import the express-rate-limit package in the webapp/src/router.ts file.
  3. Configure the rate limiter with appropriate settings.
  4. Apply the rate limiter to the routes that use the unauthedMiddlewareChain.
Suggested changeset 2
webapp/src/router.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/webapp/src/router.ts b/webapp/src/router.ts
--- a/webapp/src/router.ts
+++ b/webapp/src/router.ts
@@ -33,2 +33,8 @@
 import { PlanLimitsKeys, pricingMatrix, SubscriptionPlan } from 'struct/billing';
+import rateLimit from 'express-rate-limit';
+
+const limiter = rateLimit({
+    windowMs: 15 * 60 * 1000, // 15 minutes
+    max: 100, // limit each IP to 100 requests per windowMs
+});
 
@@ -236,3 +242,3 @@
 	// Non team endpoints
-	server.get('/', unauthedMiddlewareChain, homeRedirect);
+	server.get('/', limiter, unauthedMiddlewareChain, homeRedirect);
 	server.get(
@@ -245,2 +251,3 @@
 		'/register',
+		limiter,
 		unauthedMiddlewareChain,
@@ -249,5 +256,6 @@
 	);
-	server.get('/verify', unauthedMiddlewareChain, renderStaticPage(app, '/verify'));
+	server.get('/verify', limiter, unauthedMiddlewareChain, renderStaticPage(app, '/verify'));
 	server.get(
 		'/account',
+		limiter,
 		unauthedMiddlewareChain,
@@ -261,2 +269,3 @@
 		'/welcome',
+		limiter,
 		unauthedMiddlewareChain,
EOF
@@ -33,2 +33,8 @@
import { PlanLimitsKeys, pricingMatrix, SubscriptionPlan } from 'struct/billing';
import rateLimit from 'express-rate-limit';

const limiter = rateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 100, // limit each IP to 100 requests per windowMs
});

@@ -236,3 +242,3 @@
// Non team endpoints
server.get('/', unauthedMiddlewareChain, homeRedirect);
server.get('/', limiter, unauthedMiddlewareChain, homeRedirect);
server.get(
@@ -245,2 +251,3 @@
'/register',
limiter,
unauthedMiddlewareChain,
@@ -249,5 +256,6 @@
);
server.get('/verify', unauthedMiddlewareChain, renderStaticPage(app, '/verify'));
server.get('/verify', limiter, unauthedMiddlewareChain, renderStaticPage(app, '/verify'));
server.get(
'/account',
limiter,
unauthedMiddlewareChain,
@@ -261,2 +269,3 @@
'/welcome',
limiter,
unauthedMiddlewareChain,
webapp/package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/webapp/package.json b/webapp/package.json
--- a/webapp/package.json
+++ b/webapp/package.json
@@ -127,3 +127,4 @@
     "tsconfig-paths": "^4.2.0",
-    "uuid": "^9.0.1"
+    "uuid": "^9.0.1",
+    "express-rate-limit": "^7.4.1"
   },
EOF
@@ -127,3 +127,4 @@
"tsconfig-paths": "^4.2.0",
"uuid": "^9.0.1"
"uuid": "^9.0.1",
"express-rate-limit": "^7.4.1"
},
This fix introduces these dependencies
Package Version Security advisories
express-rate-limit (npm) 7.4.1 None
Copilot is powered by AI and may make mistakes. Always verify output.
Comment thread webapp/src/controllers/airbyte.ts Fixed
Comment thread webapp/src/controllers/airbyte.ts Fixed
Comment thread webapp/src/components/connectorform/DynamicConnectorForm.tsx Fixed
@NaderRNA NaderRNA linked an issue Oct 21, 2024 that may be closed by this pull request
@NaderRNA NaderRNA self-assigned this Oct 21, 2024
…ill need to patch bugs, fix issues, make dynamic
Comment thread webapp/src/router.ts Fixed
Comment thread webapp/src/components/connectorform/DynamicConnectorForm.tsx Fixed
@NaderRNA

NaderRNA commented Oct 22, 2024

Copy link
Copy Markdown
Collaborator Author

Current status of the code:

  1. Successfully authenticates the user with specifically HubSpot currently.
  2. Passes the refreshToken back through the callback function (OAuth controller)
  3. Callback redirects back to datasource/add page with information required to set up datasource
  4. CreateDatasourceForm makes a POST with the sourceConfig set to the required config for the given oauth connector
  5. That then creates the TestDatasource object using the OAuth credentials
  6. Continue datasource creation as standard using the existing flow

The next few commits will likely be refactoring the implementation to do the following in order:

  1. pass the datasource name and datasource description through the flow
  2. Provide the user with better feedback indicating what's happening (adding a new loader for OAuth to say that authentication was successful and Agent Cloud is testing the datasource)
  3. Make the entire flow more compatible for other oauth providers, not just hubspot

Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/lib/middleware/auth/fetchsession.ts Fixed
Comment thread webapp/src/lib/middleware/auth/fetchsession.ts Fixed
Comment thread webapp/src/lib/oauthsecret/index.ts Fixed
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/lib/oauthsecret/index.ts Fixed
Comment thread webapp/src/lib/struct/oauth.ts Fixed
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/lib/oauthsecret/index.ts Fixed
Comment thread webapp/src/lib/oauthsecret/index.ts Fixed
Comment thread webapp/src/lib/oauthsecret/index.ts Fixed
Comment thread webapp/src/lib/oauthsecret/index.ts Fixed
Comment thread webapp/src/lib/oauthsecret/index.ts Fixed
Comment thread webapp/src/lib/oauthsecret/index.ts Fixed
Comment thread webapp/src/router.ts Fixed
Comment thread webapp/src/router.ts Fixed
Comment thread webapp/src/router.ts
oauthRouter.get(
'/hubspot/callback',
useSession,
useJWT,

Check failure

Code scanning / CodeQL

Missing rate limiting

This route handler performs [authorization](1), but is not rate-limited.

Copilot Autofix

AI almost 2 years ago

To fix the problem, we need to introduce rate limiting to the routes that use the useJWT middleware. The best way to achieve this is by using the express-rate-limit package, which allows us to easily set up and apply rate limiting to specific routes or the entire application.

  1. Install the express-rate-limit package.
  2. Import the express-rate-limit package in the webapp/src/router.ts file.
  3. Set up a rate limiter with appropriate configuration (e.g., maximum of 100 requests per 15 minutes).
  4. Apply the rate limiter to the routes that use the useJWT middleware.
Suggested changeset 2
webapp/src/router.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/webapp/src/router.ts b/webapp/src/router.ts
--- a/webapp/src/router.ts
+++ b/webapp/src/router.ts
@@ -33,2 +33,8 @@
 import { PlanLimitsKeys, pricingMatrix, SubscriptionPlan } from 'struct/billing';
+import RateLimit from 'express-rate-limit';
+
+const limiter = RateLimit({
+	windowMs: 15 * 60 * 1000, // 15 minutes
+	max: 100, // max 100 requests per windowMs
+});
 
@@ -103,2 +109,3 @@
 		'/google/callback',
+		limiter,
 		useSession,
@@ -130,2 +137,3 @@
 		'/hubspot/callback',
+		limiter,
 		useSession,
EOF
@@ -33,2 +33,8 @@
import { PlanLimitsKeys, pricingMatrix, SubscriptionPlan } from 'struct/billing';
import RateLimit from 'express-rate-limit';

const limiter = RateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 100, // max 100 requests per windowMs
});

@@ -103,2 +109,3 @@
'/google/callback',
limiter,
useSession,
@@ -130,2 +137,3 @@
'/hubspot/callback',
limiter,
useSession,
webapp/package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/webapp/package.json b/webapp/package.json
--- a/webapp/package.json
+++ b/webapp/package.json
@@ -127,3 +127,4 @@
     "tsconfig-paths": "^4.2.0",
-    "uuid": "^9.0.1"
+    "uuid": "^9.0.1",
+    "express-rate-limit": "^7.4.1"
   },
EOF
@@ -127,3 +127,4 @@
"tsconfig-paths": "^4.2.0",
"uuid": "^9.0.1"
"uuid": "^9.0.1",
"express-rate-limit": "^7.4.1"
},
This fix introduces these dependencies
Package Version Security advisories
express-rate-limit (npm) 7.4.1 None
Copilot is powered by AI and may make mistakes. Always verify output.
Comment thread webapp/src/router.ts
oauthRouter.get(
'/salesforce/callback',
useSession,
useJWT,

Check failure

Code scanning / CodeQL

Missing rate limiting

This route handler performs [authorization](1), but is not rate-limited.
Comment thread webapp/src/router.ts
oauthRouter.get(
'/xero/callback',
useSession,
useJWT,

Check failure

Code scanning / CodeQL

Missing rate limiting

This route handler performs [authorization](1), but is not rate-limited.

Copilot Autofix

AI almost 2 years ago

To fix the problem, we need to introduce rate limiting to the route handlers that perform authorization. The best way to do this is by using the express-rate-limit package, which allows us to easily set up rate limiting middleware. We will configure the rate limiter to allow a maximum of 100 requests per 15 minutes and apply it to the relevant routes.

  1. Install the express-rate-limit package.
  2. Import the express-rate-limit package in the webapp/src/router.ts file.
  3. Set up the rate limiter with the desired configuration.
  4. Apply the rate limiter to the relevant routes.
Suggested changeset 2
webapp/src/router.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/webapp/src/router.ts b/webapp/src/router.ts
--- a/webapp/src/router.ts
+++ b/webapp/src/router.ts
@@ -33,2 +33,8 @@
 import { PlanLimitsKeys, pricingMatrix, SubscriptionPlan } from 'struct/billing';
+import RateLimit from 'express-rate-limit';
+
+const limiter = RateLimit({
+    windowMs: 15 * 60 * 1000, // 15 minutes
+    max: 100, // max 100 requests per windowMs
+});
 
@@ -130,2 +136,3 @@
 		'/hubspot/callback',
+		limiter,
 		useSession,
@@ -151,2 +158,3 @@
 		'/salesforce/callback',
+		limiter,
 		useSession,
@@ -172,2 +180,3 @@
 		'/xero/callback',
+		limiter,
 		useSession,
EOF
@@ -33,2 +33,8 @@
import { PlanLimitsKeys, pricingMatrix, SubscriptionPlan } from 'struct/billing';
import RateLimit from 'express-rate-limit';

const limiter = RateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 100, // max 100 requests per windowMs
});

@@ -130,2 +136,3 @@
'/hubspot/callback',
limiter,
useSession,
@@ -151,2 +158,3 @@
'/salesforce/callback',
limiter,
useSession,
@@ -172,2 +180,3 @@
'/xero/callback',
limiter,
useSession,
webapp/package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/webapp/package.json b/webapp/package.json
--- a/webapp/package.json
+++ b/webapp/package.json
@@ -127,3 +127,4 @@
     "tsconfig-paths": "^4.2.0",
-    "uuid": "^9.0.1"
+    "uuid": "^9.0.1",
+    "express-rate-limit": "^7.4.1"
   },
EOF
@@ -127,3 +127,4 @@
"tsconfig-paths": "^4.2.0",
"uuid": "^9.0.1"
"uuid": "^9.0.1",
"express-rate-limit": "^7.4.1"
},
This fix introduces these dependencies
Package Version Security advisories
express-rate-limit (npm) 7.4.1 None
Copilot is powered by AI and may make mistakes. Always verify output.
Comment thread webapp/src/router.ts
oauthRouter.get(
'/airtable/callback',
useSession,
useJWT,

Check failure

Code scanning / CodeQL

Missing rate limiting

This route handler performs [authorization](1), but is not rate-limited.

Copilot Autofix

AI almost 2 years ago

To fix the problem, we need to introduce rate limiting to the routes that use the useJWT middleware. The best way to do this is by using the express-rate-limit package, which allows us to easily set up rate limiting for our Express routes. We will create a rate limiter and apply it to the relevant routes to ensure that they are protected against excessive requests.

  1. Install the express-rate-limit package if it is not already installed.
  2. Import the express-rate-limit package in the webapp/src/router.ts file.
  3. Create a rate limiter with appropriate settings (e.g., maximum of 100 requests per 15 minutes).
  4. Apply the rate limiter to the routes that use the useJWT middleware.
Suggested changeset 2
webapp/src/router.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/webapp/src/router.ts b/webapp/src/router.ts
--- a/webapp/src/router.ts
+++ b/webapp/src/router.ts
@@ -33,2 +33,8 @@
 import { PlanLimitsKeys, pricingMatrix, SubscriptionPlan } from 'struct/billing';
+import RateLimit from 'express-rate-limit';
+
+const limiter = RateLimit({
+    windowMs: 15 * 60 * 1000, // 15 minutes
+    max: 100, // max 100 requests per windowMs
+});
 
@@ -172,2 +178,3 @@
 		'/xero/callback',
+		limiter,
 		useSession,
@@ -208,2 +215,3 @@
 		'/airtable/callback',
+		limiter,
 		useSession,
EOF
@@ -33,2 +33,8 @@
import { PlanLimitsKeys, pricingMatrix, SubscriptionPlan } from 'struct/billing';
import RateLimit from 'express-rate-limit';

const limiter = RateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 100, // max 100 requests per windowMs
});

@@ -172,2 +178,3 @@
'/xero/callback',
limiter,
useSession,
@@ -208,2 +215,3 @@
'/airtable/callback',
limiter,
useSession,
webapp/package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/webapp/package.json b/webapp/package.json
--- a/webapp/package.json
+++ b/webapp/package.json
@@ -127,3 +127,4 @@
     "tsconfig-paths": "^4.2.0",
-    "uuid": "^9.0.1"
+    "uuid": "^9.0.1",
+    "express-rate-limit": "^7.4.1"
   },
EOF
@@ -127,3 +127,4 @@
"tsconfig-paths": "^4.2.0",
"uuid": "^9.0.1"
"uuid": "^9.0.1",
"express-rate-limit": "^7.4.1"
},
This fix introduces these dependencies
Package Version Security advisories
express-rate-limit (npm) 7.4.1 None
Copilot is powered by AI and may make mistakes. Always verify output.
@NaderRNA

NaderRNA commented Nov 5, 2024

Copy link
Copy Markdown
Collaborator Author

Somewhere along the line here a bug was introduced that replaced the file upload/drop area with the connectors dropdown

@ragyabraham

Copy link
Copy Markdown
Contributor

@NaderRNA what's happening with this PR?? where are we with this??

@NaderRNA

NaderRNA commented Nov 12, 2024

Copy link
Copy Markdown
Collaborator Author

Currently the branch supports custom OAuth implementation for any OAuth provider where we have a Client ID and Client Secret for the oauth. Currently HubSpot is completley functional using OAuth but there are some issues with out Airtable, Salesforce and Xero app credentials so I'm just trying to iron those out with Andrew.
If you'd like a version that can go to prod alongside BYO VectorDBs I can modify this code so it won't break prod and will work for HubSpot.
But currently if you know the OAuth provider endpoint and required scopes you can create any custom OAuth implementation using this implementation of the code, nothing is hardcoded so introduction of new OAuth implementation is a lot easier for both account creation and datasource auth.

Comment thread webapp/src/components/CreateDatasourceForm.tsx Fixed
Comment thread webapp/src/components/CreateDatasourceForm.tsx Fixed
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/lib/middleware/passportmanager.ts Fixed
Comment thread webapp/src/lib/middleware/passportmanager.ts Fixed
Comment thread webapp/src/lib/middleware/passportmanager.ts Fixed
Comment thread webapp/src/lib/middleware/passportmanager.ts Fixed
Comment thread webapp/src/lib/middleware/passportmanager.ts Fixed
This reverts commit cea520e.
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/controllers/oauth.ts Fixed
Comment thread webapp/src/lib/struct/oauth.ts Fixed
@NaderRNA

NaderRNA commented Nov 21, 2024

Copy link
Copy Markdown
Collaborator Author

reverting back because Airtable was creating more problems as I was implementing it. Using custom strategies on passport seems to be broken with our current versions of passport.js. To use airtable or anything not supported by passport.js, a custom implementation is required that skips passport implementation and is completely end to end handled by the webapp

@ragyabraham

Copy link
Copy Markdown
Contributor

@iandjx where are we at with this.... this would be a really good addition

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

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

Custom Passport.js strategies for unsupported OAuth providers Better OAuth implementation for datasource/airbyte

4 participants