Modify management schema

to be able to set extra parameters for authorize and token endpoints
rabbitmq · Sep 20, 2024 · 3a210d7 · 3a210d7
1 parent a30a2fa
commit 3a210d7
Show file tree

Hide file tree

Showing 12 changed files with 259 additions and 109 deletions.
diff --git a/deps/oauth2_client/include/types.hrl b/deps/oauth2_client/include/types.hrl
@@ -37,7 +37,8 @@
 -record(access_token_request, {
     client_id :: string() | binary(),
     client_secret :: string() | binary(),
-    scope :: string() | binary() | undefined,
+    scope :: option(string() | binary()),
+    extra_parameters :: option(query_list()),
     timeout :: option(integer())
 }).
 

diff --git a/deps/oauth2_client/src/oauth2_client.erl b/deps/oauth2_client/src/oauth2_client.erl
@@ -470,33 +470,50 @@ ensure_oauth_provider_has_id_property(OAuth2ProviderId, OAuth2Provider) ->
     end.
 
 build_access_token_request_body(Request) ->
-    uri_string:compose_query([
-        grant_type_request_parameter(?CLIENT_CREDENTIALS_GRANT_TYPE),
-        client_id_request_parameter(Request#access_token_request.client_id),
-        client_secret_request_parameter(Request#access_token_request.client_secret)]
-        ++ scope_request_parameter_or_default(Request#access_token_request.scope, [])).
+    uri_string:compose_query(
+        append_extra_parameters(Request, 
+            append_scope_request_parameter(Request#access_token_request.scope, [
+                grant_type_request_parameter(?CLIENT_CREDENTIALS_GRANT_TYPE),
+                client_id_request_parameter(
+                    Request#access_token_request.client_id),
+                client_secret_request_parameter(
+                        Request#access_token_request.client_secret)]))).
 
 build_refresh_token_request_body(Request) ->
-    uri_string:compose_query([
-        grant_type_request_parameter(?REFRESH_TOKEN_GRANT_TYPE),
-        refresh_token_request_parameter(Request#refresh_token_request.refresh_token),
-        client_id_request_parameter(Request#refresh_token_request.client_id),
-        client_secret_request_parameter(Request#refresh_token_request.client_secret)]
-        ++ scope_request_parameter_or_default(Request#refresh_token_request.scope, [])).
+    uri_string:compose_query(
+        append_scope_request_parameter(Request#refresh_token_request.scope, [
+            grant_type_request_parameter(?REFRESH_TOKEN_GRANT_TYPE),
+            refresh_token_request_parameter(Request),
+            client_id_request_parameter(Request#refresh_token_request.client_id),
+            client_secret_request_parameter(
+                Request#refresh_token_request.client_secret)])).
 
 grant_type_request_parameter(Type) ->
     {?REQUEST_GRANT_TYPE, Type}.
-client_id_request_parameter(Client_id) ->
-    {?REQUEST_CLIENT_ID, binary_to_list(Client_id)}.
-client_secret_request_parameter(Client_secret) ->
-    {?REQUEST_CLIENT_SECRET, binary_to_list(Client_secret)}.
-refresh_token_request_parameter(RefreshToken) ->
-    {?REQUEST_REFRESH_TOKEN, RefreshToken}.
-scope_request_parameter_or_default(Scope, Default) ->
+
+client_id_request_parameter(ClientId) ->
+    {?REQUEST_CLIENT_ID, 
+        binary_to_list(ClientId)}.
+
+client_secret_request_parameter(ClientSecret) ->
+    {?REQUEST_CLIENT_SECRET, 
+        binary_to_list(ClientSecret)}.
+
+refresh_token_request_parameter(Request) ->
+    {?REQUEST_REFRESH_TOKEN, Request#refresh_token_request.refresh_token}.
+
+append_scope_request_parameter(Scope, QueryList) ->
     case Scope of
-        undefined -> Default;
-        <<>> -> Default;
-        Scope -> [{?REQUEST_SCOPE, Scope}]
+        undefined -> QueryList;
+        <<>> -> QueryList;
+        Scope -> [{?REQUEST_SCOPE, Scope} | QueryList]
+    end.
+
+append_extra_parameters(Request, QueryList) ->
+    case Request#access_token_request.extra_parameters of
+        undefined -> QueryList;
+        [] -> QueryList;
+        Params -> Params ++ QueryList
     end.
 
 get_ssl_options_if_any(OAuthProvider) ->

diff --git a/deps/oauth2_client/test/system_SUITE.erl b/deps/oauth2_client/test/system_SUITE.erl
@@ -33,6 +33,7 @@ all() ->
 
 groups() ->
 [
+
     {with_all_oauth_provider_settings, [], [
         {group, verify_get_oauth_provider}
     ]},
@@ -402,6 +403,23 @@ grants_access_token(Config) ->
     ?assertEqual(proplists:get_value(token_type, JsonPayload), TokenType),
     ?assertEqual(proplists:get_value(access_token, JsonPayload), AccessToken).
 
+grants_access_token_optional_parameters(Config) ->
+    #{request := #{parameters := Parameters},
+        response := [ {code, 200}, {content_type, _CT}, {payload, JsonPayload}] }
+        = lookup_expectation(token_endpoint, Config),
+
+    AccessTokenRequest0 = build_access_token_request(Parameters),
+    AccessTokenRequest = AccessTokenRequest0#access_token_request{
+        scope = "some-scope",
+        extra_parameters = [{"param1", "value1"}]
+    },
+    {ok, #successful_access_token_response{access_token = AccessToken,
+        token_type = TokenType} } =
+            oauth2_client:get_access_token(?config(oauth_provider, Config),
+                AccessTokenRequest),
+    ?assertEqual(proplists:get_value(token_type, JsonPayload), TokenType),
+    ?assertEqual(proplists:get_value(access_token, JsonPayload), AccessToken).
+
 grants_refresh_token(Config) ->
     #{request := #{parameters := Parameters},
         response := [ {code, 200}, {content_type, _CT}, {payload, JsonPayload}] }

diff --git a/deps/rabbitmq_auth_backend_oauth2/priv/schema/rabbitmq_auth_backend_oauth2.schema b/deps/rabbitmq_auth_backend_oauth2/priv/schema/rabbitmq_auth_backend_oauth2.schema
@@ -158,16 +158,6 @@
  "rabbitmq_auth_backend_oauth2.authorization_endpoint",
  [{datatype, string}, {validators, ["uri", "https_uri"]}]}.
 
-{mapping,
- "auth_oauth2.authorization_endpoint_params.$param",
- "rabbitmq_auth_backend_oauth2.authorization_endpoint_params",
- [{datatype, string}]}.
-
-{translation, "rabbitmq_auth_backend_oauth2.authorization_endpoint_params",
- fun(Conf) ->
-    oauth2_schema:translate_endpoint_params("authorization_endpoint_params", Conf)
- end}.
-
 {mapping,
  "auth_oauth2.discovery_endpoint_path",
  "rabbitmq_auth_backend_oauth2.discovery_endpoint_path",
@@ -189,22 +179,7 @@
  [{datatype, string}]}.
 
 {mapping,
- "auth_oauth2.token_endpoint_params.$param",
- "rabbitmq_auth_backend_oauth2.token_endpoint_params",
- [{datatype, string}]}.
-
-{translation, "rabbitmq_auth_backend_oauth2.token_endpoint_params",
- fun(Conf) ->
-    oauth2_schema:translate_endpoint_params("token_endpoint_params", Conf)
- end}.
-
-{mapping,
- "auth_oauth2.oauth_providers.$name.authorization_endpoint_params.$param",
- "rabbitmq_auth_backend_oauth2.oauth_providers",
- [{datatype, string}]}.
-
-{mapping,
- "auth_oauth2.oauth_providers.$name.token_endpoint_params.$param",
+ "auth_oauth2.oauth_providers.$name.discovery_endpoint_path",
  "rabbitmq_auth_backend_oauth2.oauth_providers",
  [{datatype, string}]}.
 

diff --git a/deps/rabbitmq_auth_backend_oauth2/src/oauth2_schema.erl b/deps/rabbitmq_auth_backend_oauth2/src/oauth2_schema.erl
@@ -41,8 +41,6 @@ translate_oauth_providers(Conf) ->
     merge_list_of_maps([
         extract_oauth_providers_properties(Settings),
         extract_oauth_providers_endpoint_params(discovery_endpoint_params, Settings),
-        extract_oauth_providers_endpoint_params(authorization_endpoint_params, Settings),
-        extract_oauth_providers_endpoint_params(token_endpoint_params, Settings),
         extract_oauth_providers_algorithm(Settings),
         extract_oauth_providers_https(Settings),
         extract_oauth_providers_signing_keys(Settings)
@@ -122,13 +120,7 @@ mapOauthProviderProperty({Key, Value}) ->
         token_endpoint -> validator_https_uri(Key, Value);
         jwks_uri -> validator_https_uri(Key, Value);
         end_session_endpoint -> validator_https_uri(Key, Value);
-        authorization_endpoint -> validator_https_uri(Key, Value);
-        token_endpoint_params ->
-            cuttlefish:invalid(io_lib:format(
-                "Invalid attribute (~p) value: should be a map of Key,Value pairs", [Key]));
-        authorization_endpoint_params ->
-            cuttlefish:invalid(io_lib:format(
-                "Invalid attribute (~p) value: should be a map of Key,Value pairs", [Key]));
+        authorization_endpoint -> validator_https_uri(Key, Value);        
         discovery_endpoint_params ->
             cuttlefish:invalid(io_lib:format(
                 "Invalid attribute (~p) value: should be a map of Key,Value pairs", [Key]));

diff --git a/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwks.erl b/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwks.erl
@@ -1,7 +1,7 @@
 -module(uaa_jwks).
 -export([get/2]).
 
--spec get(string() | binary(), term()) -> {ok, term()} | {error, term()}.
+-spec get(uri_string:uri_string(), list()) -> {ok, term()} | {error, term()}.
 get(JwksUrl, SslOptions) ->
     Options = [{timeout, 60000}] ++ [{ssl, SslOptions}],
     httpc:request(get, {JwksUrl, []}, Options, []).
diff --git a/deps/rabbitmq_auth_backend_oauth2/test/oauth2_schema_SUITE.erl b/deps/rabbitmq_auth_backend_oauth2/test/oauth2_schema_SUITE.erl
@@ -45,9 +45,7 @@ test_without_resource_servers(_) ->
     #{} = oauth2_schema:translate_resource_servers([]).
 
 test_without_endpoint_params(_) ->
-    #{} = translate_endpoint_params("discovery_endpoint_params", []),
-    #{} = translate_endpoint_params("token_endpoint_params", []),
-    #{} = translate_endpoint_params("authorization_endpoint_params", []).
+    #{} = translate_endpoint_params("oauth_discovery_endpoint_params", []).
 
 test_with_invalid_endpoint_params(_) ->
     try translate_endpoint_params("discovery_endpoint_params", [
@@ -60,16 +58,10 @@ test_with_invalid_endpoint_params(_) ->
 test_with_endpoint_params(_) ->
     Conf = [
         {["auth_oauth2","discovery_endpoint_params","param1"], "some-value1"},
-        {["auth_oauth2","discovery_endpoint_params","param2"], "some-value2"},
-        {["auth_oauth2","token_endpoint_params","audience"], "some-audience"},
-        {["auth_oauth2","authorization_endpoint_params","resource"], "some-resource"}
+        {["auth_oauth2","discovery_endpoint_params","param2"], "some-value2"}
     ],
     #{ <<"param1">> := <<"some-value1">>, <<"param2">> := <<"some-value2">> } =
-        translate_endpoint_params("discovery_endpoint_params", Conf),
-    #{ <<"audience">> := <<"some-audience">>} =
-        translate_endpoint_params("token_endpoint_params", Conf),
-    #{ <<"resource">> := <<"some-resource">>} =
-        translate_endpoint_params("authorization_endpoint_params", Conf).
+        translate_endpoint_params("discovery_endpoint_params", Conf).
 
 test_invalid_oauth_providers_endpoint_params(_) ->
     try oauth2_schema:translate_oauth_providers([
@@ -83,17 +75,15 @@ test_without_oauth_providers_with_endpoint_params(_) ->
     Conf = [
         {["auth_oauth2","oauth_providers", "A", "discovery_endpoint_params","param1"], "some-value1"},
         {["auth_oauth2","oauth_providers", "A", "discovery_endpoint_params","param2"], "some-value2"},
-        {["auth_oauth2","oauth_providers", "B", "token_endpoint_params","audience"], "some-audience"},
-        {["auth_oauth2","oauth_providers", "C", "authorization_endpoint_params","resource"], "some-resource"}
+        {["auth_oauth2","oauth_providers", "B", "discovery_endpoint_params","param3"], "some-value3"}
     ],
 
     #{
         <<"A">> := [{discovery_endpoint_params,
                     #{ <<"param1">> := <<"some-value1">>, <<"param2">> := <<"some-value2">> }}],
-        <<"B">> := [{token_endpoint_params,
-                    #{ <<"audience">> := <<"some-audience">>}}],
-        <<"C">> := [{authorization_endpoint_params,
-                    #{ <<"resource">> := <<"some-resource">>}}]
+        <<"B">> := [{discovery_endpoint_params,
+                    #{ <<"param3">> := <<"some-value3">>}}
+        ]
     } = translate_oauth_providers(Conf).
 
 test_with_one_oauth_provider(_) ->
@@ -110,11 +100,13 @@ test_with_one_resource_server(_) ->
 
 test_with_many_oauth_providers(_) ->
     Conf = [{["auth_oauth2","oauth_providers","keycloak","issuer"],"https://keycloak"},
-            {["auth_oauth2","oauth_providers","uaa","issuer"],"https://uaa"}
+            {["auth_oauth2","oauth_providers","uaa","issuer"],"https://uaa"},
+            {["auth_oauth2","oauth_providers","uaa","discovery_endpoint_path"],"/some-path"}
             ],
-    #{<<"keycloak">> := [{issuer, <<"https://keycloak">>}
+    #{<<"keycloak">> := [{issuer, <<"https://keycloak">>}                
                         ],
-      <<"uaa">> := [{issuer, <<"https://uaa">>}
+      <<"uaa">> := [{issuer, <<"https://uaa">>},
+                    {discovery_endpoint_path, <<"/some-path">>}
                     ]
     } = oauth2_schema:translate_oauth_providers(Conf).
 

diff --git a/deps/rabbitmq_management/BUILD.bazel b/deps/rabbitmq_management/BUILD.bazel
@@ -130,6 +130,11 @@ rabbitmq_suite(
     ],
 )
 
+rabbitmq_suite(
+    name = "rabbit_mgmt_schema_SUITE",
+    size = "small"    
+)
+
 rabbitmq_integration_suite(
     name = "clustering_prop_SUITE",
     size = "large",

diff --git a/deps/rabbitmq_management/app.bzl b/deps/rabbitmq_management/app.bzl
@@ -30,6 +30,7 @@ def all_beam_files(name = "all_beam_files"):
             "src/rabbit_mgmt_load_definitions.erl",
             "src/rabbit_mgmt_login.erl",
             "src/rabbit_mgmt_nodes.erl",
+            "src/rabbit_mgmt_schema.erl",
             "src/rabbit_mgmt_oauth_bootstrap.erl",
             "src/rabbit_mgmt_reset_handler.erl",
             "src/rabbit_mgmt_stats.erl",
@@ -163,6 +164,7 @@ def all_test_beam_files(name = "all_test_beam_files"):
             "src/rabbit_mgmt_load_definitions.erl",
             "src/rabbit_mgmt_login.erl",
             "src/rabbit_mgmt_nodes.erl",
+            "src/rabbit_mgmt_schema.erl",
             "src/rabbit_mgmt_oauth_bootstrap.erl",
             "src/rabbit_mgmt_reset_handler.erl",
             "src/rabbit_mgmt_stats.erl",
@@ -387,6 +389,7 @@ def all_srcs(name = "all_srcs"):
             "src/rabbit_mgmt_load_definitions.erl",
             "src/rabbit_mgmt_login.erl",
             "src/rabbit_mgmt_nodes.erl",
+            "src/rabbit_mgmt_schema.erl",
             "src/rabbit_mgmt_oauth_bootstrap.erl",
             "src/rabbit_mgmt_reset_handler.erl",
             "src/rabbit_mgmt_stats.erl",
@@ -495,6 +498,15 @@ def all_srcs(name = "all_srcs"):
     )
 
 def test_suite_beam_files(name = "test_suite_beam_files"):
+    erlang_bytecode(
+        name = "rabbit_mgmt_schema_SUITE_beam_files",
+        testonly = True,
+        srcs = ["test/rabbit_mgmt_schema_SUITE.erl"],
+        outs = ["test/rabbit_mgmt_schema_SUITE.beam"],
+        app_name = "rabbitmq_management",
+        erlc_opts = "//:test_erlc_opts",
+        deps = ["@proper//:erlang_app"],
+    )
     erlang_bytecode(
         name = "cache_SUITE_beam_files",
         testonly = True,

diff --git a/deps/rabbitmq_management/priv/schema/rabbitmq_management.schema b/deps/rabbitmq_management/priv/schema/rabbitmq_management.schema
@@ -472,6 +472,26 @@ end}.
 {mapping, "management.oauth_response_type", "rabbitmq_management.oauth_response_type",
     [{datatype, string}]}.
 
+%% Configure OAuth2 authorization_endpoint additional request parameters
+{mapping, "management.oauth_authorization_endpoint_params.$name", 
+    "rabbitmq_management.oauth_authorization_endpoint_params",
+    [{datatype, string}]}.
+
+{translation, "rabbitmq_management.oauth_authorization_endpoint_params",
+ fun(Conf) ->
+    rabbit_mgmt_schema:translate_endpoint_params("oauth_authorization_endpoint_params", Conf)
+ end}.
+
+%% Configure OAuth2 token_endpoint additional request parameters
+{mapping, "management.oauth_token_endpoint_params.$name", 
+    "rabbitmq_management.oauth_token_endpoint_params",
+    [{datatype, string}]}.
+
+{translation, "rabbitmq_management.oauth_token_endpoint_params",
+ fun(Conf) ->
+    rabbit_mgmt_schema:translate_endpoint_params("oauth_token_endpoint_params", Conf)
+ end}.
+
 %% The scopes RabbitMq should claim during the authorization flow. Defaults to "openid profile"
 {mapping, "management.oauth_scopes", "rabbitmq_management.oauth_scopes",
       [{datatype, string}]}.
@@ -513,8 +533,6 @@ end}.
   [{datatype, string}]
 }.
 
-
-
 {mapping,
   "management.oauth_resource_servers.$name.oauth_client_id",
   "rabbitmq_management.oauth_resource_servers",
@@ -533,7 +551,6 @@ end}.
   [{datatype, string}]
 }.
 
-
 {mapping,
   "management.oauth_resource_servers.$name.oauth_scopes",
   "rabbitmq_management.oauth_resource_servers",
@@ -551,36 +568,17 @@ end}.
     "rabbitmq_management.oauth_resource_servers",
   [{datatype, {enum, [sp_initiated, idp_initiated]}}]}.
 
+{mapping, "management.oauth_resource_servers.$name.authorization_endpoint_params.$name", 
+    ""rabbitmq_management.oauth_resource_servers",
+    [{datatype, string}]}.
+
+{mapping, "management.oauth_resource_servers.$name.token_endpoint_params.$name", 
+    ""rabbitmq_management.oauth_resource_servers",
+    [{datatype, string}]}.
+
 {translation, "rabbitmq_management.oauth_resource_servers",
  fun(Conf) ->
-    Settings = cuttlefish_variable:filter_by_prefix("management.oauth_resource_servers", Conf),
-    ResourceServers  = [{Name, {list_to_atom(Key), V}} || {["management","oauth_resource_servers", Name, Key], V} <- Settings ],
-    KeyFun = fun({Name,_}) -> list_to_binary(Name) end,
-    ValueFun = fun({_,V}) -> V end,
-    NewGroup = maps:groups_from_list(KeyFun, ValueFun, ResourceServers),
-    ListOrSingleFun = fun(K, List) ->
-      case K of
-        key_config -> proplists:get_all_values(K, List);
-        _ ->
-          case proplists:lookup_all(K, List) of
-            [One] -> proplists:get_value(K, List);
-            [One|_] = V -> V
-          end
-      end
-    end,
-    GroupKeyConfigFun = fun(K, List) ->
-        ListKeys = proplists:get_keys(List),
-        [ {K,ListOrSingleFun(K,List)} || K <- ListKeys ]
-     end,
-    NewGroupTwo = maps:map(GroupKeyConfigFun, NewGroup),
-    IndexByIdOrElseNameFun = fun(K, V, NewMap) ->
-      case proplists:get_value(id, V) of
-        undefined -> maps:put(K, V, NewMap);
-        ID when is_binary(ID) -> maps:put(ID, V, NewMap);
-        ID -> maps:put(list_to_binary(ID), V, NewMap)
-      end
-    end,
-    maps:fold(IndexByIdOrElseNameFun,#{}, NewGroupTwo)
+     rabbit_mgmt_schema:translate_resource_servers(Conf)
  end}.
 
 %% ===========================================================================