Merge pull request #4666 from rabbitmq/mergify/bp/v3.10.x/pr-4665

michaelklishin · web-flow · commit edc5d6ef0a37 · 2022-04-27T23:34:00.000+04:00
OAuth 2: expand all scope aliases provided (backport #4665)
diff --git a/deps/rabbit_common/src/rabbit_data_coercion.erl b/deps/rabbit_common/src/rabbit_data_coercion.erl
@@ -8,7 +8,7 @@
 -module(rabbit_data_coercion).
 
 -export([to_binary/1, to_list/1, to_atom/1, to_integer/1, to_proplist/1, to_map/1]).
--export([to_atom/2, atomize_keys/1]).
+-export([to_atom/2, atomize_keys/1, to_list_of_binaries/1]).
 
 -spec to_binary(Val :: binary() | list() | atom() | integer()) -> binary().
 to_binary(Val) when is_list(Val)    -> list_to_binary(Val);
@@ -51,4 +51,17 @@ to_map(Val) when is_list(Val) -> maps:from_list(Val).
 atomize_keys(Val) when is_list(Val) ->
   [{to_atom(K), V} || {K, V} <- Val];
 atomize_keys(Val) when is_map(Val) ->
-  maps:from_list(atomize_keys(maps:to_list(Val))).
+  maps:from_list(atomize_keys(maps:to_list(Val))).
+
+-spec to_list_of_binaries(Val :: undefined | [atom() | list() | binary() | integer()]) -> [binary()].
+to_list_of_binaries(Value) ->
+    case Value of
+      undefined ->
+          [];
+      List when is_list(List) ->
+          [to_binary(LI) || LI <- List];
+      Bin when is_binary(Bin) ->
+           [Bin];
+      Other ->
+           [to_binary(Other)]
+    end.
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
@@ -164,7 +164,9 @@ check_token(Token) ->
     Settings = application:get_all_env(?APP),
     case uaa_jwt:decode_and_verify(Token) of
         {error, Reason} -> {refused, {error, Reason}};
-        {true, Payload} -> validate_payload(post_process_payload(Payload, Settings));
+        {true, Payload} ->
+            ct:pal("post_process_payload(Payload, Settings): ~p", [post_process_payload(Payload, Settings)]),
+            validate_payload(post_process_payload(Payload, Settings));
         {false, _}      -> {refused, signature_invalid}
     end.
 
@@ -246,31 +248,25 @@ post_process_payload_with_scope_alias_in_extra_scopes_source(Payload, AppEnv) ->
 post_process_payload_with_scope_alias_field_named(Payload, undefined, _ScopeAliasMapping) ->
     Payload;
 post_process_payload_with_scope_alias_field_named(Payload, FieldName, ScopeAliasMapping) ->
-    ExistingScopes = maps:get(?SCOPE_JWT_FIELD, Payload, []),
-    AdditionalScopes = case FieldName of
-        undefined -> [];
-        []        -> [];
-        _Value    ->
-            ScopeAlias = maps:get(FieldName, Payload, undefined),
-            case ScopeAlias of
-                undefined -> [];
-                []        -> [];
-                [Value1]   ->
-                    rabbit_data_coercion:to_list(maps:get(Value1, ScopeAliasMapping, []));
-                Value2 when is_binary(Value2)  ->
-                    maps:get(Value2, ScopeAliasMapping, []);
-                Value3 when is_list(Value3)  ->
-                    maps:get(list_to_binary(Value3), ScopeAliasMapping, [])
-            end
-    end,
-
-    case AdditionalScopes of
-        []      -> Payload;
-        List when is_list(List) ->
-            maps:put(?SCOPE_JWT_FIELD, AdditionalScopes ++ ExistingScopes, Payload);
-        Bin when is_binary(Bin)   ->
-            maps:put(?SCOPE_JWT_FIELD, [Bin | ExistingScopes], Payload)
-    end.
+      Scopes0 = maps:get(FieldName, Payload, []),
+      Scopes = rabbit_data_coercion:to_list_of_binaries(Scopes0),
+      %% for all scopes, look them up in the scope alias map, and if they are
+      %% present, add the alias to the final scope list. Note that we also preserve
+      %% the original scopes, it should not hurt.
+      ExpandedScopes =
+          lists:foldl(fun(ScopeListItem, Acc) ->
+                        case maps:get(ScopeListItem, ScopeAliasMapping, undefined) of
+                            undefined ->
+                                Acc;
+                            MappedList when is_list(MappedList) ->
+                                Binaries = rabbit_data_coercion:to_list_of_binaries(MappedList),
+                                Acc ++ Binaries;
+                            Value ->
+                                Binaries = rabbit_data_coercion:to_list_of_binaries(Value),
+                                Acc ++ Binaries
+                        end
+                      end, Scopes, Scopes),
+       maps:put(?SCOPE_JWT_FIELD, ExpandedScopes, Payload).
 
 
 -spec does_include_complex_claim_field(Payload :: map()) -> boolean().
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
@@ -98,24 +98,24 @@ fixture_token(ExtraScopes) ->
 fixture_token_with_full_permissions() ->
     fixture_token_with_scopes(full_permission_scopes()).
 
-token_with_scope_alias_in_scope_field(Alias) ->
+token_with_scope_alias_in_scope_field(Value) ->
     %% expiration is a timestamp with precision in seconds
     #{<<"exp">> => default_expiration_moment(),
       <<"kid">> => <<"token-key">>,
       <<"iss">> => <<"unit_test">>,
       <<"foo">> => <<"bar">>,
       <<"aud">> => [<<"rabbitmq">>],
-      <<"scope">> => Alias}.
+      <<"scope">> => Value}.
 
-token_with_scope_alias_in_claim_field(Alias, Scopes) ->
+token_with_scope_alias_in_claim_field(Claims, Scopes) ->
     %% expiration is a timestamp with precision in seconds
     #{<<"exp">> => default_expiration_moment(),
       <<"kid">> => <<"token-key">>,
       <<"iss">> => <<"unit_test">>,
       <<"foo">> => <<"bar">>,
       <<"aud">> => [<<"rabbitmq">>],
       <<"scope">>  => Scopes,
-      <<"claims">> => Alias}.
+      <<"claims">> => Claims}.
 
 seconds_in_the_future() ->
     seconds_in_the_future(30).
diff --git a/deps/rabbitmq_auth_backend_oauth2/test/system_SUITE.erl b/deps/rabbitmq_auth_backend_oauth2/test/system_SUITE.erl
@@ -55,9 +55,11 @@ groups() ->
      ]},
 
      {scope_aliases, [], [
-                       test_successful_connection_with_with_scope_alias_in_extra_scopes_source,
+                       test_successful_connection_with_with_single_scope_alias_in_extra_scopes_source,
+                       test_successful_connection_with_with_multiple_scope_aliases_in_extra_scopes_source,
                        test_successful_connection_with_scope_alias_in_scope_field_case1,
                        test_successful_connection_with_scope_alias_in_scope_field_case2,
+                       test_successful_connection_with_scope_alias_in_scope_field_case3,
                        test_failed_connection_with_with_non_existent_scope_alias_in_extra_scopes_source,
                        test_failed_connection_with_non_existent_scope_alias_in_scope_field
                       ]}
@@ -72,7 +74,9 @@ groups() ->
 -define(EXTRA_SCOPES_SOURCE, <<"additional_rabbitmq_scopes">>).
 -define(CLAIMS_FIELD, <<"claims">>).
 
--define(SCOPE_ALIAS_NAME, <<"role-1">>).
+-define(SCOPE_ALIAS_NAME,   <<"role-1">>).
+-define(SCOPE_ALIAS_NAME_2, <<"role-2">>).
+-define(SCOPE_ALIAS_NAME_3, <<"role-3">>).
 
 init_per_suite(Config) ->
     rabbit_ct_helpers:log_environment(),
@@ -126,7 +130,7 @@ init_per_testcase(Testcase, Config) when Testcase =:= test_successful_connection
   rabbit_ct_helpers:testcase_started(Config, Testcase),
   Config;
 
-init_per_testcase(Testcase, Config) when Testcase =:= test_successful_connection_with_with_scope_alias_in_extra_scopes_source ->
+init_per_testcase(Testcase, Config) when Testcase =:= test_successful_connection_with_with_single_scope_alias_in_extra_scopes_source ->
     rabbit_ct_broker_helpers:add_vhost(Config, <<"vhost1">>),
     ok = rabbit_ct_broker_helpers:rpc(Config, 0, application, set_env,
         [rabbitmq_auth_backend_oauth2, extra_scopes_source, ?CLAIMS_FIELD]),
@@ -141,6 +145,25 @@ init_per_testcase(Testcase, Config) when Testcase =:= test_successful_connection
     rabbit_ct_helpers:testcase_started(Config, Testcase),
     Config;
 
+init_per_testcase(Testcase, Config) when Testcase =:= test_successful_connection_with_with_multiple_scope_aliases_in_extra_scopes_source ->
+    rabbit_ct_broker_helpers:add_vhost(Config, <<"vhost4">>),
+    ok = rabbit_ct_broker_helpers:rpc(Config, 0, application, set_env,
+        [rabbitmq_auth_backend_oauth2, extra_scopes_source, ?CLAIMS_FIELD]),
+    ok = rabbit_ct_broker_helpers:rpc(Config, 0, application, set_env,
+        [rabbitmq_auth_backend_oauth2, scope_aliases, #{
+            ?SCOPE_ALIAS_NAME => [
+                <<"rabbitmq.configure:vhost4/*">>
+            ],
+            ?SCOPE_ALIAS_NAME_2 => [
+                <<"rabbitmq.write:vhost4/*">>
+            ],
+            ?SCOPE_ALIAS_NAME_3 => [
+                <<"rabbitmq.read:vhost4/*">>
+            ]
+        }]),
+    rabbit_ct_helpers:testcase_started(Config, Testcase),
+    Config;
+
 init_per_testcase(Testcase, Config) when Testcase =:= test_successful_connection_with_scope_alias_in_scope_field_case1 orelse
                                          Testcase =:= test_successful_connection_with_scope_alias_in_scope_field_case2 ->
     rabbit_ct_broker_helpers:add_vhost(Config, <<"vhost2">>),
@@ -154,6 +177,22 @@ init_per_testcase(Testcase, Config) when Testcase =:= test_successful_connection
         ]),
     rabbit_ct_helpers:testcase_started(Config, Testcase),
     Config;
+init_per_testcase(Testcase, Config) when Testcase =:= test_successful_connection_with_scope_alias_in_scope_field_case3 ->
+    rabbit_ct_broker_helpers:add_vhost(Config, <<"vhost3">>),
+    ok = rabbit_ct_broker_helpers:rpc(Config, 0, application, set_env,
+        [rabbitmq_auth_backend_oauth2, scope_aliases, #{
+            ?SCOPE_ALIAS_NAME => [
+                <<"rabbitmq.configure:vhost3/*">>
+            ],
+            ?SCOPE_ALIAS_NAME_2 => [
+                <<"rabbitmq.write:vhost3/*">>
+            ],
+            ?SCOPE_ALIAS_NAME_3 => [
+                <<"rabbitmq.read:vhost3/*">>
+            ]
+        }]),
+    rabbit_ct_helpers:testcase_started(Config, Testcase),
+    Config;
 
 init_per_testcase(Testcase, Config) ->
     rabbit_ct_helpers:testcase_started(Config, Testcase),
@@ -179,7 +218,7 @@ end_per_testcase(Testcase, Config) when Testcase =:= test_successful_connection_
   rabbit_ct_helpers:testcase_finished(Config, Testcase),
   Config;
 
-end_per_testcase(Testcase, Config) when Testcase =:= test_successful_connection_with_with_scope_alias_in_extra_scopes_source ->
+end_per_testcase(Testcase, Config) when Testcase =:= test_successful_connection_with_with_single_scope_alias_in_extra_scopes_source ->
   rabbit_ct_broker_helpers:delete_vhost(Config, <<"vhost1">>),
   ok = rabbit_ct_broker_helpers:rpc(Config, 0, application, unset_env,
         [rabbitmq_auth_backend_oauth2, scope_aliases]),
@@ -188,6 +227,15 @@ end_per_testcase(Testcase, Config) when Testcase =:= test_successful_connection_
   rabbit_ct_helpers:testcase_finished(Config, Testcase),
   Config;
 
+end_per_testcase(Testcase, Config) when Testcase =:= test_successful_connection_with_with_multiple_scope_aliases_in_extra_scopes_source ->
+  rabbit_ct_broker_helpers:delete_vhost(Config, <<"vhost4">>),
+  ok = rabbit_ct_broker_helpers:rpc(Config, 0, application, unset_env,
+        [rabbitmq_auth_backend_oauth2, scope_aliases]),
+  ok = rabbit_ct_broker_helpers:rpc(Config, 0, application, unset_env,
+        [rabbitmq_auth_backend_oauth2, extra_scopes_source]),
+  rabbit_ct_helpers:testcase_finished(Config, Testcase),
+  Config;
+
 end_per_testcase(Testcase, Config) when Testcase =:= test_successful_connection_with_scope_alias_in_scope_field_case1 orelse
                                         Testcase =:= test_successful_connection_with_scope_alias_in_scope_field_case2 ->
   rabbit_ct_broker_helpers:delete_vhost(Config, <<"vhost2">>),
@@ -196,6 +244,13 @@ end_per_testcase(Testcase, Config) when Testcase =:= test_successful_connection_
   rabbit_ct_helpers:testcase_finished(Config, Testcase),
   Config;
 
+end_per_testcase(Testcase, Config) when Testcase =:= test_successful_connection_with_scope_alias_in_scope_field_case3 ->
+  rabbit_ct_broker_helpers:delete_vhost(Config, <<"vhost3">>),
+  ok = rabbit_ct_broker_helpers:rpc(Config, 0, application, unset_env,
+        [rabbitmq_auth_backend_oauth2, scope_aliases]),
+  rabbit_ct_helpers:testcase_finished(Config, Testcase),
+  Config;
+
 end_per_testcase(Testcase, Config) ->
     rabbit_ct_broker_helpers:delete_vhost(Config, <<"vhost1">>),
     rabbit_ct_helpers:testcase_finished(Config, Testcase),
@@ -447,26 +502,38 @@ test_failed_token_refresh_case2(Config) ->
     close_connection(Conn).
 
 
-test_successful_connection_with_with_scope_alias_in_extra_scopes_source(Config) ->
+test_successful_connection_with_with_single_scope_alias_in_extra_scopes_source(Config) ->
+    test_successful_connection_with_with_scope_aliases_in_extra_scopes_source(Config, ?SCOPE_ALIAS_NAME, <<"vhost1">>).
+
+test_successful_connection_with_with_multiple_scope_aliases_in_extra_scopes_source(Config) ->
+    Claims = [?SCOPE_ALIAS_NAME, ?SCOPE_ALIAS_NAME_2, ?SCOPE_ALIAS_NAME_3],
+    test_successful_connection_with_with_scope_aliases_in_extra_scopes_source(Config, Claims, <<"vhost4">>).
+
+test_successful_connection_with_with_scope_aliases_in_extra_scopes_source(Config, Claims, VHost) ->
     {_Algo, Token} = generate_valid_token_with_extra_fields(
         Config,
-        #{<<"claims">> => ?SCOPE_ALIAS_NAME}
+        #{<<"claims">> => Claims}
     ),
-    Conn     = open_unmanaged_connection(Config, 0, <<"vhost1">>, <<"username">>, Token),
+    Conn     = open_unmanaged_connection(Config, 0, VHost, <<"username">>, Token),
     {ok, Ch} = amqp_connection:open_channel(Conn),
     #'queue.declare_ok'{} =
         amqp_channel:call(Ch, #'queue.declare'{queue = <<"one">>, exclusive = true}),
     close_connection_and_channel(Conn, Ch).
 
+
 test_successful_connection_with_scope_alias_in_scope_field_case1(Config) ->
-    test_successful_connection_with_scope_alias_in_scope_field_case(Config, ?SCOPE_ALIAS_NAME).
+    test_successful_connection_with_scope_alias_in_scope_field_case(Config, ?SCOPE_ALIAS_NAME, <<"vhost2">>).
 
 test_successful_connection_with_scope_alias_in_scope_field_case2(Config) ->
-    test_successful_connection_with_scope_alias_in_scope_field_case(Config, [?SCOPE_ALIAS_NAME]).
+    test_successful_connection_with_scope_alias_in_scope_field_case(Config, [?SCOPE_ALIAS_NAME], <<"vhost2">>).
 
-test_successful_connection_with_scope_alias_in_scope_field_case(Config, Scopes) ->
+test_successful_connection_with_scope_alias_in_scope_field_case3(Config) ->
+    Scopes = [?SCOPE_ALIAS_NAME, ?SCOPE_ALIAS_NAME_2, ?SCOPE_ALIAS_NAME_3],
+    test_successful_connection_with_scope_alias_in_scope_field_case(Config, Scopes, <<"vhost3">>).
+
+test_successful_connection_with_scope_alias_in_scope_field_case(Config, Scopes, VHost) ->
     {_Algo, Token} = generate_valid_token(Config, Scopes),
-    Conn     = open_unmanaged_connection(Config, 0, <<"vhost2">>, <<"username">>, Token),
+    Conn     = open_unmanaged_connection(Config, 0, VHost, <<"username">>, Token),
     {ok, Ch} = amqp_connection:open_channel(Conn),
     #'queue.declare_ok'{} =
         amqp_channel:call(Ch, #'queue.declare'{queue = <<"one">>, exclusive = true}),
diff --git a/deps/rabbitmq_auth_backend_oauth2/test/unit_SUITE.erl b/deps/rabbitmq_auth_backend_oauth2/test/unit_SUITE.erl
