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

Parameter interpolation in datalog blocks #66

Closed
divarvel opened this issue May 6, 2022 · 3 comments
Closed

Parameter interpolation in datalog blocks #66

divarvel opened this issue May 6, 2022 · 3 comments

Comments

@divarvel
Copy link
Collaborator

divarvel commented May 6, 2022

Currently, datalog is constructed from strings that are parsed at runtime. Integrating dynamic values in those strings can be done two ways:

  1. string concatenation: this is heavily discouraged for security reasons. all the SQL injection issues can apply, and it requires great care with escaping / formatting data
  2. variable substitution: the trick is to put datalog variables as placeholders, and then to call .set() on the fact (or rule, or check, or policy) to replace relevant variables
let f = Fact::from_str("parent(\"alice\", $child)")?.set("child", "Bob")?;

While the option 2 prevents injections and security issues, it's not satisfying for a couple reasons:

  • it repurposes datalog variables into something they are not:
    • that can be confusing
    • that can lead to collisions between regular variable names and variable-only-really-used-for-interpolation names
  • it only works were variables can appear (that will be an issue with 3rd party blocks, where we will want to inject public keys in scope annotations)

I suggest a dedicated interpolation syntax instead, which:

  • makes it clear we're injecting values from the host language
  • avoids collisions between datalog variable names and parameter names
  • can appear in places variables can't
  • will play nice with a macro for compile-time-parsing

Incidentally, that's what is done in the haskell library (for now parameter interpolation only works for compile-time parsing)

Proposed solution

  • add a new Param(String) alternative in datalog::Term (we don't need string interning here)
  • recognize {paramName} in the datalog syntax where it makes sense (currently, everywhere a term can appear, including inside set literals)
  • either:
    • add a set(Map<String, impl ToParamValue>) function on BlockBuilder and AuthorizerBuilder that would walk the tree and perform replacements. The ToParamValue trait would allow directly passing regular rust values without having to wrap them in a Term manually (and would also, when 3rd party blocks are implemented, let the caller provide a public key where it makes sense)
    • add a Map<String, impl ToParamValue> parameter to BiscuitBuilder
  • check that all parameters have been replaced when the builder is used

Currently there are multiple ways to do the same things, when it comes to parsing datalog from strings. This feature could be the occasion to rationalize things a bit. Especially since this feature will be a first step towards compile-time parsing

@Geal
Copy link
Contributor

Geal commented May 6, 2022

that looks fine. I'm not sure ToParamValue is needed, Into<Term> should be enough since there are From implementations for Term already https://docs.rs/biscuit-auth/latest/biscuit_auth/builder/enum.Term.html#impl-From%3C%26%27_%20%5Bu8%5D%3E

@divarvel
Copy link
Collaborator Author

divarvel commented May 6, 2022

Yeah, it would only be needed for public keys in scope declarations (these are not terms)

@divarvel
Copy link
Collaborator Author

Done in #69

It will require updates with third party blocks, but we're not there yet

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

No branches or pull requests

2 participants