Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Deprecate two OAuth2 settings: auth_oauth2.jwks_url and management.metadata_url #12399

Merged
merged 7 commits into from
Oct 9, 2024
Merged

Deprecate two OAuth2 settings: auth_oauth2.jwks_url and management.metadata_url #12399

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b21a222
Remove management.oauth_metadata_url
MarcialRosales Sep 27, 2024
322a9a9
Rename jkws_url to jwks_uri
MarcialRosales Sep 27, 2024
ee8d5f7
Deprecate jwks_url but it is still supported
MarcialRosales Oct 2, 2024
c9d5ddf
Deprecate oauth_metadata_url
MarcialRosales Oct 2, 2024
0835c7e
Resolve merge conflicts
MarcialRosales Oct 4, 2024
0c8dadd
Fix failing test cases
MarcialRosales Oct 9, 2024
0f1b876
Fix issue
MarcialRosales Oct 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 20 additions & 17 deletions deps/oauth2_client/src/oauth2_client.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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_url, JwksEndPoint} | proplists:delete(jwks_url, 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;
Expand Down Expand Up @@ -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]),
Expand Down Expand Up @@ -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
[] ->
Expand Down Expand Up @@ -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,
Expand All @@ -393,7 +391,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 = 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),
Expand Down Expand Up @@ -437,15 +435,17 @@ 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.

% 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 ->
Expand All @@ -464,7 +464,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.
Expand Down Expand Up @@ -535,8 +536,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) ->
Expand Down Expand Up @@ -615,7 +617,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),
Expand Down
108 changes: 83 additions & 25 deletions deps/oauth2_client/test/system_SUITE.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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).

Expand All @@ -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,
Expand All @@ -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
]},
Expand All @@ -78,10 +81,16 @@ groups() ->

init_per_suite(Config) ->
[
{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()} ]}
{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()} ]}
| Config].

end_per_suite(Config) ->
Expand All @@ -95,7 +104,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(
Expand Down Expand Up @@ -198,21 +207,38 @@ 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 } ] ++
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) ->
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,
Expand All @@ -232,9 +258,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);
Expand All @@ -248,6 +283,9 @@ init_per_testcase(TestCase, Config) ->
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,
Expand All @@ -256,13 +294,16 @@ 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),
application:unset_env(rabbitmq_auth_backend_oauth2, key_config),
case ?config(group, Config) of
https ->
stop_https_auth_server();
without_all_oauth_providers_settings ->
stop_https_auth_server();
_ ->
do_nothing
end,
Expand Down Expand Up @@ -466,16 +507,15 @@ 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,
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 = proplists:get_value(jwks_url,
application:get_env(rabbitmq_auth_backend_oauth2, key_config, [])),
ExpectedIssuer = get_env(issuer),
ExpectedTokenEndPoint = get_env(token_endpoint),
ExpectedJwks_uri = get_env(jwks_uri),
?assertEqual(root, Id),
?assertEqual(ExpectedIssuer, Issuer),
?assertEqual(ExpectedTokenEndPoint, TokenEndPoint),
Expand All @@ -492,9 +532,9 @@ 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_oauth_provider_from_key_config();
verify_get_oauth_provider_returns_root_oauth_provider();
DefaultOAuthProviderId ->
verify_get_oauth_provider_returns_default_oauth_provider(DefaultOAuthProviderId)
end;
Expand Down Expand Up @@ -525,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),
Expand Down Expand Up @@ -564,6 +603,17 @@ 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, 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(get_env(jwks_uri), Jwks_uri).


%%% HELPERS
Expand All @@ -584,10 +634,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 {
Expand Down Expand Up @@ -623,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
Expand Down Expand Up @@ -677,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}.
Expand Down
Loading
Loading