Skip to content
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

Enhance Airbyte Integration with Connection Checks and Conditional Features #626

Merged
merged 4 commits into from
Nov 11, 2024

Conversation

iandjx
Copy link
Collaborator

@iandjx iandjx commented Nov 1, 2024

  • Added a function to check Airbyte connection status and integrated it into various components.
  • Enhanced DatasourceTable and Datasources pages to conditionally enable features based on Airbyte availability.
  • Implemented a new route and controller method to handle Airbyte connection checks.
  • Updated Airbyte setup to include status checks and initialization logic.

@iandjx iandjx changed the title Add check on airbyte availability and run airbyte setup init when needed Enhance Airbyte Integration with Connection Checks and Conditional Features Nov 1, 2024
Copy link

github-actions bot commented Nov 1, 2024

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

Minimum allowed coverage is 80%

Generated by 🐒 cobertura-action against 6728867

@iandjx iandjx marked this pull request as ready for review November 1, 2024 07:27
@@ -37,6 +40,12 @@
const [deletingMap, setDeletingMap] = useState({});
const [confirmClose, setConfirmClose] = useState(false);

const goToDatasourcePage = (id: string) => {
if (isAirbyteEnabled) {
router.push(`/${resourceSlug}/datasource/${id}`);

Check warning

Code scanning / CodeQL

Client-side URL redirect Medium

Untrusted URL redirection depends on a
user-provided value
.

Copilot Autofix AI 9 days ago

To fix the problem, we should avoid using user input directly in constructing the redirect URL. Instead, we can maintain a list of authorized redirects and choose from that list based on the user input. This ensures that only valid and safe URLs are used for redirection.

  1. Create a list of authorized resourceSlug values.
  2. Check if the resourceSlug from router.query is in the list of authorized values.
  3. Only perform the redirection if the resourceSlug is authorized.
Suggested changeset 1
webapp/src/components/DatasourceTable.tsx

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/components/DatasourceTable.tsx b/webapp/src/components/DatasourceTable.tsx
--- a/webapp/src/components/DatasourceTable.tsx
+++ b/webapp/src/components/DatasourceTable.tsx
@@ -42,5 +42,8 @@
 
+	const authorizedResourceSlugs = ['validSlug1', 'validSlug2']; // Add all authorized slugs here
 	const goToDatasourcePage = (id: string) => {
-		if (isAirbyteEnabled) {
+		if (isAirbyteEnabled && authorizedResourceSlugs.includes(resourceSlug)) {
 			router.push(`/${resourceSlug}/datasource/${id}`);
+		} else {
+			toast.error('Unauthorized resource slug');
 		}
EOF
@@ -42,5 +42,8 @@

const authorizedResourceSlugs = ['validSlug1', 'validSlug2']; // Add all authorized slugs here
const goToDatasourcePage = (id: string) => {
if (isAirbyteEnabled) {
if (isAirbyteEnabled && authorizedResourceSlugs.includes(resourceSlug)) {
router.push(`/${resourceSlug}/datasource/${id}`);
} else {
toast.error('Unauthorized resource slug');
}
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@ragyabraham
Copy link
Contributor

@iandjx

This approach looks to test the "/api/v1/health" endpoint. There might be a situation where the health check returns 200 but the bearer token stored in the webapp is invalid (since the health check endpoint doesn't require auth). Might be a good idea to add a second endpoint check to ensure that both airbyte is up and that the webapp has a valid bearer token

…t request, adding ENV variable to env example to ensure that lack of the variable doesn't break the app
@NaderRNA
Copy link
Collaborator

NaderRNA commented Nov 4, 2024

@iandjx

This approach looks to test the "/api/v1/health" endpoint. There might be a situation where the health check returns 200 but the bearer token stored in the webapp is invalid (since the health check endpoint doesn't require auth). Might be a good idea to add a second endpoint check to ensure that both airbyte is up and that the webapp has a valid bearer token

Addressed these issues in latest commits.

@NaderRNA NaderRNA merged commit c0a5823 into develop Nov 11, 2024
7 checks passed
@NaderRNA NaderRNA deleted the handle-airbyte-availability branch November 11, 2024 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants