From b21a222abd0a812c1e71c7f8dc40e01e9ac52f60 Mon Sep 17 00:00:00 2001 From: Marcial Rosales Date: Fri, 27 Sep 2024 15:36:05 +0200 Subject: [PATCH 1/7] Remove management.oauth_metadata_url --- .../rabbitmq_auth_backend_oauth2.schema | 4 +-- .../priv/schema/rabbitmq_management.schema | 11 -------- .../src/rabbit_mgmt_wm_auth.erl | 6 ++-- .../test/rabbit_mgmt_wm_auth_SUITE.erl | 28 ++----------------- 4 files changed, 7 insertions(+), 42 deletions(-) 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 index 5379f87560d..a7cacdbdf15 100644 --- 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 @@ -144,8 +144,8 @@ [{datatype, string}, {validators, ["uri", "https_uri"]}]}. {mapping, - "auth_oauth2.jwks_url", - "rabbitmq_auth_backend_oauth2.key_config.jwks_url", + "auth_oauth2.jwks_uri", + "rabbitmq_auth_backend_oauth2.key_config.jwks_uri", [{datatype, string}, {validators, ["uri", "https_uri"]}]}. {mapping, diff --git a/deps/rabbitmq_management/priv/schema/rabbitmq_management.schema b/deps/rabbitmq_management/priv/schema/rabbitmq_management.schema index 244a4626146..a3ff550eec8 100644 --- a/deps/rabbitmq_management/priv/schema/rabbitmq_management.schema +++ b/deps/rabbitmq_management/priv/schema/rabbitmq_management.schema @@ -496,11 +496,6 @@ end}. {mapping, "management.oauth_scopes", "rabbitmq_management.oauth_scopes", [{datatype, string}]}. -%% The URL of the OIDC discovery url where the provider is listening on -%% by default it is /.well-known/openid-configuration which is the -%% default OIDC discovery endpoint -{mapping, "management.oauth_metadata_url", "rabbitmq_management.oauth_metadata_url", - [{datatype, string}]}. %% Configure the OAuth 2 type allowed for the end users to logon to the management UI %% Default type is sp_initiated meaning the standard OAuth 2.0 mode where users come without any token @@ -557,12 +552,6 @@ end}. [{datatype, string}] }. -{mapping, - "management.oauth_resource_servers.$name.oauth_metadata_url", - "rabbitmq_management.oauth_resource_servers", - [{datatype, string}] -}. - {mapping, "management.oauth_resource_servers.$name.oauth_initiated_logon_type", "rabbitmq_management.oauth_resource_servers", diff --git a/deps/rabbitmq_management/src/rabbit_mgmt_wm_auth.erl b/deps/rabbitmq_management/src/rabbit_mgmt_wm_auth.erl index c8db33e1d77..a0478e7d83a 100644 --- a/deps/rabbitmq_management/src/rabbit_mgmt_wm_auth.erl +++ b/deps/rabbitmq_management/src/rabbit_mgmt_wm_auth.erl @@ -34,7 +34,7 @@ merge_property(Key, List, MapIn) -> extract_oauth_provider_info_props_as_map(ManagementProps) -> lists:foldl(fun(K, Acc) -> merge_property(K, ManagementProps, Acc) end, #{}, [oauth_provider_url, - oauth_metadata_url, oauth_authorization_endpoint_params, + oauth_authorization_endpoint_params, oauth_token_endpoint_params]). merge_oauth_provider_info(OAuthResourceServer, MgtResourceServer, ManagementProps) -> @@ -55,8 +55,8 @@ oauth_provider_to_map(OAuthProvider) -> Map0 = case OAuthProvider#oauth_provider.issuer of undefined -> #{}; Issuer -> #{ oauth_provider_url => Issuer, - oauth_metadata_url => OAuthProvider#oauth_provider.discovery_endpoint - } + oauth_metadata_url => OAuthProvider#oauth_provider.discovery_endpoint + } end, case OAuthProvider#oauth_provider.end_session_endpoint of undefined -> Map0; diff --git a/deps/rabbitmq_management/test/rabbit_mgmt_wm_auth_SUITE.erl b/deps/rabbitmq_management/test/rabbit_mgmt_wm_auth_SUITE.erl index 970630b6aaf..3a295484080 100644 --- a/deps/rabbitmq_management/test/rabbit_mgmt_wm_auth_SUITE.erl +++ b/deps/rabbitmq_management/test/rabbit_mgmt_wm_auth_SUITE.erl @@ -96,10 +96,7 @@ groups() -> should_return_mgt_oauth_metadata_url_url1, {with_mgt_oauth_provider_url_url0, [], [ should_return_mgt_oauth_provider_url_url0, - should_return_mgt_oauth_metadata_url_url1, - {with_mgt_oauth_metadata_url_url0, [], [ - should_return_mgt_oauth_metadata_url_url0 - ]} + should_return_mgt_oauth_metadata_url_url1 ]} ]} ]} @@ -205,10 +202,7 @@ groups() -> should_return_oauth_resource_server_a_with_oauth_metadata_url_url1, {with_mgt_oauth_resource_server_a_with_oauth_provider_url_url1, [], [ should_return_oauth_resource_server_rabbit_with_oauth_provider_url_url0, - should_return_oauth_resource_server_a_with_oauth_provider_url_url1, - {with_mgt_oauth_resource_server_a_with_oauth_metadata_url_url0, [], [ - should_return_oauth_resource_server_a_with_oauth_metadata_url_url0 - ]} + should_return_oauth_resource_server_a_with_oauth_provider_url_url1 ]} ]} ]} @@ -409,9 +403,6 @@ init_per_group(with_mgt_oauth_client_secret_q, Config) -> init_per_group(with_mgt_oauth_provider_url_url0, Config) -> set_env(rabbitmq_management, oauth_provider_url, ?config(url0, Config)), Config; -init_per_group(with_mgt_oauth_metadata_url_url0, Config) -> - set_env(rabbitmq_management, oauth_metadata_url, ?config(meta_url0, Config)), - Config; init_per_group(with_root_issuer_url1, Config) -> set_env(rabbitmq_auth_backend_oauth2, issuer, ?config(url1, Config)), Config; @@ -460,10 +451,6 @@ init_per_group(with_mgt_oauth_resource_server_a_with_oauth_provider_url_url1, Co set_attribute_in_entry_for_env_variable(rabbitmq_management, oauth_resource_servers, ?config(a, Config), oauth_provider_url, ?config(url1, Config)), Config; -init_per_group(with_mgt_oauth_resource_server_a_with_oauth_metadata_url_url0, Config) -> - set_attribute_in_entry_for_env_variable(rabbitmq_management, oauth_resource_servers, - ?config(a, Config), oauth_metadata_url, ?config(meta_url0, Config)), - Config; init_per_group(with_mgt_resource_server_a_with_client_id_x, Config) -> set_attribute_in_entry_for_env_variable(rabbitmq_management, oauth_resource_servers, ?config(a, Config), oauth_client_id, ?config(x, Config)), @@ -538,9 +525,6 @@ end_per_group(with_resource_server_id_rabbit, Config) -> end_per_group(with_mgt_oauth_provider_url_url0, Config) -> unset_env(rabbitmq_management, oauth_provider_url), Config; -end_per_group(with_mgt_oauth_metadata_url_url0, Config) -> - unset_env(rabbitmq_management, oauth_metadata_url), - Config; end_per_group(with_root_issuer_url1, Config) -> unset_env(rabbitmq_auth_backend_oauth2, issuer), unset_env(rabbitmq_auth_backend_oauth2, discovery_endpoint), @@ -574,10 +558,6 @@ end_per_group(with_mgt_oauth_resource_server_a_with_oauth_provider_url_url1, Con remove_attribute_from_entry_from_env_variable(rabbitmq_management, oauth_resource_servers, ?config(a, Config), oauth_provider_url), Config; -end_per_group(with_mgt_oauth_resource_server_a_with_oauth_metadata_url_url0, Config) -> - remove_attribute_from_entry_from_env_variable(rabbitmq_management, oauth_resource_servers, - ?config(a, Config), oauth_metadata_url), - Config; end_per_group(with_mgt_resource_server_a_with_oauth_initiated_logon_type_sp_initiated, Config) -> remove_attribute_from_entry_from_env_variable(rabbitmq_management, oauth_resource_servers, ?config(a, Config), oauth_initiated_logon_type), @@ -705,10 +685,6 @@ should_return_oauth_resource_server_a_with_oauth_metadata_url_url1(Config) -> assertEqual_on_attribute_for_oauth_resource_server(authSettings(), Config, a, oauth_metadata_url, meta_url1). -should_return_oauth_resource_server_a_with_oauth_metadata_url_url0(Config) -> - assertEqual_on_attribute_for_oauth_resource_server(authSettings(), - Config, a, oauth_metadata_url, meta_url0). - should_return_oauth_resource_server_a_with_oauth_provider_url_url0(Config) -> assertEqual_on_attribute_for_oauth_resource_server(authSettings(), Config, a, oauth_provider_url, url0). From 322a9a9f9f0e30276c2e46369ed6c9b2bac24f45 Mon Sep 17 00:00:00 2001 From: Marcial Rosales Date: Fri, 27 Sep 2024 16:10:01 +0200 Subject: [PATCH 2/7] Rename jkws_url to jwks_uri --- deps/oauth2_client/src/oauth2_client.erl | 21 +++++++++++++-------- deps/oauth2_client/test/system_SUITE.erl | 4 ++-- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/deps/oauth2_client/src/oauth2_client.erl b/deps/oauth2_client/src/oauth2_client.erl index a6a75f0bbef..e72c9ca0109 100644 --- a/deps/oauth2_client/src/oauth2_client.erl +++ b/deps/oauth2_client/src/oauth2_client.erl @@ -218,7 +218,7 @@ do_update_oauth_provider_endpoints_configuration(OAuthProvider) when List = get_env(key_config, []), ModifiedList = case OAuthProvider#oauth_provider.jwks_uri of undefined -> List; - JwksEndPoint -> [{jwks_url, JwksEndPoint} | proplists:delete(jwks_url, List)] + JwksEndPoint -> [{jwks_uri, JwksEndPoint} | proplists:delete(jwks_uri, List)] end, set_env(key_config, ModifiedList), rabbit_log:debug("Updated oauth_provider details: ~p ", @@ -393,7 +393,7 @@ lookup_oauth_provider_from_keyconfig() -> id = root, issuer = Issuer, discovery_endpoint = DiscoverEndpoint, - jwks_uri = maps:get(jwks_url, Map, undefined), %% jwks_url not uri . _url is the legacy name + jwks_uri = maps:get(jwks_uri, Map, undefined), token_endpoint = get_env(token_endpoint), authorization_endpoint = get_env(authorization_endpoint), end_session_endpoint = get_env(end_session_endpoint), @@ -437,7 +437,8 @@ extract_ssl_options_as_list(Map) -> ++ case maps:get(hostname_verification, Map, none) of wildcard -> - [{customize_hostname_check, [{match_fun, public_key:pkix_verify_hostname_match_fun(https)}]}]; + [{customize_hostname_check, [{match_fun, + public_key:pkix_verify_hostname_match_fun(https)}]}]; none -> [] end. @@ -445,7 +446,8 @@ extract_ssl_options_as_list(Map) -> % Replace peer_verification with verify to make it more consistent with other % ssl_options in RabbitMQ and Erlang's ssl options % Eventually, peer_verification will be removed. For now, both are allowed --spec get_verify_or_peer_verification(#{atom() => any()}, verify_none | verify_peer ) -> verify_none | verify_peer. +-spec get_verify_or_peer_verification(#{atom() => + any()}, verify_none | verify_peer ) -> verify_none | verify_peer. get_verify_or_peer_verification(Ssl_options, Default) -> case maps:get(verify, Ssl_options, undefined) of undefined -> @@ -464,7 +466,8 @@ lookup_oauth_provider_config(OAuth2ProviderId) -> undefined -> {error, {oauth_provider_not_found, OAuth2ProviderId}}; OAuthProvider -> - ensure_oauth_provider_has_id_property(OAuth2ProviderId, OAuthProvider) + ensure_oauth_provider_has_id_property(OAuth2ProviderId, + OAuthProvider) end; _ -> {error, invalid_oauth_provider_configuration} end. @@ -535,8 +538,9 @@ get_timeout_of_default(Timeout) -> is_json(?CONTENT_JSON) -> true; is_json(_) -> false. --spec decode_body(string(), string() | binary() | term()) -> 'false' | 'null' | 'true' | - binary() | [any()] | number() | map() | {error, term()}. +-spec decode_body(string(), string() | binary() | term()) -> + 'false' | 'null' | 'true' | binary() | [any()] | number() | map() | + {error, term()}. decode_body(_, []) -> []; decode_body(?CONTENT_JSON, Body) -> @@ -615,7 +619,8 @@ format_ssl_options(TlsOptions) -> [] -> 0; Certs -> length(Certs) end, - lists:flatten(io_lib:format("{verify: ~p, fail_if_no_peer_cert: ~p, crl_check: ~p, depth: ~p, cacertfile: ~p, cacerts(count): ~p }", [ + lists:flatten(io_lib:format("{verify: ~p, fail_if_no_peer_cert: ~p, " ++ + "crl_check: ~p, depth: ~p, cacertfile: ~p, cacerts(count): ~p }", [ proplists:get_value(verify, TlsOptions), proplists:get_value(fail_if_no_peer_cert, TlsOptions), proplists:get_value(crl_check, TlsOptions), diff --git a/deps/oauth2_client/test/system_SUITE.erl b/deps/oauth2_client/test/system_SUITE.erl index 97ae8a4a5e5..8a3930d052b 100644 --- a/deps/oauth2_client/test/system_SUITE.erl +++ b/deps/oauth2_client/test/system_SUITE.erl @@ -198,7 +198,7 @@ configure_all_oauth_provider_settings(Config) -> OAuthProvider#oauth_provider.end_session_endpoint), application:set_env(rabbitmq_auth_backend_oauth2, authorization_endpoint, OAuthProvider#oauth_provider.authorization_endpoint), - KeyConfig = [ { jwks_url, OAuthProvider#oauth_provider.jwks_uri } ] ++ + KeyConfig = [ { jwks_uri, OAuthProvider#oauth_provider.jwks_uri } ] ++ case OAuthProvider#oauth_provider.ssl_options of undefined -> []; @@ -474,7 +474,7 @@ verify_get_oauth_provider_returns_oauth_provider_from_key_config() -> oauth2_client:get_oauth_provider([issuer, token_endpoint, jwks_uri]), ExpectedIssuer = application:get_env(rabbitmq_auth_backend_oauth2, issuer, undefined), ExpectedTokenEndPoint = application:get_env(rabbitmq_auth_backend_oauth2, token_endpoint, undefined), - ExpectedJwks_uri = proplists:get_value(jwks_url, + ExpectedJwks_uri = proplists:get_value(jwks_uri, application:get_env(rabbitmq_auth_backend_oauth2, key_config, [])), ?assertEqual(root, Id), ?assertEqual(ExpectedIssuer, Issuer), From ee8d5f7fb0dc93fb81cca19f48b03113761348c5 Mon Sep 17 00:00:00 2001 From: Marcial Rosales Date: Wed, 2 Oct 2024 10:11:06 +0200 Subject: [PATCH 3/7] Deprecate jwks_url but it is still supported jwks_uri takes precedence when both are set --- deps/oauth2_client/src/oauth2_client.erl | 20 +++--- deps/oauth2_client/test/system_SUITE.erl | 72 +++++++++++++++---- deps/rabbitmq_auth_backend_oauth2/README.md | 10 +-- .../rabbitmq_auth_backend_oauth2.schema | 9 ++- .../src/uaa_jwt.erl | 2 +- .../rabbitmq_auth_backend_oauth2.snippets | 4 ++ .../test/jwks_SUITE.erl | 35 +++++---- .../test/rabbit_oauth2_provider_SUITE.erl | 4 +- 8 files changed, 109 insertions(+), 47 deletions(-) diff --git a/deps/oauth2_client/src/oauth2_client.erl b/deps/oauth2_client/src/oauth2_client.erl index e72c9ca0109..c6e07c46c10 100644 --- a/deps/oauth2_client/src/oauth2_client.erl +++ b/deps/oauth2_client/src/oauth2_client.erl @@ -215,12 +215,10 @@ do_update_oauth_provider_endpoints_configuration(OAuthProvider) when undefined -> do_nothing; EndSessionEndpoint -> set_env(end_session_endpoint, EndSessionEndpoint) end, - List = get_env(key_config, []), - ModifiedList = case OAuthProvider#oauth_provider.jwks_uri of - undefined -> List; - JwksEndPoint -> [{jwks_uri, JwksEndPoint} | proplists:delete(jwks_uri, List)] + case OAuthProvider#oauth_provider.jwks_uri of + undefined -> do_nothing; + JwksUri -> set_env(jwks_uri, JwksUri) end, - set_env(key_config, ModifiedList), rabbit_log:debug("Updated oauth_provider details: ~p ", [format_oauth_provider(OAuthProvider)]), OAuthProvider; @@ -271,7 +269,7 @@ unlock(LockId) -> -spec get_oauth_provider(list()) -> {ok, oauth_provider()} | {error, any()}. get_oauth_provider(ListOfRequiredAttributes) -> case get_env(default_oauth_provider) of - undefined -> get_oauth_provider_from_keyconfig(ListOfRequiredAttributes); + undefined -> get_root_oauth_provider(ListOfRequiredAttributes); DefaultOauthProviderId -> rabbit_log:debug("Using default_oauth_provider ~p", [DefaultOauthProviderId]), @@ -303,9 +301,9 @@ ensure_oauth_provider_has_attributes(OAuthProvider, ListOfRequiredAttributes) -> {error, {missing_oauth_provider_attributes, Attrs}} end. -get_oauth_provider_from_keyconfig(ListOfRequiredAttributes) -> - OAuthProvider = lookup_oauth_provider_from_keyconfig(), - rabbit_log:debug("Using oauth_provider ~p from keyconfig", +get_root_oauth_provider(ListOfRequiredAttributes) -> + OAuthProvider = lookup_root_oauth_provider(), + rabbit_log:debug("Using root oauth_provider ~p", [format_oauth_provider(OAuthProvider)]), case find_missing_attributes(OAuthProvider, ListOfRequiredAttributes) of [] -> @@ -384,7 +382,7 @@ find_missing_attributes(#oauth_provider{} = OAuthProvider, RequiredAttributes) - Filtered = filter_undefined_props(PropList), intersection(Filtered, RequiredAttributes). -lookup_oauth_provider_from_keyconfig() -> +lookup_root_oauth_provider() -> Map = maps:from_list(get_env(key_config, [])), Issuer = get_env(issuer), DiscoverEndpoint = build_openid_discovery_endpoint(Issuer, @@ -393,7 +391,7 @@ lookup_oauth_provider_from_keyconfig() -> id = root, issuer = Issuer, discovery_endpoint = DiscoverEndpoint, - jwks_uri = maps:get(jwks_uri, Map, undefined), + jwks_uri = get_env(jwks_uri, maps:get(jwks_url, Map, undefined)), token_endpoint = get_env(token_endpoint), authorization_endpoint = get_env(authorization_endpoint), end_session_endpoint = get_env(end_session_endpoint), diff --git a/deps/oauth2_client/test/system_SUITE.erl b/deps/oauth2_client/test/system_SUITE.erl index 8a3930d052b..4a5bc1fe543 100644 --- a/deps/oauth2_client/test/system_SUITE.erl +++ b/deps/oauth2_client/test/system_SUITE.erl @@ -28,6 +28,7 @@ all() -> {group, https_down}, {group, https}, {group, with_all_oauth_provider_settings} + % {group, without_all_oauth_providers_settings} ]. @@ -35,10 +36,12 @@ groups() -> [ {with_all_oauth_provider_settings, [], [ - {group, verify_get_oauth_provider} + {group, verify_get_oauth_provider}, + jwks_uri_takes_precedence_over_jwks_url, + jwks_url_is_used_in_absense_of_jwks_uri ]}, {without_all_oauth_providers_settings, [], [ - {group, verify_get_oauth_provider} + {group, verify_get_oauth_provider} ]}, {verify_openid_configuration, [], [ get_openid_configuration, @@ -57,7 +60,7 @@ groups() -> expiration_time_in_token ]}, {verify_get_oauth_provider, [], [ - get_oauth_provider, + get_oauth_provider, {with_default_oauth_provider, [], [ get_oauth_provider ]}, @@ -78,6 +81,8 @@ groups() -> init_per_suite(Config) -> [ + {jwks_url, build_jwks_uri("https", "/certs4url")}, + {jwks_uri, build_jwks_uri("https")}, {denies_access_token, [ {token_endpoint, denies_access_token_expectation()} ]}, {auth_server_error, [ {token_endpoint, auth_server_error_when_access_token_request_expectation()} ]}, {non_json_payload, [ {token_endpoint, non_json_payload_when_access_token_request_expectation()} ]}, @@ -95,7 +100,7 @@ init_per_group(https, Config) -> CertsDir = ?config(rmq_certsdir, Config0), CaCertFile = filename:join([CertsDir, "testca", "cacert.pem"]), WrongCaCertFile = filename:join([CertsDir, "server", "server.pem"]), - [{group, https}, + [{group, https}, {oauth_provider_id, <<"uaa">>}, {oauth_provider, build_https_oauth_provider(<<"uaa">>, CaCertFile)}, {oauth_provider_with_issuer, keep_only_issuer_and_ssl_options( @@ -198,17 +203,34 @@ configure_all_oauth_provider_settings(Config) -> OAuthProvider#oauth_provider.end_session_endpoint), application:set_env(rabbitmq_auth_backend_oauth2, authorization_endpoint, OAuthProvider#oauth_provider.authorization_endpoint), - KeyConfig = [ { jwks_uri, OAuthProvider#oauth_provider.jwks_uri } ] ++ + KeyConfig0 = case OAuthProvider#oauth_provider.ssl_options of undefined -> []; _ -> [ {peer_verification, proplists:get_value(verify, OAuthProvider#oauth_provider.ssl_options) }, - {cacertfile, proplists:get_value(cacertfile, + {cacertfile, proplists:get_value(cacertfile, OAuthProvider#oauth_provider.ssl_options) } ] end, + KeyConfig = + case ?config(jwks_uri_type_of_config, Config) of + undefined -> + application:set_env(rabbitmq_auth_backend_oauth2, jwks_uri, + OAuthProvider#oauth_provider.jwks_uri), + KeyConfig0; + only_jwks_uri -> + application:set_env(rabbitmq_auth_backend_oauth2, jwks_uri, + OAuthProvider#oauth_provider.jwks_uri), + KeyConfig0; + only_jwks_url -> + [ { jwks_url, ?config(jwks_url, Config) } | KeyConfig0 ]; + both -> + application:set_env(rabbitmq_auth_backend_oauth2, jwks_uri, + OAuthProvider#oauth_provider.jwks_uri), + [ { jwks_url, ?config(jwks_url, Config) } | KeyConfig0 ] + end, application:set_env(rabbitmq_auth_backend_oauth2, key_config, KeyConfig). configure_minimum_oauth_provider_settings(Config) -> @@ -232,9 +254,18 @@ configure_minimum_oauth_provider_settings(Config) -> end, application:set_env(rabbitmq_auth_backend_oauth2, key_config, KeyConfig). -init_per_testcase(TestCase, Config) -> +init_per_testcase(TestCase, Config0) -> application:set_env(rabbitmq_auth_backend_oauth2, use_global_locks, false), + Config = [case TestCase of + jwks_url_is_used_in_absense_of_jwks_uri -> + {jwks_uri_type_of_config, only_jwks_url}; + jwks_uri_takes_precedence_over_jwks_url -> + {jwks_uri_type_of_config, both}; + _ -> + {jwks_uri_type_of_config, only_jwks_uri} + end | Config0], + case ?config(with_all_oauth_provider_settings, Config) of false -> configure_minimum_oauth_provider_settings(Config); true -> configure_all_oauth_provider_settings(Config); @@ -256,6 +287,7 @@ init_per_testcase(TestCase, Config) -> end_per_testcase(_, Config) -> application:unset_env(rabbitmq_auth_backend_oauth2, oauth_providers), application:unset_env(rabbitmq_auth_backend_oauth2, issuer), + application:unset_env(rabbitmq_auth_backend_oauth2, jwks_uri), application:unset_env(rabbitmq_auth_backend_oauth2, token_endpoint), application:unset_env(rabbitmq_auth_backend_oauth2, authorization_endpoint), application:unset_env(rabbitmq_auth_backend_oauth2, end_session_endpoint), @@ -466,7 +498,7 @@ ssl_connection_error(Config) -> {error, {failed_connect, _} } = oauth2_client:get_access_token( ?config(oauth_provider_with_wrong_ca, Config), build_access_token_request(Parameters)). -verify_get_oauth_provider_returns_oauth_provider_from_key_config() -> +verify_get_oauth_provider_returns_root_oauth_provider() -> {ok, #oauth_provider{id = Id, issuer = Issuer, token_endpoint = TokenEndPoint, @@ -474,8 +506,7 @@ verify_get_oauth_provider_returns_oauth_provider_from_key_config() -> oauth2_client:get_oauth_provider([issuer, token_endpoint, jwks_uri]), ExpectedIssuer = application:get_env(rabbitmq_auth_backend_oauth2, issuer, undefined), ExpectedTokenEndPoint = application:get_env(rabbitmq_auth_backend_oauth2, token_endpoint, undefined), - ExpectedJwks_uri = proplists:get_value(jwks_uri, - application:get_env(rabbitmq_auth_backend_oauth2, key_config, [])), + ExpectedJwks_uri = application:get_env(rabbitmq_auth_backend_oauth2, jwks_uri, undefined), ?assertEqual(root, Id), ?assertEqual(ExpectedIssuer, Issuer), ?assertEqual(ExpectedTokenEndPoint, TokenEndPoint), @@ -494,7 +525,7 @@ get_oauth_provider(Config) -> true -> case application:get_env(rabbitmq_auth_backend_oauth2, default_oauth_provider, undefined) of undefined -> - verify_get_oauth_provider_returns_oauth_provider_from_key_config(); + verify_get_oauth_provider_returns_root_oauth_provider(); DefaultOAuthProviderId -> verify_get_oauth_provider_returns_default_oauth_provider(DefaultOAuthProviderId) end; @@ -564,6 +595,20 @@ get_oauth_provider_given_oauth_provider_id(Config) -> Jwks_uri) end. +jwks_url_is_used_in_absense_of_jwks_uri(Config) -> + {ok, #oauth_provider{ + jwks_uri = Jwks_uri}} = oauth2_client:get_oauth_provider([jwks_uri]), + ?assertEqual( + proplists:get_value(jwks_url, + application:get_env(rabbitmq_auth_backend_oauth2, key_config, []), undefined), + Jwks_uri). + +jwks_uri_takes_precedence_over_jwks_url(Config) -> + {ok, #oauth_provider{ + jwks_uri = Jwks_uri}} = oauth2_client:get_oauth_provider([jwks_uri]), + ?assertEqual( + application:get_env(rabbitmq_auth_backend_oauth2, jwks_uri, undefined), + Jwks_uri). %%% HELPERS @@ -584,10 +629,13 @@ build_token_endpoint_uri(Scheme) -> path => "/token"}). build_jwks_uri(Scheme) -> + build_jwks_uri(Scheme, "/certs"). + +build_jwks_uri(Scheme, Path) -> uri_string:recompose(#{scheme => Scheme, host => "localhost", port => rabbit_data_coercion:to_integer(?AUTH_PORT), - path => "/certs"}). + path => Path}). build_access_token_request(Request) -> #access_token_request { diff --git a/deps/rabbitmq_auth_backend_oauth2/README.md b/deps/rabbitmq_auth_backend_oauth2/README.md index 1d72c5af3e0..13bf62b5bad 100644 --- a/deps/rabbitmq_auth_backend_oauth2/README.md +++ b/deps/rabbitmq_auth_backend_oauth2/README.md @@ -149,13 +149,13 @@ In that case, the configuration would look like this: {rabbitmq_auth_backend_oauth2, [ {resource_server_id, <<"my_rabbit_server">>}, {key_config, [ - {jwks_url, <<"https://jwt-issuer.my-domain.local/jwks.json">>} + {jwks_uri, <<"https://jwt-issuer.my-domain.local/jwks.json">>} ]} ]}, ]. ``` -Note: if both are configured, `jwks_url` takes precedence over `signing_keys`. +Note: if both are configured, `jwks_uri` takes precedence over `signing_keys`. ### Variables Configurable in rabbitmq.conf @@ -166,7 +166,7 @@ Note: if both are configured, `jwks_url` takes precedence over `signing_keys`. | `auth_oauth2.additional_scopes_key` | Key to fetch additional scopes from (maps to `additional_rabbitmq_scopes` in the `advanced.config` format) | `auth_oauth2.default_key` | ID (name) of the default signing key | `auth_oauth2.signing_keys` | Paths to signing key files -| `auth_oauth2.jwks_url` | The URL of key server. According to the [JWT Specification](https://datatracker.ietf.org/doc/html/rfc7515#section-4.1.2) key server URL must be https +| `auth_oauth2.jwks_uri` | The URL of key server. According to the [JWT Specification](https://datatracker.ietf.org/doc/html/rfc7515#section-4.1.2) key server URL must be https | `auth_oauth2.https.cacertfile` | Path to a file containing PEM-encoded CA certificates. The CA certificates are used during key server [peer verification](https://rabbitmq.com/ssl.html#peer-verification) | `auth_oauth2.https.depth` | The maximum number of non-self-issued intermediate certificates that may follow the peer certificate in a valid [certification path](https://rabbitmq.com/ssl.html#peer-verification-depth). Default is 10. | `auth_oauth2.https.peer_verification` | Should [peer verification](https://rabbitmq.com/ssl.html#peer-verification) be enabled Available values: `verify_none`, `verify_peer`. Default is `verify_none`. It is recommended to configure `verify_peer`. Peer verification requires a certain amount of setup and is more secure. @@ -194,7 +194,7 @@ auth_oauth2.algorithms.2 = RS256 ``` auth_oauth2.resource_server_id = new_resource_server_id -auth_oauth2.jwks_url = https://my-jwt-issuer/jwks.json +auth_oauth2.jwks_uri = https://my-jwt-issuer/jwks.json auth_oauth2.https.cacertfile = test/config_schema_SUITE_data/certs/cacert.pem auth_oauth2.https.peer_verification = verify_peer auth_oauth2.https.depth = 5 @@ -234,7 +234,7 @@ resolve the user's identity: `username`, `user_name`, `email`, `sub`, `client_id {resource_server_id, <<"my_rabbit_server">>}, {preferred_username_claims, [ <<"username">>, <<"user_name">>, <<"email">> ]} {key_config, [ - {jwks_url, <<"https://jwt-issuer.my-domain.local/jwks.json">>} + {jwks_uri, <<"https://jwt-issuer.my-domain.local/jwks.json">>} ]} ]}, ]. 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 index a7cacdbdf15..bd81ee3a55a 100644 --- 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 @@ -143,9 +143,16 @@ "rabbitmq_auth_backend_oauth2.token_endpoint", [{datatype, string}, {validators, ["uri", "https_uri"]}]}. +%% DEPRECATES auth_oauth2.jwks_url {mapping, "auth_oauth2.jwks_uri", - "rabbitmq_auth_backend_oauth2.key_config.jwks_uri", + "rabbitmq_auth_backend_oauth2.jwks_uri", + [{datatype, string}, {validators, ["uri", "https_uri"]}]}. + +%% DEPRECATED +{mapping, + "auth_oauth2.jwks_url", + "rabbitmq_auth_backend_oauth2.key_config.jwks_url", [{datatype, string}, {validators, ["uri", "https_uri"]}]}. {mapping, diff --git a/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl b/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl index 46a46cd4117..d95e74ee5c0 100644 --- a/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl +++ b/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl @@ -124,7 +124,7 @@ get_jwk(KeyId, InternalOAuthProvider, AllowUpdateJwks) -> case update_jwks_signing_keys(OAuthProvider) of ok -> get_jwk(KeyId, InternalOAuthProvider, false); - {error, no_jwks_url} -> + {error, no_jwks_uri} -> {error, key_not_found}; {error, _} = Err -> Err 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 index 4638312ecb5..0d991be472a 100644 --- 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 @@ -11,6 +11,7 @@ auth_oauth2.default_key = id1 auth_oauth2.signing_keys.id1 = test/config_schema_SUITE_data/certs/key.pem auth_oauth2.signing_keys.id2 = test/config_schema_SUITE_data/certs/cert.pem + auth_oauth2.jwks_uri = https://my-jwt-issuer/jwks.json auth_oauth2.jwks_url = https://my-jwt-issuer/jwks.json auth_oauth2.issuer = https://my-jwt-issuer auth_oauth2.https.cacertfile = test/config_schema_SUITE_data/certs/cacert.pem @@ -36,6 +37,7 @@ {discovery_endpoint_params, [ {<<"param1">>, <<"value1">>} ]}, + {jwks_uri, "https://my-jwt-issuer/jwks.json"}, {key_config, [ {default_key, <<"id1">>}, {signing_keys, @@ -69,6 +71,7 @@ auth_oauth2.default_key = id1 auth_oauth2.signing_keys.id1 = test/config_schema_SUITE_data/certs/key.pem auth_oauth2.signing_keys.id2 = test/config_schema_SUITE_data/certs/cert.pem + auth_oauth2.jwks_uri = https://my-jwt-issuer/jwks.json auth_oauth2.jwks_url = https://my-jwt-issuer/jwks.json auth_oauth2.https.cacertfile = test/config_schema_SUITE_data/certs/cacert.pem auth_oauth2.https.peer_verification = verify_none @@ -90,6 +93,7 @@ {extra_scopes_source, <<"my_custom_scope_key">>}, {preferred_username_claims, [<<"user_name">>, <<"username">>, <<"email">>]}, {verify_aud, true}, + {jwks_uri, "https://my-jwt-issuer/jwks.json"}, {resource_servers, #{ <<"rabbitmq-operations">> => [ diff --git a/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl b/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl index c3f32406353..438a06a6bb4 100644 --- a/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl +++ b/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl @@ -170,12 +170,18 @@ end_per_suite(Config) -> ] ++ rabbit_ct_broker_helpers:teardown_steps()). init_per_group(no_peer_verification, Config) -> +<<<<<<< HEAD KeyConfig = set_config(?config(key_config, Config), [ {jwks_url, ?config(non_strict_jwks_url, Config)}, {peer_verification, verify_none} ]), ok = rpc_set_env(Config,key_config, KeyConfig), set_config(Config, {key_config, KeyConfig}); +======= + KeyConfig = rabbit_ct_helpers:set_config(?config(key_config, Config), [{jwks_uri, ?config(non_strict_jwks_uri, Config)}, {peer_verification, verify_none}]), + ok = rabbit_ct_broker_helpers:rpc(Config, 0, application, set_env, [rabbitmq_auth_backend_oauth2, key_config, KeyConfig]), + rabbit_ct_helpers:set_config(Config, {key_config, KeyConfig}); +>>>>>>> 2586207266 (Deprecate jwks_url but it is still supported) init_per_group(without_kid, Config) -> set_config(Config, [{include_kid, false}]); @@ -224,7 +230,6 @@ init_per_group(with_oauth_provider_A_with_jwks_with_one_signing_key, Config) -> OAuthProvider = maps:get(<<"A">>, OAuthProviders0, []), OAuthProviders1 = maps:put(<<"A">>, [ {jwks_uri, strict_jwks_url(Config, "/jwksA")} | OAuthProvider], - OAuthProviders0), ok = rpc_set_env(Config, oauth_providers, OAuthProviders1), Config; @@ -269,7 +274,7 @@ init_per_group(with_root_oauth_provider_with_two_static_keys_and_one_jwks_key, C ?UTIL_MOD:token_key(Jwks2) => {json, Jwks2} }, KeyConfig1 = [{signing_keys, SigningKeys}, - {jwks_url, strict_jwks_url(Config, "/jwks")}| KeyConfig], + {jwks_url, strict_jwks_uri(Config, "/jwks")}| KeyConfig], ok = rpc_set_env(Config, key_config, KeyConfig1), Config; init_per_group(with_root_oauth_provider_with_default_key_1, Config) -> @@ -296,7 +301,7 @@ init_per_group(with_oauth_provider_B_with_one_static_key_and_jwks_with_two_signi }, OAuthProviders1 = maps:put(<<"B">>, [ {signing_keys, SigningKeys}, - {jwks_uri, strict_jwks_url(Config, "/jwksB")} | OAuthProvider], + {jwks_uri, strict_jwks_uri(Config, "/jwksB")} | OAuthProvider], OAuthProviders0), ok = rpc_set_env(Config, oauth_providers, OAuthProviders1), @@ -331,7 +336,7 @@ end_per_group(without_kid, Config) -> end_per_group(no_peer_verification, Config) -> KeyConfig = set_config(?config(key_config, Config), [ - {jwks_url, ?config(strict_jwks_url, Config)}, + {jwks_uri, ?config(strict_jwks_uri, Config)}, {peer_verification, verify_peer}]), ok = rpc_set_env(Config, key_config, KeyConfig), set_config(Config, {key_config, KeyConfig}); @@ -460,8 +465,8 @@ start_jwks_server(Config0) -> %% Both URLs direct to the same JWKS server %% The NonStrictJwksUrl identity cannot be validated while StrictJwksUrl identity can be validated - NonStrictJwksUrl = non_strict_jwks_url(Config), - StrictJwksUrl = strict_jwks_url(Config), + NonStrictJwksUri = non_strict_jwks_uri(Config), + StrictJwksUri = strict_jwks_uri(Config), {ok, _} = application:ensure_all_started(ssl), {ok, _} = application:ensure_all_started(cowboy), @@ -474,13 +479,13 @@ start_jwks_server(Config0) -> {"/jwks1", [Jwk1, Jwk3]}, {"/jwks2", [Jwk2]} ]), - KeyConfig = [{jwks_url, StrictJwksUrl}, + KeyConfig = [{jwks_uri, StrictJwksUri}, {peer_verification, verify_peer}, {cacertfile, filename:join([CertsDir, "testca", "cacert.pem"])}], ok = rpc_set_env(Config, key_config, KeyConfig), set_config(Config, [ - {non_strict_jwks_url, NonStrictJwksUrl}, - {strict_jwks_url, StrictJwksUrl}, + {non_strict_jwks_uri, NonStrictJwksUri}, + {strict_jwks_uri, StrictJwksUri}, {key_config, KeyConfig}, {fixture_static_1, Jwk7}, {fixture_static_2, Jwk8}, @@ -494,13 +499,13 @@ start_jwks_server(Config0) -> {fixture_jwks_1, [Jwk1, Jwk3]}, {fixture_jwks_2, [Jwk2]} ]). -strict_jwks_url(Config) -> - strict_jwks_url(Config, "/jwks"). -strict_jwks_url(Config, Path) -> +strict_jwks_uri(Config) -> + strict_jwks_uri(Config, "/jwks"). +strict_jwks_uri(Config, Path) -> "https://localhost:" ++ integer_to_list(?config(jwksServerPort, Config)) ++ Path. -non_strict_jwks_url(Config) -> - non_strict_jwks_url(Config, "/jwks"). -non_strict_jwks_url(Config, Path) -> +non_strict_jwks_uri(Config) -> + non_strict_jwks_uri(Config, "/jwks"). +non_strict_jwks_uri(Config, Path) -> "https://127.0.0.1:" ++ integer_to_list(?config(jwksServerPort, Config)) ++ Path. diff --git a/deps/rabbitmq_auth_backend_oauth2/test/rabbit_oauth2_provider_SUITE.erl b/deps/rabbitmq_auth_backend_oauth2/test/rabbit_oauth2_provider_SUITE.erl index 9f830585aa1..956155cb694 100644 --- a/deps/rabbitmq_auth_backend_oauth2/test/rabbit_oauth2_provider_SUITE.erl +++ b/deps/rabbitmq_auth_backend_oauth2/test/rabbit_oauth2_provider_SUITE.erl @@ -110,7 +110,7 @@ init_per_group(oauth_provider_with_jwks_uri, Config) -> URL = case ?config(oauth_provider_id, Config) of root -> RootUrl = build_url_to_oauth_provider(<<"/keys">>), - set_env(key_config, [{jwks_url, RootUrl}]), + set_env(key_config, [{jwks_uri, RootUrl}]), RootUrl; <<"A">> -> AUrl = build_url_to_oauth_provider(<<"/A/keys">>), @@ -211,7 +211,7 @@ end_per_group(oauth_provider_with_issuer, Config) -> Config; end_per_group(oauth_provider_with_jwks_uri, Config) -> case ?config(oauth_provider_id, Config) of - root -> unset_env(jwks_url); + root -> unset_env(jwks_uri); Id -> unset_oauth_provider_properties(Id, [jwks_uri]) end, Config; From c9d5ddf89ff33611fc26253ca58c956f57f4bf64 Mon Sep 17 00:00:00 2001 From: Marcial Rosales Date: Wed, 2 Oct 2024 12:37:21 +0200 Subject: [PATCH 4/7] Deprecate oauth_metadata_url If oauth_metadata_url is configured, RabbitMQ uses it. Else it uses the discovery_endpoint url calculated from issuer and discovery_endpoint_path --- .../priv/schema/rabbitmq_management.schema | 13 + .../src/rabbit_mgmt_wm_auth.erl | 297 ++++++++++-------- .../test/rabbit_mgmt_wm_auth_SUITE.erl | 30 +- 3 files changed, 208 insertions(+), 132 deletions(-) diff --git a/deps/rabbitmq_management/priv/schema/rabbitmq_management.schema b/deps/rabbitmq_management/priv/schema/rabbitmq_management.schema index a3ff550eec8..ceabe77a6e4 100644 --- a/deps/rabbitmq_management/priv/schema/rabbitmq_management.schema +++ b/deps/rabbitmq_management/priv/schema/rabbitmq_management.schema @@ -472,6 +472,13 @@ end}. {mapping, "management.oauth_response_type", "rabbitmq_management.oauth_response_type", [{datatype, string}]}. +%% THIS VARIABLE IS DEPRECATED. CHECKOUT auth_oauth2.discovery_endpoint_path VARIABLE. +%% The URL of the OIDC discovery url where the provider is listening on +%% by default it is /.well-known/openid-configuration which is the +%% default OIDC discovery endpoint +{mapping, "management.oauth_metadata_url", "rabbitmq_management.oauth_metadata_url", + [{datatype, string}]}. + %% Configure OAuth2 authorization_endpoint additional request parameters {mapping, "management.oauth_authorization_endpoint_params.$name", "rabbitmq_management.oauth_authorization_endpoint_params", @@ -552,6 +559,12 @@ end}. [{datatype, string}] }. +{mapping, + "management.oauth_resource_servers.$name.oauth_metadata_url", + "rabbitmq_management.oauth_resource_servers", + [{datatype, string}] +}. + {mapping, "management.oauth_resource_servers.$name.oauth_initiated_logon_type", "rabbitmq_management.oauth_resource_servers", diff --git a/deps/rabbitmq_management/src/rabbit_mgmt_wm_auth.erl b/deps/rabbitmq_management/src/rabbit_mgmt_wm_auth.erl index a0478e7d83a..5b7a333e44b 100644 --- a/deps/rabbitmq_management/src/rabbit_mgmt_wm_auth.erl +++ b/deps/rabbitmq_management/src/rabbit_mgmt_wm_auth.erl @@ -23,162 +23,199 @@ variances(Req, Context) -> {[<<"accept-encoding">>, <<"origin">>], Req, Context}. content_types_provided(ReqData, Context) -> - {rabbit_mgmt_util:responder_map(to_json), ReqData, Context}. + {rabbit_mgmt_util:responder_map(to_json), ReqData, Context}. merge_property(Key, List, MapIn) -> - case proplists:get_value(Key, List) of - undefined -> MapIn; - V0 -> MapIn#{Key => V0} - end. + case proplists:get_value(Key, List) of + undefined -> MapIn; + V0 -> MapIn#{Key => V0} + end. extract_oauth_provider_info_props_as_map(ManagementProps) -> - lists:foldl(fun(K, Acc) -> - merge_property(K, ManagementProps, Acc) end, #{}, [oauth_provider_url, - oauth_authorization_endpoint_params, - oauth_token_endpoint_params]). - -merge_oauth_provider_info(OAuthResourceServer, MgtResourceServer, ManagementProps) -> - OAuthProviderResult = case proplists:get_value(oauth_provider_id, OAuthResourceServer) of - undefined -> oauth2_client:get_oauth_provider([issuer]); - OauthProviderId -> oauth2_client:get_oauth_provider(OauthProviderId, [issuer]) - end, - OAuthProviderInfo0 = case OAuthProviderResult of - {ok, OAuthProvider} -> oauth_provider_to_map(OAuthProvider); - {error, _} -> #{} - end, - OAuthProviderInfo1 = maps:merge(OAuthProviderInfo0, - extract_oauth_provider_info_props_as_map(ManagementProps)), - maps:merge(OAuthProviderInfo1, proplists:to_map(MgtResourceServer)). + lists:foldl(fun(K, Acc) -> + merge_property(K, ManagementProps, Acc) end, #{}, + [oauth_provider_url, + oauth_metadata_url, + oauth_authorization_endpoint_params, + oauth_token_endpoint_params]). + +merge_oauth_provider_info(OAuthResourceServer, MgtResourceServer, + ManagementProps) -> + OAuthProviderResult = + case proplists:get_value(oauth_provider_id, OAuthResourceServer) of + undefined -> + oauth2_client:get_oauth_provider([issuer]); + OauthProviderId -> + oauth2_client:get_oauth_provider(OauthProviderId, [issuer]) + end, + OAuthProviderInfo0 = + case OAuthProviderResult of + {ok, OAuthProvider} -> oauth_provider_to_map(OAuthProvider); + {error, _} -> #{} + end, + OAuthProviderInfo1 = maps:merge(OAuthProviderInfo0, + extract_oauth_provider_info_props_as_map(ManagementProps)), + maps:merge(OAuthProviderInfo1, proplists:to_map(MgtResourceServer)). oauth_provider_to_map(OAuthProvider) -> - % only include issuer and end_session_endpoint for now. The other endpoints are resolved by oidc-client library - Map0 = case OAuthProvider#oauth_provider.issuer of - undefined -> #{}; - Issuer -> #{ oauth_provider_url => Issuer, - oauth_metadata_url => OAuthProvider#oauth_provider.discovery_endpoint - } - end, - case OAuthProvider#oauth_provider.end_session_endpoint of - undefined -> Map0; - V -> maps:put(end_session_endpoint, V, Map0) - end. + % only include issuer and end_session_endpoint for now. + % The other endpoints are resolved by oidc-client library + Map0 = case OAuthProvider#oauth_provider.issuer of + undefined -> + #{}; + Issuer -> + #{ + oauth_provider_url => Issuer, + oauth_metadata_url => + OAuthProvider#oauth_provider.discovery_endpoint + } + end, + case OAuthProvider#oauth_provider.end_session_endpoint of + undefined -> Map0; + V -> maps:put(end_session_endpoint, V, Map0) + end. -skip_unknown_mgt_resource_servers(MgtOauthResources, OAuth2Resources) -> - maps:filter(fun(Key, _Value) -> maps:is_key(Key, OAuth2Resources) end, MgtOauthResources). +skip_unknown_mgt_resource_servers(ManagementProps, OAuth2Resources) -> + maps:filter(fun(Key, _Value) -> maps:is_key(Key, OAuth2Resources) end, + proplists:get_value(oauth_resource_servers, ManagementProps, #{})). skip_disabled_mgt_resource_servers(MgtOauthResources) -> - maps:filter(fun(_Key, Value) -> not proplists:get_value(disabled, Value, false) end, MgtOauthResources). + maps:filter(fun(_Key, Value) -> + not proplists:get_value(disabled, Value, false) end, + MgtOauthResources). extract_oauth2_and_mgt_resources(OAuth2BackendProps, ManagementProps) -> - OAuth2Resources = getAllDeclaredOauth2Resources(OAuth2BackendProps), - MgtResources0 = skip_unknown_mgt_resource_servers(proplists:get_value(oauth_resource_servers, - ManagementProps, #{}), OAuth2Resources), - MgtResources1 = maps:merge(MgtResources0, maps:filtermap(fun(K,_V) -> - case maps:is_key(K, MgtResources0) of - true -> false; - false -> {true, [{id, K}]} - end end, OAuth2Resources)), - MgtResources = maps:map( - fun(K,V) -> merge_oauth_provider_info(maps:get(K, OAuth2Resources, #{}), V, ManagementProps) end, - skip_disabled_mgt_resource_servers(MgtResources1)), - case maps:size(MgtResources) of - 0 -> {}; - _ -> {MgtResources} - end. + OAuth2Resources = getAllDeclaredOauth2Resources(OAuth2BackendProps), + MgtResources0 = skip_unknown_mgt_resource_servers(ManagementProps, + OAuth2Resources), + MgtResources1 = maps:merge(maps:filtermap(fun(K,_V) -> + case maps:is_key(K, MgtResources0) of + true -> false; + false -> {true, [{id, K}]} + end end, OAuth2Resources), MgtResources0), + MgtResources = maps:map( + fun(K,V) -> merge_oauth_provider_info( + maps:get(K, OAuth2Resources, #{}), V, ManagementProps) end, + skip_disabled_mgt_resource_servers(MgtResources1)), + case maps:size(MgtResources) of + 0 -> {}; + _ -> {MgtResources} + end. getAllDeclaredOauth2Resources(OAuth2BackendProps) -> - OAuth2Resources = proplists:get_value(resource_servers, OAuth2BackendProps, #{}), - case proplists:get_value(resource_server_id, OAuth2BackendProps) of - undefined -> OAuth2Resources; - Id -> maps:put(Id, buildRootResourceServerIfAny(Id, OAuth2BackendProps), - OAuth2Resources) - end. + OAuth2Resources = proplists:get_value(resource_servers, OAuth2BackendProps, + #{}), + case proplists:get_value(resource_server_id, OAuth2BackendProps) of + undefined -> + OAuth2Resources; + Id -> + maps:put(Id, buildRootResourceServerIfAny(Id, OAuth2BackendProps), + OAuth2Resources) + end. buildRootResourceServerIfAny(Id, Props) -> - [ {id, Id}, - {oauth_client_id, - proplists:get_value(oauth_client_id, Props)}, - {oauth_client_secret, - proplists:get_value(oauth_client_secret, Props)}, - {oauth_response_type, - proplists:get_value(oauth_response_type, Props)}, - {oauth_authorization_endpoint_params, - proplists:get_value(oauth_authorization_endpoint_params, Props)}, - {oauth_token_endpoint_params, - proplists:get_value(oauth_token_endpoint_params, Props)} - ]. + [ + {id, Id}, + {oauth_provider_id, proplists:get_value(oauth_provider_id, Props)} + ]. authSettings() -> - ManagementProps = application:get_all_env(rabbitmq_management), - OAuth2BackendProps = application:get_all_env(rabbitmq_auth_backend_oauth2), - EnableOAUTH = proplists:get_value(oauth_enabled, ManagementProps, false), - case EnableOAUTH of - false -> [{oauth_enabled, false}]; - true -> - case extract_oauth2_and_mgt_resources(OAuth2BackendProps, ManagementProps) of - {MgtResources} -> produce_auth_settings(MgtResources, ManagementProps); - {} -> [{oauth_enabled, false}] - end + ManagementProps = application:get_all_env(rabbitmq_management), + OAuth2BackendProps = application:get_all_env(rabbitmq_auth_backend_oauth2), + EnableOAUTH = proplists:get_value(oauth_enabled, ManagementProps, false), + case EnableOAUTH of + false -> [{oauth_enabled, false}]; + true -> + case extract_oauth2_and_mgt_resources(OAuth2BackendProps, + ManagementProps) of + {MgtResources} -> + produce_auth_settings(MgtResources, ManagementProps); + {} -> + [{oauth_enabled, false}] + end end. -skip_mgt_resource_servers_without_oauth_client_id_with_sp_initiated_logon(MgtResourceServers, ManagementProps) -> - DefaultOauthInitiatedLogonType = proplists:get_value(oauth_initiated_logon_type, ManagementProps, sp_initiated), - maps:filter(fun(_K,ResourceServer) -> - SpInitiated = case maps:get(oauth_initiated_logon_type, ResourceServer, DefaultOauthInitiatedLogonType) of - sp_initiated -> true; - _ -> false - end, - not SpInitiated or - not is_invalid([maps:get(oauth_client_id, ResourceServer, undefined)]) end, MgtResourceServers). - - -filter_mgt_resource_servers_without_oauth_client_id_for_sp_initiated(MgtResourceServers, ManagementProps) -> - case is_invalid([proplists:get_value(oauth_client_id, ManagementProps)]) of - true -> skip_mgt_resource_servers_without_oauth_client_id_with_sp_initiated_logon(MgtResourceServers, ManagementProps); - false -> MgtResourceServers - end. +% invalid -> those resources that dont have an oauth_client_id and +% their login_type is sp_initiated +skip_invalid_mgt_resource_servers(MgtResourceServers, ManagementProps) -> + DefaultOauthInitiatedLogonType = proplists:get_value( + oauth_initiated_logon_type, ManagementProps, sp_initiated), + maps:filter(fun(_K,ResourceServer) -> + SpInitiated = + case maps:get(oauth_initiated_logon_type, ResourceServer, + DefaultOauthInitiatedLogonType) of + sp_initiated -> true; + _ -> false + end, + not SpInitiated or not is_invalid([maps:get(oauth_client_id, + ResourceServer, undefined)]) + end, MgtResourceServers). + +% filter -> include only those resources with an oauth_client_id +% or those whose logon type is not sp_initiated +filter_out_invalid_mgt_resource_servers(MgtResourceServers, ManagementProps) -> + case is_invalid([proplists:get_value(oauth_client_id, ManagementProps)]) of + true -> + skip_invalid_mgt_resource_servers(MgtResourceServers, + ManagementProps); + false -> + MgtResourceServers + end. filter_mgt_resource_servers_without_oauth_provider_url(MgtResourceServers) -> - maps:filter(fun(_K1,V1) -> maps:is_key(oauth_provider_url, V1) end, MgtResourceServers). + maps:filter(fun(_K1,V1) -> maps:is_key(oauth_provider_url, V1) end, + MgtResourceServers). ensure_oauth_resource_server_properties_are_binaries(Key, Value) -> - case Key of - oauth_authorization_endpoint_params -> Value; - oauth_token_endpoint_params -> Value; - _ -> to_binary(Value) - end. + case Key of + oauth_authorization_endpoint_params -> Value; + oauth_token_endpoint_params -> Value; + _ -> to_binary(Value) + end. produce_auth_settings(MgtResourceServers, ManagementProps) -> - ConvertValuesToBinary = fun(_K,V) -> [ - {K1, ensure_oauth_resource_server_properties_are_binaries(K1, V1)} || {K1,V1} - <- maps:to_list(V)] end, - FilteredMgtResourceServers = filter_mgt_resource_servers_without_oauth_provider_url( - filter_mgt_resource_servers_without_oauth_client_id_for_sp_initiated(MgtResourceServers, ManagementProps)), - - case maps:size(FilteredMgtResourceServers) of - 0 -> [{oauth_enabled, false}]; - _ -> - filter_empty_properties([ - {oauth_enabled, true}, - {oauth_resource_servers, maps:map(ConvertValuesToBinary, FilteredMgtResourceServers)}, - to_tuple(oauth_disable_basic_auth, ManagementProps, fun to_binary/1, true), - to_tuple(oauth_client_id, ManagementProps), - to_tuple(oauth_client_secret, ManagementProps), - to_tuple(oauth_scopes, ManagementProps), - case proplists:get_value(oauth_initiated_logon_type, ManagementProps, sp_initiated) of - sp_initiated -> {}; - idp_initiated -> {oauth_initiated_logon_type, <<"idp_initiated">>} - end, - to_tuple(oauth_authorization_endpoint_params, ManagementProps, undefined, undefined), - to_tuple(oauth_token_endpoint_params, ManagementProps, undefined, undefined) - ]) + ConvertValuesToBinary = fun(_K,V) -> + [ + {K1, ensure_oauth_resource_server_properties_are_binaries(K1, V1)} + || {K1,V1} <- maps:to_list(V) + ] end, + FilteredMgtResourceServers = + filter_mgt_resource_servers_without_oauth_provider_url( + filter_out_invalid_mgt_resource_servers(MgtResourceServers, + ManagementProps)), + + case maps:size(FilteredMgtResourceServers) of + 0 -> + [{oauth_enabled, false}]; + _ -> + filter_empty_properties([ + {oauth_enabled, true}, + {oauth_resource_servers, + maps:map(ConvertValuesToBinary, FilteredMgtResourceServers)}, + to_tuple(oauth_disable_basic_auth, ManagementProps, + fun to_binary/1, true), + to_tuple(oauth_client_id, ManagementProps), + to_tuple(oauth_client_secret, ManagementProps), + to_tuple(oauth_scopes, ManagementProps), + case proplists:get_value(oauth_initiated_logon_type, + ManagementProps, sp_initiated) of + sp_initiated -> + {}; + idp_initiated -> + {oauth_initiated_logon_type, <<"idp_initiated">>} + end, + to_tuple(oauth_authorization_endpoint_params, ManagementProps, + undefined, undefined), + to_tuple(oauth_token_endpoint_params, ManagementProps, + undefined, undefined) + ]) end. filter_empty_properties(ListOfProperties) -> - lists:filter(fun(Prop) -> - case Prop of - {} -> false; - _ -> true - end - end, ListOfProperties). + lists:filter(fun(Prop) -> + case Prop of + {} -> false; + _ -> true + end + end, ListOfProperties). to_binary(Value) when is_boolean(Value)-> Value; to_binary(Value) -> rabbit_data_coercion:to_binary(Value). diff --git a/deps/rabbitmq_management/test/rabbit_mgmt_wm_auth_SUITE.erl b/deps/rabbitmq_management/test/rabbit_mgmt_wm_auth_SUITE.erl index 3a295484080..07d7ab98a0e 100644 --- a/deps/rabbitmq_management/test/rabbit_mgmt_wm_auth_SUITE.erl +++ b/deps/rabbitmq_management/test/rabbit_mgmt_wm_auth_SUITE.erl @@ -96,7 +96,10 @@ groups() -> should_return_mgt_oauth_metadata_url_url1, {with_mgt_oauth_provider_url_url0, [], [ should_return_mgt_oauth_provider_url_url0, - should_return_mgt_oauth_metadata_url_url1 + should_return_mgt_oauth_metadata_url_url1, + {with_mgt_oauth_resource_server_rabbit_with_oauth_metadata_url_url1, [], [ + should_return_oauth_resource_server_rabbit_with_oauth_metadata_url_url1 + ]} ]} ]} ]} @@ -215,6 +218,9 @@ groups() -> {with_mgt_oauth_client_id_z, [], [ should_return_oauth_resource_server_rabbit_with_oauth_provider_url_idp1_url, should_return_oauth_resource_server_rabbit_with_oauth_metadata_url_idp1_url, + {with_mgt_oauth_resource_server_rabbit_with_oauth_metadata_url_url1, [], [ + should_return_oauth_resource_server_rabbit_with_oauth_metadata_url_url1 + ]}, {with_root_issuer_url1, [], [ should_return_oauth_resource_server_rabbit_with_oauth_provider_url_idp1_url ]}, @@ -223,7 +229,10 @@ groups() -> should_return_oauth_resource_server_rabbit_with_oauth_metadata_url_idp1_url, {with_mgt_oauth_resource_server_a_with_oauth_provider_url_url1, [], [ should_return_oauth_resource_server_rabbit_with_oauth_provider_url_url0, - should_return_oauth_resource_server_a_with_oauth_provider_url_url1 + should_return_oauth_resource_server_a_with_oauth_provider_url_url1, + {with_mgt_oauth_resource_server_a_with_oauth_metadata_url_url1, [], [ + should_return_oauth_resource_server_a_with_oauth_metadata_url_url1 + ]} ]} ]} ]} @@ -459,6 +468,9 @@ init_per_group(with_mgt_resource_server_a_with_client_id_x, Config) -> init_per_group(with_default_oauth_provider_idp1, Config) -> set_env(rabbitmq_auth_backend_oauth2, default_oauth_provider, ?config(idp1, Config)), Config; +init_per_group(with_mgt_oauth_resource_server_rabbit_with_oauth_metadata_url_url1, Config) -> + set_env(rabbitmq_management, oauth_metadata_url, ?config(meta_url1, Config)), + Config; init_per_group(with_default_oauth_provider_idp3, Config) -> set_env(rabbitmq_auth_backend_oauth2, default_oauth_provider, ?config(idp3, Config)), Config; @@ -489,6 +501,10 @@ init_per_group(with_mgt_resource_server_a_with_authorization_endpoint_params_1, set_attribute_in_entry_for_env_variable(rabbitmq_management, oauth_resource_servers, ?config(a, Config), oauth_authorization_endpoint_params, ?config(authorization_params_1, Config)), Config; +init_per_group(with_mgt_oauth_resource_server_a_with_oauth_metadata_url_url1, Config) -> + set_attribute_in_entry_for_env_variable(rabbitmq_management, oauth_resource_servers, + ?config(a, Config), oauth_metadata_url, ?config(meta_url1, Config)), + Config; init_per_group(with_mgt_resource_server_a_with_token_endpoint_params_1, Config) -> set_attribute_in_entry_for_env_variable(rabbitmq_management, oauth_resource_servers, ?config(a, Config), oauth_token_endpoint_params, ?config(token_params_1, Config)), @@ -522,9 +538,15 @@ end_per_group(with_oauth_disable_basic_auth_false, Config) -> end_per_group(with_resource_server_id_rabbit, Config) -> unset_env(rabbitmq_auth_backend_oauth2, resource_server_id), Config; +end_per_group(with_default_oauth_provider_idp1, Config) -> + unset_env(rabbitmq_auth_backend_oauth2, default_oauth_provider), + Config; end_per_group(with_mgt_oauth_provider_url_url0, Config) -> unset_env(rabbitmq_management, oauth_provider_url), Config; +end_per_group(with_mgt_oauth_resource_server_rabbit_with_oauth_metadata_url_url1, Config) -> + unset_env(rabbitmq_management, oauth_metadata_url), + Config; end_per_group(with_root_issuer_url1, Config) -> unset_env(rabbitmq_auth_backend_oauth2, issuer), unset_env(rabbitmq_auth_backend_oauth2, discovery_endpoint), @@ -558,6 +580,10 @@ end_per_group(with_mgt_oauth_resource_server_a_with_oauth_provider_url_url1, Con remove_attribute_from_entry_from_env_variable(rabbitmq_management, oauth_resource_servers, ?config(a, Config), oauth_provider_url), Config; +end_per_group(with_mgt_oauth_resource_server_a_with_oauth_metadata_url_url1, Config) -> + remove_attribute_from_entry_from_env_variable(rabbitmq_management, oauth_resource_servers, + ?config(a, Config), oauth_metadata_url), + Config; end_per_group(with_mgt_resource_server_a_with_oauth_initiated_logon_type_sp_initiated, Config) -> remove_attribute_from_entry_from_env_variable(rabbitmq_management, oauth_resource_servers, ?config(a, Config), oauth_initiated_logon_type), From 0835c7ecf4035a1f6d0ee7ff7be1eac3a8cc29ed Mon Sep 17 00:00:00 2001 From: Marcial Rosales Date: Fri, 4 Oct 2024 14:32:13 +0200 Subject: [PATCH 5/7] Resolve merge conflicts --- deps/oauth2_client/test/system_SUITE.erl | 4 +- .../test/jwks_SUITE.erl | 44 ++++++------------- .../test/rabbit_oauth2_provider_SUITE.erl | 2 +- .../test/unit_SUITE.erl | 6 ++- 4 files changed, 20 insertions(+), 36 deletions(-) diff --git a/deps/oauth2_client/test/system_SUITE.erl b/deps/oauth2_client/test/system_SUITE.erl index 4a5bc1fe543..4c6b92feff7 100644 --- a/deps/oauth2_client/test/system_SUITE.erl +++ b/deps/oauth2_client/test/system_SUITE.erl @@ -27,8 +27,8 @@ all() -> [ {group, https_down}, {group, https}, - {group, with_all_oauth_provider_settings} - % {group, without_all_oauth_providers_settings} + {group, with_all_oauth_provider_settings}, + {group, without_all_oauth_providers_settings} ]. diff --git a/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl b/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl index 438a06a6bb4..0a0be86ba83 100644 --- a/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl +++ b/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl @@ -27,7 +27,9 @@ -import(rabbit_ct_broker_helpers, [ rpc/5 ]). --import(rabbit_mgmt_test_util, [amqp_port/1]). +-import(rabbit_mgmt_test_util, [ + amqp_port/1 +]). all() -> [ @@ -170,30 +172,21 @@ end_per_suite(Config) -> ] ++ rabbit_ct_broker_helpers:teardown_steps()). init_per_group(no_peer_verification, Config) -> -<<<<<<< HEAD KeyConfig = set_config(?config(key_config, Config), [ - {jwks_url, ?config(non_strict_jwks_url, Config)}, + {jwks_url, ?config(non_strict_jwks_uri, Config)}, {peer_verification, verify_none} ]), - ok = rpc_set_env(Config,key_config, KeyConfig), + ok = rpc_set_env(Config, key_config, KeyConfig), set_config(Config, {key_config, KeyConfig}); -======= - KeyConfig = rabbit_ct_helpers:set_config(?config(key_config, Config), [{jwks_uri, ?config(non_strict_jwks_uri, Config)}, {peer_verification, verify_none}]), - ok = rabbit_ct_broker_helpers:rpc(Config, 0, application, set_env, [rabbitmq_auth_backend_oauth2, key_config, KeyConfig]), - rabbit_ct_helpers:set_config(Config, {key_config, KeyConfig}); ->>>>>>> 2586207266 (Deprecate jwks_url but it is still supported) - init_per_group(without_kid, Config) -> set_config(Config, [{include_kid, false}]); - init_per_group(with_resource_servers_rabbitmq1_with_oauth_provider_A, Config) -> ResourceServersConfig0 = rpc_get_env(Config, resource_servers, #{}), - Resource0 = maps:get(<<"rabbitmq1">>, - ResourceServersConfig0, [{id, <<"rabbitmq1">>}]), + Resource0 = maps:get(<<"rabbitmq1">>, ResourceServersConfig0, + [{id, <<"rabbitmq1">>}]), ResourceServersConfig1 = maps:put(<<"rabbitmq1">>, [{oauth_provider_id, <<"A">>} | Resource0], ResourceServersConfig0), ok = rpc_set_env(Config, resource_servers, ResourceServersConfig1); - init_per_group(with_oauth_providers_A_B_and_C, Config) -> OAuthProviders = #{ <<"A">> => [ @@ -211,26 +204,22 @@ init_per_group(with_oauth_providers_A_B_and_C, Config) -> }, ok = rpc_set_env(Config, oauth_providers, OAuthProviders), Config; - init_per_group(with_default_oauth_provider_B, Config) -> ok = rpc_set_env(Config, default_oauth_provider, <<"B">>); - init_per_group(with_oauth_providers_A_with_default_key, Config) -> {ok, OAuthProviders0} = rpc_get_env(Config, oauth_providers), OAuthProvider = maps:get(<<"A">>, OAuthProviders0, []), OAuthProviders1 = maps:put(<<"A">>, [ {default_key, ?UTIL_MOD:token_key(?config(fixture_jwksA, Config))} | OAuthProvider], OAuthProviders0), - ok = rpc_set_env(Config, oauth_providers, OAuthProviders1), Config; - init_per_group(with_oauth_provider_A_with_jwks_with_one_signing_key, Config) -> {ok, OAuthProviders0} = rpc_get_env(Config, oauth_providers), OAuthProvider = maps:get(<<"A">>, OAuthProviders0, []), OAuthProviders1 = maps:put(<<"A">>, [ - {jwks_uri, strict_jwks_url(Config, "/jwksA")} | OAuthProvider], - + {jwks_uri, strict_jwks_uri(Config, "/jwksA")} | OAuthProvider], + OAuthProviders0), ok = rpc_set_env(Config, oauth_providers, OAuthProviders1), Config; init_per_group(with_resource_servers_rabbitmq2, Config) -> @@ -239,7 +228,8 @@ init_per_group(with_resource_servers_rabbitmq2, Config) -> [{id, <<"rabbitmq2">>}]), ResourceServersConfig1 = maps:put(<<"rabbitmq2">>, Resource0, ResourceServersConfig0), - ok = rpc_set_env(Config, resource_servers, ResourceServersConfig1); + ok = rpc_set_env(Config, resource_servers, ResourceServersConfig1), + Config; init_per_group(with_oauth_providers_B_with_default_key_static_key, Config) -> {ok, OAuthProviders0} = rpc_get_env(Config, oauth_providers), OAuthProvider = maps:get(<<"B">>, OAuthProviders0, []), @@ -247,7 +237,6 @@ init_per_group(with_oauth_providers_B_with_default_key_static_key, Config) -> {default_key, ?UTIL_MOD:token_key(?config(fixture_staticB, Config))} | proplists:delete(default_key, OAuthProvider)], OAuthProviders0), - ok = rpc_set_env(Config,oauth_providers, OAuthProviders1), Config; init_per_group(with_oauth_provider_C_with_two_static_keys, Config) -> @@ -264,7 +253,6 @@ init_per_group(with_oauth_provider_C_with_two_static_keys, Config) -> ok = rpc_set_env(Config, oauth_providers, OAuthProviders1), Config; - init_per_group(with_root_oauth_provider_with_two_static_keys_and_one_jwks_key, Config) -> KeyConfig = rpc_get_env(Config, key_config, []), Jwks1 = ?config(fixture_static_1, Config), @@ -291,7 +279,6 @@ init_per_group(with_root_oauth_provider_with_default_jwks_key, Config) -> | KeyConfig], ok = rpc_set_env(Config, key_config, KeyConfig1), Config; - init_per_group(with_oauth_provider_B_with_one_static_key_and_jwks_with_two_signing_keys, Config) -> {ok, OAuthProviders0} = rpc_get_env(Config, oauth_providers), OAuthProvider = maps:get(<<"B">>, OAuthProviders0, []), @@ -306,16 +293,13 @@ init_per_group(with_oauth_provider_B_with_one_static_key_and_jwks_with_two_signi ok = rpc_set_env(Config, oauth_providers, OAuthProviders1), Config; - init_per_group(with_resource_servers_rabbitmq3_with_oauth_provider_C, Config) -> ResourceServersConfig0 = rpc_get_env(Config, resource_servers, #{}), Resource0 = maps:get(<<"rabbitmq3">>, ResourceServersConfig0, [ {id, <<"rabbitmq3">>},{oauth_provider_id, <<"C">>}]), ResourceServersConfig1 = maps:put(<<"rabbitmq3">>, Resource0, ResourceServersConfig0), - ok = rpc_set_env(Config, resource_servers, ResourceServersConfig1); - init_per_group(with_oauth_providers_C_with_default_key_static_key_1, Config) -> {ok, OAuthProviders0} = rpc_get_env(Config, oauth_providers), OAuthProvider = maps:get(<<"C">>, OAuthProviders0, []), @@ -323,10 +307,8 @@ init_per_group(with_oauth_providers_C_with_default_key_static_key_1, Config) -> OAuthProviders1 = maps:put(<<"C">>, [ {default_key, ?UTIL_MOD:token_key(Jwks)} | OAuthProvider], OAuthProviders0), - ok = rpc_set_env(Config, oauth_providers, OAuthProviders1), Config; - init_per_group(_Group, Config) -> ok = rpc_set_env(Config, resource_server_id, ?RESOURCE_SERVER_ID), Config. @@ -461,7 +443,7 @@ start_jwks_server(Config0) -> %% Assume we don't have more than 100 ports allocated for tests PortBase = rabbit_ct_broker_helpers:get_node_config(Config0, 0, tcp_ports_base), JwksServerPort = PortBase + 100, - Config = rabbit_ct_helpers:set_config(Config0, [{jwksServerPort, JwksServerPort}]), + Config = set_config(Config0, [{jwksServerPort, JwksServerPort}]), %% Both URLs direct to the same JWKS server %% The NonStrictJwksUrl identity cannot be validated while StrictJwksUrl identity can be validated @@ -479,7 +461,7 @@ start_jwks_server(Config0) -> {"/jwks1", [Jwk1, Jwk3]}, {"/jwks2", [Jwk2]} ]), - KeyConfig = [{jwks_uri, StrictJwksUri}, + KeyConfig = [{jwks_url, StrictJwksUri}, {peer_verification, verify_peer}, {cacertfile, filename:join([CertsDir, "testca", "cacert.pem"])}], ok = rpc_set_env(Config, key_config, KeyConfig), diff --git a/deps/rabbitmq_auth_backend_oauth2/test/rabbit_oauth2_provider_SUITE.erl b/deps/rabbitmq_auth_backend_oauth2/test/rabbit_oauth2_provider_SUITE.erl index 956155cb694..ac3ca2b67e8 100644 --- a/deps/rabbitmq_auth_backend_oauth2/test/rabbit_oauth2_provider_SUITE.erl +++ b/deps/rabbitmq_auth_backend_oauth2/test/rabbit_oauth2_provider_SUITE.erl @@ -110,7 +110,7 @@ init_per_group(oauth_provider_with_jwks_uri, Config) -> URL = case ?config(oauth_provider_id, Config) of root -> RootUrl = build_url_to_oauth_provider(<<"/keys">>), - set_env(key_config, [{jwks_uri, RootUrl}]), + set_env(jwks_uri, RootUrl), RootUrl; <<"A">> -> AUrl = build_url_to_oauth_provider(<<"/A/keys">>), diff --git a/deps/rabbitmq_auth_backend_oauth2/test/unit_SUITE.erl b/deps/rabbitmq_auth_backend_oauth2/test/unit_SUITE.erl index aaeb0b92960..04d4639f3aa 100644 --- a/deps/rabbitmq_auth_backend_oauth2/test/unit_SUITE.erl +++ b/deps/rabbitmq_auth_backend_oauth2/test/unit_SUITE.erl @@ -1105,8 +1105,8 @@ test_incorrect_kid(_) -> AltKid = <<"other-token-key">>, Username = <<"username">>, Jwk = ?UTIL_MOD:fixture_jwk(), - set_env(resource_server_id, - <<"rabbitmq">>), + unset_env(key_config), + set_env(resource_server_id, <<"rabbitmq">>), Token = ?UTIL_MOD:sign_token_hs( ?UTIL_MOD:token_with_sub(?UTIL_MOD:fixture_token(), Username), Jwk, AltKid, true), @@ -1298,6 +1298,8 @@ normalize_token_scope_without_scope_claim(_) -> set_env(Par, Var) -> application:set_env(rabbitmq_auth_backend_oauth2, Par, Var). +unset_env(Par) -> + application:unset_env(rabbitmq_auth_backend_oauth2, Par). assert_vhost_access_granted(AuthUser, VHost) -> assert_vhost_access_response(true, AuthUser, VHost). From 0c8dadd6623f6f1e73fb24a6789362f3c5a67e79 Mon Sep 17 00:00:00 2001 From: Marcial Rosales Date: Wed, 9 Oct 2024 10:07:44 +0200 Subject: [PATCH 6/7] Fix failing test cases --- deps/oauth2_client/test/system_SUITE.erl | 52 ++++++++++++++---------- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/deps/oauth2_client/test/system_SUITE.erl b/deps/oauth2_client/test/system_SUITE.erl index 4c6b92feff7..6bf8064fd9a 100644 --- a/deps/oauth2_client/test/system_SUITE.erl +++ b/deps/oauth2_client/test/system_SUITE.erl @@ -12,7 +12,8 @@ -include_lib("oauth2_client.hrl"). -import(oauth2_client, [ - build_openid_discovery_endpoint/3]). + build_openid_discovery_endpoint/3 +]). -compile(export_all). @@ -27,8 +28,7 @@ all() -> [ {group, https_down}, {group, https}, - {group, with_all_oauth_provider_settings}, - {group, without_all_oauth_providers_settings} + {group, with_all_oauth_provider_settings} ]. @@ -83,10 +83,14 @@ init_per_suite(Config) -> [ {jwks_url, build_jwks_uri("https", "/certs4url")}, {jwks_uri, build_jwks_uri("https")}, - {denies_access_token, [ {token_endpoint, denies_access_token_expectation()} ]}, - {auth_server_error, [ {token_endpoint, auth_server_error_when_access_token_request_expectation()} ]}, - {non_json_payload, [ {token_endpoint, non_json_payload_when_access_token_request_expectation()} ]}, - {grants_refresh_token, [ {token_endpoint, grants_refresh_token_expectation()} ]} + {denies_access_token, [ + {token_endpoint, denies_access_token_expectation()} ]}, + {auth_server_error, [ + {token_endpoint, auth_server_error_when_access_token_request_expectation()} ]}, + {non_json_payload, [ + {token_endpoint, non_json_payload_when_access_token_request_expectation()} ]}, + {grants_refresh_token, [ + {token_endpoint, grants_refresh_token_expectation()} ]} | Config]. end_per_suite(Config) -> @@ -234,7 +238,7 @@ configure_all_oauth_provider_settings(Config) -> application:set_env(rabbitmq_auth_backend_oauth2, key_config, KeyConfig). configure_minimum_oauth_provider_settings(Config) -> - OAuthProvider = ?config(oauth_provider_with_issuer, Config), + OAuthProvider = ?config(oauth_provider, Config), OAuthProviders = #{ ?config(oauth_provider_id, Config) => oauth_provider_to_proplist(OAuthProvider) }, application:set_env(rabbitmq_auth_backend_oauth2, oauth_providers, @@ -279,6 +283,9 @@ init_per_testcase(TestCase, Config0) -> https -> start_https_oauth_server(?AUTH_PORT, ?config(rmq_certsdir, Config), ListOfExpectations); + without_all_oauth_providers_settings -> + start_https_oauth_server(?AUTH_PORT, ?config(rmq_certsdir, Config), + ListOfExpectations); _ -> do_nothing end, @@ -295,6 +302,8 @@ end_per_testcase(_, Config) -> case ?config(group, Config) of https -> stop_https_auth_server(); + without_all_oauth_providers_settings -> + stop_https_auth_server(); _ -> do_nothing end, @@ -504,9 +513,9 @@ verify_get_oauth_provider_returns_root_oauth_provider() -> token_endpoint = TokenEndPoint, jwks_uri = Jwks_uri}} = oauth2_client:get_oauth_provider([issuer, token_endpoint, jwks_uri]), - ExpectedIssuer = application:get_env(rabbitmq_auth_backend_oauth2, issuer, undefined), - ExpectedTokenEndPoint = application:get_env(rabbitmq_auth_backend_oauth2, token_endpoint, undefined), - ExpectedJwks_uri = application:get_env(rabbitmq_auth_backend_oauth2, jwks_uri, undefined), + ExpectedIssuer = get_env(issuer), + ExpectedTokenEndPoint = get_env(token_endpoint), + ExpectedJwks_uri = get_env(jwks_uri), ?assertEqual(root, Id), ?assertEqual(ExpectedIssuer, Issuer), ?assertEqual(ExpectedTokenEndPoint, TokenEndPoint), @@ -523,7 +532,7 @@ verify_get_oauth_provider_returns_default_oauth_provider(DefaultOAuthProviderId) get_oauth_provider(Config) -> case ?config(with_all_oauth_provider_settings, Config) of true -> - case application:get_env(rabbitmq_auth_backend_oauth2, default_oauth_provider, undefined) of + case get_env(default_oauth_provider) of undefined -> verify_get_oauth_provider_returns_root_oauth_provider(); DefaultOAuthProviderId -> @@ -556,8 +565,7 @@ get_oauth_provider_given_oauth_provider_id(Config) -> [issuer, token_endpoint, jwks_uri, authorization_endpoint, end_session_endpoint]), - OAuthProviders = application:get_env(rabbitmq_auth_backend_oauth2, - oauth_providers, #{}), + OAuthProviders = get_env(oauth_providers, #{}), ExpectedProvider = maps:get(Id, OAuthProviders, []), ?assertEqual(proplists:get_value(issuer, ExpectedProvider), Issuer), @@ -599,16 +607,13 @@ jwks_url_is_used_in_absense_of_jwks_uri(Config) -> {ok, #oauth_provider{ jwks_uri = Jwks_uri}} = oauth2_client:get_oauth_provider([jwks_uri]), ?assertEqual( - proplists:get_value(jwks_url, - application:get_env(rabbitmq_auth_backend_oauth2, key_config, []), undefined), + proplists:get_value(jwks_url, get_env(key_config, []), undefined), Jwks_uri). jwks_uri_takes_precedence_over_jwks_url(Config) -> {ok, #oauth_provider{ jwks_uri = Jwks_uri}} = oauth2_client:get_oauth_provider([jwks_uri]), - ?assertEqual( - application:get_env(rabbitmq_auth_backend_oauth2, jwks_uri, undefined), - Jwks_uri). + ?assertEqual(get_env(jwks_uri), Jwks_uri). %%% HELPERS @@ -671,11 +676,11 @@ oauth_provider_to_proplist(#oauth_provider{ authorization_endpoint = AuthorizationEndpoint, ssl_options = SslOptions, jwks_uri = Jwks_uri}) -> - [ { issuer, Issuer}, + [ {issuer, Issuer}, {token_endpoint, TokenEndpoint}, {end_session_endpoint, EndSessionEndpoint}, {authorization_endpoint, AuthorizationEndpoint}, - { https, + {https, case SslOptions of undefined -> []; Value -> Value @@ -725,6 +730,11 @@ token(ExpiresIn) -> EncodedToken. +get_env(Par) -> + application:get_env(rabbitmq_auth_backend_oauth2, Par, undefined). +get_env(Par, Default) -> + application:get_env(rabbitmq_auth_backend_oauth2, Par, Default). + build_http_mock_behaviour(Request, Response) -> #{request => Request, response => Response}. From 0f1b8760a47ec13cc51a4b767eee4064ad489084 Mon Sep 17 00:00:00 2001 From: Marcial Rosales Date: Wed, 9 Oct 2024 16:43:01 +0200 Subject: [PATCH 7/7] Fix issue --- deps/rabbitmq_auth_backend_oauth2/test/unit_SUITE.erl | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/deps/rabbitmq_auth_backend_oauth2/test/unit_SUITE.erl b/deps/rabbitmq_auth_backend_oauth2/test/unit_SUITE.erl index 04d4639f3aa..9255c9a6361 100644 --- a/deps/rabbitmq_auth_backend_oauth2/test/unit_SUITE.erl +++ b/deps/rabbitmq_auth_backend_oauth2/test/unit_SUITE.erl @@ -73,16 +73,10 @@ groups() -> init_per_suite(Config) -> application:load(rabbitmq_auth_backend_oauth2), Env = application:get_all_env(rabbitmq_auth_backend_oauth2), - Config1 = rabbit_ct_helpers:set_config(Config, {env, Env}), - rabbit_ct_helpers:run_setup_steps(Config1, []). + lists:foreach(fun({K, _V}) -> unset_env(K) end, Env), + rabbit_ct_helpers:run_setup_steps(Config, []). end_per_suite(Config) -> - Env = ?config(env, Config), - lists:foreach( - fun({K, V}) -> - set_env(K, V) - end, - Env), rabbit_ct_helpers:run_teardown_steps(Config). init_per_group(with_rabbitmq_node, Config) ->