Always use list() type for urls

MarcialRosales · MarcialRosales · commit 2f1a87854df7 · 2024-09-27T09:35:45.000+02:00
diff --git a/deps/rabbitmq_auth_backend_oauth2/src/oauth2_schema.erl b/deps/rabbitmq_auth_backend_oauth2/src/oauth2_schema.erl
@@ -69,8 +69,7 @@ translate_list_of_signing_keys(ListOfKidPath) ->
 -spec translate_endpoint_params(list(), [{list(), binary()}]) -> map().
 translate_endpoint_params(Variable, Conf) ->
     Params0 = cuttlefish_variable:filter_by_prefix("auth_oauth2." ++ Variable, Conf),
-    Params = [{Param, V} || {["auth_oauth2", _, Param], V} <- Params0],
-    maps:from_list(Params).
+    [{list_to_binary(Param), list_to_binary(V)} || {["auth_oauth2", _, Param], V} <- Params0].
 
 validator_file_exists(Attr, Filename) ->
     case file:read_file(Filename) of
@@ -81,10 +80,21 @@ validator_file_exists(Attr, Filename) ->
             cuttlefish:invalid(io_lib:format(
                 "Invalid attribute (~p) value: file ~p does not exist or cannot be read by the node", [Attr, Filename]))
     end.
+
+validator_uri(Attr, Uri) when is_binary(Uri) ->
+    validator_uri(Attr, binary_to_list(Uri));
+validator_uri(Attr, Uri) when is_list(Uri) ->
+    case uri_string:parse(Uri) of
+        {error, _, _} = Error ->
+            cuttlefish:invalid(io_lib:format(
+                "Invalid attribute (~p) value: ~p (~p)", [Attr, Uri, Error]));
+        _ -> Uri
+    end.
+
 validator_https_uri(Attr, Uri) when is_binary(Uri) ->
-    list_to_binary(validator_https_uri(Attr, binary_to_list(Uri)));
+    validator_https_uri(Attr, binary_to_list(Uri));
 
-validator_https_uri(Attr, Uri) ->
+validator_https_uri(Attr, Uri) when is_list(Uri) ->
     case string:nth_lexeme(Uri, 1, "://") == "https" of
         true -> Uri;
         false ->
@@ -120,6 +130,7 @@ mapOauthProviderProperty({Key, Value}) ->
         jwks_uri -> validator_https_uri(Key, Value);
         end_session_endpoint -> validator_https_uri(Key, Value);
         authorization_endpoint -> validator_https_uri(Key, Value);
+        discovery_endpoint_path -> validator_uri(Key, Value);
         discovery_endpoint_params ->
             cuttlefish:invalid(io_lib:format(
                 "Invalid attribute (~p) value: should be a map of Key,Value pairs", [Key]));
@@ -167,7 +178,7 @@ extract_oauth_providers_endpoint_params(Variable, Settings) ->
     IndexedParams = [{Name, {list_to_binary(ParamName), list_to_binary(V)}} ||
         {["auth_oauth2","oauth_providers", Name, EndpointVar, ParamName], V}
             <- Settings, EndpointVar == atom_to_list(Variable) ],
-    maps:map(fun(_K,V)-> [{Variable, maps:from_list(V)}] end,
+    maps:map(fun(_K,V)-> [{Variable, V}] end,
         maps:groups_from_list(KeyFun, fun({_, V}) -> V end, IndexedParams)).
 
 extract_oauth_providers_signing_keys(Settings) ->
diff --git a/deps/rabbitmq_auth_backend_oauth2/test/config_schema_SUITE_data/rabbitmq_auth_backend_oauth2.snippets b/deps/rabbitmq_auth_backend_oauth2/test/config_schema_SUITE_data/rabbitmq_auth_backend_oauth2.snippets
@@ -33,9 +33,9 @@
             {verify_aud, true},
             {issuer, "https://my-jwt-issuer"},
             {discovery_endpoint_path, "/.well-known/openid-configuration"},
-            {discovery_endpoint_params, #{
-                "param1" => "value1"
-            }},
+            {discovery_endpoint_params, [
+                {<<"param1">>, <<"value1">>}
+            ]},
             {key_config, [
               {default_key, <<"id1">>},
               {signing_keys,
@@ -142,6 +142,8 @@
         auth_oauth2.oauth_providers.keycloak.https.depth = 2
         auth_oauth2.oauth_providers.keycloak.default_key = token-key
         auth_oauth2.oauth_providers.keycloak.signing_keys.id1 = test/config_schema_SUITE_data/certs/key.pem
+        auth_oauth2.oauth_providers.keycloak.discovery_endpoint_path = /.well-known/openid-configuration
+        auth_oauth2.oauth_providers.keycloak.discovery_endpoint_params.param1 = value1
         auth_oauth2.oauth_providers.keycloak.algorithms.1 = HS256
         auth_oauth2.oauth_providers.keycloak.algorithms.2 = RS256",
         [
@@ -166,14 +168,18 @@
                     {cacertfile, "test/config_schema_SUITE_data/certs/cacert.pem"}
                   ]},
                   {algorithms, [<<"HS256">>, <<"RS256">>]},
+                  {discovery_endpoint_params, [
+                     {<<"param1">>, <<"value1">>}
+                  ]},
+                  {discovery_endpoint_path, "/.well-known/openid-configuration"},
                   {default_key, <<"token-key">>},
-                  {end_session_endpoint, <<"https://keycloak/logout">>},
-                  {authorization_endpoint, <<"https://keycloak/authorize">>},
-                  {jwks_uri, <<"https://keycloak/keys">>},
-                  {token_endpoint, <<"https://keycloak/token">>}
+                  {end_session_endpoint, "https://keycloak/logout"},
+                  {authorization_endpoint, "https://keycloak/authorize"},
+                  {jwks_uri, "https://keycloak/keys"},
+                  {token_endpoint, "https://keycloak/token"}
                 ],
                <<"uaa">> => [
-                  {issuer, <<"https://uaa">>}
+                  {issuer, "https://uaa"}
                ]
 
               }
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,7 +45,7 @@ test_without_resource_servers(_) ->
     #{} = oauth2_schema:translate_resource_servers([]).
 
 test_without_endpoint_params(_) ->
-    #{} = translate_endpoint_params("oauth_discovery_endpoint_params", []).
+    [] = translate_endpoint_params("oauth_discovery_endpoint_params", []).
 
 test_with_invalid_endpoint_params(_) ->
     try translate_endpoint_params("discovery_endpoint_params", [
@@ -60,7 +60,7 @@ test_with_endpoint_params(_) ->
         {["auth_oauth2","discovery_endpoint_params","param1"], "some-value1"},
         {["auth_oauth2","discovery_endpoint_params","param2"], "some-value2"}
     ],
-    #{ "param1" := "some-value1", "param2" := "some-value2" } =
+    [ {<<"param1">>, <<"some-value1">>}, {<<"param2">>, <<"some-value2">>} ] =
         translate_endpoint_params("discovery_endpoint_params", Conf).
 
 test_invalid_oauth_providers_endpoint_params(_) ->
@@ -79,17 +79,20 @@ test_without_oauth_providers_with_endpoint_params(_) ->
     ],
 
     #{
-        <<"A">> := [{discovery_endpoint_params,
-                    #{ <<"param1">> := <<"some-value1">>, <<"param2">> := <<"some-value2">> }}],
-        <<"B">> := [{discovery_endpoint_params,
-                    #{ <<"param3">> := <<"some-value3">>}}
-        ]
+        <<"A">> := [{discovery_endpoint_params, [
+                      {<<"param1">>, <<"some-value1">>},
+                      {<<"param2">>, <<"some-value2">>}
+                    ]}],
+        <<"B">> := [{discovery_endpoint_params, [
+                      {<<"param3">>, <<"some-value3">>}
+                    ]}]
+
     } = translate_oauth_providers(Conf).
 
 test_with_one_oauth_provider(_) ->
     Conf = [{["auth_oauth2","oauth_providers","keycloak","issuer"],"https://rabbit"}
             ],
-    #{<<"keycloak">> := [{issuer, <<"https://rabbit">>}]
+    #{<<"keycloak">> := [{issuer, "https://rabbit"}]
     } = oauth2_schema:translate_oauth_providers(Conf).
 
 test_with_one_resource_server(_) ->
@@ -103,10 +106,10 @@ test_with_many_oauth_providers(_) ->
             {["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">>},
-                    {discovery_endpoint_path, <<"/some-path">>}
+      <<"uaa">> := [{issuer, "https://uaa"},
+                    {discovery_endpoint_path, "/some-path"}
                     ]
     } = oauth2_schema:translate_oauth_providers(Conf).
 
@@ -126,7 +129,7 @@ test_oauth_providers_attributes(_) ->
             {["auth_oauth2","oauth_providers","keycloak","default_key"],"token-key"}
             ],
     #{<<"keycloak">> := [{default_key, <<"token-key">>},
-                         {issuer, <<"https://keycloak">>}
+                         {issuer, "https://keycloak"}
                         ]
     } = sort_settings(oauth2_schema:translate_oauth_providers(Conf)).
 
@@ -173,7 +176,7 @@ test_oauth_providers_algorithms(_) ->
             {["auth_oauth2","oauth_providers","keycloak","algorithms","1"],"RS256"}
             ],
     #{<<"keycloak">> := [{algorithms, [<<"RS256">>, <<"HS256">>]},
-                         {issuer, <<"https://keycloak">>}
+                         {issuer, "https://keycloak"}
                          ]
     } = sort_settings(oauth2_schema:translate_oauth_providers(Conf)).
 
@@ -196,7 +199,7 @@ test_oauth_providers_https(Conf) ->
                                   {fail_if_no_peer_cert, true},
                                   {cacertfile, _CaCertFile}
                                   ]},
-                         {issuer, <<"https://keycloak">>}
+                         {issuer, "https://keycloak"}
                          ]
     } = sort_settings(oauth2_schema:translate_oauth_providers(CuttlefishConf)).
 
@@ -216,7 +219,7 @@ test_oauth_providers_signing_keys(Conf) ->
             {["auth_oauth2","oauth_providers","keycloak","signing_keys","2"], cert_filename(Conf)},
             {["auth_oauth2","oauth_providers","keycloak","signing_keys","1"], cert_filename(Conf)}
             ],
-    #{<<"keycloak">> := [{issuer, <<"https://keycloak">>},
+    #{<<"keycloak">> := [{issuer, "https://keycloak"},
                          {signing_keys, SigningKeys}
                          ]
     } = sort_settings(oauth2_schema:translate_oauth_providers(CuttlefishConf)),
diff --git a/deps/rabbitmq_management/src/rabbit_mgmt_schema.erl b/deps/rabbitmq_management/src/rabbit_mgmt_schema.erl
@@ -39,7 +39,7 @@ translate_oauth_resource_servers(Conf) ->
 -spec translate_endpoint_params(list(), [{list(), binary()}]) -> map().
 translate_endpoint_params(Variable, Conf) ->
     Params0 = cuttlefish_variable:filter_by_prefix("management." ++ Variable, Conf),
-    Params = [{Param, list_to_binary(V)} || {["management", _, Param], V} <- Params0].
+    [{list_to_binary(Param), list_to_binary(V)} || {["management", _, Param], V} <- Params0].
 
 merge_list_of_maps(ListOfMaps) ->
     lists:foldl(fun(Elem, AccIn) -> maps:merge_with(fun(_K,V1,V2) -> V1 ++ V2 end,
@@ -62,7 +62,7 @@ extract_resource_server_endpoint_params(Variable, Settings) ->
     KeyFun = fun extract_key/1,
 
     rabbit_log:debug("extract_resource_server_endpoint_params ~p ~p", [Variable, Settings]),
-    IndexedParams = [{Name, {ParamName, list_to_binary(V)}} ||
+    IndexedParams = [{Name, {list_to_binary(ParamName), list_to_binary(V)}} ||
         {["management","oauth_resource_servers", Name, EndpointVar, ParamName], V}
             <- Settings, EndpointVar == atom_to_list(Variable) ],
     maps:map(fun(_K,V)-> [{Variable, V}] end,
diff --git a/deps/rabbitmq_management/test/config_schema_SUITE_data/rabbitmq_management.snippets b/deps/rabbitmq_management/test/config_schema_SUITE_data/rabbitmq_management.snippets
@@ -627,7 +627,7 @@
   [
    {rabbitmq_management, [
                           {oauth_authorization_endpoint_params, [
-                            {"param1", <<"value1">>}
+                            {<<"param1">>, <<"value1">>}
                           ]},
                           {oauth_enabled, true},
                           {oauth_provider_url, "http://localhost:8080"},
@@ -667,7 +667,7 @@
                              ],
                              "resource-one" => [
                                 {oauth_token_endpoint_params, [
-                                   {"param2", <<"value2">>}
+                                   {<<"param2">>, <<"value2">>}
                                 ]},
                                 {oauth_scopes, "openid profile rabbitmq.*"},
                                 {oauth_client_id, "one"},
@@ -677,7 +677,7 @@
                               ],
                               "resource-two" => [
                                 {oauth_authorization_endpoint_params, [
-                                   {"param1", <<"value1">>}
+                                   {<<"param1">>, <<"value1">>}
                                 ]},
                                 {oauth_client_id, "two"},
                                 {id, "resource-two"},
diff --git a/deps/rabbitmq_management/test/rabbit_mgmt_schema_SUITE.erl b/deps/rabbitmq_management/test/rabbit_mgmt_schema_SUITE.erl
@@ -24,8 +24,8 @@ all() ->
 
 
 test_empty_endpoint_params(_) ->
-    #{} = translate_endpoint_params("oauth_authorization_endpoint_params", []),
-    #{} = translate_endpoint_params("oauth_token_endpoint_params", []).
+    [] = translate_endpoint_params("oauth_authorization_endpoint_params", []),
+    [] = translate_endpoint_params("oauth_token_endpoint_params", []).
 
 test_invalid_endpoint_params(_) ->
     try translate_endpoint_params("oauth_authorization_endpoint_params", [
@@ -36,7 +36,7 @@ test_invalid_endpoint_params(_) ->
     end.
 
 test_translate_endpoint_params(_) ->
-    #{ "param1" := "some-value1" } =
+    [ {<<"param1">>, <<"some-value1">>} ] =
         translate_endpoint_params("oauth_authorization_endpoint_params", [
             {["management","oauth_authorization_endpoint_params","param1"], "some-value1"}
         ]).
diff --git a/deps/rabbitmq_management/test/rabbit_mgmt_wm_auth_SUITE.erl b/deps/rabbitmq_management/test/rabbit_mgmt_wm_auth_SUITE.erl
@@ -361,10 +361,10 @@ init_per_suite(Config) ->
     {w, <<"w">>},
     {z, <<"z">>},
     {x, <<"x">>},
-    {authorization_params_0, [{"a-param0", "value0"}]},
-    {authorization_params_1, [{"a-param1", "value1"}]},
-    {token_params_0, [{"t-param0", "value0"}]},
-    {token_params_1, [{"t-param1", "value1"}]},
+    {authorization_params_0, [{<<"a-param0">>, <<"value0">>}]},
+    {authorization_params_1, [{<<"a-param1">>, <<"value1">>}]},
+    {token_params_0, [{<<"t-param0">>, <<"value0">>}]},
+    {token_params_1, [{<<"t-param1">>, <<"value1">>}]},
     {admin_mgt, <<"admin mgt">>},
     {read_write, <<"read write">>} | Config].
 
