Principal extraction from nested username claim was broken

[mstruk]

When extracting a user id from JWT token by using oauth.username.claim or oauth.fallback.username.claim it only worked for top level attributes, not for nested attributes. For example, by configuring: "oauth.username.claim=auth.userid", and given a JWT token:

{
    ...
   "auth": {
      "userid": "alice"
   }
}

Extraction would not find 'userid' key under top level 'auth' object, rather it was looking for 'auth.userid' top level key.

This PR adds an option to use JsonPath to target nested attributes.
If the claim specification starts with an opening square bracket '[', it is interpreted as a JsonPath query.
Otherwise, it is interpreted as a top level attribute name.

userId                    ... use top level attribute named 'userId'
user.id                   ... use top level attribute named 'user.id'
$userid                   ... use top level attribute named '$userid'
['userInfo'].id           ... use nested attribute 'id' under 'userInfo' top level attribute
['user.info']['user.id']  ... use nested attribute 'user.id' under 'user.info' top level attribute
['user.info'].['user.id'] ... use nested attribute 'user.id' under 'user.info' top level attribute (optional dot)

[scholzj]

I think this deserves some description to expain what this change does. You seem to have really just swicthed two parameters around without any change to the method you call. So it is hard to understand the logic behind it.

[mstruk]

@scholzj Thanks for pointing this out. Upon a second look I realised that the fix is not good as it potentially introduces backwards compatibility issues. I'll describe the problem in more detail, and I'm working on a proper fix.

[tombentley]

How does this handle the input [foo].bar[baz]?

[mstruk]

This should be illegal. It should throw an exception. I'll fix it. Once you start your claim spec with '[' the characters between ']' and '[' should be limited to arbitrary number of spaces and a single '.'.

[mstruk]

Fixed.

[tombentley]

How does this handle the input ['foo'bar']?

[mstruk]

Currently it looks for the claim called foo'bar. I can't imagine a sane person using ', [ or ] inside an attribute name, so I don't think we have to support that. But I'm also not a fan of going out of our way to error on such strange characters beyond them interfering with parsing rules.

For example you can't have ']' in the name, but you can have '['. In the same vein I don't see a problem allowing ' in the middle of names.

[tombentley]

I accept that this is an edge case, but I find it really surprising anything should accept 'foo'bar' as if it were a literal containing the characters foo'bar. I can't think of any examples (e.g. in programming languages) where things are parsed where such input wouldn't be rejected. Such parsing is usually based on tokenisation and the idea of paired delimiters. If people need to include ' in the middle of a literal like that the right way to do it is require escaping. You're violating a very very common expectation here. That would be OK if there was a valid use for supporting this, but you agree that no one should want or need this flexibility.

I think it's far safer to reject inputs like this, since it's more than likely due to a user messing up their config than something intentional. And rejection would appear to be easy enough: Anything inside the quotes should match [a-zA-Z0-9.]+ or similar.

In general it's much easier to be strict from that start, and relax once its clear that the rules are too strict for valid use cases that users have, than to try to restrict inputs later on. By that point you don't know what weird and wonderful things people might be using and expecting to continue working.

[mstruk]

@tombentley I modified the implementation to use JsonPath for nested attributes. This should remove any inconsistencies in special character matching ...

[mstruk]

@tombentley I hope I adequately addressed the comments. WDYT?

[tombentley]

Thanks @mstruk