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 2020/03/10 01:21:05 UTC

[GitHub] [couchdb] atrauzzi opened a new pull request #2648: Feature - Add JWT support

atrauzzi opened a new pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648

## Overview

👋 Hello! This PR is introduces support for JWT authentication in CouchDB.

Before any details, I would like to give a big thank-you to @rnewson for his guidance and help. He's been a great ambassador! 👏

### Why?

I'm starting out with the familiar scenario of a JavaScript application using PouchDB that I wish to sync to CouchDB.
What's different is that my JS application performs authentication against an OpenID Connect/Oauth2 server that is not running on CouchDB. Think Auth0, Azure AD B2C, OpenIddict Core, etc...

At present, in order to authenticate with CouchDB from clients that are already performing authentication outside of the CouchDB server, I am forced to consider some undesirable duplications of the concept of "identity" in my ecosystem. This all while a de-facto standard already exists for the notion of a portable identity token: JWT.

My hope is that this implementation weighed against the complexity and less deliberate nature of any one workaround makes a good case for itself.

---

## Testing recommendations

After enabling the authorization handler and setting a matching JWT secret in your couchdb ini file, you should be able to authenticate to CouchDB using an HTTP `Authorization: Bearer [token]` header.

## Checklist

- [x] Code is written and works correctly
- [ ] Changes are covered by tests
- [x] 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

## Meta

I'm relatively new to Erlang but am interested enough to get this through review. I've tried to keep my changes as additive as possible however if any refactors or structural work ends up being required, I may need detailed guidance.

Finally, I feel this functionality would be valuable not just for my own personal use, but for future users considering CouchDB for their own mixed ecosystems. Many cloud vendors offer HTTP-centric API gateway services which are a very good match for CouchDB.

CouchDB is in an interesting spot because it's made to be directly accessed by clients. This intersects nicely with the overall design and portability of JWT bearer tokens. Not that I want to sound too product-y, but I think having JWT support built in would be a great way to broaden CouchDBs reach! 🙂

----------------------------------------------------------------
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.

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

With regards,
Apache Git Services

[GitHub] [couchdb] jjrodrig commented on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

jjrodrig commented on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-597338929
 
 
   Hi @atrauzzi 
   
   I'm currently working on the js to elixir porting.  I can try to help you with the elixir part. 
   
   This contribution looks great. Thanks!

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391281729
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,25 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {undefined, _} -> Req;
+        {_, undefined} -> Req;
+        {Secret, "Bearer " ++ Jwt} ->
+            RequiredClaims = re:split(config:get("jwt_auth", "required_claims", "sub, exp"), "\s*,\s*"),
+            AllowedAlgorithms = re:split(config:get("jwt_auth", "allowed_algorithms", "HS256"), "\s*,\s*"),
+            TimestampLeniency = config:get_integer("jwt_auth", "timestamp_leniency", "0"),
 
 Review comment:
   Apologies, I'm so used to having a compiler for type checking small fixes 🙂

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391945025
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,31 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {Secret, "Bearer " ++ Jwt} when Secret /= undefined ->
+            RequiredClaims = get_configured_claims(),
+            case jwtf:decode(?l2b(Jwt), RequiredClaims, fun(_,_) -> Secret end) of
+                {ok, {Claims}} ->
+                    {_, User} = lists:keyfind(<<"sub">>, 1, Claims),
+                    Req#httpd{user_ctx=#user_ctx{
+                        name=User
+                    }};
+                {error, Reason} ->
+                    throw({unauthorized, Reason})
+            end;
+        {_, _} -> Req
+    end.
+
+get_configured_claims() ->
+    jwt_default_claims(re:split(config:get("jwt_auth", "required_claims", "sub"), "\s*,\s*")).
+
+jwt_default_claims(Claims) ->
+    DefaultClaims = [<<"sub">>],
+    case Claims of
+        "" -> DefaultClaims;
 
 Review comment:
   re:split won't ever return `""` so this clause is unreachable.
   
   ```
   jwt_default_claims(Claims) when is_list(Claims) ->
       lists:usort([<<"sub">> | Claims]).
   ```
   
   would seem to suffice.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-597117809

Most of the objection to JWT in those articles appears to be that the tokens can't be expired ahead of their built-in exp field, a criticism that can equally be levelled at our session cookies.

On that point, this PR does not directly validate any of the time fields (iat, nbf and exp). If the jwt library is doing that, we should say so, as this validation is mandatory.

The other objection I could determine is cryptographic, in that the tokens themselves dictate which encryption scheme to use to validate them, and there's at least one known weakness where a substitution can be an issue.

We should get ahead of this and have server side controls over what we'll accept. I can think of a few settings off the top of my head but this should be thorough before merging;

* a config setting listing which of the JWT schemes will be allowed.
* a config setting listing which asymmetric signing keys are permitted.
* configurable leniency on the various timestamp fields, defaulting to 0 (no leniency).

Finally, if we're doing this for CouchDB I would prefer to use the JWT library I developed for Cloudant rather than this one. I will, of course, pursue the licensing steps, etc.

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

With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391330484
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,25 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {undefined, _} -> Req;
+        {_, undefined} -> Req;
+        {Secret, "Bearer " ++ Jwt} ->
+            RequiredClaims = re:split(config:get("jwt_auth", "required_claims", "sub, exp"), "\s*,\s*"),
+            AllowedAlgorithms = re:split(config:get("jwt_auth", "allowed_algorithms", "HS256"), "\s*,\s*"),
+            TimestampLeniency = config:get_integer("jwt_auth", "timestamp_leniency", "0"),
+            case jwtf:decode(?l2b(Jwt), RequiredClaims, fun(_,_) -> Secret end) of
+                {ok, {Claims}} ->
+                    {_, User} = lists:keyfind(<<"sub">>, 1, Claims),
 
 Review comment:
   Done!

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r392114748
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,27 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {Secret, "Bearer " ++ Jwt} when Secret /= undefined ->
+            RequiredClaims = get_configured_claims(),
+            case jwtf:decode(?l2b(Jwt), RequiredClaims, fun(_,_) -> Secret end) of
 
 Review comment:
   the one conceptual wrinkle is that we don't tie the `secret` value to any particular algorithm, we'll attempt the decode for any algorithm, as dictated by the JWT header. I think, for safety, we need a property for allowed algorithm (singular) for this first JWT merge. Might need to enhance jwtf to verify this. Thoughts?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi commented on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi commented on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-597367015
 
 
   @jjrodrig - Thanks, that would be lovely! Let me know how you'd like to proceed.
   
   I'm hanging out on the CouchDB slack most of the time lately if you prefer something slightly more synchronous.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-598157980
 
 
   fyi, we'll import jwtf and its history directly into the project: https://github.com/apache/couchdb/pull/2658

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] wohali commented on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

wohali commented on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-597302288
 
 
   Yes, our integration tests are over in JS, not in eunit largely.
   
   We're migrating all of our tests out of JS into Elixir. You should use Elixir to write any new integration tests.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] wohali commented on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

wohali commented on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-597189439
 
 
   Hi there - thanks for the essay. I'm not a security expert and won't pretend to be one. I'm comfortable knowing that the issues have been reviewed. I wasn't planning on vetoing JWT support, in any case.
   
   Carry on.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391281850
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,25 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {undefined, _} -> Req;
+        {_, undefined} -> Req;
+        {Secret, "Bearer " ++ Jwt} ->
+            RequiredClaims = re:split(config:get("jwt_auth", "required_claims", "sub, exp"), "\s*,\s*"),
+            AllowedAlgorithms = re:split(config:get("jwt_auth", "allowed_algorithms", "HS256"), "\s*,\s*"),
+            TimestampLeniency = config:get_integer("jwt_auth", "timestamp_leniency", "0"),
+            case jwtf:decode(?l2b(Jwt), RequiredClaims, fun(_,_) -> Secret end) of
+                {ok, {Claims}} ->
+                    {_, User} = lists:keyfind(<<"sub">>, 1, Claims),
 
 Review comment:
   Are you suggesting we prepend/append `<<"sub">>` to `RequiredClaims`?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391269387
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,25 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {undefined, _} -> Req;
+        {_, undefined} -> Req;
+        {Secret, "Bearer " ++ Jwt} ->
+            RequiredClaims = re:split(config:get("jwt_auth", "required_claims", "sub, exp"), "\s*,\s*"),
+            AllowedAlgorithms = re:split(config:get("jwt_auth", "allowed_algorithms", "HS256"), "\s*,\s*"),
+            TimestampLeniency = config:get_integer("jwt_auth", "timestamp_leniency", "0"),
 
 Review comment:
   you've run this? should be an error to say `"0"` as integer default versus `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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391304723
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,25 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {undefined, _} -> Req;
+        {_, undefined} -> Req;
+        {Secret, "Bearer " ++ Jwt} ->
+            RequiredClaims = re:split(config:get("jwt_auth", "required_claims", "sub, exp"), "\s*,\s*"),
+            AllowedAlgorithms = re:split(config:get("jwt_auth", "allowed_algorithms", "HS256"), "\s*,\s*"),
+            TimestampLeniency = config:get_integer("jwt_auth", "timestamp_leniency", "0"),
+            case jwtf:decode(?l2b(Jwt), RequiredClaims, fun(_,_) -> Secret end) of
+                {ok, {Claims}} ->
+                    {_, User} = lists:keyfind(<<"sub">>, 1, Claims),
 
 Review comment:
   Does erlang or couchdb have a fancy utility method to intersect two lists?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r390155870
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,26 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    Secret = config:get("couch_httpd_auth", "jwt_secret", ""),
+    UserClaim = config:get("couch_httpd_auth", "jwt_user_claim", "sub"),
+
+    case header_value(Req, "Authorization") of
+        "Bearer " ++ Jwt ->
+            case jwt:decode(?l2b(Jwt), ?l2b(Secret)) of
+                {ok, Claims} ->
+                    User = maps:get(?l2b(UserClaim), Claims),
+
+                    Req#httpd{user_ctx=#user_ctx{
+                        name=User,
+                        roles=[]
+                    }};
+                {error, Reason} ->
+		            throw({unauthorized, Reason})
 
 Review comment:
   indentation should be 4 spaces.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391588085
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,26 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    DefaultClaims = [<<"sub">>],
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {Secret, "Bearer " ++ Jwt} when Secret /= undefined ->
+            RequiredClaims = case re:split(config:get("jwt_auth", "required_claims", "sub"), "\s*,\s*") of
+                "" -> DefaultClaims;
+                ConfiguredClaims ->
+                    sets:to_list(sets:from_list(lists:merge(DefaultClaims, ConfiguredClaims)))
 
 Review comment:
   `RequiredClaims = lists:usort([<<"sub">> | ConfiguredClaims]).` is the concisest I can think of right now.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391968373
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,31 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {Secret, "Bearer " ++ Jwt} when Secret /= undefined ->
+            RequiredClaims = get_configured_claims(),
+            case jwtf:decode(?l2b(Jwt), RequiredClaims, fun(_,_) -> Secret end) of
+                {ok, {Claims}} ->
+                    {_, User} = lists:keyfind(<<"sub">>, 1, Claims),
+                    Req#httpd{user_ctx=#user_ctx{
+                        name=User
+                    }};
+                {error, Reason} ->
+                    throw({unauthorized, Reason})
+            end;
+        {_, _} -> Req
+    end.
+
+get_configured_claims() ->
+    jwt_default_claims(re:split(config:get("jwt_auth", "required_claims", "sub"), "\s*,\s*")).
+
+jwt_default_claims(Claims) ->
+    DefaultClaims = [<<"sub">>],
+    case Claims of
+        "" -> DefaultClaims;
 
 Review comment:
   Done!

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r390155782
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,26 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    Secret = config:get("couch_httpd_auth", "jwt_secret", ""),
+    UserClaim = config:get("couch_httpd_auth", "jwt_user_claim", "sub"),
+
+    case header_value(Req, "Authorization") of
+        "Bearer " ++ Jwt ->
+            case jwt:decode(?l2b(Jwt), ?l2b(Secret)) of
+                {ok, Claims} ->
+                    User = maps:get(?l2b(UserClaim), Claims),
+
+                    Req#httpd{user_ctx=#user_ctx{
+                        name=User,
+                        roles=[]
 
 Review comment:
   this line can be removed (I realise it's mine) for clarity that JWT auth does not set 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] wohali commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

wohali commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r390068836
 
 

 ##########
 File path: rel/overlay/etc/default.ini
 ##########
 @@ -131,7 +131,7 @@ prefer_minimal = Cache-Control, Content-Length, Content-Range, Content-Type, ETa
 max_db_number_for_dbs_info_req = 100
 
 ; authentication handlers
-; authentication_handlers = {chttpd_auth, cookie_authentication_handler}, {chttpd_auth, default_authentication_handler}
+authentication_handlers = {chttpd_auth, jwt_authentication_handler}, {chttpd_auth, cookie_authentication_handler}, {chttpd_auth, default_authentication_handler}
 
 Review comment:
   The `default.ini` file is intended to document defaults. The line here should remain commented out.
   
   If we add JWT support as enabled out of the box, as you propose, you'd update L134 to include the new default, but you would also change the definition of the default in Erlang at https://github.com/apache/couchdb/blob/master/src/chttpd/src/chttpd.erl#L491-L492 .
   
   If we don't add JWT support as enabled by default, you could add a new example line here similar to the one at line 136.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] wohali commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

wohali commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r390593856
 
 

 ##########
 File path: rel/overlay/etc/default.ini
 ##########
 @@ -267,11 +273,11 @@ credentials = false
 ; List of origins separated by a comma, * means accept all
 ; Origins must include the scheme: http://example.com
 ; You can't set origins: * and credentials = true at the same time.
-;origins = *
+; origins = 
 
 Review comment:
   Restore the original `*` here please.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] wohali commented on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

wohali commented on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-601247238
 
 
   Hey @atrauzzi , we need documentation for this change. Would you be willing to file a PR against https://github.com/apache/couchdb-documentation ? Thanks! Both myself and @flimzy can help if you need it.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] dottorblaster commented on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

dottorblaster commented on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-597505396
 
 
   @atrauzzi don't worry about Elixir, we'll guide you if you need! 🎉 love this, I'll try to help :-)

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-597069422
 
 
   finally, we need some tests and docs, but focus on the comments above first.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391304723
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,25 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {undefined, _} -> Req;
+        {_, undefined} -> Req;
+        {Secret, "Bearer " ++ Jwt} ->
+            RequiredClaims = re:split(config:get("jwt_auth", "required_claims", "sub, exp"), "\s*,\s*"),
+            AllowedAlgorithms = re:split(config:get("jwt_auth", "allowed_algorithms", "HS256"), "\s*,\s*"),
+            TimestampLeniency = config:get_integer("jwt_auth", "timestamp_leniency", "0"),
+            case jwtf:decode(?l2b(Jwt), RequiredClaims, fun(_,_) -> Secret end) of
+                {ok, {Claims}} ->
+                    {_, User} = lists:keyfind(<<"sub">>, 1, Claims),
 
 Review comment:
   Does erlang or couchdb have a fancy utility method to intersect two lists?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391281850
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,25 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {undefined, _} -> Req;
+        {_, undefined} -> Req;
+        {Secret, "Bearer " ++ Jwt} ->
+            RequiredClaims = re:split(config:get("jwt_auth", "required_claims", "sub, exp"), "\s*,\s*"),
+            AllowedAlgorithms = re:split(config:get("jwt_auth", "allowed_algorithms", "HS256"), "\s*,\s*"),
+            TimestampLeniency = config:get_integer("jwt_auth", "timestamp_leniency", "0"),
+            case jwtf:decode(?l2b(Jwt), RequiredClaims, fun(_,_) -> Secret end) of
+                {ok, {Claims}} ->
+                    {_, User} = lists:keyfind(<<"sub">>, 1, Claims),
 
 Review comment:
   Are you suggesting we prepend/append `<<"sub">>` to the config?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r393160561
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,31 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {Secret, "Bearer " ++ Jwt} when Secret /= undefined ->
+            RequiredClaims = get_configured_claims(),
+            AllowedAlgorithms = get_configured_algorithms(),
+            case jwtf:decode(?l2b(Jwt), [{alg, AllowedAlgorithms} | RequiredClaims], fun(_,_) -> Secret end) of
+                {ok, {Claims}} ->
+                    {_, User} = lists:keyfind(<<"sub">>, 1, Claims),
+                    Req#httpd{user_ctx=#user_ctx{
+                        name=User
+                    }};
+                {error, Reason} ->
+                    throw({unauthorized, Reason})
+            end;
+        {_, _} -> Req
+    end.
+
+get_configured_algorithms() ->
+    re:split(config:get("jwt_auth", "allowed_algorithms", "HS256"), "\s*,\s*", [{return, binary}]).
+
+get_configured_claims() ->
+    jwt_default_claims(re:split(config:get("jwt_auth", "required_claims", ""), "\s*,\s*", [{return, binary}])).
+
+jwt_default_claims(Claims) when is_list(Claims) ->
+    lists:usort([<<"sub">> | Claims]).
 
 Review comment:
   calling myself out here. jwtf doesn't validate this field, so no point passing it to as a check. I'm enhancing jwtf to make this more obvious...

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391294634
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,25 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {undefined, _} -> Req;
+        {_, undefined} -> Req;
+        {Secret, "Bearer " ++ Jwt} ->
+            RequiredClaims = re:split(config:get("jwt_auth", "required_claims", "sub, exp"), "\s*,\s*"),
+            AllowedAlgorithms = re:split(config:get("jwt_auth", "allowed_algorithms", "HS256"), "\s*,\s*"),
+            TimestampLeniency = config:get_integer("jwt_auth", "timestamp_leniency", "0"),
+            case jwtf:decode(?l2b(Jwt), RequiredClaims, fun(_,_) -> Secret end) of
+                {ok, {Claims}} ->
+                    {_, User} = lists:keyfind(<<"sub">>, 1, Claims),
 
 Review comment:
   (noting also that we'll just pull the jwtf code directly into this project.)

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r392263380
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,27 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {Secret, "Bearer " ++ Jwt} when Secret /= undefined ->
+            RequiredClaims = get_configured_claims(),
+            case jwtf:decode(?l2b(Jwt), RequiredClaims, fun(_,_) -> Secret end) of
 
 Review comment:
   my point is just that nothing stops tokens signed in other ways from being presented. We should insist on verifying them with HS256 (and therefore tokens signed any other way will fail). 
   
   Agreed that JWK support can come later, if at all.
   
   the jwtf enhancement is https://github.com/apache/couchdb/pull/2661, so all we need here is a property for allowed_algorithms with a code default of HS256 only.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r390151220
 
 

 ##########
 File path: rel/overlay/etc/default.ini
 ##########
 @@ -236,6 +236,7 @@ port = 6984
 ; min_writer_size = 16777216
 
 [couch_httpd_auth]
+jwt_secret = zxczxc12zxczxc12
 
 Review comment:
   also noting that this should be under [chttpd] section.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r390152292
 
 

 ##########
 File path: rel/overlay/etc/default.ini
 ##########
 @@ -131,7 +131,7 @@ prefer_minimal = Cache-Control, Content-Length, Content-Range, Content-Type, ETa
 max_db_number_for_dbs_info_req = 100
 
 ; authentication handlers
-; authentication_handlers = {chttpd_auth, cookie_authentication_handler}, {chttpd_auth, default_authentication_handler}
+authentication_handlers = {chttpd_auth, jwt_authentication_handler}, {chttpd_auth, cookie_authentication_handler}, {chttpd_auth, default_authentication_handler}
 
 Review comment:
   We should discuss whether JWT auth handler should be in the default handler list at all.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] wohali commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

wohali commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r390068198
 
 

 ##########
 File path: rebar.config.script
 ##########
 @@ -154,6 +154,7 @@ DepDescs = [
 {fauxton,          {url, "https://github.com/apache/couchdb-fauxton"},
                    {tag, "v1.2.2"}, [raw]},
 %% Third party deps
+{jwt, {url, "https://github.com/artemeff/jwt"}, {tag, "0.1.9"}},
 
 Review comment:
   If this PR is approved, we will need to import this to an Apache repo and change the URL accordingly. All of our dependencies must be locally mirrored within Apache infrastructure.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391295094
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,25 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {undefined, _} -> Req;
+        {_, undefined} -> Req;
+        {Secret, "Bearer " ++ Jwt} ->
 
 Review comment:
   we can simplify the clauses here by adding a guard of `when Secret /= undefined`, so the previous two clauses can be removed.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi edited a comment on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi edited a comment on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-597104260

Hey @wohali - thanks for the review! Lots of obvious cleanup in there, I wasn't sure whether to leave it as part of the review process, but yes, definitely will get on the specific code changes later today 🙂

I'll speak to the concern over JWTs for the rest of this comment as I think it's important conceptually to address separately. I apologize if this is too long, or if some of my explanations dip into _the basics_, it's necessary to keep oriented while giving these articles a considered look.

---

### On JWT

> Finally, I've had a long-standing knee-jerk reaction against JWT because of these two articles. Can you comment on this?

I'm going to give them their due, but overall, I'm not sure why these posts exist. I found them poorly reasoned and by their title alone, not applicable.
If we ignore the title and take it as generalized advice on JWT (a-la _knee-jerk reaction_), I could foresee untangling it taking up a lot of space in this thread.

> Does your proposed approach address any of the (rather serious) accusations raised by Sven?

Honestly, I don't think the posts generate any actionables. If I could say one thing, it's that the purpose of this PR is not to introduce JWTs for sessions, only for identity. That might be the _tldr;_... 😆

I'll start out picking on a few notable points, but if there's anything specific that you feel applies, let me know.

<details>
<summary>click to read...</summary>

#### General

I don't see a legitimate complaint proven in the posts, the author is making it clear that they are only interested in single node monoliths and that they don't understand that JWTs are not intended to store server-side state! 😅
Nearly all the points in the posts are presenting a preference as advice on the basis of _what's good for the goose_.

To discuss the topic implying that JWTs are equivalent to sessions is a false premise. And even despite the _"A note upfront"_ section, the posts fail to demonstrate an understanding of JWTs. This is indicated by the list preceded by _"I'll define a few terms first"_ which exposes the authors mistaken assumption that JWTs are used for session data.

Another example of suspicious rationale: _"While signed cookies are more secure than unsigned cookies, this is in no way unique to JWT, and good session implementations use signed cookies as well."_

...Not exactly a well-reasoned indictment of the security or utility of JWTs.

Let's clarify:

- JWTs are a(n) (optionally expirable, self-signable & encryptable) bearer token capable of storing small bits of information, typically identifiers
- Cookies are a set of conventions to round-trip state, but are most often used to transport a bearer token that corresponds to yet more state server-side
- At the transport level, cookies and JWTs are both susceptible to the same security risks. TLS is the common ground that most popular bearer-based systems use
- Fun fact: Some people even use JWTs as the content of their identity cookie!

While cookies encode data, that data doesn't necessarily have to represent identity. Indeed many sites you visit will create a cookie without you even having taken an action! In fairness, it is _very_ easy to conflate the concepts involved given the combination of abstractions that many web frameworks offer.

I think we're seeing a bit of this in the posts, with the point being that before criticizing JWTs, the author should have spent some time getting an idea of their intended use.

It wouldn't be too out-of-hand to stop at this point and dismiss these posts as fear mongering, but I will be fair and give it it a little more analysis.

#### Easier to (horizontally) scale

This is not a primary motivator for adopting JWT. The benefit offered by JWT is a single common and stateless format to provide and validate claims in a heterogenous system.

This argument really doesn't fit with an analysis of why one shouldn't use JWTs.

#### CSRF

Yes, JWTs in authorization headers do prevent CSRF.

It's an oddly seldom known trick that you can cleverly mitigate against CSRF by explicitly requiring a header as part of a request. This header can be as simple as `content-type: application/json`, and `authorization: Bearer *` has the same effect: `<form>`s can't set headers! 🏆

JWTs in headers - as I propose them here - are _more_ secure than cookies. Though I'd also like to remind that this is not the primary reason why I'm looking to introduce them.

#### Expiration, Easier to Use, More Flexible

>This is nonsense, and not a useful feature. Expiration can be implemented server-side just as well, and many implementations do.

This is the kind of assertion that really brings these posts down. Focusing on expiration as a flaw takes the architectural experience or preferences of the author and pushes them as absolutes.

The whole point of JWT is that the token will not be visiting a single, centralized monolith. It will be shopped around to various cooperating services. Also worth noting that people often configure their cookies to live for far too long, vs say... Implementing a remember-me token, which is basically an ad-hoc refresh token built on top of cookies. Something already standardized by [OpenID Connect](https://openid.net/connect/).

The fact that JWTs can be set to self-destruct means that rotations can be forced with zero overhead to servers that will be receiving the token. Case in point, my PR doesn't require any logic to direct the client on how to obtain new tokens.
When taken as part of a complete [OpenID Connect implementation](https://github.com/openiddict/openiddict-core), you can get some very cool SDKs like [this JavaScript one ](https://github.com/IdentityModel/oidc-client-js) which will auto-rotate your tokens for you 🌟 and more 🌟!

#### Single Use

This is a strange rule imposed by the author which I reject. I could use this same argument to say that a bearer token in a cookie should be single use as well.

This point is easily disregarded as trying to kitchen-sink against JWTs.

---

The flowchart offered in the follow-up post I feel doesn't really require much attention. Some of the statements within are cynical and are too far into prior misunderstandings to really warrant concern.

Really, it just comes down to the fact that cookies and JWTs can be used for overlapping and sometimes different purposes in equally as nuanced situations. I use cookies for browser based authentication in the same application that also uses JWTs to validate API requests. There's nothing unusual about this, it's how many applications perform their oauth2 authentication and consent flows.

Finally, these posts came out in June 2016, coming up to their 4 year anniversary. If the security concerns they detail were in any way legitimate, I'm very confident the greater communities we all participate in would have discarded JWT by now. Instead, I think we can see quite the opposite and that client & server libraries as well as many large scale services have come to embrace JWT.

Again, I'd love to explore any specific concern, so do let me know if there's one in particular I didn't happen to choose to drag out here. 🙂
</details>

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

With regards,
Apache Git Services

[GitHub] [couchdb] rnewson merged pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson merged pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi commented on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi commented on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-601283094
 
 
   @wohali - Sure thing!
   
   @rnewson, would you like me to wait until you've made your changes before doing any doc work?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r390581483
 
 

 ##########
 File path: rebar.config.script
 ##########
 @@ -154,6 +154,7 @@ DepDescs = [
 {fauxton,          {url, "https://github.com/apache/couchdb-fauxton"},
                    {tag, "v1.2.2"}, [raw]},
 %% Third party deps
+{jwt, {url, "https://github.com/artemeff/jwt"}, {tag, "0.1.9"}},
 
 Review comment:
   For sure. I've just finished integrating in `cloudant/jwtf`, should make it easier to bring in something much closer to home. 🙂

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r393161481
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,31 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {Secret, "Bearer " ++ Jwt} when Secret /= undefined ->
+            RequiredClaims = get_configured_claims(),
+            AllowedAlgorithms = get_configured_algorithms(),
+            case jwtf:decode(?l2b(Jwt), [{alg, AllowedAlgorithms} | RequiredClaims], fun(_,_) -> Secret end) of
+                {ok, {Claims}} ->
+                    {_, User} = lists:keyfind(<<"sub">>, 1, Claims),
 
 Review comment:
   this will need to switch to handle the other case;
   
   ```
   case lists:keyfind(<<"sub">>, 1, Claims) of
       {_, User} ->
           ...;
       false ->
           throw something
   end
   ```

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi commented on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi commented on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-597818908
 
 
   @rnewson - Applied your suggestions (both to existing and the new code I introduced). 🙂
   
   - I reworked the configuration into its own section, definitely much nicer.
   - Most of the JWT stuff now only happens when the JWT handler has enough of a reason to run.
   - I also added new config values for which algorithms to support and timestamp leniency.
   
   Would it be okay to ask for your help in finding homes for those values & related changes that may be necessary for `jwtf` as well as the asymmetric key support? I'm not familiar with it or what kind of situations it would be used for.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi commented on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi commented on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-597104260

---

### On JWT

> Finally, I've had a long-standing knee-jerk reaction against JWT because of these two articles. Can you comment on this?

> Does your proposed approach address any of the (rather serious) accusations raised by Sven?

I'll start out picking on a few notable points, but if there's anything specific that you feel applies, let me know.

<details>
<summary>click to read...</summary>

#### General

I don't see a legitimate complaint proven in the articles so much as a comparison and then a preference being expressed. The author is making it clear that they are only interested in single node monoliths and that they don't understand that JWTs are not intended to store server-side state! 😅
Nearly all the points in the posts are simply presenting a preference on the basis of _what's good for the goose_.

To discuss the topic implying that JWTs are equivalent to sessions is a false premise. And even despite the _"A note upfront"_ section, the posts fail to act on any fundamental understanding of JWTs. This is indicated by the list preceded by _"I'll define a few terms first"_ which exposes the authors mistaken assumption that JWTs are used for session data.

Another example of suspicious rationale: _"While signed cookies are more secure than unsigned cookies, this is in no way unique to JWT, and good session implementations use signed cookies as well."_
...Not exactly a well-reasoned indictment of the security or utility of JWTs.

Let's clarify:

I think we're seeing a bit of this in the posts, with the point being that before authoring any criticism of JWT, the author should have spent some time getting an idea of their intended use.

It wouldn't be too out-of-hand to stop at this point and dismiss these posts as fear mongering, but I will be fair and give it it a little more analysis.

#### Easier to (horizontally) scale

This is not a primary motivator for adopting JWT. The benefit offered by JWT is a single common and stateless format to provide and validate claims in a heterogenous system.

This argument really doesn't fit with an analysis of why one shouldn't use JWTs.

#### CSRF

Yes, JWTs in authorization headers do prevent CSRF.

JWTs in headers - as I propose them here - are _more_ secure than cookies. Though I'd also like to remind that this is not the primary reason why I'm looking to introduce them.

#### Expiration, Easier to Use, More Flexible

>This is nonsense, and not a useful feature. Expiration can be implemented server-side just as well, and many implementations do.

This is the kind of assertion that really brings these posts down. Focusing on expiration as a flaw takes the architectural experience or preferences of the author and pushes them as absolutes.

#### Single Use

This is a strange rule imposed by the author which I reject. I could use this same argument to say that a bearer token in a cookie should be single use as well.

This point is easily disregarded as trying to kitchen-sink against JWTs.

---

Really, it just comes down to the fact that cookies and JWT can be used for overlapping and sometimes different purposes in equally as nuanced situations. I use cookies for browser based authentication in the same application that also uses JWT to validate API requests. There's nothing unusual about this, it's how many applications perform their oauth2 authentication and consent flows.

Again, I'd love to explore any specific concern, so do let me know if there's one in particular I didn't happen to choose to drag out here. 🙂
</details>

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

With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391057800
 
 

 ##########
 File path: rel/overlay/etc/default.ini
 ##########
 @@ -125,6 +125,7 @@ require_valid_user = false
 ; List of headers that will be kept when the header Prefer: return=minimal is included in a request.
 ; If Server header is left out, Mochiweb will add its own one in.
 prefer_minimal = Cache-Control, Content-Length, Content-Range, Content-Type, ETag, Server, Transfer-Encoding, Vary
+
 
 Review comment:
   remove this unnecesary whitespace change pls.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391057587
 
 

 ##########
 File path: rel/overlay/etc/default.ini
 ##########
 @@ -134,10 +135,15 @@ max_db_number_for_dbs_info_req = 100
 ; authentication_handlers = {chttpd_auth, cookie_authentication_handler}, {chttpd_auth, default_authentication_handler}
 ; uncomment the next line to enable proxy authentication
 ; authentication_handlers = {chttpd_auth, proxy_authentication_handler}, {chttpd_auth, cookie_authentication_handler}, {chttpd_auth, default_authentication_handler}
+; uncomment the next line to enable JWT authentication
+; authentication_handlers = {chttpd_auth, jwt_authentication_handler}, {chttpd_auth, cookie_authentication_handler}, {chttpd_auth, default_authentication_handler}
 
 ; prevent non-admins from accessing /_all_dbs
 ; admin_only_all_dbs = true
 
+; Symmetric secret to be used when checking JWT token signatures
+; jwt_secret = 
 
 Review comment:
   let's put this in its own section (`[jwt_auth]` has a nice ring to it), the property then becomes just `secret`. That it's a secret key (rather than private/public key) should be specified by another property.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] wohali commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

wohali commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r390457352
 
 

 ##########
 File path: rel/overlay/etc/default.ini
 ##########
 @@ -131,7 +131,7 @@ prefer_minimal = Cache-Control, Content-Length, Content-Range, Content-Type, ETa
 max_db_number_for_dbs_info_req = 100
 
 ; authentication handlers
-; authentication_handlers = {chttpd_auth, cookie_authentication_handler}, {chttpd_auth, default_authentication_handler}
+authentication_handlers = {chttpd_auth, jwt_authentication_handler}, {chttpd_auth, cookie_authentication_handler}, {chttpd_auth, default_authentication_handler}
 
 Review comment:
   Agree - I do not think it should be in the default list.
   
   No one answered if CORS must be enabled for JWT support. If so, we have the unfortuate situation where enabling JWT support requires changing 3 or 4 other settings, and allowing CORS from everywhere seems a bad choice.
   
   You're right below that putting this into the Setup wizard workflow would make the most sense. If we do that, then the admin can be prompted _explicitly_ (via Fauxton, otherwise a mandatory field in the wizard setup process) that CORS must be turned on for JWT, and also prompted for a list of domains to allow in.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r390154837
 
 

 ##########
 File path: rel/overlay/etc/default.ini
 ##########
 @@ -236,6 +236,7 @@ port = 6984
 ; min_writer_size = 16777216
 
 [couch_httpd_auth]
+jwt_secret = zxczxc12zxczxc12
 
 Review comment:
   I'd rather not see us repeat the mistake we had with the cookie secret, as it caused a cluster to generate a unique value on each node (the upshot being a session cookie on one node is invalid on another. A similar effect would happen for the JWT tokens).
   
   So, a commenting describing this new setting is fine, and the code should default to a nil value and then skip JWT auth if that's the case. Separately, we'd want the setup wizard to establish a cluster-wide jwt secret.
   
   The public/private key option should also work, and we should explicitly specify which of the JWT auth mechanisms the server will use in order to mitigate some of the known security problems with JWT.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-598628592
 
 
   morning, I think we're almost there on the code change, just a few small comments today.
   
   We need some tests (a positive and negative auth) and docs. I will take on the doc side of things if you like, as I want to expand on the varieties of JWT auth allowed.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391058617
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,23 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    RequiredClaims = ["sub", "exp"],
+    case { config:get("chttpd", "jwt_secret"), header_value(Req, "Authorization") } of
 
 Review comment:
   our code style does not have spaces around braces, so `case {config:get...` etc.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-597180420
 
 
   Hi, IBM Cloudant has released its JWT library under the ASLv2 at https://github.com/cloudant/jwtf, please update to use this. This obsoletes the extra libraries this PR currently references.
   
   The jwtf library will be contributed to CouchDB as part of this effort.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391058844
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,23 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    RequiredClaims = ["sub", "exp"],
+    case { config:get("chttpd", "jwt_secret"), header_value(Req, "Authorization") } of
+        { undefined, _ } -> Req;
+        { _, undefined } -> Req;
+        { Secret, "Bearer " ++ Jwt } ->
+            case jwtf:decode(?l2b(Jwt), RequiredClaims, fun(_,_) -> Secret end) of
+                {ok, { Claims } } ->
+                    { _, User } = lists:keyfind(<<"sub">>, 1, Claims),
+
 
 Review comment:
   remove blank line

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391294425
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,25 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {undefined, _} -> Req;
+        {_, undefined} -> Req;
+        {Secret, "Bearer " ++ Jwt} ->
+            RequiredClaims = re:split(config:get("jwt_auth", "required_claims", "sub, exp"), "\s*,\s*"),
+            AllowedAlgorithms = re:split(config:get("jwt_auth", "allowed_algorithms", "HS256"), "\s*,\s*"),
+            TimestampLeniency = config:get_integer("jwt_auth", "timestamp_leniency", "0"),
+            case jwtf:decode(?l2b(Jwt), RequiredClaims, fun(_,_) -> Secret end) of
+                {ok, {Claims}} ->
+                    {_, User} = lists:keyfind(<<"sub">>, 1, Claims),
 
 Review comment:
   that seems the right move here. noting also that we do nothing with AllowedAlgorithms or TimestampLeniency as yet.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] wohali commented on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

wohali commented on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-597312679
 
 
   Elixir's pretty easy. You might be able to get some help from @dottorblaster who's been on an Elixir kick as of late.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi edited a comment on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi edited a comment on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-597125533
 
 
   For signature verification, HS256 signed, unencrypted seems to be the norm in the implementations and testers/tools I've seen.
   
   If we can get the JWT implementation you made, that should help with the transitive dependencies (base64 and jsx). I'm assuming it would also help with the `nbf` and `iat` claims as well.
   
   edit: The `jwt` library here does validate `exp`, for what it's worth, but again, I suspect your implementation will be a better fit if/when it's available.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r392179420
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,27 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {Secret, "Bearer " ++ Jwt} when Secret /= undefined ->
+            RequiredClaims = get_configured_claims(),
+            case jwtf:decode(?l2b(Jwt), RequiredClaims, fun(_,_) -> Secret end) of
 
 Review comment:
   Hmm, it's by no means exhaustive, but I can't immediately think of a reason why this would be necessary for the most basic interpretation of the JWT spec.
   
   Maybe it's worth looking at when going for [full JWK](https://tools.ietf.org/html/rfc7517#section-4.5) support?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391281850
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,25 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {undefined, _} -> Req;
+        {_, undefined} -> Req;
+        {Secret, "Bearer " ++ Jwt} ->
+            RequiredClaims = re:split(config:get("jwt_auth", "required_claims", "sub, exp"), "\s*,\s*"),
+            AllowedAlgorithms = re:split(config:get("jwt_auth", "allowed_algorithms", "HS256"), "\s*,\s*"),
+            TimestampLeniency = config:get_integer("jwt_auth", "timestamp_leniency", "0"),
+            case jwtf:decode(?l2b(Jwt), RequiredClaims, fun(_,_) -> Secret end) of
+                {ok, {Claims}} ->
+                    {_, User} = lists:keyfind(<<"sub">>, 1, Claims),
 
 Review comment:
   Are you suggesting we prepend/append `<<"sub">>` to the list?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391587360
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,26 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    DefaultClaims = [<<"sub">>],
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {Secret, "Bearer " ++ Jwt} when Secret /= undefined ->
+            RequiredClaims = case re:split(config:get("jwt_auth", "required_claims", "sub"), "\s*,\s*") of
+                "" -> DefaultClaims;
+                ConfiguredClaims ->
+                    sets:to_list(sets:from_list(lists:merge(DefaultClaims, ConfiguredClaims)))
 
 Review comment:
   hrm, maybe that ukeymerge thing isn't right here, but I don't much like the sets to/from either. the goal is to deduplicate? if so, lists:usort will do 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-597127994
 
 
   yes, my library insists on valid tokens (current time must be between iat and exp) before it returns the payload, etc. I realise you can't see it to confirm that right now.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r390155554
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,26 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    Secret = config:get("couch_httpd_auth", "jwt_secret", ""),
+    UserClaim = config:get("couch_httpd_auth", "jwt_user_claim", "sub"),
 
 Review comment:
   the JWT `sub` field is defined as indicating the subject of the token, I don't think it makes sense to make this configurable.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi commented on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi commented on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-597301311
 
 
   Updated the code and cleaned up the configuration. This now uses `jwtf`, which I'm guessing someone will have to pull in as an apache repo.
   
   I haven't added tests yet as I'm not sure if it makes sense to until there's a consensus on the final shape of things.
   
   On that: Am I right in understanding that the tests for auth handlers are all done in JS?
   
   🙂 I'll wait for the next round of suggestions!

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391945025
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,31 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {Secret, "Bearer " ++ Jwt} when Secret /= undefined ->
+            RequiredClaims = get_configured_claims(),
+            case jwtf:decode(?l2b(Jwt), RequiredClaims, fun(_,_) -> Secret end) of
+                {ok, {Claims}} ->
+                    {_, User} = lists:keyfind(<<"sub">>, 1, Claims),
+                    Req#httpd{user_ctx=#user_ctx{
+                        name=User
+                    }};
+                {error, Reason} ->
+                    throw({unauthorized, Reason})
+            end;
+        {_, _} -> Req
+    end.
+
+get_configured_claims() ->
+    jwt_default_claims(re:split(config:get("jwt_auth", "required_claims", "sub"), "\s*,\s*")).
+
+jwt_default_claims(Claims) ->
+    DefaultClaims = [<<"sub">>],
+    case Claims of
+        "" -> DefaultClaims;
 
 Review comment:
   re:split won't ever return `""` so this clause is unreachable.
   
   ```
   jwt_default_claims(Claims) when is_list(Claims) ->
       lists:usort([<<"sub">>], Claims).
   ```
   
   would seem to suffice.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi edited a comment on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi edited a comment on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-597125533
 
 
   For signature verification, HS256 signed, unencrypted seems to be the norm in the implementations and testers/tools I've seen.
   
   If we can get the JWT implementation you made, that should help with the transitive dependencies (base64 and jsx). I'm assuming it would also help with the `nbf` and `iat` claims as well.
   
   edit: The JWT one does validate `exp`, for what it's worth.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391295167
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,25 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {undefined, _} -> Req;
+        {_, undefined} -> Req;
+        {Secret, "Bearer " ++ Jwt} ->
+            RequiredClaims = re:split(config:get("jwt_auth", "required_claims", "sub, exp"), "\s*,\s*"),
+            AllowedAlgorithms = re:split(config:get("jwt_auth", "allowed_algorithms", "HS256"), "\s*,\s*"),
+            TimestampLeniency = config:get_integer("jwt_auth", "timestamp_leniency", 0),
+            case jwtf:decode(?l2b(Jwt), RequiredClaims, fun(_,_) -> Secret end) of
+                {ok, {Claims}} ->
+                    {_, User} = lists:keyfind(<<"sub">>, 1, Claims),
+                    Req#httpd{user_ctx=#user_ctx{
+                        name=User
+                    }};
+                {error, Reason} ->
+                    throw({unauthorized, Reason})
+            end;
+        {Secret, _} -> Req
 
 Review comment:
   and this can simply be `{_, _} -> Req` 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r390503460
 
 

 ##########
 File path: rel/overlay/etc/default.ini
 ##########
 @@ -166,7 +166,7 @@ allow_jsonp = false
 ; For more socket options, consult Erlang's module 'inet' man page.
 ;socket_options = [{recbuf, undefined}, {sndbuf, 262144}, {nodelay, true}]
 socket_options = [{sndbuf, 262144}]
-enable_cors = false
+enable_cors = true
 
 Review comment:
   Yeah, we should leave anything to do with CORS up to whoever is configuring the server. 👍

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi commented on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi commented on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-597125533
 
 
   For signature verification, HS256 signed, unencrypted seems to be the norm in the implementations and testers/tools I've seen.
   
   If we can get the JWT implementation you made, that should help with the transitive dependencies (base64 and jsx). I'm assuming it would also help with the `nbf` and `iat` claims as well.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r392112656
 
 

 ##########
 File path: rel/overlay/etc/default.ini
 ##########
 @@ -134,10 +134,18 @@ max_db_number_for_dbs_info_req = 100
 ; authentication_handlers = {chttpd_auth, cookie_authentication_handler}, {chttpd_auth, default_authentication_handler}
 ; uncomment the next line to enable proxy authentication
 ; authentication_handlers = {chttpd_auth, proxy_authentication_handler}, {chttpd_auth, cookie_authentication_handler}, {chttpd_auth, default_authentication_handler}
+; uncomment the next line to enable JWT authentication
+; authentication_handlers = {chttpd_auth, jwt_authentication_handler}, {chttpd_auth, cookie_authentication_handler}, {chttpd_auth, default_authentication_handler}
 
 ; prevent non-admins from accessing /_all_dbs
 ; admin_only_all_dbs = true
 
+;[jwt_auth]
+; Symmetric secret to be used when checking JWT token signatures
+; secret =
+; List of claims to validate
+; required_claims = sub, exp
 
 Review comment:
   the default here is just "exp", as we forcibly add "sub" in the code.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391586558
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,26 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    DefaultClaims = [<<"sub">>],
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {Secret, "Bearer " ++ Jwt} when Secret /= undefined ->
+            RequiredClaims = case re:split(config:get("jwt_auth", "required_claims", "sub"), "\s*,\s*") of
+                "" -> DefaultClaims;
+                ConfiguredClaims ->
+                    sets:to_list(sets:from_list(lists:merge(DefaultClaims, ConfiguredClaims)))
 
 Review comment:
   `lists:ukeymerge(1, ConfiguredClaims, DefaultClaims)` is how we do this pattern elsewhere.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r390155325
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,26 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    Secret = config:get("couch_httpd_auth", "jwt_secret", ""),
 
 Review comment:
   a simple config:get("chttpd", "jwt_secret") here, and then handle the case of `undefined` explicitly (as previous comment, to mean "don't attempt JWT auth").

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] wohali commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

wohali commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r390069370
 
 

 ##########
 File path: rel/overlay/etc/default.ini
 ##########
 @@ -166,7 +166,7 @@ allow_jsonp = false
 ; For more socket options, consult Erlang's module 'inet' man page.
 ;socket_options = [{recbuf, undefined}, {sndbuf, 262144}, {nodelay, true}]
 socket_options = [{sndbuf, 262144}]
-enable_cors = false
+enable_cors = true
 
 Review comment:
   This is an unrelated change, and should be removed, unless CORS support is mandatory for JWT support.
   
   Similar to the change above, you would also need to change the Erlang-based default, this time at https://github.com/apache/couchdb/blob/master/src/chttpd/src/chttpd_cors.erl#L283 .

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391215317
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,26 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {undefined, _} -> Req;
+        {_, undefined} -> Req;
+        {Secret, "Bearer " ++ Jwt} ->
+            RequiredClaims = re:split(config:get("jwt_auth", "required_claims", "sub, exp"), "\s*,\s*"),
+            AllowedAlgorithms = re:split(config:get("jwt_auth", "allowed_algorithms", "HS256"), "\s*,\s*"),
+            TimestampLeniencyConfig = config:get("jwt_auth", "timestamp_leniency", "0"),
 
 Review comment:
   config:get_integer

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] wohali commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

wohali commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r390593997
 
 

 ##########
 File path: rel/overlay/etc/default.ini
 ##########
 @@ -267,11 +273,11 @@ credentials = false
 ; List of origins separated by a comma, * means accept all
 ; Origins must include the scheme: http://example.com
 ; You can't set origins: * and credentials = true at the same time.
-;origins = *
+; origins = 
 ; List of accepted headers separated by a comma
-; headers =
+; headers = 
 ; List of accepted methods
-; methods =
+; methods = 
 
 Review comment:
   No trailing whitespace, please.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r392112220
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,27 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {Secret, "Bearer " ++ Jwt} when Secret /= undefined ->
+            RequiredClaims = get_configured_claims(),
+            case jwtf:decode(?l2b(Jwt), RequiredClaims, fun(_,_) -> Secret end) of
+                {ok, {Claims}} ->
+                    {_, User} = lists:keyfind(<<"sub">>, 1, Claims),
+                    Req#httpd{user_ctx=#user_ctx{
+                        name=User
+                    }};
+                {error, Reason} ->
+                    throw({unauthorized, Reason})
+            end;
+        {_, _} -> Req
+    end.
+
+get_configured_claims() ->
+    jwt_default_claims(re:split(config:get("jwt_auth", "required_claims", "sub"), "\s*,\s*")).
 
 Review comment:
   let's be explicit that we need binaries returned by re:split (that is the default now but I think input format (config:get returns lists) in older erlang releases. it's just an extra argument of value `[binary]`.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi commented on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi commented on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-597548915
 
 
   @dottorblaster Great!
   
   I think that just leaves the "wizard". Is that worth having someone more familiar help out with?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391586740
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,26 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    DefaultClaims = [<<"sub">>],
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {Secret, "Bearer " ++ Jwt} when Secret /= undefined ->
+            RequiredClaims = case re:split(config:get("jwt_auth", "required_claims", "sub"), "\s*,\s*") of
+                "" -> DefaultClaims;
+                ConfiguredClaims ->
+                    sets:to_list(sets:from_list(lists:merge(DefaultClaims, ConfiguredClaims)))
 
 Review comment:
   I'd also break this out into its own function now that things are getting a little more involved.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] wohali commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

wohali commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r390071068
 
 

 ##########
 File path: rel/overlay/etc/default.ini
 ##########
 @@ -267,11 +268,11 @@ credentials = false
 ; List of origins separated by a comma, * means accept all
 ; Origins must include the scheme: http://example.com
 ; You can't set origins: * and credentials = true at the same time.
-;origins = *
+origins = *
 ; List of accepted headers separated by a comma
-; headers =
+headers = authorization,content-type
 ; List of accepted methods
-; methods =
+methods = GET,POST
 
 Review comment:
   Again, these are unrelated changes, unless CORS support is mandatory for JWT, and only if we enable JWT by default. (I'm now less emphatic about that myself.)

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] wohali commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

wohali commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r390070493
 
 

 ##########
 File path: rel/overlay/etc/default.ini
 ##########
 @@ -236,6 +236,7 @@ port = 6984
 ; min_writer_size = 16777216
 
 [couch_httpd_auth]
+jwt_secret = zxczxc12zxczxc12
 
 Review comment:
   First off, it's very bad practice to put any sort of fixed value in our default ini file. This would end up in millions of installations of CouchDB default.
   
   Since we need a unique, random value here, that should be inserted into `local.ini` at first startup by the JWT subsystem, if not already found. You could borrow this code - it's how we generate a random UUID on startup for a brand new installation:
   
   https://github.com/apache/couchdb/blob/master/src/couch/src/couch_server.erl#L65-L69
   
   You can probably just use the exact same call to `couch_uuids:random/0` since the value doesn't really matter. Just `config:set` the secret under `[couch_httpd_auth] jwt_secret` as you propose here. That'll save the file to the user's `local.ini` file, where it belongs.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r392280587
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,27 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {Secret, "Bearer " ++ Jwt} when Secret /= undefined ->
+            RequiredClaims = get_configured_claims(),
+            case jwtf:decode(?l2b(Jwt), RequiredClaims, fun(_,_) -> Secret end) of
 
 Review comment:
   Alright, sounds good!

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391057869
 
 

 ##########
 File path: rel/overlay/etc/default.ini
 ##########
 @@ -267,7 +273,7 @@ credentials = false
 ; List of origins separated by a comma, * means accept all
 ; Origins must include the scheme: http://example.com
 ; You can't set origins: * and credentials = true at the same time.
-;origins = *
+; origins = *
 
 Review comment:
   remove this unnecesary whitespace change pls.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] rnewson commented on a change in pull request #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

rnewson commented on a change in pull request #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#discussion_r391270049
 
 

 ##########
 File path: src/couch/src/couch_httpd_auth.erl
 ##########
 @@ -186,6 +188,25 @@ proxy_auth_user(Req) ->
             end
     end.
 
+jwt_authentication_handler(Req) ->
+    case {config:get("jwt_auth", "secret"), header_value(Req, "Authorization")} of
+        {undefined, _} -> Req;
+        {_, undefined} -> Req;
+        {Secret, "Bearer " ++ Jwt} ->
+            RequiredClaims = re:split(config:get("jwt_auth", "required_claims", "sub, exp"), "\s*,\s*"),
+            AllowedAlgorithms = re:split(config:get("jwt_auth", "allowed_algorithms", "HS256"), "\s*,\s*"),
+            TimestampLeniency = config:get_integer("jwt_auth", "timestamp_leniency", "0"),
+            case jwtf:decode(?l2b(Jwt), RequiredClaims, fun(_,_) -> Secret end) of
+                {ok, {Claims}} ->
+                    {_, User} = lists:keyfind(<<"sub">>, 1, Claims),
 
 Review comment:
   since `sub` is mandatory, an admin that doesn't include it in the `required_claims` property is just going to cause a `badarg` 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] atrauzzi commented on issue #2648: Feature - Add JWT support

Posted by GitBox <gi...@apache.org>.

atrauzzi commented on issue #2648: Feature - Add JWT support
URL: https://github.com/apache/couchdb/pull/2648#issuecomment-597309803
 
 
   Alright, I'll get on the additional configs.
   
   The wizard might be more of a challenge for me, same goes for authoring any new handler tests in elixir. (I was hoping to build off of the JS ones as examples)

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services