Skip to content

Commit

Permalink
Move query building for access checks off the request handling path e…
Browse files Browse the repository at this point in the history
…xcept for list records where the current filter-where-clause construction doesn't allow for it.
  • Loading branch information
ignatz committed Dec 17, 2024
1 parent 312d161 commit bd0b33e
Show file tree
Hide file tree
Showing 5 changed files with 243 additions and 183 deletions.
8 changes: 4 additions & 4 deletions Cargo.lock

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

19 changes: 14 additions & 5 deletions trailbase-core/src/listing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::borrow::Cow;
use std::collections::HashMap;
use thiserror::Error;

use crate::records::json_to_sql::simple_json_value_to_param;
use crate::records::json_to_sql::json_string_to_value;
use crate::table_metadata::TableOrViewMetadata;
use crate::util::b64_to_id;

Expand Down Expand Up @@ -141,6 +141,16 @@ pub fn parse_query(query: Option<String>) -> Option<QueryParseResult> {
continue;
};

if !k
.chars()
.all(|c| c.is_alphanumeric() || c == '.' || c == '-' || c == '_')
{
#[cfg(debug_assertions)]
debug!("skipping non-trivial query param: {key}={value}");

continue;
}

if value.is_empty() {
continue;
}
Expand Down Expand Up @@ -183,6 +193,8 @@ pub fn build_filter_where_clause(
)));
}

// IMPORTANT: We only include parameters with known columns to avoid building an invalid
// query early and forbid injections.
let Some((col, _col_meta)) = table_metadata.column_by_name(&column_name) else {
return Err(WhereClauseError::UnrecognizedParam(format!(
"Unrecognized parameter: {column_name}"
Expand All @@ -195,10 +207,7 @@ pub fn build_filter_where_clause(
continue;
};

match simple_json_value_to_param(
col.data_type,
serde_json::Value::String(query_param.value.clone()),
) {
match json_string_to_value(col.data_type, query_param.value) {
Ok(value) => {
where_clauses.push(format!("{column_name} {op} :{column_name}"));
params.push((format!(":{column_name}").into(), value));
Expand Down
5 changes: 4 additions & 1 deletion trailbase-core/src/records/json_to_sql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,10 @@ fn try_json_array_to_blob(arr: &Vec<serde_json::Value>) -> Result<Value, ParamsE
return Ok(Value::Blob(byte_array));
}

fn json_string_to_value(data_type: ColumnDataType, value: String) -> Result<Value, ParamsError> {
pub(crate) fn json_string_to_value(
data_type: ColumnDataType,
value: String,
) -> Result<Value, ParamsError> {
return Ok(match data_type {
ColumnDataType::Null => Value::Null,
// Strict/storage types
Expand Down
16 changes: 6 additions & 10 deletions trailbase-core/src/records/list_records.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ use axum::{
extract::{Path, RawQuery, State},
Json,
};
use itertools::Itertools;
use std::borrow::Cow;
use trailbase_sqlite::Value;

use crate::app_state::AppState;
use crate::auth::user::User;
Expand Down Expand Up @@ -48,24 +50,19 @@ pub async fn list_records_handler(
.map_err(|_err| RecordError::BadRequest("Invalid filter params"))?;

if let Some(cursor) = cursor {
params.push((
Cow::Borrowed(":cursor"),
trailbase_sqlite::Value::Blob(cursor.to_vec()),
));
params.push((Cow::Borrowed(":cursor"), Value::Blob(cursor.to_vec())));
clause = format!("{clause} AND _ROW_.id < :cursor");
}

// User properties
params.extend_from_slice(&[
(
Cow::Borrowed(":limit"),
trailbase_sqlite::Value::Integer(limit_or_default(limit) as i64),
Value::Integer(limit_or_default(limit) as i64),
),
(
Cow::Borrowed(":__user_id"),
user.map_or(trailbase_sqlite::Value::Null, |u| {
trailbase_sqlite::Value::Blob(u.uuid.into())
}),
user.map_or(Value::Null, |u| Value::Blob(u.uuid.into())),
),
]);

Expand All @@ -75,7 +72,7 @@ pub async fn list_records_handler(
// TODO: Should this be a separate access rule? Maybe one wants users to access a specific
// record but not list all the records.
if let Some(read_access) = api.access_rule(Permission::Read) {
clause = format!("({clause}) AND {read_access}");
clause = format!("({read_access}) AND ({clause})");
}

let default_ordering = || {
Expand All @@ -94,7 +91,6 @@ pub async fn list_records_handler(
}
)
})
.collect::<Vec<_>>()
.join(", ");

let query = format!(
Expand Down
Loading

0 comments on commit bd0b33e

Please sign in to comment.