Comments (5)
There are few different approaches for report errors as return value instead exceptions; either/try monad, [result, error]
vector, return exception as is if error is happens, are some examples of that.
Choise one will make the code opinionated on one concrete solution and I don't want to put opinions in that.
Create a simple wrapper on buddy-sign functions for the appropriate call signature (and return value) adapted to your application needs is very simple and is the recommended approach in my opinion instead of exposing additional and opinionated solution from the buddy-sign side.
If you think that I'm wrong, please feel free to expose more arguments about your proposal and I'm gladly try to reconsider may posture.
from buddy-sign.
Hi,
thank you for considering my feedback.
I am trying to argue that an unsuccessful unsign
due to an invalid signature or expired token (in JWT) is just another result, there is nothing special about them. Why not return that result the same way you return the result of an unsign
where the signature happened to be valid. These are all results that caller should expect and should be able to handle them.
You will have to choose a particular representation for these non-success results either way, why not just use the simpler way to communicate them to the caller as return values rather than through the more complicated and less efficient exception machinery.
Wrapping all other results other than the valid signature into an exception makes your code actually more opinionated, as you made two decisions instead of one.
To reiterate on my earlier point, exceptions IMHO should be reserved to signal unexpected/exceptional situations/errors where the caller may not be in the position to handle them and where exceptions are more convenient mechanism to propagate these errors higher up the caller chain where they can be appropriately handled without requiring the levels in between to know about every possible downstream exception.
With that said, I think buddy
is a fantastic and very high quality addition to the Clojure open-source ecosystem.
from buddy-sign.
I mostly agree with your arguments. But I'm not clearly understand some parts.
Are you suggesting return the same result in successful and unsuccessful validation of the token?
In this case, this seems like a bad idea because it is completely counterintuitive and not evident.
Also, unfortunately to our opinion about exceptions, they seems like the most used way to handle this kind of stuff in the clojure ecosystem and this is the reason of using them as default approach.
In order to expose additional api that return errors as values, a proper way to signal the validation error should be choiced. Returning the same value for success and validation error is not an option. As I said previously is very confusing and not evident to the final user. Additionally, not all validation errors can return the data (on JWE as example valid tokens but encrypted with different key, will cause a validation error but the data can't be retrieved).
On the end, I don't know that abstraction is better for create the new functions that return validation errors as value. I think that vector of [result, error]
is the proper approach for it, but I'm not pretty sure...
from buddy-sign.
Thanks,
that is good additional insight. Having a fresh look at the public API, it occurred to me that unsign
actually does two things
- check signature of unsigned data
- remove signature and return the naked data
This is perfectly fine, as you may do it in this order and step 2. is likely to be conditioned on step 1. 99% of the time. I'd like to have those two operations a la carte as well as the convenience method of having them as one.
I think in this case using exceptions seems semantically correct. The way you use it it almost suggests that good data with invalid signature is invalid data, i.e an invalid argument to unsign
, which is widely handled via throwing an exception (IllegalArgument or whatever)....
So if I had 1) and 2) as separate methods, I think my argument for returning all results as return values of the function would be stronger. There is still the efficiency argument though, I think, but I understand it is far less compelling.
Now as to how should you return successful results and errors, you are right, you need to wrap them into a container that indicates to the caller all useful outcomes of an unsign
.
I don't necessarily agree that in the Clojure ecosystem it is uniformly accepted that one should use exceptions in this case. The Prismatic (now Plumatic) library uses exceptions in their validate
function, whereas its semantic equivalent conform
in the upcoming Clojure.Spec library returns an value indicating an error.
There are different approaches to this, as you noted, using [:ok {..naked data..}] vs. [:error {some data describing the error}] is perfectly idiomatic and play well with destructuring. This is also the preferred way in ML languages.
Finally, the matter of evidence. I think it depends on ones previous experiences and exposure to other libraries, common practices etc. Everyone will read the docstring at least once (hopefully). :)
from buddy-sign.
On the end, I think that define own functions that adapts the buddy-sign to the needs of the concrete application is very very easy. And it there are a lot of different ways to represent the error as value, I prefer leave it to the user.
This is a very quick example that the user can do in the application code to adapt buddy-sign function for return errors as value in form of a vector ([err result]
):
(defn sign*
[& args]
(try
[nil (apply sign args)]
(catch clojure.lang.ExceptionInfo e
(let [error (assoc (ex-data e) :message (.getMessage e))]
[error nil]))))
(defn unsign*
[& args]
(try
[nil (apply unsign args)]
(catch clojure.lang.ExceptionInfo e
(let [error (assoc (ex-data e) :message (.getMessage e))]
[error nil]))))
Other people may want to use some kind of Either or Try monad (see more in funcool/cats library) and I can't offer all posible variants for that.
Thank you very much for your feedback and for this issue ;)
from buddy-sign.
Related Issues (20)
- Cannot get "kid" of JWT token from decode-header HOT 2
- Is ES384 omitted purposefully? HOT 6
- Supporting custom headers HOT 3
- `iss` validation should be against a collection ("whitelist") HOT 2
- Support for looking up public keys via OIDC discovery HOT 19
- Handle java.security.SignatureException for JWS applications HOT 1
- Where is jws/to-timestamp? HOT 1
- EdDSA JWS support HOT 1
- JWS ECDSA Signatures are generated in incorrect format HOT 2
- New release? HOT 1
- JWT signatures invalid according to jwt.io website HOT 2
- 2.* -> 3.*: what's the upgrade path? What are the breaking changes? HOT 1
- Problem upgrading Buddy-sign from 0.3.0 to 2.2.0 HOT 1
- Suggestion: Allow audience validation against collection of valid audiences HOT 4
- JWS unsigned alg should be read out of the JOSE header if available HOT 3
- Errors in using :eddsa algo for claims signing HOT 1
- [PERFORMANCE] - avoid reflective calls HOT 6
- buddy.sign.jwt/unsign fails when hs256 is not used and alg is unspecified HOT 4
- Buddy JWT doesn't support the NONE algorithm HOT 1
- Tag and Changelog Version 3.4.351 Wrong HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from buddy-sign.