'Contains' for check against lists

smancke commented 7 years ago

I plan to support a list of groups, which a user can have in loginsrv. It would be cool to also have support for that in caddy-jwt. I could think of a syntax like:

allow claim > value
deny claim > value

Where claim may be an json array of strings.

What do you think about that? Should I implement this and do a PR? Any objections or better ideas about the syntax?

BTBurke commented 7 years ago

Sebastian,

I appreciate the contributions but I'm not too crazy about this one. I think it adds a lot of complexity and the possibility of bugs by relying on JSON parsing. It would be better if you could just list multiple allow values for each claim.

For example:


allow group users

allow group admin

I believe this works now, but there might not be a test to cover this case. If people have so many groups that they can't list them out on multiple lines, then something is wrong with the way their system is designed or their use-case is so advanced that they should be dealing with authorization on their own.

I'd rather keep this plugin simple because I don't have the time to support users with advanced use cases. They should be doing that stuff on their own.

On Tue, Dec 13, 2016, at 04:36 AM, Sebastian Mancke wrote:

I plan to support a list of groups, which a user can have in loginsrv[1]. It would be cool to also have support for that in caddy-jwt. I could think of a syntax like:

allow claim > value deny claim > value

Where claim may be an json array of strings.

What do you think about that?

Should I implement this and do a PR?

Any objections or better ideas about the syntax?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub[2], or mute the thread[3].

Links:

smancke commented 7 years ago

Hi Bryan,

yes, this one is not so obvious, so I'm talking about this first, before doing a PR ;-)

I think, you have misunderstood, my objective. I don't want a simple syntax for multiple rules. What I want to implement is the support for list types within the JWT claim. In my case, a user can be part of multiple groups. And I want to check, if the user is at least part of one group. e.g.

{
  "sub": "bob",
  "groups": ["admin", "user", "operator"]
}

If I read you comment, I think for the configuration part it would be the best, not to introduce any special syntax for lists, but just use the given syntax and do a contains check in the cases, where the claim is a list. e.g.

ALLOW groups admin

This way, the configuration would stay pretty simple.

smancke commented 7 years ago

Hi Bryan,

I continued with implementing this, because I'll now need this for a project of mine. Looks fine for me. For me, it looks very understandable and straight forward. What do you think about it?

It adds up in PR #13, because I did not separate both things in branches. If you prefer, I can separate both commits.

BTBurke commented 7 years ago

Merged. Thanks for the contribution.

BTBurke / caddy-jwt

'Contains' for check against lists #14