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/21 17:10:15 UTC
[GitHub] [couchdb] atrauzzi opened a new pull request #2694: Add support for
roles to be obtained from JWTs.
atrauzzi opened a new pull request #2694: Add support for roles to be obtained from JWTs.
URL: https://github.com/apache/couchdb/pull/2694
As a followon to https://github.com/apache/couchdb/pull/2648, I'd like to include support for roles in JWTs.
Chalk this up to my newness with couchdb, I thought simply being an admin user would inherit roles 😉
Basically I need a way to allow my servers to generate short lived `_admin` tokens to perform admin commands from my main application logic.
----------------------------------------------------------------
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 #2694: Add
support for roles to be obtained from JWTs.
Posted by GitBox <gi...@apache.org>.
rnewson commented on a change in pull request #2694: Add support for roles to be obtained from JWTs.
URL: https://github.com/apache/couchdb/pull/2694#discussion_r396025629
##########
File path: src/couch/src/couch_httpd_auth.erl
##########
@@ -198,7 +198,13 @@ jwt_authentication_handler(Req) ->
case lists:keyfind(<<"sub">>, 1, Claims) of
false -> throw({unauthorized, <<"Token missing sub claim.">>});
{_, User} -> Req#httpd{user_ctx=#user_ctx{
- name=User
+ name=User,
+ roles=case lists:keyfind(<<"roles">>, 1, Claims) of
+ false -> [];
+ {_, Roles} ->
+ erlang:display(Roles),
Review comment:
remove the display 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] atrauzzi commented on a change in pull request #2694: Add
support for roles to be obtained from JWTs.
Posted by GitBox <gi...@apache.org>.
atrauzzi commented on a change in pull request #2694: Add support for roles to be obtained from JWTs.
URL: https://github.com/apache/couchdb/pull/2694#discussion_r396724895
##########
File path: test/elixir/test/jwtauth_test.exs
##########
@@ -103,13 +103,14 @@ defmodule JwtAuthTest do
end
def test_fun(alg, key) do
- {:ok, token} = :jwtf.encode({[{"alg", alg}, {"typ", "JWT"}]}, {[{"sub", "couch@apache.org"}]}, key)
+ {:ok, token} = :jwtf.encode({[{"alg", alg}, {"typ", "JWT"}]}, {[{"sub", "couch@apache.org"}, {"roles", ["testing"]}]}, key)
resp = Couch.get("/_session",
headers: [authorization: "Bearer #{token}"]
)
assert resp.body["userCtx"]["name"] == "couch@apache.org"
+ assert resp.body["userCtx"]["roles"] == [<<"testing">>]
assert resp.body["info"]["authenticated"] == "jwt"
Review comment:
I've added an assertion to this portion of the test to ensure we're verifying what comes out of the session endpoint.
----------------------------------------------------------------
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 #2694: Add
support for roles to be obtained from JWTs.
Posted by GitBox <gi...@apache.org>.
rnewson commented on a change in pull request #2694: Add support for roles to be obtained from JWTs.
URL: https://github.com/apache/couchdb/pull/2694#discussion_r397028323
##########
File path: test/elixir/test/jwtauth_test.exs
##########
@@ -103,13 +103,14 @@ defmodule JwtAuthTest do
end
def test_fun(alg, key) do
- {:ok, token} = :jwtf.encode({[{"alg", alg}, {"typ", "JWT"}]}, {[{"sub", "couch@apache.org"}]}, key)
+ {:ok, token} = :jwtf.encode({[{"alg", alg}, {"typ", "JWT"}]}, {[{"sub", "couch@apache.org"}, {"roles", ["testing"]}]}, key)
resp = Couch.get("/_session",
headers: [authorization: "Bearer #{token}"]
)
assert resp.body["userCtx"]["name"] == "couch@apache.org"
+ assert resp.body["userCtx"]["roles"] == [<<"testing">>]
Review comment:
this should be `["testing"]` for parity with the input and in deference to elixir's syntax where strings are binaries.
----------------------------------------------------------------
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 #2694: Add
support for roles to be obtained from JWTs.
Posted by GitBox <gi...@apache.org>.
atrauzzi commented on a change in pull request #2694: Add support for roles to be obtained from JWTs.
URL: https://github.com/apache/couchdb/pull/2694#discussion_r396724895
##########
File path: test/elixir/test/jwtauth_test.exs
##########
@@ -103,13 +103,14 @@ defmodule JwtAuthTest do
end
def test_fun(alg, key) do
- {:ok, token} = :jwtf.encode({[{"alg", alg}, {"typ", "JWT"}]}, {[{"sub", "couch@apache.org"}]}, key)
+ {:ok, token} = :jwtf.encode({[{"alg", alg}, {"typ", "JWT"}]}, {[{"sub", "couch@apache.org"}, {"roles", ["testing"]}]}, key)
resp = Couch.get("/_session",
headers: [authorization: "Bearer #{token}"]
)
assert resp.body["userCtx"]["name"] == "couch@apache.org"
+ assert resp.body["userCtx"]["roles"] == [<<"testing">>]
assert resp.body["info"]["authenticated"] == "jwt"
Review comment:
I've added assertions to this portion of the test to ensure we're verifying what comes out of the session endpoint.
----------------------------------------------------------------
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 #2694: Add
support for roles to be obtained from JWTs.
Posted by GitBox <gi...@apache.org>.
rnewson commented on a change in pull request #2694: Add support for roles to be obtained from JWTs.
URL: https://github.com/apache/couchdb/pull/2694#discussion_r396124423
##########
File path: test/elixir/test/jwtauth_test.exs
##########
@@ -16,17 +16,43 @@ defmodule JwtAuthTest do
]
run_on_modified_server(server_config, fn ->
- test_fun()
+ resp = Couch.get("/_session",
+ headers: [authorization: "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJjb3VjaEBhcGFjaGUub3JnIiwicm9sZXMiOlsiX2FkbWluIl19.JF_vunmn95R-XxQK1UtYhlKdLJdg4ms3sBeZRxpXgkw"]
+ )
+
+ assert resp.body["userCtx"]["name"] == "couch@apache.org"
+ assert resp.body["userCtx"]["roles"] == [<<"_admin">>]
+ assert resp.body["info"]["authenticated"] == "jwt"
+ end)
+ end
+
+ test "jwt auth with secret and without roles", _context do
+
+ secret = "zxczxc12zxczxc12"
+
+ server_config = [
+ %{
+ :section => "jwt_auth",
+ :key => "secret",
+ :value => secret
+ }
+ ]
+
+ run_on_modified_server(server_config, fn ->
+ run_on_modified_server(server_config, fn ->
+ resp = Couch.get("/_session",
+ headers: [authorization: "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJjb3VjaEBhcGFjaGUub3JnIn0.KYHmGXWj0HNHzZCjfOfsIfZWdguEBSn31jUdDUA9118"]
Review comment:
see in my open PR how to encode the token directly (https://github.com/apache/couchdb/pull/2687/files#diff-b72decaf14c27ef5e205025f5fa0c011R106). that way the test is clearer to the reader.
----------------------------------------------------------------
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 #2694: Add support for roles
to be obtained from JWTs.
Posted by GitBox <gi...@apache.org>.
rnewson commented on issue #2694: Add support for roles to be obtained from JWTs.
URL: https://github.com/apache/couchdb/pull/2694#issuecomment-603141670
@atrauzzi one tiny change to make and then please rebase and squash this so I can merge. And 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] rnewson commented on a change in pull request #2694: Add
support for roles to be obtained from JWTs.
Posted by GitBox <gi...@apache.org>.
rnewson commented on a change in pull request #2694: Add support for roles to be obtained from JWTs.
URL: https://github.com/apache/couchdb/pull/2694#discussion_r396025700
##########
File path: test/elixir/test/jwtauth_test.exs
##########
@@ -22,10 +22,11 @@ defmodule JwtAuthTest do
def test_fun() do
resp = Couch.get("/_session",
- headers: [authorization: "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJjb3VjaEBhcGFjaGUub3JnIn0.KYHmGXWj0HNHzZCjfOfsIfZWdguEBSn31jUdDUA9118"]
+ headers: [authorization: "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJjb3VjaEBhcGFjaGUub3JnIiwicm9sZXMiOiJfYWRtaW4ifQ.hP_nxaHADCkx5cNzHGQUFm2j0OEbtYvL7c1fEdmBQSU"]
Review comment:
let's have a test without roles and another with 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] rnewson commented on a change in pull request #2694: Add
support for roles to be obtained from JWTs.
Posted by GitBox <gi...@apache.org>.
rnewson commented on a change in pull request #2694: Add support for roles to be obtained from JWTs.
URL: https://github.com/apache/couchdb/pull/2694#discussion_r396040722
##########
File path: src/couch/src/couch_httpd_auth.erl
##########
@@ -198,7 +198,11 @@ jwt_authentication_handler(Req) ->
case lists:keyfind(<<"sub">>, 1, Claims) of
false -> throw({unauthorized, <<"Token missing sub claim.">>});
{_, User} -> Req#httpd{user_ctx=#user_ctx{
- name=User
+ name=User,
+ roles=case lists:keyfind(<<"roles">>, 1, Claims) of
Review comment:
`roles = couch_util:get_value(<<"roles">>, Claims, [])`
----------------------------------------------------------------
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 merged pull request #2694: Add support for roles
to be obtained from JWTs.
Posted by GitBox <gi...@apache.org>.
rnewson merged pull request #2694: Add support for roles to be obtained from JWTs.
URL: https://github.com/apache/couchdb/pull/2694
----------------------------------------------------------------
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