Return 422 when auth fails, begin adding more tests

Author: lukebakken

deps/rabbitmq_auth_backend_ldap/src/rabbit_auth_backend_ldap_mgmt.erl

@@ -35,7 +35,7 @@ content_types_accepted(ReqData, Context) ->
    {[{'*', accept_content}], ReqData, Context}.
 
 allowed_methods(ReqData, Context) ->
-    {[<<"HEAD">>, <<"PUT">>, <<"OPTIONS">>], ReqData, Context}.
+    {[<<"PUT">>, <<"OPTIONS">>], ReqData, Context}.
 
 resource_exists(ReqData, Context) ->
     {true, ReqData, Context}.
@@ -65,16 +65,16 @@ accept_content(ReqData0, Context) ->
                                 ok ->
                                     {true, ReqData1, Context};
                                 {error, invalidCredentials} ->
-                                    rabbit_mgmt_util:not_authorised("invalid credentials", ReqData1, Context);
+                                    rabbit_mgmt_util:unprocessable_entity("Invalid LDAP credentials", ReqData1, Context);
                                 {error, unwillingToPerform} ->
-                                    rabbit_mgmt_util:not_authorised("invalid credentials", ReqData1, Context);
+                                    rabbit_mgmt_util:unprocessable_entity("Invalid LDAP credentials", ReqData1, Context);
                                 {error, E} ->
                                     Reason = unicode_format(E),
-                                    rabbit_mgmt_util:bad_request(Reason, ReqData1, Context)
+                                    rabbit_mgmt_util:unprocessable_entity(Reason, ReqData1, Context)
                             end;
                         Error ->
                             Reason = unicode_format(Error),
-                            rabbit_mgmt_util:bad_request(Reason, ReqData1, Context)
+                            rabbit_mgmt_util:unprocessable_entity(Reason, ReqData1, Context)
                     end,
                     eldap:close(LDAP),
                     Result;

deps/rabbitmq_auth_backend_ldap/test/system_SUITE.erl

@@ -316,6 +316,42 @@ validate_ldap_configuration_via_api(Config) ->
             'servers' => ["localhost"],
             'port' => LdapPort
         }, ?METHOD_NOT_ALLOWED),
+    %% Invalid JSON should return 400 Bad Request
+    rabbit_mgmt_test_util:http_put_raw(Config, "/ldap/validate/simple-bind",
+        "{invalid json", ?BAD_REQUEST),
+
+    %% HTTP Method coverage tests
+    %% GET method - should return 405 Method Not Allowed
+    ?assertMatch({ok, {{_, ?METHOD_NOT_ALLOWED, _}, _Headers, _ResBody}},
+        rabbit_mgmt_test_util:req(Config, 0, get, "/ldap/validate/simple-bind",
+                                  [rabbit_mgmt_test_util:auth_header("guest", "guest")])),
+
+    %% HEAD method - should return 405 Method Not Allowed (same as GET)
+    ?assertMatch({ok, {{_, ?METHOD_NOT_ALLOWED, _}, _Headers, _ResBody}},
+        rabbit_mgmt_test_util:req(Config, 0, head, "/ldap/validate/simple-bind",
+                                  [rabbit_mgmt_test_util:auth_header("guest", "guest")])),
+
+    %% POST method - should return 405 Method Not Allowed
+    ?assertMatch({ok, {{_, ?METHOD_NOT_ALLOWED, _}, _Headers, _ResBody}},
+        rabbit_mgmt_test_util:req(Config, 0, post, "/ldap/validate/simple-bind",
+                                  [rabbit_mgmt_test_util:auth_header("guest", "guest")], "{}")),
+
+    %% DELETE method - should return 405 Method Not Allowed
+    ?assertMatch({ok, {{_, ?METHOD_NOT_ALLOWED, _}, _Headers, _ResBody}},
+        rabbit_mgmt_test_util:req(Config, 0, delete, "/ldap/validate/simple-bind",
+                                  [rabbit_mgmt_test_util:auth_header("guest", "guest")])),
+
+    %% OPTIONS method - should return 200 with Allow header showing only PUT, OPTIONS
+    {ok, {{_, OptionsCode, _}, OptionsHeaders, _OptionsResBody}} =
+        rabbit_mgmt_test_util:req(Config, 0, options, "/ldap/validate/simple-bind",
+                                  [rabbit_mgmt_test_util:auth_header("guest", "guest")]),
+    ?assertEqual(?OK, OptionsCode),
+    AllowHeader = proplists:get_value("allow", OptionsHeaders),
+    ?assert(string:str(string:to_upper(AllowHeader), "PUT") > 0),
+    ?assert(string:str(string:to_upper(AllowHeader), "OPTIONS") > 0),
+    %% Should NOT contain GET or HEAD
+    ?assertEqual(0, string:str(string:to_upper(AllowHeader), "GET")),
+    ?assertEqual(0, string:str(string:to_upper(AllowHeader), "HEAD")),
     http_put(Config, "/ldap/validate/simple-bind",
         #{
             'user_dn' => AliceUserDN,
@@ -329,7 +365,7 @@ validate_ldap_configuration_via_api(Config) ->
             'password' => Password,
             'servers' => ["localhost"],
             'port' => LdapPort
-        }, ?NOT_AUTHORISED),
+        }, ?UNPROCESSABLE_ENTITY),
     http_put(Config, "/ldap/validate/simple-bind",
         #{
             'user_dn' => AliceUserDN,

deps/rabbitmq_ct_helpers/include/rabbit_mgmt_test.hrl

@@ -8,6 +8,7 @@
 -define(BAD_REQUEST, 400).
 -define(NOT_AUTHORISED, 401).
 -define(METHOD_NOT_ALLOWED, 405).
+-define(UNPROCESSABLE_ENTITY, 422).
 %%-define(NOT_FOUND, 404). Defined for AMQP by amqp_client.hrl (as 404)
 %% httpc seems to get racy when using HTTP 1.1
 -define(HTTPC_OPTS, [{version, "HTTP/1.0"}, {autoredirect, false}]).

deps/rabbitmq_management/src/rabbit_mgmt_util.erl

@@ -20,6 +20,7 @@
 -export([user/1]).
 -export([bad_request/3, service_unavailable/3, not_authorised/3, bad_request_exception/4,
          internal_server_error/3, internal_server_error/4, precondition_failed/3,
+         unprocessable_entity/3,
          id/2, parse_bool/1, parse_int/1, redirect_to_home/3]).
 -export([with_decode/4, not_found/3]).
 -export([with_channel/4, with_channel/5]).
@@ -665,6 +666,9 @@ a2b(B)                 -> B.
 bad_request(Reason, ReqData, Context) ->
     halt_response(400, bad_request, Reason, ReqData, Context).
 
+unprocessable_entity(Reason, ReqData, Context) ->
+    halt_response(422, unprocessable_entity, Reason, ReqData, Context).
+
 service_unavailable(Reason, ReqData, Context) ->
     halt_response(503, service_unavailable, Reason, ReqData, Context).