biscuit-auth / biscuit-go

Apache License 2.0
70 stars 21 forks source link

Add != #142

Open mauromorales opened 4 months ago

mauromorales commented 4 months ago

Hi, I got referred to your project and I find it quite interesting. Here's my attempt at adding != which seems to be needed for v3. Looking forward to your feedback

relates to https://github.com/biscuit-auth/biscuit-go/issues/117

mauromorales commented 4 months ago

@lu-zero I saw your post on x, not sure if you're the right person to ping, but maybe you can point me to the right person. Thanks!

divarvel commented 4 months ago

Thanks for your PR!

The general idea is good, but since biscuit blocks are versioned, the new operations cannot be directly added, they need to be checked against the block number (ie a block with version=3 cannot contain a != operator, and conversely, serializing a block that contains this operator must have a minimum version of 4).

Additionally, the samples.json file is copied from the spec https://github.com/biscuit-auth/biscuit/blob/main/samples/current/samples.json and should not be modified directly, as it is kept in sync with biscuit files as well.

mauromorales commented 4 months ago

@divarvel let me see if I understand correctly, the current implementation is testing against https://github.com/biscuit-auth/biscuit/blob/main/samples/deprecated/v2/samples.json

So for adding != it would need to be a version 3 of the library, and test against https://github.com/biscuit-auth/biscuit/blob/main/samples/current/samples.json

I'll need a bit of further explanation about what is a block of version 3 and a block version 4 are we talking about the same versions than in the samples, or is this a different kind of version?

Geal commented 4 months ago

@mauromorales thank you for looking into this!

I'll need a bit of further explanation about what is a block of version 3 and a block version 4 are we talking about the same versions than in the samples, or is this a different kind of version?

Biscuit token contain multiple blocks, each one corresponding to one level of attenuation. Each of those blocks holds a version number: https://github.com/biscuit-auth/biscuit/blob/main/schema.proto#L45 That version number indicates the presence of newer features (ex: new ways to check in datalog), to make sure that older implementations cannot accept newer tokens without validating the new features. From the specification:

Each block contains a version field, indicating at which format version it was generated. Since a Biscuit implementation at version N can receive a valid token generated at version N-1, new implementations must be able to recognize older formats. Moreover, when appending a new block, they cannot convert the old blocks to the new format (since that would invalidate the signature). So each block must carry its own version.

  • An implementation must refuse a token containing blocks with a newer format than the range they know.

  • An implementation must refuse a token containing blocks with an older format than the range they know.

  • An implementation may generate blocks with older formats to help with backwards compatibility, when possible, especially for biscuit versions that are only additive in terms of features.

  • The lowest supported biscuit version is 3;

  • The highest supported biscuit version is 4;

In general, we make sure that when generating a block, we use the lowest version number possible (if no new features are used, keep a lower number) so that they could still be validated by older versions.

Geal commented 4 months ago

this is where we check the version when parsing the token: https://github.com/biscuit-auth/biscuit-go/blob/61386fc9e1be296472c2cc63f42a87f9b227ae51/converters.go#L64-L110

It looks like this implementation does not minimize the version number when serializing though: https://github.com/biscuit-auth/biscuit-go/blob/61386fc9e1be296472c2cc63f42a87f9b227ae51/builder.go#L115-L122