Open rishabhpoddar opened 1 year ago
Some notes for our call tomorrow:
Adding global claim validators on the frontend is not that useful - cause you still have to do checks and handle errors on each route using Session.validateClaim anyway. If we are expecting users to create their own centralised place where they call and handle validateClaim , then there is no need for users to add claims via our global validator function.
They can use the invalidClaims
array of the session context. In most cases, that should be enough.
If claims validators are being added to SessionAuth, that by itself doesn’t stop the child from rendering in case the claim fails. Whilst this is what we wanted, it’s very counter intuitive from the user’s point of view. We should add an on failure handler in the claim itself which perhaps returns a component that’s rendered in case that claim fails. In case of email verification claim, we could return a redirection component.
This was the idea originally (or somewhere along the way). The main argument against it was that people might get confused if we don't do this for APIs that fail with a claim error. That should be fine in general since the re-validation on the frontend should get the same conclusion. Other than that:
<SessionAuth>
On the backend, claims have a key, but in the validators, and on the frontend claims have an id. So the backend should also be called id instead of key.
On both the backend and front, validators have id
s.
Session claims on the backend have a key
not present on the FE. We might want to remove the key from the user facing interface since it's only ever used in addClaimFromOtherRecipe
to check for duplicates outside of the claim classes themselves.
On claim failure from the backend, the payload as a id for the validator’s ID. But on the frontend, the result of validateClaim has validatorId which is inconsistent
It's called validatorId
because it'd be easy to mistake it for the key/id of the claim, which is not necessarily (but almost always) the same.
For custom UI, ideally the user should not have to call Session.validateClaim themselves in all their routes - it’s very easy to miss.
This is similar to checking if a session exists in all their routes. If we are not bundling those two together (and I don't think we should), I'm not sure what we can do about it.
We should make the refresh function on the frontend claims optional. For things like 2fa claim, we are have to define an empty function, which is weird.
We can make that optional. 👍
On the frontend, the refresh function is in claim and in claim validator - as opposed to the backend in which the fetchValue function is only in the claim and the claim validator has a reference to the claim. We should change the frontend to make the claim validator have a reference to the claim as well.
The original intention was to have a very limited claim concept on the FE, but we can expand it like this. 👍