jtanguy / hmacaroons

Pure haskell implementation of macaroons
https://jtanguy.github.io/hmacaroons
BSD 3-Clause "New" or "Revised" License
14 stars 7 forks source link

Better verifiers #11

Open jtanguy opened 6 years ago

jtanguy commented 6 years ago

The verifier logic could benefit from a small refactoring, mainly from two things:

What are your thoughts about it @ptitfred @clementd-fretlink ?

clementd-fretlink commented 6 years ago

The MonadIO constraint is not that terrible in practice (a pure version alongside the IO would still be nice). I'm not sure what a new monad transformer would bring.

For verifiers that need access to other caveats, I first build verifiers by inspecting caveats, which works okay. Maybe giving access to the other verifiers in a zipper fashion would be nice indeed.

My main gripe with the verifiers currently is that VerifierResult embeds too many cases (macaroon-level errors), so its monoid instance has quite nonsensical combinations (that's why the implementation was partial in the first case).

clementd-fretlink commented 6 years ago

Something like

data VerifierResult =
    Unrelated
  | Refused Text
  | Verified

would improve things a lot. Having a separate type for macaroon validation (with SigMismatch, and RemainingCaveats (NonEmpty (Caveat, [Text]))) would allow to improve UX

clementd-fretlink commented 6 years ago

Another idea (even though I realize it's a bit more far-fetched) would be to add Deferred to the verifier result, as well as a Deferred (NonEmpty Caveat) case to the macaroon validation result, to allow multiple-step validation.

clementd-fretlink commented 6 years ago

@jtanguy what do you think about these ideas?

jtanguy commented 6 years ago

I do like the changes to the VerifierResult. I am less sure about the Deferred/RemainingCaveats part because I believe there may be a way to statically define verifiers as long as they can inspect the caveat structure (and the previous/next caveats the ListZipper offers)

jtanguy commented 6 years ago

That would be a departure from the rest of the macaroons libraries (which I'm not specifically against), and so if the need to have multiple-step macaroon verification arises, that would be a good opportunity to contribute back these propositions to the broader macaroons community.

clementd-fretlink commented 6 years ago

Yeah the Deferred thingy is a departure from the other implementations. Its need arose in practice, as there was a natural distinction between "technical" and "business" caveats. No problem keeping out of the library

jtanguy commented 6 years ago

I just would like this to be opt-in

clementd-fretlink commented 6 years ago

I'm trying out the new data types I've proposed, it works quite well and it allows a significant cleanup of the validation code.

fmenou commented 6 years ago

That would be a departure from the rest of the macaroons libraries (which I'm not specifically against), and so if the need to have multiple-step macaroon verification arises, that would be a good opportunity to contribute back these propositions to the broader macaroons community.

Its need arose in practice, as there was a natural distinction between "technical" and "business" caveats

Those 2 quotes let me believe we should add examples in the library to illustrate those various use-cases, don't you both think?

clementd-fretlink commented 6 years ago

You may find it funny, but I've given some more thought to this, and I don't think that multi-step validation is necessary for our use case anymore (since we're already accumulating verifiers). I'll give a shot at merging the two steps to see how it goes.