Comments (22)
Also - as I'm thinking about it, it seems weird to me to say (in code) is_not_before(leeway)
. It seems to me that the leeway maybe should also be set on a per-instance basis...because what you are saying is "I'm willing to be within this margin of error on my server"...which, to me, is "instance" - not "function".
from lua-resty-jwt.
Another change I'd like to propose is in the naming of the validators. Instead of (for example) naming the optional one equals_any_of
and the required one required_equals_any_of
, I would propose the optional one (which, IMO, will be used less) be named opt_equals_any_of
and the required one (the more common case) be named equals_any_of
. "Required" is kind of implied, I think.
from lua-resty-jwt.
sounds like my proposal combined with @fermaem 's.
have a little discusstion at #19 , check it out
from lua-resty-jwt.
Looks great - I'd love to join in the discussion and/or help put together something that would also be a bit more flexible (i.e. to add the ability to validate a subject, or something else application-specific). Would that be best to do on #19 or would it be better to work up some kind of proposal (in the form of a PR) and carry on the discussion in a new issue (or this issue)?
from lua-resty-jwt.
hmm...
let's discuss in this issue 🎉
from lua-resty-jwt.
Agreed that this now makes the validators table look kind of weird. I would almost say that we replace the timeframe and iss validator options with some exposed functions that can be used just as a table of claims.
from lua-resty-jwt.
yep, i agree.
hi @fermaem , any thoughts?
from lua-resty-jwt.
I'm not sure that every claim validation should be allowed to be overridden now (ie. YAGNI): For instance, the lifecycle validator. On the other hand, it may make sense to provide the user with a callback for him/her to feed/override the base validator with some additional data.
@toonetown Any chance you could wrap up a client API proposal mock?
from lua-resty-jwt.
WARNING: Super long post follows.
I actually really like the simplicity of the API that is used by the auth
function by https://github.com/auth0/nginx-jwt (which, BTW, uses this project for their underlying JWT work). Particularly, I really like both the readability and simplicity of defining claims to be checked as a table that matches the claims themselves. i.e.:
{
sub = "^[a-z]+$",
iss = function(val)
for _, value in pairs({ "first", "second" }) do
if value == val then return true end
end
return false
end
}
The auth0/nginx-jwt project contains a single helper function - table_contains
that would allow you to simplify this second statement thusly:
{
iss = function(val) return nginx_jwt.table_contains({ "first", "second" }, val) end
}
I would, however, propose to simplify the API even one step further by defining it as this:
local verified = jwt:verify_jwt_obj(key, jwt_obj [, claim_spec])
-- OR --
local verified = jwt:verify(key, jet_token [, claim_spec])
From a purely "readability" point of view, this tells me "verify the object with the given key against the given claim_spec
."
I would define a claim_spec
as a lua table where each key in the table is the name of a claim which is being verified. The value corresponding to each key is a function which has the following signature:
function(val, claim, jwt_obj)
Where val
is the value of the claim from jwt_obj
(or nil if it doesn't exist), claim
is the name of the claim that is being verified, and jwt_obj
is the object that is being verified. This function would return either true
or false
(That is, I would simplify the API a bit from what auth0/nginx-jwt has done and remove the ability to specify a string value). Any validator MAY raise an error, and the validation will be treated as a failure, and the error that was raised will be put into the reason
field of the resulting object.
Then, I would provide a library of "validator" functions that would return an appropriate validator function. Since I would anticipate this set of validator functions, I might suggest placing them in a separate file and module (jwt-validators.lua
in this example). One could then verify a token (or alternatively, a loaded object using verify_jwt_obj
) by doing:
local jwt = require "resty.jwt"
local validators = require "resty.jwt_validators"
local leeway = 300
local verified = jwt:verify(key, jwt_token, {
sub = validators.required_matches("^[a-z]+$"),
iss = validators.required_equals_any_of({ "first", "second" }),
nbf = validators.required_not_before(leeway),
exp = validators.required_expiration(leeway),
myclaim = validators.required(),
...
})
Or, put another way, what is now defined as:
local jwt_obj = jwt:verify(key, jwt_token,
{
lifetime_grace_period = 120,
require_exp_claim = true,
valid_issuers = { "my-trusted-issuer", "my-other-trusteed-issuer" },
claims = { sub = "My Test Subject" }
}
)
Could be done as:
local jwt_obj = jwt:verify(key, jwt_token,
{
exp = validators.required_expiration(120)
iss = validators.required_equals_any_of({ "my-trusted-issuer", "my-other-trusteed-issuer" }),
sub = validators.required_equals("My Test Subject")
}
)
It just seems a bit clearer what is trying to be verified, and what is happening. It also cleans up code which uses this library a bit and makes it more readable.
Suggested functions to include in the jwt_validators
module (all of these functions would actually return a validator function - one that takes a single parameter and returns true or false if they match):
--[[
Returns a validator that returns false if a value doesn't exist. If
the value exists and a chain_function is specified, then the value of
chain_function(val, claim, jwt_obj)
will be returned, otherwise, true will be returned. This allows for
specifying that a value is both required *and* it must match some
additional check. This function will be used in the "required_*" shortcut
functions for simplification.
]]-
function jwt_validators.required([chain_function])
--[[
Returns a validator that checks if a value exactly equals the given check_value.
If the value is nil, then this check succeeds.
]]-
function jwt_validators.equals(check_val)
--[[
Returns a validator that checks if a value matches the given pattern. If the
value is nil, then this check succeeds.
]]-
function jwt_validators.matches(pattern)
--[[
Returns a validator that checks if a value exactly equals any of the given values.
If the value is nil, then this check succeeds.
]]-
function jwt_validators.equals_any_of(check_values)
--[[
Returns a validator that checks if a value matches any of the given patterns.
If the value is nil, then this check succeeds.
]]-
function jwt_validators.matches_any_of(check_patterns)
--[[
Returns a validator that checks if a date is not before the current time (within
the given optional leeway). If leeway is not specified, it defaults to 0. An
optional date_value can also be given - which is helpful for writing unit tests.
If date_value is not specified, then the current time is used. If the value is
nil, then this check succeeds.
]]--
function jwt_validators.not_before([leeway [, date_value]])
--[[
Returns a validator that checks if the current time is equal to or before
a date (within the given optional leeway). If leeway is not specified, it defaults
to 0. An optional date_value can also be given - which is helpful for writing
unit tests. If date_value is not specified, then the current time is used. If the
value is nil, then this check succeeds.
]]--
function jwt_validators.expiration([leeway [, date_value]])
--[[
These functions are just shortcuts to prevent the need to type, for example:
jwt_validators.required(jwt_validators.equals(check_val)). They take the same
parameters as their non-required counterparts (i.e required_equals_any_of takes the
same parameters as equals_any_of).
]]--
function jwt_validators.required_equals(check_val)
function jwt_validators.required_matches(pattern)
function jwt_validators.required_equals_any_of(check_values)
function jwt_validators.required_matches_any_of(check_patterns)
function jwt_validators.required_not_before([leeway [, date_value]])
function jwt_validators.required_expiration([leeway [, date_value]])
I think that this will provide flexibility as well as ease of use of the library. It also seems like it simplifies things within the library so that there are fewer places for potential bugs. Also, the functions provided by the validators library can be expanded upon and improved over time.
I'd appreciate hearing any feedback or suggestions about this that you would have. I'm also willing to code this up and submit it as a PR if it would be easier to evaluate in that format.
from lua-resty-jwt.
It might even be nice to have the verify
functions take multiple claim_spec
parameters, and it merges them all together into a single claim_spec
to use (later ones override earlier-specified ones). Then, an adapter function could be provided in jwt_validators
that returns an appropriate claim_spec
which takes validation_options
(as they are defined currently surrounding lifecycle). For example:
--[[
Returns a `claim_spec` table follows the lifetime-specific validation_options
]]--
function jwt_validators.lifetime_spec(validation_options)
You could then do something like this:
local jwt_obj = jwt:verify(key, jwt_token, validators.lifetime_spec(
{
lifetime_grace_period = 120,
require_exp_claim = true
}
))
Or, to use the full above example:
local jwt_obj = jwt:verify(key, jwt_token,
validators.lifetime_spec({
lifetime_grace_period = 120,
require_exp_claim = true
}), {
iss = validators.required_equals_any_of({ "my-trusted-issuer", "my-other-trusteed-issuer" }),
sub = "My Test Subject"
}
)
That's just a thought. I kind of like the ability to merge multiple claim_spec
tables.
from lua-resty-jwt.
local jwt_obj = jwt:verify(key, jwt_token,
{
exp = validators.required_expiration(120)
iss = validators.required_equals_any_of({ "my-trusted-issuer", "my-other-trusteed-issuer" }),
sub = validators.required_equals("My Test Subject")
}
)
i personally like the readability and the power to control everything.
but @fermaem got's his point, this lib should be easy to use without digging into complex manuals.
how about we add a function that takes current validate_options
and produce a validators table shown above?
user can still easily control the validation behaviors by using the validate_options
without worrying compromising basic validations.
but they can also gain full power by creating their own validators table directly.
from lua-resty-jwt.
@SkyLothar
how about we add a function that takes current validate_options and produce a validators table shown above?
user can still easily control the validation behaviors by using the validate_options without worrying compromising basic validations.
but they can also gain full power by creating their own validators table directly.
That is the route that I am planning on going. I have the first bit of it in place, but I'm working under the following assumptions:
- If you pass one of the verify functions a single
claim_spec
- AND If that claim spec has at least one of the following keys:
lifetime_grace_period
require_nbf_claim
require_exp_claim
valid_issuers
claims
(see below)
- THEN it will operate in "legacy" mode (I don't care about the naming...that's just what I've called it so far).
When in "legacy" mode, the functions behave identically to how they are behaving right now.
I would like to fully remove the claims
support of triggering "legacy" mode...I'm the one who added it in #25 - and I'm fine with backing out that change in favor of this one.
In the PR so far, I have "deprecated" legacy mode...and it logs a warning when you use it. That is mainly for me in doing testing and seeing how things are working. My goals are to support the current unit tests without modification (except for possibly the error strings that are matched against), and to add as many new unit tests as needed to test the feature completely.
from lua-resty-jwt.
Cool, let's do this. 🎉
from lua-resty-jwt.
Ok. Hopefully I'll be able to finish up PR #26 tomorrow.
from lua-resty-jwt.
Wow! That's a very detailed proposal! Thanks a lot for that! 💯 I like the general idea and the in-depth analysis.
Few thoughts, though:
- I'm not really sold on the optional
date_value
parameter. I can't think of a real life use_case when one would want to set this value. However, it's possible it's been define for the sole purpose of make the testing easier. Would that be the case, how would you feel about (maybe) setting this at the instance level? (eg.jwt.set_system_clock(function() { return 123456789 } end)
, default behavior being to returnnow
?) - Would we want to allow the user to plug in his/her own validators, it may be interesting to define a general purpose callback signature. Maybe something along the lines of
bool function is_valid(jwt_obj, claim_name, claim_value)
. This way, would the validation of a claim depend on another claim value, the implementer would be able to inspect the jwt_obj. (FWIW, being a tad paranoid, I'd favor passing a deeply cloned instance ofjwt_obj
to the callback rather than the instance by itself to avoid any potential side effects on the object itself).
Thoughts?
from lua-resty-jwt.
@fermaem
I'm not really sold on the optional date_value parameter. I can't think of a real life use_case when one would want to set this value. However, it's possible it's been define for the sole purpose of make the testing easier. Would that be the case, how would you feel about (maybe) setting this at the instance level?
I think that's a great idea - I was trying to figure out how to handle it, and I like that approach better. I'll add it to the tasks
@fermaem
Would we want to allow the user to plug in his/her own validators, it may be interesting to define a general purpose callback signature. Maybe something along the lines ofbool function is_valid(jwt_obj, claim_name, claim_value)
.
That is almost what I proposed for the callback signature - except for reversing the order of the arguments (claim_value, claim_name, jwt_obj)
- because in my view, the most common action a validator is going to take is to use the claim_value
...so a validator author can take a shortcut and just define their function as taking the claim_value
only, since that is all they are interested in.
As a side note, I also figured it might be good to be able to define a "whole object" validator...that is - one that's not tied to a claim, but is tied to the jwt_obj
itself. I don't know how to specify these (probably with a key of __jwt_obj__
or something that is very unlikely to be used as a "real" claim name). In that case, the claim_value
could either be nil
...or we could pass the entire obj in as the claim_value
as well...I haven't worked out that part yet.
@fermaem
FWIW, being a tad paranoid, I'd favor passing a deeply cloned instance of jwt_obj to the callback rather than the instance by itself to avoid any potential side effects on the object itself
I'm glad you mentioned that - because it was something that I hadn't even considered...but you are 100% correct! We definitely don't want validators changing the jwt_obj
...so passing in a deep clone is a good idea.
For performance reasons, what would you think about passing the same deep copy to all the validators? I don't know that it would be a good idea to deep copy the object for every validator. That way, the original jwt_obj
would be preserved...but there would still be the possibility of mutating the object from one function to another.
Another option is we could pass (to all validators) a json-encoded jwt_obj
...that way, in the (IMO, less than common) scenario where a validator actually needs access to the jwt_obj
, then that validator can take the performance hit of decoding the json string to get access to what it needs. That way, the more common scenario will not have as much of a performance hit (I'd assume that a single json.encode call is roughly the same performance time as a single deep copy) - but it would also have the "immutability" that making a deep copy for each validator call would have.
Thoughts?
from lua-resty-jwt.
Just a status update (aside from what I've been tracking on #26) - I am most of the way finished with updating the legacy options to using the new code, but I am not quite there. I know that @SkyLothar said you were going to do a release this weekend, but perhaps you could hold off until the PR is finished (depending on my schedule, it might be this weekend, but it might also not be ready until Monday)
from lua-resty-jwt.
Thanks for your efforts. These are great proposals!
from lua-resty-jwt.
PR #26 is ready to be merged once it has been reviewed/checked for errors. I have no more plans for changes other than anything that comes up in the review process.
Would you prefer I squash the commits into a single one and resubmit as a separate PR? I don't know if it is easier for you to review as separate commits or as a single commit.
from lua-resty-jwt.
Separate pr is great.
I can squash them into one when i merge the pr.
from lua-resty-jwt.
Done - see #27 for a single-commit version #26 has the commits separated.
from lua-resty-jwt.
#27 has been merged.
from lua-resty-jwt.
Related Issues (20)
- Can You Make this Compatible For Roblox Developers? i want To Use this kind Of Api To Manage Remote Events
- Validation of HS* tokens depends on the payload order HOT 1
- The token is not identical due to table not being ordered
- JWT token verification problem (undefined symbol: EVP_MD_CTX_create) HOT 1
- How the JWT is being verified?
- openidc.lua:819: authenticate(): id_token 'HS256' signature verification failed, client: 10... HOT 8
- Require alg parameter when verifying.
- "jwt.lua": HS256 signature validation fails which reason as "internal error" HOT 9
- Comment mention both x5c and x5u being defined when they are not HOT 1
- Confused about expected key format HOT 1
- Module 'resty.aes' not found: HOT 1
- Is this project still active? HOT 10
- attempt to index local 'jwt' (a userdata value) HOT 1
- jwt sgin err when typ is JWE HOT 2
- Civetweb integration
- /lua-resty-jwt/ Where is the directory? HOT 1
- With the JWT token, server/openresty returns INVALID_JWT
- [discuss] remove folder `vendor`
- Decode secret is not a valid cert\/public key HOT 1
- I encountered an error while checking the token of rs256
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 lua-resty-jwt.