Merge pull request #13408 from rabbitmq/mqtt-optional-password-cred

Do not propagate `none` password to http backend

Author: michaelklishin

deps/rabbitmq_auth_backend_http/src/rabbit_auth_backend_http.erl

@@ -76,8 +76,12 @@ is_internal_property(rabbit_auth_backend_http) -> true;
 is_internal_property(rabbit_auth_backend_cache) -> true;
 is_internal_property(_Other) -> false.
 
+is_internal_none_password(password, none) -> true;
+is_internal_none_password(_, _) -> false.
+
 extract_other_credentials(AuthProps) ->
-  PublicAuthProps = [{K,V} || {K,V} <-AuthProps, not is_internal_property(K)],
+  PublicAuthProps = [{K,V} || {K,V} <-AuthProps, not is_internal_property(K) and 
+                                                  not is_internal_none_password(K, V)],
   case PublicAuthProps of
     [] -> resolve_using_persisted_credentials(AuthProps);
     _ -> PublicAuthProps

deps/rabbitmq_auth_backend_http/test/auth_SUITE.erl

@@ -18,6 +18,9 @@
                         password => <<"Kocur">>,
                         expected_credentials => [username, password],
                         tags => [policymaker, monitoring]}).
+-define(ALLOWED_USER_2, #{username => <<"Ala3">>,                        
+                        expected_credentials => [username],
+                        tags => [policymaker, monitoring]}).
 -define(ALLOWED_USER_WITH_EXTRA_CREDENTIALS, #{username => <<"Ala2">>,
                                                password => <<"Kocur">>,
                                                client_id => <<"some_id">>,
@@ -46,12 +49,14 @@ shared() ->
         grants_access_to_user_passing_additional_required_authprops,
         grants_access_to_user_skipping_internal_authprops,
         grants_access_to_user_with_credentials_in_rabbit_auth_backend_http,
-        grants_access_to_user_with_credentials_in_rabbit_auth_backend_cache
+        grants_access_to_user_with_credentials_in_rabbit_auth_backend_cache,
+        grants_access_to_ssl_user_without_a_password
     ].
 
 init_per_suite(Config) ->
     rabbit_ct_helpers:run_setup_steps(Config) ++
         [{allowed_user, ?ALLOWED_USER},
+        {allowed_user_2, ?ALLOWED_USER_2},
         {allowed_user_with_extra_credentials, ?ALLOWED_USER_WITH_EXTRA_CREDENTIALS},
         {denied_user, ?DENIED_USER}].
 
@@ -65,13 +70,21 @@ init_per_group(over_http, Config) ->
 init_per_group(over_https, Config) ->
     configure_http_auth_backend("https", Config),
     {User1, Tuple1} = extractUserTuple(?ALLOWED_USER),
-    {User2, Tuple2} = extractUserTuple(?ALLOWED_USER_WITH_EXTRA_CREDENTIALS),
+    {User2, Tuple2} = extractUserTuple(?ALLOWED_USER_2),    
+    {User3, Tuple3} = extractUserTuple(?ALLOWED_USER_WITH_EXTRA_CREDENTIALS),
     CertsDir = ?config(rmq_certsdir, Config),
-    start_https_auth_server(?AUTH_PORT, CertsDir, ?USER_PATH, #{User1 => Tuple1, User2 => Tuple2}),
-    Config.
+    start_https_auth_server(?AUTH_PORT, CertsDir, ?USER_PATH, #{
+        User1 => Tuple1, 
+        User3 => Tuple3, 
+        User2 => Tuple2}),
+    Config ++ [{group, over_https}].
 
 extractUserTuple(User) ->
-    #{username := Username, password := Password, tags := Tags, expected_credentials := ExpectedCredentials} = User,
+    #{username := Username,  tags := Tags, expected_credentials := ExpectedCredentials} = User,
+    Password = case maps:get(password, User, undefined) of
+        undefined -> none;
+        P -> P
+    end,
     {Username, {Password, Tags, ExpectedCredentials}}.
 
 end_per_suite(Config) ->
@@ -91,6 +104,16 @@ grants_access_to_user(Config) ->
     ?assertMatch({U, T, AuthProps},
                  {User#auth_user.username, User#auth_user.tags, (User#auth_user.impl)()}).
 
+grants_access_to_ssl_user_without_a_password(Config) ->
+    case ?config(group, Config) of 
+        over_https -> 
+            #{username := U, tags := T} = ?config(allowed_user_2, Config),
+            {ok, User} = rabbit_auth_backend_http:user_login_authentication(U, []),
+            ?assertMatch({U, T, []},
+                        {User#auth_user.username, User#auth_user.tags, (User#auth_user.impl)()});
+        _ ->{skip, "Requires https"}
+    end.
+ 
 denies_access_to_user(Config) ->
     #{username := U, password := P} = ?config(denied_user, Config),
     ?assertMatch({refused, "Denied by the backing HTTP service", []},

deps/rabbitmq_auth_backend_http/test/auth_http_mock.erl

@@ -14,8 +14,9 @@ init(Req = #{method := <<"GET">>}, Users) ->
 %%% HELPERS
 
 authenticate(QsVals, Users) ->
+   ct:log("QsVals: ~p Users: ~p", [QsVals, Users]),
    Username = proplists:get_value(<<"username">>, QsVals),
-   Password = proplists:get_value(<<"password">>, QsVals),
+   Password = proplists:get_value(<<"password">>, QsVals, none),
    case maps:get(Username, Users, undefined) of
        {MatchingPassword, Tags, ExpectedCredentials} when Password =:= MatchingPassword ->
             case lists:all(fun(C) -> proplists:is_defined(list_to_binary(rabbit_data_coercion:to_list(C)),QsVals) end, ExpectedCredentials) of

deps/rabbitmq_mqtt/test/auth_SUITE.erl

@@ -72,7 +72,7 @@ sub_groups() ->
        [invalid_client_id_from_cert_san_dns
        ]},
      {ssl_user_with_client_id_in_cert_san_dns, [],
-       [client_id_from_cert_san_dns        
+       [client_id_from_cert_san_dns
        ]},
      {ssl_user_with_client_id_in_cert_san_dns_1, [],
        [client_id_from_cert_san_dns_1

selenium/full-suite-authnz-messaging

@@ -1,10 +1,9 @@
 authnz-messaging/auth-cache-http-backends.sh
 authnz-messaging/auth-cache-ldap-backends.sh
-authnz-messaging/auth-http-backend.sh
+authnz-messaging/auth-http-backend-with-mtls.sh
 authnz-messaging/auth-http-internal-backends-with-internal.sh
 authnz-messaging/auth-http-internal-backends.sh
 authnz-messaging/auth-internal-backend.sh
 authnz-messaging/auth-internal-mtls-backend.sh
 authnz-messaging/auth-internal-http-backends.sh
 authnz-messaging/auth-ldap-backend.sh
-authnz-messaging/auth-http-backend.sh

selenium/suites/authnz-messaging/auth-http-backend-with-mtls.sh

@@ -0,0 +1,11 @@
+#!/usr/bin/env bash
+
+SCRIPT="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
+TEST_CASES_PATH=/authnz-msg-protocols
+PROFILES="internal-user auth-http auth_backends-http auth-mtls"
+# internal-user profile is used because the client certificates to 
+# access rabbitmq are issued with the alt_name = internal-user 
+
+source $SCRIPT/../../bin/suite_template
+runWith mock-auth-backend-http

selenium/suites/authnz-messaging/auth-http-backend.sh

@@ -28,6 +28,7 @@ function getAmqpsConnectionOptions() {
 }
 function getConnectionOptions() {
   let scheme = process.env.RABBITMQ_AMQP_SCHEME || 'amqp'
+  console.log("Using AMQP protocol: " + scheme)
   switch(scheme){
     case "amqp":
       return getAmqpConnectionOptions()

selenium/test/amqp.js

@@ -29,12 +29,17 @@ describe('Having AMQP 1.0 protocol enabled and the following auth_backends: ' +
   let expectations = []
   let username = process.env.RABBITMQ_AMQP_USERNAME
   let password = process.env.RABBITMQ_AMQP_PASSWORD
+  let usemtls = process.env.AMQP_USE_MTLS
   let amqp;
 
-  before(function () {
-    if (backends.includes("http") && username.includes("http")) {
+  before(function () {    
+    if (backends.includes("http") && (username.includes("http") || usemtls)) {
       reset()
-      expectations.push(expectUser({ "username": username, "password": password}, "allow"))
+      if (!usemtls) {
+        expectations.push(expectUser({ "username": username, "password": password}, "allow"))
+      } else {
+        expectations.push(expectUser({ "username": username}, "allow"))
+      }
       expectations.push(expectVhost({ "username": username, "vhost": "/"}, "allow"))
       expectations.push(expectResource({ "username": username, "vhost": "/", "resource": "queue", "name": "my-queue", "permission":"configure", "tags":""}, "allow"))
       expectations.push(expectResource({ "username": username, "vhost": "/", "resource": "queue", "name": "my-queue", "permission":"read", "tags":""}, "allow"))
@@ -56,7 +61,7 @@ describe('Having AMQP 1.0 protocol enabled and the following auth_backends: ' +
     await untilConnectionEstablished
     var untilMessageReceived = new Promise((resolve, reject) => {
       onAmqp('message', function(context) {
-        resolve()
+        if (receivedAmqpMessageCount == 2) resolve()
       })
     })
     amqp.sender.send({body:'second message'})    

selenium/test/authnz-msg-protocols/amqp10.js

@@ -23,11 +23,23 @@ describe('Having MQTT protocol enbled and the following auth_backends: ' + backe
   let password = process.env.RABBITMQ_AMQP_PASSWORD
   let client_id = process.env.RABBITMQ_AMQP_USERNAME || 'selenium-client'
 
-  before(function () {
-    if (backends.includes("http") && username.includes("http")) {
+  before(function () {    
+    if (backends.includes("http") && (username.includes("http") || usemtls)) {
       reset()
-      expectations.push(expectUser({ "username": username, "password": password, "client_id": client_id, "vhost": "/" }, "allow"))
+      if (!usemtls) {
+        expectations.push(expectUser({ 
+          "username": username, 
+          "password": password, 
+          "client_id": client_id, 
+          "vhost": "/" }, "allow"))
+      } else {
+        expectations.push(expectUser({ 
+          "username": username, 
+          "client_id": client_id, 
+          "vhost": "/" }, "allow"))
+      }
       expectations.push(expectVhost({ "username": username, "vhost": "/"}, "allow"))
+
     } else if (backends.includes("oauth") && username.includes("oauth")) {
       let oauthProviderUrl = process.env.OAUTH_PROVIDER_URL
       let oauthClientId = process.env.OAUTH_CLIENT_ID
@@ -58,15 +70,20 @@ describe('Having MQTT protocol enbled and the following auth_backends: ' + backe
     }
   })
 
-  it('can open an MQTT connection', function () {
+  it('can open an MQTT connection', async function () {
     var client = mqtt.connect(mqttUrl, mqttOptions)
-    client.on('error', function(err) {
-      assert.fail("Mqtt connection failed due to " + err)
-      client.end()
-    })
-    client.on('connect', function(err) {
-      client.end()
+    let done = new Promise((resolve, reject) => {
+      client.on('error', function(err) {
+        reject(err)
+        client.end()
+        assert.fail("Mqtt connection failed due to " + err)        
+      }),
+      client.on('connect', function(err) {
+        resolve("ok")
+        client.end()
+      })
     })
+    assert.equal("ok", await done)
   })
 
   after(function () {