You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2022/05/27 11:57:42 UTC

[GitHub] [couchdb] big-r81 opened a new pull request, #4041: Allow and evaluate nested json claim roles

big-r81 opened a new pull request, #4041:
URL: https://github.com/apache/couchdb/pull/4041

   ## Overview
   
   In a JWT token it is possible to add an attribute for role claims. If the roles are presented as top-level attribute like
   ```json
   {
     "couchdb-roles": [
       "my_role_1",
       "my_role_2"
     ]
   }
   ```
   and setting the parameter `roles_claim_name` in the config file to 
   ```
   [jwt_auth]
   roles_claim_name = couchdb-roles
   ```
   CouchDB was able to read that attributed and take over that roles.
   
   This doesn't work, if the claim roles are nested, eg:
   ```
   {
     "my" :{
       "nested": {
          "couchdb-roles": [
            "my_role_1",
            "my_role_2"
          ]
       }
     }
   }
   ```
   To allow this and for backwards compatibility, a new config parameter `roles_claim_path` is introduced to allow nested role claims. To allow the example from above, yo can use the following syntax:
   ```
   [jwt_auth]
   roles_claim_path = my.nested
   roles_claim_name = couchdb-roles
   ```
   
   This is a first quick (with logging output) draft to allow CouchDB to read and apply nested JSON claim roles.
   
   
   ## Testing recommendations
   
   Example JWT token:
   ```
   eyJ0eXAiOiJKV1QiLCJraWQiOiJteWp3dHRlc3RrZXkiLCJhbGciOiJSUzI1NiJ9.eyJleHAiOjE2NTU0MjQwNjUsImlhdCI6MTY1MzM4ODA4NiwiYXV0aF90aW1lIjoxNjUzMzg4MDY1LCJqdGkiOiJlMTY4OTEzNy00M2U3LTQ3ZjItYTg3Zi1jMjExMWVhMzBmN2QiLCJpc3MiOiJodHRwOi8vbG9jYWxob3N0OjkwODAvYXV0aC9yZWFsbXMvb3ZhbHQiLCJhdWQiOlsid2ViLXBnYyIsImFjY291bnQiXSwic3ViIjoiZGYwNTNlZDktYTljZS00MWJmLTkxNDMtODVlNzVlZTI2NTdjIiwidHlwIjoiQmVhcmVyIiwiYXpwIjoid2ViLWNvY2twaXQiLCJub25jZSI6ImYxMjJhZmFkLWY4MzktNGYxMS04OWJhLTg0Y2Q5Y2I1NzgwMiIsInNlc3Npb25fc3RhdGUiOiI2MDZiM2M1Yy1lYjVjLTRhMjktODE0ZS0xZTk2NDEwNGRiM2UiLCJhY3IiOiIwIiwiYWxsb3dlZC1vcmlnaW5zIjpbImh0dHA6Ly9sb2NhbGhvc3Q6ODA4MCIsImh0dHA6Ly9sb2NhbGhvc3Q6MzAwMCJdLCJyZWFsbV9hY2Nlc3MiOnsicm9sZXMiOlsiZGVmYXVsdC1yb2xlcy1vdmFsdCIsIm9mZmxpbmVfYWNjZXNzIiwidW1hX2F1dGhvcml6YXRpb24iXX0sIm15Ijp7Im5lc3RlZCI6eyJjb3VjaGRiLXJvbGVzIjpbIm15X3JvbGVfMSIsIm15X3JvbGVfMiJdfX0sInJlc291cmNlX2FjY2VzcyI6eyJ3ZWItcGdjIjp7InJvbGVzIjpbInBnY191c2VyIl19LCJhY2NvdW50Ijp7InJvbGVzIjpbIm1hbmFnZS1hY2NvdW50IiwibWFuYWdlLWFjY291bnQtbGlua3MiLC
 J2aWV3LXByb2ZpbGUiXX19LCJzY29wZSI6Im9wZW5pZCBlbWFpbCBwcm9maWxlIiwic2lkIjoiNjA2YjNjNWMtZWI1Yy00YTI5LTgxNGUtMWU5NjQxMDRkYjNlIiwiZW1haWxfdmVyaWZpZWQiOmZhbHNlLCJwcmVmZXJyZWRfdXNlcm5hbWUiOiJ0ZXN0dXNlciJ9.RsB6XXxNp8wehtaPJDAXuvMYScl4AheaC4g1udImeoAkcDSfe9-kJXcJaY7Qa7ztKvVdqWJWh0gCHJS4F7WHlH97-QQqxFkvIigRoPBGQWUlwQLMAOi5uv3lWF7Tb4Ab9EjG479Z6aiPiAOeKkMMpgM5EU2wAniN2GywwOf-10CBkNqpoZ4k2xvz52KhtU0KGcW0vwEBbJoxB8udjIBeY-q1SCSbTvWNoqE2oEgiv3ImRbeN4Mb2k64sD2ptBQq3eLFwwpjQIkGeVxT041w064rmxGgWCIcY3U5JOAd2cU9wdWlFRL0TZ7FdOJGKeuslucufJyxzzcRBVpEHoY3f7A
   ```
   
   Example config file:
   ```
   [chttpd]
   authentication_handlers = {chttpd_auth, jwt_authentication_handler}, {chttpd_auth, cookie_authentication_handler}, {chttpd_auth, default_authentication_handler}
   
   [jwt_auth]
   roles_claim_path = my.nested
   roles_claim_name = couchdb-roles
   
   [jwt_keys]
   rsa:myjwttestkey = -----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA6S7asUuzq5Q/3U9rbs+P\nkDVIdjgmtgWreG5qWPsC9xXZKiMV1AiV9LXyqQsAYpCqEDM3XbfmZqGb48yLhb/X\nqZaKgSYaC/h2DjM7lgrIQAp9902Rr8fUmLN2ivr5tnLxUUOnMOc2SQtr9dgzTONY\nW5Zu3PwyvAWk5D6ueIUhLtYzpcB+etoNdL3Ir2746KIy/VUsDwAM7dhrqSK8U2xF\nCGlau4ikOTtvzDownAMHMrfE7q1B6WZQDAQlBmxRQsyKln5DIsKv6xauNsHRgBAK\nctUxZG8M4QJIx3S6Aughd3RZC4Ca5Ae9fd8L8mlNYBCrQhOZ7dS0f4at4arlLcaj\ntwIDAQAB\n-----END PUBLIC KEY-----\n
   ```
   
   ## Related Issues or Pull Requests
   
   #3758 
   #3166 
   
   ## Checklist
   
   - [ ] Code is written and works correctly
   - [ ] Changes are covered by tests
   - [ ] Any new configurable parameters are documented in `rel/overlay/etc/default.ini`
   - [ ] A PR for documentation changes has been made in https://github.com/apache/couchdb-documentation
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] big-r81 commented on a diff in pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#discussion_r885018938


##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -253,6 +246,68 @@ jwt_authentication_handler(Req) ->
             Req
     end.
 
+get_roles_claim(Claims) ->
+    Roles_Claim_Name_Param = config:get(
+        "jwt_auth", "roles_claim_name"
+    ),
+    Roles_Claim_Path_Param = config:get(
+        "jwt_auth", "roles_claim_path"
+    ),
+    Roles =
+        case Roles_Claim_Name_Param of
+            undefined ->
+                % Check for implicitly role_claim "_couchdb.roles" in JWT token
+                case lists:keyfind(<<"_couchdb.roles">>, 1, Claims) of
+                    false ->
+                        % `_couchdb.roles` not found in token
+                        % check if `roles_claim_path` is set
+                        case Roles_Claim_Path_Param of
+                            undefined ->
+                                [];
+                            _ ->
+                                % no implicit or explicit value for `_couchdb.roles` found, use `roles_claim_path`
+                                Roles_Claim_Path = [
+                                    ?l2b(Item)
+                                 || Item <- string:tokens(Roles_Claim_Path_Param, ".")
+                                ],
+                                couch_util:get_nested_json_value({Claims}, Roles_Claim_Path)
+                        end;
+                    {_, Token_CouchDBRoles} ->
+                        % `_couchdb.roles` found in token, read value
+                        couch_log:info(
+                            "Implicit value for `_couchdb.roles` found and used in token, please migrate to" ++
+                                " `roles_claim_path`!",
+                            []
+                        ),
+                        Token_CouchDBRoles
+                end;
+            _ ->
+                % other value for `roles_claim_name` found.
+                if
+                    Roles_Claim_Path_Param =/= undefined ->
+                        couch_log:info(

Review Comment:
   It was only a [suggestion](https://github.com/apache/couchdb/pull/4041#discussion_r884104705) from me, because I think it's better to only have one param instead of two. We only have to find a find a common denominator... 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] rnewson commented on a diff in pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
rnewson commented on code in PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#discussion_r884994627


##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -253,6 +246,68 @@ jwt_authentication_handler(Req) ->
             Req
     end.
 
+get_roles_claim(Claims) ->
+    Roles_Claim_Name_Param = config:get(
+        "jwt_auth", "roles_claim_name"
+    ),
+    Roles_Claim_Path_Param = config:get(
+        "jwt_auth", "roles_claim_path"
+    ),
+    Roles =
+        case Roles_Claim_Name_Param of
+            undefined ->
+                % Check for implicitly role_claim "_couchdb.roles" in JWT token
+                case lists:keyfind(<<"_couchdb.roles">>, 1, Claims) of

Review Comment:
   I don't understand this. We definitely do not let the token decide where the role param is. The couchdb server's config is the only authority on that.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] big-r81 commented on pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
big-r81 commented on PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#issuecomment-1140873430

   I updated the PR with the following logic:
   
   We have now different code paths, how a list of couchdb roles can get into the system (inclusive backwards compatibility). 
   I want to explain it in detail:
   
   | roles_claim_name | roles_claim_path | _couchdb.roles in JWT token | used in CouchDB | log message |
   | ---  | --- | --- | --- | --- |
   | `undef`  | `undef` | `undef` | `---` | `---` |
   | `undef`  | `undef` | `def` | `_couchdb.roles` |  implicit values found and used, migrate to path |
   | `undef`  | `def` | `undef` | `roles_claim_path` | --- |
   | `undef`  | `def` | `def` | `_couchdb.roles` |  implicit values found and used, migrate to path |
   | `def`  | `undef` | `undef` | `roles_claim_name` | Use of 'roles_claim_name' is deprecated. Please migrate to 'roles_claim_path'! |
   | `def`  | `undef` | `def` | `roles_claim_name` | Use of 'roles_claim_name' is deprecated. Please migrate to 'roles_claim_path'! |
   | `def`  | `def` | `undef` | `roles_claim_name` | Both, 'roles_claim_name' and 'roles_claim_path' are set. For backwards compatibility, only `roles_claim_name`is used! |
   | `def`  | `def` | `def` | `roles_claim_name` | Both, 'roles_claim_name' and 'roles_claim_path' are set. For backwards compatibility, only `roles_claim_name`is used! |
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] rnewson commented on a diff in pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
rnewson commented on code in PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#discussion_r902726091


##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -253,6 +246,59 @@ jwt_authentication_handler(Req) ->
             Req
     end.
 
+tokenize_json_path(Path, SliceStart, [], Result) ->
+    Result1 = Result ++ [?l2b(string:slice(Path, SliceStart))],
+    [?l2b(string:replace(X, "\\.", ".", all)) || X <- Result1];
+tokenize_json_path(Path, SliceStart, [[{Pos, _}] | T], Result) ->
+    Slice = string:slice(Path, SliceStart, Pos - SliceStart),
+    NewResult = Result ++ [?l2b(Slice)],
+    tokenize_json_path(Path, Pos + 1, T, NewResult).
+
+tokenize_json_path(Path, SplitPositions) ->
+    tokenize_json_path(Path, 0, SplitPositions, []).
+
+get_roles_claim(Claims) ->
+    RolesClaimPath = config:get(
+        "jwt_auth", "roles_claim_path"
+    ),
+    Result =
+        case RolesClaimPath of
+            undefined ->
+                couch_util:get_value(
+                    ?l2b(
+                        config:get(
+                            "jwt_auth", "roles_claim_name", "_couchdb.roles"
+                        )
+                    ),
+                    Claims,
+                    []
+                );
+            _ ->

Review Comment:
   `_` can only be a list, so test for that `when is_list(X) ->`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] big-r81 commented on a diff in pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#discussion_r902766142


##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -253,6 +246,59 @@ jwt_authentication_handler(Req) ->
             Req
     end.
 
+tokenize_json_path(Path, SliceStart, [], Result) ->
+    Result1 = Result ++ [?l2b(string:slice(Path, SliceStart))],
+    [?l2b(string:replace(X, "\\.", ".", all)) || X <- Result1];
+tokenize_json_path(Path, SliceStart, [[{Pos, _}] | T], Result) ->
+    Slice = string:slice(Path, SliceStart, Pos - SliceStart),
+    NewResult = Result ++ [?l2b(Slice)],
+    tokenize_json_path(Path, Pos + 1, T, NewResult).
+
+tokenize_json_path(Path, SplitPositions) ->
+    tokenize_json_path(Path, 0, SplitPositions, []).
+
+get_roles_claim(Claims) ->
+    RolesClaimPath = config:get(
+        "jwt_auth", "roles_claim_path"
+    ),
+    Result =
+        case RolesClaimPath of
+            undefined ->
+                couch_util:get_value(
+                    ?l2b(
+                        config:get(
+                            "jwt_auth", "roles_claim_name", "_couchdb.roles"
+                        )
+                    ),
+                    Claims,
+                    []
+                );
+            _ ->
+                % find all "." but no "\."
+                PathRegex = "(?<!\\\\)\\.",
+                MatchPositions =
+                    case re:run(RolesClaimPath, PathRegex, [global]) of
+                        nomatch -> [];
+                        {match, Pos} -> Pos
+                    end,
+                TokenizedJsonPath = tokenize_json_path(RolesClaimPath, MatchPositions),
+                couch_util:get_nested_json_value({Claims}, TokenizedJsonPath)
+        end,
+    case is_list(Result) of
+        true ->
+            [

Review Comment:
   ok, I "stole" this from here: https://github.com/apache/couchdb/blob/116c4e95636483143830d2c3ca9c3e2f68ff2b23/src/couch/src/couch_db.erl#L814



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] big-r81 commented on a diff in pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#discussion_r883607290


##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -227,22 +227,27 @@ jwt_authentication_handler(Req) ->
             RequiredClaims = get_configured_claims(),
             case jwtf:decode(?l2b(Jwt), [alg | RequiredClaims], fun jwtf_keystore:get/2) of
                 {ok, {Claims}} ->
+                    couch_log:info("Claims: ~p", [Claims]),

Review Comment:
   for the moment yes, i left them in for the draft, but they are not in the final pr...



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] janl commented on a diff in pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
janl commented on code in PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#discussion_r883571201


##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -227,22 +227,27 @@ jwt_authentication_handler(Req) ->
             RequiredClaims = get_configured_claims(),
             case jwtf:decode(?l2b(Jwt), [alg | RequiredClaims], fun jwtf_keystore:get/2) of
                 {ok, {Claims}} ->
+                    couch_log:info("Claims: ~p", [Claims]),

Review Comment:
   debugging leftover?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] big-r81 commented on a diff in pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#discussion_r902767976


##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -253,6 +246,59 @@ jwt_authentication_handler(Req) ->
             Req
     end.
 
+tokenize_json_path(Path, SliceStart, [], Result) ->
+    Result1 = Result ++ [?l2b(string:slice(Path, SliceStart))],
+    [?l2b(string:replace(X, "\\.", ".", all)) || X <- Result1];
+tokenize_json_path(Path, SliceStart, [[{Pos, _}] | T], Result) ->
+    Slice = string:slice(Path, SliceStart, Pos - SliceStart),
+    NewResult = Result ++ [?l2b(Slice)],
+    tokenize_json_path(Path, Pos + 1, T, NewResult).
+
+tokenize_json_path(Path, SplitPositions) ->
+    tokenize_json_path(Path, 0, SplitPositions, []).
+
+get_roles_claim(Claims) ->
+    RolesClaimPath = config:get(
+        "jwt_auth", "roles_claim_path"
+    ),
+    Result =
+        case RolesClaimPath of
+            undefined ->
+                couch_util:get_value(
+                    ?l2b(
+                        config:get(
+                            "jwt_auth", "roles_claim_name", "_couchdb.roles"
+                        )
+                    ),
+                    Claims,
+                    []
+                );
+            _ ->
+                % find all "." but no "\."
+                PathRegex = "(?<!\\\\)\\.",
+                MatchPositions =
+                    case re:run(RolesClaimPath, PathRegex, [global]) of
+                        nomatch -> [];
+                        {match, Pos} -> Pos
+                    end,
+                TokenizedJsonPath = tokenize_json_path(RolesClaimPath, MatchPositions),
+                couch_util:get_nested_json_value({Claims}, TokenizedJsonPath)
+        end,
+    case is_list(Result) of
+        true ->
+            [
+                throw(
+                    {bad_request, <<"Malformed JWT roles claim. Must be a JSON list of strings">>}

Review Comment:
   ok, will shorten that message, same as above, looked at I "stole" this from here https://github.com/apache/couchdb/blob/116c4e95636483143830d2c3ca9c3e2f68ff2b23/src/couch/src/couch_db.erl#L814



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] rnewson merged pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
rnewson merged PR #4041:
URL: https://github.com/apache/couchdb/pull/4041


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] big-r81 commented on pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
big-r81 commented on PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#issuecomment-1142236302

   I've updated the request with things discussed today. It is now possible to specify nested (& unnested) JSON paths in role_claim_path, like `roles_claim_path = foo.bar\.zonk.baz\.buu.baa.baa\.bee.roles` which is the equivalent of
   ```json
   "foo": {
     "bar.zonk": {
       "baz.buu": {
         "baa": {
           "baa.bee": {
             "roles": [
               "my_nested_role_1",
               "my_nested_role_2"
             ]
           }
         }
       }
     }
   }
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] big-r81 commented on a diff in pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#discussion_r884104705


##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -227,22 +227,27 @@ jwt_authentication_handler(Req) ->
             RequiredClaims = get_configured_claims(),
             case jwtf:decode(?l2b(Jwt), [alg | RequiredClaims], fun jwtf_keystore:get/2) of
                 {ok, {Claims}} ->
+                    couch_log:info("Claims: ~p", [Claims]),
+                    Roles_Claim_Name = config:get(
+                        "jwt_auth", "roles_claim_name", "_couchdb.roles"
+                    ),
+                    Roles_Claim_Path = [list_to_binary(Item) || Item <- string:split(config:get(
+                        "jwt_auth", "roles_claim_path", ""

Review Comment:
   Should we mark `roles_claim_name` as deprecated (log a message, documentation) to only have one setting for this in future versions? I would suggest current behavior:
   
   1. If `roles_claim_name` is found, use this - old behavior and doesn't break anything and log a info message to migrate to `roles_claim_path`
   2. If `roles_claim_path` is found
     2.1 Use this
     2.2 If `roles_claim_name`is defined too, log an info message, that this value is ignoried, because of `roles_claim_path`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] rnewson commented on a diff in pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
rnewson commented on code in PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#discussion_r885049147


##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -253,6 +246,68 @@ jwt_authentication_handler(Req) ->
             Req
     end.
 
+get_roles_claim(Claims) ->
+    Roles_Claim_Name_Param = config:get(
+        "jwt_auth", "roles_claim_name"
+    ),
+    Roles_Claim_Path_Param = config:get(
+        "jwt_auth", "roles_claim_path"
+    ),
+    Roles =
+        case Roles_Claim_Name_Param of
+            undefined ->
+                % Check for implicitly role_claim "_couchdb.roles" in JWT token
+                case lists:keyfind(<<"_couchdb.roles">>, 1, Claims) of

Review Comment:
   ```config:get("jwt_auth", "roles_claim_name", "_couchdb.roles")``` returns the roles_claim_name value from config or, if there isn't one, the default of _couchdb.roles. I see you've done the equivalent logic but in a novel and longer-form way. It would be better to stick with convention and call config:get/3 with our preferred default value.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] big-r81 commented on a diff in pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#discussion_r902770861


##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -253,6 +246,59 @@ jwt_authentication_handler(Req) ->
             Req
     end.
 
+tokenize_json_path(Path, SliceStart, [], Result) ->
+    Result1 = Result ++ [?l2b(string:slice(Path, SliceStart))],
+    [?l2b(string:replace(X, "\\.", ".", all)) || X <- Result1];
+tokenize_json_path(Path, SliceStart, [[{Pos, _}] | T], Result) ->
+    Slice = string:slice(Path, SliceStart, Pos - SliceStart),
+    NewResult = Result ++ [?l2b(Slice)],
+    tokenize_json_path(Path, Pos + 1, T, NewResult).
+
+tokenize_json_path(Path, SplitPositions) ->
+    tokenize_json_path(Path, 0, SplitPositions, []).
+
+get_roles_claim(Claims) ->
+    RolesClaimPath = config:get(
+        "jwt_auth", "roles_claim_path"
+    ),
+    Result =
+        case RolesClaimPath of
+            undefined ->
+                couch_util:get_value(
+                    ?l2b(
+                        config:get(
+                            "jwt_auth", "roles_claim_name", "_couchdb.roles"
+                        )
+                    ),
+                    Claims,
+                    []
+                );
+            _ ->

Review Comment:
   No, RolesClaimPath isn't a list, it's something like `my.nested.couchdb\.roles'



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] rnewson commented on a diff in pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
rnewson commented on code in PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#discussion_r884994784


##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -253,6 +246,68 @@ jwt_authentication_handler(Req) ->
             Req
     end.
 
+get_roles_claim(Claims) ->
+    Roles_Claim_Name_Param = config:get(

Review Comment:
   these names do not follow any convention in the rest of the codebase.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] big-r81 commented on a diff in pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#discussion_r885017465


##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -253,6 +246,68 @@ jwt_authentication_handler(Req) ->
             Req
     end.
 
+get_roles_claim(Claims) ->
+    Roles_Claim_Name_Param = config:get(

Review Comment:
   Is `RolesClaimNameParam` a better alternative, or what are the conventions?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] big-r81 commented on a diff in pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#discussion_r885017139


##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -253,6 +246,68 @@ jwt_authentication_handler(Req) ->
             Req
     end.
 
+get_roles_claim(Claims) ->
+    Roles_Claim_Name_Param = config:get(
+        "jwt_auth", "roles_claim_name"
+    ),
+    Roles_Claim_Path_Param = config:get(
+        "jwt_auth", "roles_claim_path"
+    ),
+    Roles =
+        case Roles_Claim_Name_Param of
+            undefined ->
+                % Check for implicitly role_claim "_couchdb.roles" in JWT token
+                case lists:keyfind(<<"_couchdb.roles">>, 1, Claims) of

Review Comment:
   But it's the current (default) behavior, if it's in the token. See  the current codebase, we have [this:](https://github.com/apache/couchdb/blob/83279f66b78bc714b7844b90cc9dbf71c7772c52/src/couch/src/couch_httpd_auth.erl#L237)
   ```erlang
   roles = couch_util:get_value(
         ?l2b(
               config:get(
                   "jwt_auth", "roles_claim_name", "_couchdb.roles"
               )
        ),
        Claims,
        []
   )
   ```
   This sets the default value of `roles_claim_name` to `_couchdb.roles` and if this is in the token, the roles are set from the token.
   
   I only tried to replicate the current behavior.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] jjrodrig commented on a diff in pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
jjrodrig commented on code in PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#discussion_r899953971


##########
test/elixir/test/jwt_roles_claim_test.exs:
##########
@@ -0,0 +1,126 @@
+defmodule JwtRolesClaimTest do
+  use CouchTestCase
+
+  @testdb "jwttestdb"
+
+  @global_server_config [
+    %{
+      :section => "chttpd",
+      :key => "authentication_handlers",
+      :value => [
+                  "{chttpd_auth, jwt_authentication_handler}, ",
+                  "{chttpd_auth, cookie_authentication_handler}, ",
+                  "{chttpd_auth, default_authentication_handler})"
+                ] |> Enum.join
+    },
+    %{
+      :section => "jwt_keys",
+      :key => "hmac:myjwttestkey",
+      :value => ~w(
+        NTNv7j0TuYARvmNMmWXo6fKvM4o6nv/aUi9ryX38ZH+L1bkrnD1ObOQ8JAUmHCBq7
+        Iy7otZcyAagBLHVKvvYaIpmMuxmARQ97jUVG16Jkpkp1wXOPsrF9zwew6TpczyH
+        kHgX5EuLg2MeBuiT/qJACs1J0apruOOJCg/gOtkjB4c=) |> Enum.join()
+    }
+  ]
+
+  setup do

Review Comment:
   Hi @big-r81 
   Just a question, do you need to create a testdb? It seems that the _session endpoint is the only involved here.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] big-r81 commented on a diff in pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#discussion_r899970375


##########
test/elixir/test/jwt_roles_claim_test.exs:
##########
@@ -0,0 +1,126 @@
+defmodule JwtRolesClaimTest do
+  use CouchTestCase
+
+  @testdb "jwttestdb"
+
+  @global_server_config [
+    %{
+      :section => "chttpd",
+      :key => "authentication_handlers",
+      :value => [
+                  "{chttpd_auth, jwt_authentication_handler}, ",
+                  "{chttpd_auth, cookie_authentication_handler}, ",
+                  "{chttpd_auth, default_authentication_handler})"
+                ] |> Enum.join
+    },
+    %{
+      :section => "jwt_keys",
+      :key => "hmac:myjwttestkey",
+      :value => ~w(
+        NTNv7j0TuYARvmNMmWXo6fKvM4o6nv/aUi9ryX38ZH+L1bkrnD1ObOQ8JAUmHCBq7
+        Iy7otZcyAagBLHVKvvYaIpmMuxmARQ97jUVG16Jkpkp1wXOPsrF9zwew6TpczyH
+        kHgX5EuLg2MeBuiT/qJACs1J0apruOOJCg/gOtkjB4c=) |> Enum.join()
+    }
+  ]
+
+  setup do

Review Comment:
   Hi, oh yes, we dont' need it! Good catch! First I wanted to evaluate `$db/_security` and found the way to the `_session` endpoint. So I can remove setup/0 and tear_down/0.  



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] rnewson commented on a diff in pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
rnewson commented on code in PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#discussion_r902721938


##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -253,6 +246,59 @@ jwt_authentication_handler(Req) ->
             Req
     end.
 
+tokenize_json_path(Path, SliceStart, [], Result) ->
+    Result1 = Result ++ [?l2b(string:slice(Path, SliceStart))],
+    [?l2b(string:replace(X, "\\.", ".", all)) || X <- Result1];
+tokenize_json_path(Path, SliceStart, [[{Pos, _}] | T], Result) ->
+    Slice = string:slice(Path, SliceStart, Pos - SliceStart),
+    NewResult = Result ++ [?l2b(Slice)],
+    tokenize_json_path(Path, Pos + 1, T, NewResult).
+
+tokenize_json_path(Path, SplitPositions) ->
+    tokenize_json_path(Path, 0, SplitPositions, []).
+
+get_roles_claim(Claims) ->
+    RolesClaimPath = config:get(
+        "jwt_auth", "roles_claim_path"
+    ),
+    Result =
+        case RolesClaimPath of
+            undefined ->
+                couch_util:get_value(
+                    ?l2b(
+                        config:get(
+                            "jwt_auth", "roles_claim_name", "_couchdb.roles"
+                        )
+                    ),
+                    Claims,
+                    []
+                );
+            _ ->
+                % find all "." but no "\."
+                PathRegex = "(?<!\\\\)\\.",
+                MatchPositions =
+                    case re:run(RolesClaimPath, PathRegex, [global]) of
+                        nomatch -> [];
+                        {match, Pos} -> Pos
+                    end,
+                TokenizedJsonPath = tokenize_json_path(RolesClaimPath, MatchPositions),
+                couch_util:get_nested_json_value({Claims}, TokenizedJsonPath)
+        end,
+    case is_list(Result) of
+        true ->
+            [
+                throw(
+                    {bad_request, <<"Malformed JWT roles claim. Must be a JSON list of strings">>}

Review Comment:
   `{bad_request, <<"Malformed token">>}` to match the other errors, we try not to reveal useful information to failed authentication requests.



##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -253,6 +246,59 @@ jwt_authentication_handler(Req) ->
             Req
     end.
 
+tokenize_json_path(Path, SliceStart, [], Result) ->
+    Result1 = Result ++ [?l2b(string:slice(Path, SliceStart))],
+    [?l2b(string:replace(X, "\\.", ".", all)) || X <- Result1];
+tokenize_json_path(Path, SliceStart, [[{Pos, _}] | T], Result) ->
+    Slice = string:slice(Path, SliceStart, Pos - SliceStart),
+    NewResult = Result ++ [?l2b(Slice)],
+    tokenize_json_path(Path, Pos + 1, T, NewResult).
+
+tokenize_json_path(Path, SplitPositions) ->
+    tokenize_json_path(Path, 0, SplitPositions, []).
+
+get_roles_claim(Claims) ->
+    RolesClaimPath = config:get(
+        "jwt_auth", "roles_claim_path"
+    ),
+    Result =
+        case RolesClaimPath of
+            undefined ->
+                couch_util:get_value(
+                    ?l2b(
+                        config:get(
+                            "jwt_auth", "roles_claim_name", "_couchdb.roles"
+                        )
+                    ),
+                    Claims,
+                    []
+                );
+            _ ->
+                % find all "." but no "\."
+                PathRegex = "(?<!\\\\)\\.",
+                MatchPositions =
+                    case re:run(RolesClaimPath, PathRegex, [global]) of
+                        nomatch -> [];
+                        {match, Pos} -> Pos
+                    end,
+                TokenizedJsonPath = tokenize_json_path(RolesClaimPath, MatchPositions),
+                couch_util:get_nested_json_value({Claims}, TokenizedJsonPath)
+        end,
+    case is_list(Result) of
+        true ->
+            [
+                throw(
+                    {bad_request, <<"Malformed JWT roles claim. Must be a JSON list of strings">>}
+                )
+             || R <- Result, not is_binary(R)
+            ],
+            Result;
+        false ->
+            throw(
+                {bad_request, <<"Malformed JWT roles claim. Needs to be a JSON list of strings.">>}

Review Comment:
   `{bad_request, <<"Malformed token">>}` to match the other errors, we try not to reveal useful information to failed authentication requests.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] rnewson commented on a diff in pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
rnewson commented on code in PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#discussion_r902724405


##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -253,6 +246,59 @@ jwt_authentication_handler(Req) ->
             Req
     end.
 
+tokenize_json_path(Path, SliceStart, [], Result) ->
+    Result1 = Result ++ [?l2b(string:slice(Path, SliceStart))],
+    [?l2b(string:replace(X, "\\.", ".", all)) || X <- Result1];
+tokenize_json_path(Path, SliceStart, [[{Pos, _}] | T], Result) ->
+    Slice = string:slice(Path, SliceStart, Pos - SliceStart),
+    NewResult = Result ++ [?l2b(Slice)],
+    tokenize_json_path(Path, Pos + 1, T, NewResult).
+
+tokenize_json_path(Path, SplitPositions) ->
+    tokenize_json_path(Path, 0, SplitPositions, []).
+
+get_roles_claim(Claims) ->
+    RolesClaimPath = config:get(
+        "jwt_auth", "roles_claim_path"
+    ),
+    Result =
+        case RolesClaimPath of
+            undefined ->
+                couch_util:get_value(
+                    ?l2b(
+                        config:get(
+                            "jwt_auth", "roles_claim_name", "_couchdb.roles"
+                        )
+                    ),
+                    Claims,
+                    []
+                );
+            _ ->
+                % find all "." but no "\."
+                PathRegex = "(?<!\\\\)\\.",
+                MatchPositions =
+                    case re:run(RolesClaimPath, PathRegex, [global]) of
+                        nomatch -> [];
+                        {match, Pos} -> Pos
+                    end,
+                TokenizedJsonPath = tokenize_json_path(RolesClaimPath, MatchPositions),
+                couch_util:get_nested_json_value({Claims}, TokenizedJsonPath)
+        end,
+    case is_list(Result) of
+        true ->
+            [

Review Comment:
   a list comprehension for its side-effects is a bad practice, the resulting list is constructed by discard. consider https://www.erlang.org/doc/man/lists.html#all-2 like `case lists:all(fun erlang:is_binary/1, Result) of true -> X; false -> Y`



##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -253,6 +246,59 @@ jwt_authentication_handler(Req) ->
             Req
     end.
 
+tokenize_json_path(Path, SliceStart, [], Result) ->
+    Result1 = Result ++ [?l2b(string:slice(Path, SliceStart))],
+    [?l2b(string:replace(X, "\\.", ".", all)) || X <- Result1];
+tokenize_json_path(Path, SliceStart, [[{Pos, _}] | T], Result) ->
+    Slice = string:slice(Path, SliceStart, Pos - SliceStart),
+    NewResult = Result ++ [?l2b(Slice)],
+    tokenize_json_path(Path, Pos + 1, T, NewResult).
+
+tokenize_json_path(Path, SplitPositions) ->
+    tokenize_json_path(Path, 0, SplitPositions, []).
+
+get_roles_claim(Claims) ->
+    RolesClaimPath = config:get(
+        "jwt_auth", "roles_claim_path"
+    ),
+    Result =
+        case RolesClaimPath of
+            undefined ->
+                couch_util:get_value(
+                    ?l2b(
+                        config:get(
+                            "jwt_auth", "roles_claim_name", "_couchdb.roles"
+                        )
+                    ),
+                    Claims,
+                    []
+                );
+            _ ->
+                % find all "." but no "\."
+                PathRegex = "(?<!\\\\)\\.",
+                MatchPositions =
+                    case re:run(RolesClaimPath, PathRegex, [global]) of
+                        nomatch -> [];
+                        {match, Pos} -> Pos
+                    end,
+                TokenizedJsonPath = tokenize_json_path(RolesClaimPath, MatchPositions),
+                couch_util:get_nested_json_value({Claims}, TokenizedJsonPath)
+        end,
+    case is_list(Result) of
+        true ->
+            [

Review Comment:
   a list comprehension for its side-effects is a bad practice, the resulting list is constructed but discarded. consider https://www.erlang.org/doc/man/lists.html#all-2 like `case lists:all(fun erlang:is_binary/1, Result) of true -> X; false -> Y`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] rnewson commented on a diff in pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
rnewson commented on code in PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#discussion_r883670506


##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -227,22 +227,27 @@ jwt_authentication_handler(Req) ->
             RequiredClaims = get_configured_claims(),
             case jwtf:decode(?l2b(Jwt), [alg | RequiredClaims], fun jwtf_keystore:get/2) of
                 {ok, {Claims}} ->
+                    couch_log:info("Claims: ~p", [Claims]),
+                    Roles_Claim_Name = config:get(
+                        "jwt_auth", "roles_claim_name", "_couchdb.roles"
+                    ),
+                    Roles_Claim_Path = [list_to_binary(Item) || Item <- string:split(config:get(
+                        "jwt_auth", "roles_claim_path", ""

Review Comment:
   I think it would be cleaner if roles_claim_path was independent of roles_claim_name. so you'd set it to "foo.bar.baz.roles". Ignore roles_claim_name if roles_claim_path is defined.
   
   And obviously remove the couch_log:info lines.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] rnewson commented on a diff in pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
rnewson commented on code in PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#discussion_r883670836


##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -227,22 +227,27 @@ jwt_authentication_handler(Req) ->
             RequiredClaims = get_configured_claims(),
             case jwtf:decode(?l2b(Jwt), [alg | RequiredClaims], fun jwtf_keystore:get/2) of
                 {ok, {Claims}} ->
+                    couch_log:info("Claims: ~p", [Claims]),
+                    Roles_Claim_Name = config:get(
+                        "jwt_auth", "roles_claim_name", "_couchdb.roles"
+                    ),
+                    Roles_Claim_Path = [list_to_binary(Item) || Item <- string:split(config:get(
+                        "jwt_auth", "roles_claim_path", ""

Review Comment:
   and use the shorter` ?l2b` and `?b2l` macros



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb] rnewson commented on a diff in pull request #4041: Allow and evaluate nested json claim roles

Posted by GitBox <gi...@apache.org>.
rnewson commented on code in PR #4041:
URL: https://github.com/apache/couchdb/pull/4041#discussion_r884995242


##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -253,6 +246,68 @@ jwt_authentication_handler(Req) ->
             Req
     end.
 
+get_roles_claim(Claims) ->
+    Roles_Claim_Name_Param = config:get(
+        "jwt_auth", "roles_claim_name"
+    ),
+    Roles_Claim_Path_Param = config:get(
+        "jwt_auth", "roles_claim_path"
+    ),
+    Roles =
+        case Roles_Claim_Name_Param of
+            undefined ->
+                % Check for implicitly role_claim "_couchdb.roles" in JWT token
+                case lists:keyfind(<<"_couchdb.roles">>, 1, Claims) of
+                    false ->
+                        % `_couchdb.roles` not found in token
+                        % check if `roles_claim_path` is set
+                        case Roles_Claim_Path_Param of
+                            undefined ->
+                                [];
+                            _ ->
+                                % no implicit or explicit value for `_couchdb.roles` found, use `roles_claim_path`
+                                Roles_Claim_Path = [
+                                    ?l2b(Item)
+                                 || Item <- string:tokens(Roles_Claim_Path_Param, ".")
+                                ],
+                                couch_util:get_nested_json_value({Claims}, Roles_Claim_Path)
+                        end;
+                    {_, Token_CouchDBRoles} ->
+                        % `_couchdb.roles` found in token, read value
+                        couch_log:info(
+                            "Implicit value for `_couchdb.roles` found and used in token, please migrate to" ++
+                                " `roles_claim_path`!",
+                            []
+                        ),
+                        Token_CouchDBRoles
+                end;
+            _ ->
+                % other value for `roles_claim_name` found.
+                if
+                    Roles_Claim_Path_Param =/= undefined ->
+                        couch_log:info(

Review Comment:
   logging this on every occurrence is not great. You've also decided that roles_claim_name is deprecated but there's been no suggestion of that so far and it doesn't seem reasonable to me.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org