From 947df15d30aa0fe9570660112fbbd74adec18f2e Mon Sep 17 00:00:00 2001 From: Marcial Rosales Date: Tue, 18 Apr 2023 16:28:33 +0200 Subject: [PATCH 1/2] Fix issue #7178 (cherry picked from commit 6227dfd15d7644b02f833ee43666ae33e9fb410c) (cherry picked from commit 25bd6692e42b545fa69abaff6701f6ce9925add1) --- .../src/rabbit_auth_backend_oauth2.erl | 63 +++++++++++++--- .../rabbit_auth_backend_oauth2_test_util.erl | 2 + .../test/scope_SUITE.erl | 74 +++++++++++++++++++ .../test/unit_SUITE.erl | 20 +++++ 4 files changed, 147 insertions(+), 12 deletions(-) diff --git a/deps/rabbitmq_auth_backend_oauth2/src/rabbit_auth_backend_oauth2.erl b/deps/rabbitmq_auth_backend_oauth2/src/rabbit_auth_backend_oauth2.erl index 853e9df15c7b..8f014d411c64 100644 --- a/deps/rabbitmq_auth_backend_oauth2/src/rabbit_auth_backend_oauth2.erl +++ b/deps/rabbitmq_auth_backend_oauth2/src/rabbit_auth_backend_oauth2.erl @@ -4,7 +4,6 @@ %% %% Copyright (c) 2007-2023 VMware, Inc. or its affiliates. All rights reserved. %% - -module(rabbit_auth_backend_oauth2). -include_lib("rabbit_common/include/rabbit.hrl"). @@ -18,7 +17,7 @@ check_topic_access/4, check_token/1, state_can_expire/0, update_state/2]). % for testing --export([post_process_payload/1]). +-export([post_process_payload/1, get_expanded_scopes/2]). -import(rabbit_data_coercion, [to_map/1]). @@ -80,7 +79,7 @@ user_login_authorization(Username, AuthProps) -> check_vhost_access(#auth_user{impl = DecodedTokenFun}, VHost, _AuthzData) -> with_decoded_token(DecodedTokenFun(), - fun() -> + fun(_Token) -> Scopes = get_scopes(DecodedTokenFun()), ScopeString = rabbit_oauth2_scope:concat_scopes(Scopes, ","), rabbit_log:debug("Matching virtual host '~s' against the following scopes: ~s", [VHost, ScopeString]), @@ -90,16 +89,16 @@ check_vhost_access(#auth_user{impl = DecodedTokenFun}, check_resource_access(#auth_user{impl = DecodedTokenFun}, Resource, Permission, _AuthzContext) -> with_decoded_token(DecodedTokenFun(), - fun() -> - Scopes = get_scopes(DecodedTokenFun()), + fun(Token) -> + Scopes = get_scopes(Token), rabbit_oauth2_scope:resource_access(Resource, Permission, Scopes) end). check_topic_access(#auth_user{impl = DecodedTokenFun}, Resource, Permission, Context) -> with_decoded_token(DecodedTokenFun(), - fun() -> - Scopes = get_scopes(DecodedTokenFun()), + fun(Token) -> + Scopes = get_expanded_scopes(Token, Resource), rabbit_oauth2_scope:topic_access(Resource, Permission, Context, Scopes) end). @@ -133,15 +132,15 @@ authenticate(_, AuthProps0) -> {refused, Err} -> {refused, "Authentication using an OAuth 2/JWT token failed: ~p", [Err]}; {ok, DecodedToken} -> - Func = fun() -> + Func = fun(Token0) -> Username = username_from( application:get_env(?APP, ?PREFERRED_USERNAME_CLAIMS, []), - DecodedToken), - Tags = tags_from(DecodedToken), + Token0), + Tags = tags_from(Token0), {ok, #auth_user{username = Username, tags = Tags, - impl = fun() -> DecodedToken end}} + impl = fun() -> Token0 end}} end, case with_decoded_token(DecodedToken, Func) of {error, Err} -> @@ -153,7 +152,7 @@ authenticate(_, AuthProps0) -> with_decoded_token(DecodedToken, Fun) -> case validate_token_expiry(DecodedToken) of - ok -> Fun(); + ok -> Fun(DecodedToken); {error, Msg} = Err -> rabbit_log:error(Msg), Err @@ -536,6 +535,46 @@ check_aud(Aud, ResourceServerId) -> get_scopes(#{?SCOPE_JWT_FIELD := Scope}) -> Scope. +-spec get_expanded_scopes(map(), #resource{}) -> [binary()]. +get_expanded_scopes(Token, #resource{virtual_host = VHost}) -> + Context = #{ token => Token , vhost => VHost}, + case maps:get(?SCOPE_JWT_FIELD, Token, []) of + [] -> []; + Scopes -> lists:map(fun(Scope) -> list_to_binary(parse_scope(Scope, Context)) end, Scopes) + end. + +parse_scope(Scope, Context) -> + { Acc0, _} = lists:foldl(fun(Elem, { Acc, Stage }) -> parse_scope_part(Elem, Acc, Stage, Context) end, + { [], undefined }, re:split(Scope,"([\{.*\}])",[{return,list},trim])), + Acc0. + +parse_scope_part(Elem, Acc, Stage, Context) -> + case Stage of + error -> {Acc, error}; + undefined -> + case Elem of + "{" -> { Acc, fun capture_var_name/3}; + Value -> { Acc ++ Value, Stage} + end; + _ -> Stage(Elem, Acc, Context) + end. + +capture_var_name(Elem, Acc, #{ token := Token, vhost := Vhost}) -> + { Acc ++ resolve_scope_var(Elem, Token, Vhost), fun expect_closing_var/3}. + +expect_closing_var("}" , Acc, _Context) -> { Acc , undefined }; +expect_closing_var(_ , _Acc, _Context) -> {"", error}. + +resolve_scope_var(Elem, Token, Vhost) -> + ElemAsBinary = list_to_binary(Elem), + case Elem of + "vhost" -> binary_to_list(Vhost); + _ -> binary_to_list(case maps:get(ElemAsBinary, Token, ElemAsBinary) of + Value when is_binary(Value) -> Value; + _ -> ElemAsBinary + end) + end. + %% A token may be present in the password credential or in the rabbit_auth_backend_oauth2 %% credential. The former is the most common scenario for the first time authentication. %% However, there are scenarios where the same user (on the same connection) is authenticated diff --git a/deps/rabbitmq_auth_backend_oauth2/test/rabbit_auth_backend_oauth2_test_util.erl b/deps/rabbitmq_auth_backend_oauth2/test/rabbit_auth_backend_oauth2_test_util.erl index f5cd2ca864fa..51d0a9435513 100644 --- a/deps/rabbitmq_auth_backend_oauth2/test/rabbit_auth_backend_oauth2_test_util.erl +++ b/deps/rabbitmq_auth_backend_oauth2/test/rabbit_auth_backend_oauth2_test_util.erl @@ -89,6 +89,8 @@ fixture_token() -> token_with_sub(TokenFixture, Sub) -> maps:put(<<"sub">>, Sub, TokenFixture). +token_with_scopes(TokenFixture, Scopes) -> + maps:put(<<"scope">>, Scopes, TokenFixture). fixture_token(ExtraScopes) -> Scopes = [<<"rabbitmq.configure:vhost/foo">>, diff --git a/deps/rabbitmq_auth_backend_oauth2/test/scope_SUITE.erl b/deps/rabbitmq_auth_backend_oauth2/test/scope_SUITE.erl index 63260430988a..84effae77cd0 100644 --- a/deps/rabbitmq_auth_backend_oauth2/test/scope_SUITE.erl +++ b/deps/rabbitmq_auth_backend_oauth2/test/scope_SUITE.erl @@ -14,12 +14,86 @@ all() -> [ + variable_expansion, permission_all, permission_vhost, permission_resource, permission_topic ]. +variable_expansion(_Config) -> + Scenarios = [ + { "Emtpy Scopes", + #{ + <<"client_id">> => <<"some_client">>, + <<"scope">> => [] + }, <<"default">>, [] + }, + { "No Scopes", + #{ + <<"client_id">> => <<"some_client">> + }, <<"default">>, [] + }, + { "Expand token's var and vhost", + #{ + <<"client_id">> => <<"some_client">>, + <<"scope">> => [<<"read:{vhost}/{client_id}-*">>] + }, <<"default">>, [<<"read:default/some_client-*">>] + }, + { "Expand token's var and vhost on several scopes", + #{ + <<"client_id">> => <<"some_client">>, + <<"username">> => <<"some_user">>, + <<"scope">> => [<<"read:{vhost}/{client_id}-*">>, <<"write:{vhost}/{client_id}/{username}">>] + }, <<"default">>, [<<"read:default/some_client-*">>, <<"write:default/some_client/some_user">>] + }, + { "Expand token's var", + #{ + <<"client_id">> => <<"some_client">>, + <<"username">> => <<"some_user">>, + <<"scope">> => [<<"read:{client_id}/*/{username}">>] + }, <<"other">>, [<<"read:some_client/*/some_user">>] + }, + { "No Expansion required", + #{ + <<"client_id">> => <<"some_client">>, + <<"scope">> => [<<"read:client_id/vhost-*">>] + }, <<"default">>, [<<"read:client_id/vhost-*">>] + }, + { "Missing var", + #{ + <<"scope">> => [<<"read:{client_id}/*">>] + }, <<"default">>, [<<"read:client_id/*">>] + }, + { "Var with other than single binary value", + #{ + <<"foo">> => [<<"bar">>], + <<"scope">> => [<<"read:{foo}/*">>] + }, <<"default">>, [<<"read:foo/*">>] + }, + { "Empty var", + #{ + <<"scope">> => [<<"read:{}/*">>] + }, <<"default">>, [<<"read:/*">>] + }, + { "Missing closing variable character", + #{ + <<"scope">> => [<<"read:{/*">>] + }, <<"default">>, [<<"">>] + }, + { "Unexpected closing variable character", + #{ + <<"scope">> => [<<"read:var}/*">>] + }, <<"default">>, [<<"read:var}/*">>] + } + + ], + lists:foreach(fun({ Comment, Token, Vhost, ExpectedScopes}) -> + ?assertEqual(ExpectedScopes, + rabbit_auth_backend_oauth2:get_expanded_scopes(Token, #resource{virtual_host = Vhost}), Comment) + end + , Scenarios). + permission_all(_Config) -> WildcardScopeWrite = <<"write:*/*">>, WildcardScopeWriteTopic = <<"write:*/*/*">>, diff --git a/deps/rabbitmq_auth_backend_oauth2/test/unit_SUITE.erl b/deps/rabbitmq_auth_backend_oauth2/test/unit_SUITE.erl index 6b61403aaad5..399233c60f95 100644 --- a/deps/rabbitmq_auth_backend_oauth2/test/unit_SUITE.erl +++ b/deps/rabbitmq_auth_backend_oauth2/test/unit_SUITE.erl @@ -20,6 +20,7 @@ all() -> test_validate_payload, test_validate_payload_when_verify_aud_false, test_successful_access_with_a_token, + test_successful_access_with_a_token_with_variables_in_scopes, test_successful_access_with_a_parsed_token, test_successful_access_with_a_token_that_has_tag_scopes, test_unsuccessful_access_with_a_bogus_token, @@ -631,6 +632,25 @@ test_successful_access_with_a_token(_) -> assert_topic_access_granted(User, VHost, <<"bar">>, read, #{routing_key => <<"#/foo">>}). +test_successful_access_with_a_token_with_variables_in_scopes(_) -> + %% Generate a token with JOSE + %% Check authorization with the token + %% Check user access granted by token + Jwk = ?UTIL_MOD:fixture_jwk(), + UaaEnv = [{signing_keys, #{<<"token-key">> => {map, Jwk}}}], + application:set_env(rabbitmq_auth_backend_oauth2, key_config, UaaEnv), + application:set_env(rabbitmq_auth_backend_oauth2, resource_server_id, <<"rabbitmq">>), + + VHost = <<"my-vhost">>, + Username = <<"username">>, + Token = ?UTIL_MOD:sign_token_hs( + ?UTIL_MOD:token_with_sub(?UTIL_MOD:fixture_token([<<"rabbitmq.read:{vhost}/*/{sub}">>]), Username), + Jwk), + {ok, #auth_user{username = Username} = User} = + rabbit_auth_backend_oauth2:user_login_authentication(Username, #{password => Token}), + + assert_topic_access_granted(User, VHost, <<"bar">>, read, #{routing_key => Username}). + test_successful_access_with_a_parsed_token(_) -> Jwk = ?UTIL_MOD:fixture_jwk(), UaaEnv = [{signing_keys, #{<<"token-key">> => {map, Jwk}}}], From 5806a27a8215ab70a420635dbc083d89c0b1740a Mon Sep 17 00:00:00 2001 From: Marcial Rosales Date: Tue, 18 Apr 2023 17:06:05 +0200 Subject: [PATCH 2/2] Minor code change (cherry picked from commit de4fa24444d26296bff396bc564f151e36505f75) (cherry picked from commit f8d31cc4b0ceb74763238e7583d1220c40e05bb3) --- .../src/rabbit_auth_backend_oauth2.erl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/deps/rabbitmq_auth_backend_oauth2/src/rabbit_auth_backend_oauth2.erl b/deps/rabbitmq_auth_backend_oauth2/src/rabbit_auth_backend_oauth2.erl index 8f014d411c64..768c48ecf6bf 100644 --- a/deps/rabbitmq_auth_backend_oauth2/src/rabbit_auth_backend_oauth2.erl +++ b/deps/rabbitmq_auth_backend_oauth2/src/rabbit_auth_backend_oauth2.erl @@ -566,10 +566,11 @@ expect_closing_var("}" , Acc, _Context) -> { Acc , undefined }; expect_closing_var(_ , _Acc, _Context) -> {"", error}. resolve_scope_var(Elem, Token, Vhost) -> - ElemAsBinary = list_to_binary(Elem), case Elem of "vhost" -> binary_to_list(Vhost); - _ -> binary_to_list(case maps:get(ElemAsBinary, Token, ElemAsBinary) of + _ -> + ElemAsBinary = list_to_binary(Elem), + binary_to_list(case maps:get(ElemAsBinary, Token, ElemAsBinary) of Value when is_binary(Value) -> Value; _ -> ElemAsBinary end)