Deprecate jwks_url but it is still supported

jwks_uri takes precedence when both are set
rabbitmq · Oct 2, 2024 · 2586207 · 2586207
1 parent 9a9a449
commit 2586207
Show file tree

Hide file tree

Showing 8 changed files with 105 additions and 48 deletions.
diff --git 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
@@ -28,17 +28,20 @@ all() ->
     {group, https_down},
     {group, https},
     {group, with_all_oauth_provider_settings}
+   % {group, without_all_oauth_providers_settings}
 
 ].
 
 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,16 +498,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_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
@@ -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
@@ -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
@@ -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/...q_auth_backend_oauth2/test/config_schema_SUITE_data/rabbitmq_auth_backend_oauth2.snippets b/...q_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">> => [