Skip to content

Commit

Permalink
feat: allow s3 file download/preview from inside apps (#5004)
Browse files Browse the repository at this point in the history
* feat: allow s3 file download/preview from inside apps

* improve security + handle image preview

* fix build

* fix build
  • Loading branch information
HugoCasa authored Jan 4, 2025
1 parent e8fcea2 commit 0c19171
Show file tree
Hide file tree
Showing 7 changed files with 210 additions and 11 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion backend/ee-repo-ref.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
586b02014d57f862a5c4313dd1e529d50c315c30
a1094bec38924de76936392b1148829b02628174
134 changes: 132 additions & 2 deletions backend/windmill-api/src/apps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,14 @@ use crate::{
#[cfg(feature = "parquet")]
use crate::{
job_helpers_ee::{
get_random_file_name, get_s3_resource, get_workspace_s3_resource, upload_file_from_req,
UploadFileResponse,
download_s3_file_internal, get_random_file_name, get_s3_resource,
get_workspace_s3_resource, load_image_preview_internal, upload_file_from_req,
DownloadFileQuery, LoadImagePreviewQuery, UploadFileResponse,
},
users::fetch_api_authed_from_permissioned_as,
};
#[cfg(feature = "parquet")]
use axum::response::Response;
use axum::{
extract::{Extension, Json, Path, Query},
response::IntoResponse,
Expand Down Expand Up @@ -93,6 +96,11 @@ pub fn unauthed_service() -> Router {
Router::new()
.route("/execute_component/*path", post(execute_component))
.route("/upload_s3_file/*path", post(upload_s3_file_from_app))
.route("/download_s3_file/*path", get(download_s3_file_from_app))
.route(
"/load_image_preview/*path",
get(load_s3_file_image_preview_from_app),
)
.route("/public_app/:secret", get(get_public_app_by_secret))
.route("/public_resource/*path", get(get_public_resource))
}
Expand Down Expand Up @@ -1718,6 +1726,128 @@ async fn upload_s3_file_from_app(
return Ok(Json(UploadFileResponse { file_key }));
}

#[cfg(not(feature = "parquet"))]
async fn download_s3_file_from_app() -> Result<()> {
return Err(Error::BadRequest(
"This endpoint requires the parquet feature to be enabled".to_string(),
));
}

#[cfg(feature = "parquet")]
async fn get_on_behalf_authed_from_app(
db: &DB,
path: &str,
w_id: &str,
opt_authed: &Option<ApiAuthed>,
) -> Result<ApiAuthed> {
let policy_o = sqlx::query_scalar!(
"SELECT policy from app WHERE path = $1 AND workspace_id = $2",
path,
w_id
)
.fetch_optional(db)
.await?;

let policy = policy_o
.map(|p| serde_json::from_value::<Policy>(p).map_err(to_anyhow))
.transpose()?
.unwrap_or_else(|| Policy {
execution_mode: ExecutionMode::Viewer,
triggerables: None,
triggerables_v2: None,
on_behalf_of: None,
on_behalf_of_email: None,
s3_inputs: None,
});

let (username, permissioned_as, email) =
get_on_behalf_details_from_policy_and_authed(&policy, &opt_authed).await?;

let on_behalf_authed =
fetch_api_authed_from_permissioned_as(permissioned_as, email, &w_id, &db, Some(username))
.await?;

Ok(on_behalf_authed)
}

#[cfg(feature = "parquet")]
async fn check_if_allowed_to_access_s3_file_from_app(
db: &DB,
opt_authed: &Option<ApiAuthed>,
file_key: &str,
w_id: &str,
path: &str,
) -> Result<()> {
// if anonymous, check that the file was the result of an app script ran by an anonymous user in the last 3 hours
// otherwise, if logged in, allow any file (TODO: change that when we implement better s3 policy)

let allowed = opt_authed.is_some()
|| sqlx::query_scalar!(
r#"SELECT EXISTS (
SELECT 1 FROM completed_job
WHERE workspace_id = $2
AND (job_kind = 'appscript' OR job_kind = 'preview')
AND created_by = 'anonymous'
AND started_at > now() - interval '3 hours'
AND script_path LIKE $3 || '/%'
AND result @> ('{"s3":"' || $1 || '"}')::jsonb
)"#,
file_key,
w_id,
path,
)
.fetch_one(db)
.await?
.unwrap_or(false);

if !allowed {
Err(Error::BadRequest("File restricted".to_string()))
} else {
Ok(())
}
}

#[cfg(feature = "parquet")]
async fn download_s3_file_from_app(
OptAuthed(opt_authed): OptAuthed,
Extension(db): Extension<DB>,
Path((w_id, path)): Path<(String, StripPath)>,
Query(query): Query<DownloadFileQuery>,
) -> Result<Response> {
let path = path.to_path();

let on_behalf_authed = get_on_behalf_authed_from_app(&db, &path, &w_id, &opt_authed).await?;

check_if_allowed_to_access_s3_file_from_app(&db, &opt_authed, &query.file_key, &w_id, &path)
.await?;

download_s3_file_internal(on_behalf_authed, &db, None, "", &w_id, query).await
}

#[cfg(not(feature = "parquet"))]
async fn load_s3_file_image_preview_from_app() -> Result<()> {
return Err(Error::BadRequest(
"This endpoint requires the parquet feature to be enabled".to_string(),
));
}

#[cfg(feature = "parquet")]
async fn load_s3_file_image_preview_from_app(
OptAuthed(opt_authed): OptAuthed,
Extension(db): Extension<DB>,
Path((w_id, path)): Path<(String, StripPath)>,
Query(query): Query<LoadImagePreviewQuery>,
) -> Result<Response> {
let path = path.to_path();

let on_behalf_authed = get_on_behalf_authed_from_app(&db, &path, &w_id, &opt_authed).await?;

check_if_allowed_to_access_s3_file_from_app(&db, &opt_authed, &query.file_key, &w_id, &path)
.await?;

load_image_preview_internal(on_behalf_authed, &db, "", &w_id, query).await
}

fn get_on_behalf_of(policy: &Policy) -> Result<(String, String)> {
let permissioned_as = policy
.on_behalf_of
Expand Down
42 changes: 42 additions & 0 deletions backend/windmill-api/src/job_helpers_ee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,26 @@ use bytes::Bytes;
#[cfg(feature = "parquet")]
use futures::Stream;

#[cfg(feature = "parquet")]
use axum::response::Response;
#[cfg(feature = "parquet")]
use serde::Deserialize;

#[derive(Serialize)]
pub struct UploadFileResponse {
pub file_key: String,
}

#[derive(Deserialize)]
pub struct LoadImagePreviewQuery {
pub file_key: String,
}

#[derive(Deserialize)]
pub struct DownloadFileQuery {
pub file_key: String,
}

pub fn workspaced_service() -> Router {
Router::new()
}
Expand Down Expand Up @@ -82,3 +97,30 @@ pub async fn upload_file_internal(
"Not implemented in Windmill's Open Source repository".to_string(),
))
}

#[cfg(feature = "parquet")]
pub async fn download_s3_file_internal(
_authed: ApiAuthed,
_db: &DB,
_user_db: Option<UserDB>,
_token: &str,
_w_id: &str,
_query: DownloadFileQuery,
) -> error::Result<Response> {
Err(error::Error::InternalErr(
"Not implemented in Windmill's Open Source repository".to_string(),
))
}

#[cfg(feature = "parquet")]
pub async fn load_image_preview_internal(
_authed: ApiAuthed,
_db: &DB,
_token: &str,
_w_id: &str,
_query: LoadImagePreviewQuery,
) -> error::Result<Response> {
Err(error::Error::InternalErr(
"Not implemented in Windmill's Open Source repository".to_string(),
))
}
9 changes: 5 additions & 4 deletions frontend/src/lib/components/DisplayResult.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
export let drawerOpen = false
export let nodeId: string | undefined = undefined
export let language: string | undefined = undefined
export let appPath: string | undefined = undefined
const IMG_MAX_SIZE = 10000000
const TABLE_MAX_SIZE = 5000000
Expand Down Expand Up @@ -645,7 +646,7 @@
>
</button>
{:else if !result?.disable_download}
<FileDownload {workspaceId} s3object={result} />
<FileDownload {workspaceId} s3object={result} {appPath} />
<button
class="text-secondary underline text-2xs whitespace-nowrap"
on:click={() => {
Expand All @@ -662,7 +663,7 @@
{/if}
</div>
{#if typeof result?.s3 === 'string'}
{#if result?.s3?.endsWith('.parquet') || result?.s3?.endsWith('.csv')}
{#if !appPath && (result?.s3?.endsWith('.parquet') || result?.s3?.endsWith('.csv'))}
{#key result.s3}
<ParqetTableRenderer
disable_download={result?.disable_download}
Expand All @@ -676,15 +677,15 @@
<img
alt="preview rendered"
class="w-auto h-full"
src={`/api/w/${workspaceId}/job_helpers/load_image_preview?file_key=${result.s3}` +
src={`/api/w/${workspaceId}/${appPath ? 'apps_u/load_image_preview/' + appPath : 'job_helpers/load_image_preview'}?file_key=${result.s3}` +
(result.storage ? `&storage=${result.storage}` : '')}
/>
</div>
{:else if result?.s3?.endsWith('.pdf')}
<div class="h-96 mt-2 border">
<PdfViewer
allowFullscreen
source={`/api/w/${workspaceId}/job_helpers/load_image_preview?file_key=${result.s3}` +
source={`/api/w/${workspaceId}/${appPath ? 'apps_u/load_image_preview/' + appPath : 'job_helpers/load_image_preview'}?file_key=${result.s3}` +
(result.storage ? `&storage=${result.storage}` : '')}
/>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
export let configuration: RichConfigurations
const requireHtmlApproval = getContext<boolean | undefined>(IS_APP_PUBLIC_CONTEXT_KEY)
const { app, worldStore, componentControl, workspace } =
const { app, worldStore, componentControl, workspace, appPath } =
getContext<AppViewerContext>('AppViewerContext')
let result: any = undefined
Expand Down Expand Up @@ -96,6 +96,7 @@
{result}
{requireHtmlApproval}
disableExpand={resolvedConfig?.hideDetails}
appPath={$appPath}
/>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,17 @@
export let s3object: any
export let workspaceId: string | undefined = undefined
export let appPath: string | undefined = undefined
</script>

<a
class="relative center-center flex w-full text-center font-normal text-tertiary text-sm
border border-dashed border-gray-400 hover:border-blue-500
focus-within:border-blue-500 hover:bg-blue-50 dark:hover:bg-frost-900 focus-within:bg-blue-50
duration-200 rounded-lg p-1 gap-2"
href={`${base}/api/w/${workspaceId ?? $workspaceStore}/job_helpers/download_s3_file?file_key=${
s3object?.s3
}${s3object?.storage ? `&storage=${s3object.storage}` : ''}`}
href={`${base}/api/w/${workspaceId ?? $workspaceStore}${
appPath ? `/apps_u/download_s3_file/${appPath}` : '/job_helpers/download_s3_file'
}?file_key=${s3object?.s3}${s3object?.storage ? `&storage=${s3object.storage}` : ''}`}
download={s3object?.s3.split('/').pop() ?? 'unnamed_download.file'}
>
<Download />
Expand Down

0 comments on commit 0c19171

Please sign in to comment.