Merge pull request #10624 from rabbitmq/fixes-10612

Fix issue #10612

Author: michaelklishin

deps/oauth2_client/BUILD.bazel

@@ -125,4 +125,10 @@ rabbitmq_integration_suite(
     ],
 )
 
+
+rabbitmq_suite(
+    name = "unit_SUITE",
+    size = "small",
+)
+
 assert_suites()

deps/oauth2_client/app.bzl

@@ -81,3 +81,12 @@ def test_suite_beam_files(name = "test_suite_beam_files"):
         app_name = "oauth2_client",
         erlc_opts = "//:test_erlc_opts",
     )
+    erlang_bytecode(
+        name = "unit_SUITE_beam_files",
+        testonly = True,
+        srcs = ["test/unit_SUITE.erl"],
+        outs = ["test/unit_SUITE.beam"],
+        hdrs = ["include/oauth2_client.hrl"],
+        app_name = "oauth2_client",
+        erlc_opts = "//:test_erlc_opts",
+    )

deps/oauth2_client/src/oauth2_client.erl

@@ -7,7 +7,8 @@
 -module(oauth2_client).
 -export([get_access_token/2,
         refresh_access_token/2,
-        get_oauth_provider/1,get_oauth_provider/2
+        get_oauth_provider/1, get_oauth_provider/2,
+        extract_ssl_options_as_list/1
         ]).
 
 -include("oauth2_client.hrl").
@@ -270,31 +271,46 @@ lookup_oauth_provider_from_keyconfig() ->
 
 -spec extract_ssl_options_as_list(#{atom() => any()}) -> proplists:proplist().
 extract_ssl_options_as_list(Map) ->
-  Verify = case maps:get(peer_verification, Map, verify_peer) of
+  {Verify, CaCerts, CaCertFile} = case maps:get(peer_verification, Map, verify_peer) of
     verify_peer ->
       case maps:get(cacertfile, Map, undefined) of
         undefined ->
           case public_key:cacerts_get() of
-            [] -> verify_none;
-            _ -> verify_peer
+            [] -> {verify_none, undefined, undefined};
+            Certs -> {verify_peer, Certs, undefined}
           end;
-        _ -> verify_peer
+        CaCert -> {verify_peer, undefined, CaCert}
       end;
-    verify_none -> verify_none
+    verify_none -> {verify_none, undefined, undefined}
   end,
 
-  [ {verify, Verify},
-    {cacertfile, maps:get(cacertfile, Map, "")},
-    {depth, maps:get(depth, Map, 10)},
-    {crl_check, maps:get(crl_check, Map, false)},
-    {fail_if_no_peer_cert, maps:get(fail_if_no_peer_cert, Map, false)}
-  ] ++
-  case maps:get(hostname_verification, Map, none) of
+  [ {verify, Verify} ]
+    ++
+    case Verify of
+      verify_none -> [];
+      _ ->
+        [
+          {depth, maps:get(depth, Map, 10)},
+          {crl_check, maps:get(crl_check, Map, false)},
+          {fail_if_no_peer_cert, maps:get(fail_if_no_peer_cert, Map, false)}
+        ]
+    end
+    ++
+    case Verify of
+      verify_none -> [];
+      _ ->
+        case {CaCerts, CaCertFile} of
+          {_, undefined} -> [{cacerts, CaCerts}];
+          {undefined, _} -> [{cacertfile, CaCertFile}]
+        end
+    end
+    ++
+    case maps:get(hostname_verification, Map, none) of
       wildcard ->
           [{customize_hostname_check, [{match_fun, public_key:pkix_verify_hostname_match_fun(https)}]}];
       none ->
           []
-  end.
+    end.
 
 lookup_oauth_provider_config(OAuth2ProviderId) ->
   case application:get_env(rabbitmq_auth_backend_oauth2, oauth_providers) of

deps/oauth2_client/test/system_SUITE.erl

@@ -177,7 +177,7 @@ groups() ->
     ssl_connection_error,
     {group, with_all_oauth_provider_settings},
     {group, without_all_oauth_providers_settings}
-  ]}
+  ]} 
 ].
 
 init_per_suite(Config) ->

deps/oauth2_client/test/unit_SUITE.erl

@@ -0,0 +1,68 @@
+%% This Source Code Form is subject to the terms of the Mozilla Public
+%% License, v. 2.0. If a copy of the MPL was not distributed with this
+%% file, You can obtain one at https://mozilla.org/MPL/2.0/.
+%%
+%% Copyright (c) 2007-2024 Broadcom. All Rights Reserved. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved.
+%%
+
+-module(unit_SUITE).
+
+-include_lib("common_test/include/ct.hrl").
+-include_lib("eunit/include/eunit.hrl").
+
+-include_lib("oauth2_client.hrl").
+-include_lib("public_key/include/public_key.hrl").
+
+-compile(export_all).
+
+
+all() ->
+[
+   {group, ssl_options}
+].
+
+groups() ->
+[
+  {ssl_options, [], [
+    no_ssl_options_triggers_verify_peer,
+    peer_verification_verify_none,
+    peer_verification_verify_peer_with_cacertfile
+  ]}
+].
+
+no_ssl_options_triggers_verify_peer(_) ->
+  ?assertMatch([
+    {verify, verify_peer},
+    {depth, 10},
+    {crl_check,false},
+    {fail_if_no_peer_cert,false},
+    {cacerts, _CaCerts}
+  ], oauth2_client:extract_ssl_options_as_list(#{})).
+
+peer_verification_verify_none(_) ->
+  Expected1 = [
+    {verify, verify_none}
+  ],
+  ?assertEqual(Expected1, oauth2_client:extract_ssl_options_as_list(#{peer_verification => verify_none})),
+
+  Expected2 = [
+    {verify, verify_none}
+  ],
+  ?assertEqual(Expected2, oauth2_client:extract_ssl_options_as_list(#{
+    peer_verification => verify_none,
+    cacertfile => "/tmp"
+  })).
+
+
+peer_verification_verify_peer_with_cacertfile(_) ->
+  Expected = [
+    {verify, verify_peer},
+    {depth, 10},
+    {crl_check,false},
+    {fail_if_no_peer_cert,false},
+    {cacertfile, "/tmp"}
+  ],
+  ?assertEqual(Expected, oauth2_client:extract_ssl_options_as_list(#{
+    cacertfile => "/tmp",
+    peer_verification => verify_peer
+  })).

deps/rabbitmq_auth_backend_oauth2/src/uaa_jwks.erl

@@ -2,8 +2,10 @@
 -export([get/2, ssl_options/1]).
 
 -spec get(string() | binary(), term()) -> {ok, term()} | {error, term()}.
-get(JwksUrl, KeyConfig) ->
-    httpc:request(get, {JwksUrl, []}, [{ssl, ssl_options(KeyConfig)}, {timeout, 60000}], []).
+get(JwksUrl, SslOptions) ->
+    Options = [{timeout, 60000}] ++ [{ssl, SslOptions}],
+    rabbit_log:debug("get signing keys using options ~p", Options),
+    httpc:request(get, {JwksUrl, []}, Options, []).
 
 -spec ssl_options(term()) -> list().
 ssl_options(KeyConfig) ->

deps/rabbitmq_auth_backend_oauth2/test/unit_SUITE.erl

@@ -37,8 +37,6 @@ all() ->
         test_post_process_payload_rich_auth_request_using_regular_expression_with_cluster,
         test_unsuccessful_access_with_a_token_that_uses_missing_scope_alias_in_scope_field,
         test_unsuccessful_access_with_a_token_that_uses_missing_scope_alias_in_extra_scope_source_field,
-        test_default_ssl_options,
-        test_default_ssl_options_with_cacertfile,
         test_username_from,
         {group, with_rabbitmq_node}
     ].
@@ -125,10 +123,6 @@ init_per_testcase(test_post_process_payload_rich_auth_request_using_regular_expr
   application:set_env(rabbitmq_auth_backend_oauth2, resource_server_id, <<"rabbitmq-test">>),
   Config;
 
-init_per_testcase(test_default_ssl_options_with_cacertfile, Config) ->
-  application:set_env(rabbitmq_auth_backend_oauth2, key_config, [{ cacertfile, filename:join(["testca", "cacert.pem"]) }] ),
-  Config;
-
 init_per_testcase(_, Config) ->
   Config.
 
@@ -137,10 +131,6 @@ end_per_testcase(test_post_process_token_payload_complex_claims, Config) ->
   application:set_env(rabbitmq_auth_backend_oauth2, resource_server_id, undefined),
   Config;
 
-end_per_testcase(test_default_ssl_options_with_cacertfile, Config) ->
-  application:set_env(rabbitmq_auth_backend_oauth2, key_config, undefined),
-  Config;
-
 end_per_testcase(_, Config) ->
   Config.
 
@@ -1331,25 +1321,6 @@ test_validate_payload_when_verify_aud_false(_) ->
                         <<"scope">> => [<<"bar">>, <<"other.third">>]}},
                  rabbit_auth_backend_oauth2:validate_payload(?RESOURCE_SERVER_ID, WithAudWithUnknownResourceId, ?DEFAULT_SCOPE_PREFIX)).
 
-test_default_ssl_options(_) ->
-  ?assertEqual([
-              {verify, verify_none},
-              {depth, 10},
-              {fail_if_no_peer_cert, false},
-              {crl_check, false},
-              {crl_cache, {ssl_crl_cache, {internal, [{http, 10000}]}}}
-              ], uaa_jwks:ssl_options(rabbit_oauth2_config:get_key_config())).
-
-test_default_ssl_options_with_cacertfile(_) ->
-  ?assertEqual([
-              {verify, verify_none},
-              {depth, 10},
-              {fail_if_no_peer_cert, false},
-              {crl_check, false},
-              {crl_cache, {ssl_crl_cache, {internal, [{http, 10000}]}}},
-              {cacertfile, filename:join(["testca", "cacert.pem"])}
-              ], uaa_jwks:ssl_options(rabbit_oauth2_config:get_key_config())).
-
 %%
 %% Helpers
 %%

deps/rabbitmq_management/selenium/full-suite-management-ui

@@ -10,6 +10,7 @@ authnz-mgt/oauth-idp-initiated-with-uaa-and-prefix.sh
 authnz-mgt/oauth-idp-initiated-with-uaa-via-proxy.sh
 authnz-mgt/oauth-idp-initiated-with-uaa.sh
 authnz-mgt/oauth-with-keycloak.sh
+authnz-mgt/oauth-with-keycloak-with-verify-none.sh
 authnz-mgt/oauth-with-uaa-and-mgt-prefix.sh
 authnz-mgt/oauth-with-uaa-down-but-with-basic-auth.sh
 authnz-mgt/oauth-with-uaa-down.sh

deps/rabbitmq_management/selenium/suites/authnz-mgt/oauth-with-keycloak-with-verify-none.sh

@@ -0,0 +1,10 @@
+#!/usr/bin/env bash
+
+SCRIPT="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
+TEST_CASES_PATH=/oauth/with-sp-initiated
+TEST_CONFIG_PATH=/oauth
+PROFILES="keycloak keycloak-verify-none-oauth-provider keycloak-mgt-oauth-provider tls"
+
+source $SCRIPT/../../bin/suite_template $@
+runWith keycloak

deps/rabbitmq_management/selenium/test/oauth/rabbitmq.keycloak-verify-none-oauth-provider.conf

@@ -0,0 +1,2 @@
+auth_oauth2.issuer = ${OAUTH_PROVIDER_URL}
+auth_oauth2.https.peer_verification = verify_none