Refactor HTTP API responses

deps/oauth2_client/src/jwt_helper.erl

@@ -18,5 +18,6 @@ decode(Token) ->
         {error, {invalid_token, Type, Err, Stacktrace}}
     end.
 
-get_expiration_time(#{<<"exp">> := Exp}) when is_integer(Exp) -> {ok, Exp};
+get_expiration_time(#{<<"exp">> := Exp}) when is_number(Exp) -> {ok, trunc(Exp)};
+get_expiration_time(#{<<"exp">> := _}) -> {error, invalid_exp_field};
 get_expiration_time(#{}) -> {error, missing_exp_field}.

deps/rabbit/src/rabbit_auth_backend_internal.erl

@@ -137,9 +137,26 @@ internal_check_user_login(Username, Fun) ->
                 _    -> Refused
             end;
         {error, not_found} ->
+            %% Equalise timing regardless of whether the username exists to
+            %% prevent username enumeration via a timing side-channel.
+            %% The result is always discarded.
+            _ = Fun(dummy_sentinel_user()),
             Refused
     end.
 
+%% A sentinel internal_user whose password hash will never match any real
+%% cleartext. Used solely to perform a dummy hash computation on the
+%% not-found path, equalising timing with the found-but-wrong-password path.
+%% Layout: 4 bytes zeroed salt + 32 bytes zeroed SHA-256 payload.
+-define(DUMMY_PASSWORD_HASH, <<0:32, 0:256>>).
+
+dummy_sentinel_user() ->
+    HashingMod = rabbit_password:hashing_mod(),
+    internal_user:set_password_hash(
+        internal_user:new({hashing_algorithm, HashingMod}),
+        ?DUMMY_PASSWORD_HASH,
+        HashingMod).
+
 check_vhost_access(#auth_user{username = Username}, VHostPath, _AuthzData) ->
     rabbit_db_user:get_user_permissions(Username, VHostPath) =/= undefined.
 

deps/rabbit/test/unit_access_control_SUITE.erl

@@ -32,6 +32,7 @@ groups() ->
       {sequential_tests, [], [
           login_with_credentials_but_no_password,
           login_of_passwordless_user,
+          login_of_nonexistent_user,
           set_tags_for_passwordless_user,
           change_password,
           auth_backend_internal_expand_topic_permission
@@ -188,6 +189,26 @@ login_of_passwordless_user1(_Config) ->
 
     passed.
 
+login_of_nonexistent_user(Config) ->
+    passed = rabbit_ct_broker_helpers:rpc(Config, 0,
+      ?MODULE, login_of_nonexistent_user1, [Config]).
+
+login_of_nonexistent_user1(_Config) ->
+    Username = <<"login_of_nonexistent_user-user">>,
+    Password = <<"login_of_nonexistent_user-password">>,
+    %% Ensure the user does not exist before the test.
+    case rabbit_auth_backend_internal:lookup_user(Username) of
+        {ok, _} -> rabbit_auth_backend_internal:delete_user(Username, <<"acting-user">>);
+        _       -> ok
+    end,
+    %% Authentication of a non-existent user must return {refused, ...} and
+    %% must not crash or return an error tuple.
+    ?assertMatch(
+       {refused, _Message, [Username]},
+       rabbit_auth_backend_internal:user_login_authentication(
+           Username, [{password, Password}])),
+    passed.
+
 
 set_tags_for_passwordless_user(Config) ->
     passed = rabbit_ct_broker_helpers:rpc(Config, 0,

deps/rabbitmq_auth_backend_oauth2/src/rabbit_auth_backend_oauth2.erl

@@ -160,7 +160,6 @@ update_state(AuthUser, NewToken) ->
 -spec expiry_timestamp(rabbit_types:auth_user()) -> integer() | never.
 expiry_timestamp(#auth_user{impl = DecodedTokenFun}) ->
     case DecodedTokenFun() of
-        %% "exp" may be a fractional NumericDate (RFC 7519); accept floats too.
         #{<<"exp">> := Exp} when is_number(Exp) ->
             trunc(Exp);
         _ ->
@@ -236,14 +235,15 @@ ensure_same_username(PreferredUsernameClaims, CurrentDecodedToken, NewDecodedTok
         _ -> {error, mismatch_username_after_token_refresh}
     end.
 
-%% "exp" may be a fractional NumericDate (RFC 7519); accept floats too.
 validate_token_expiry(#{<<"exp">> := Exp}) when is_number(Exp) ->
-    ExpSeconds = trunc(Exp),
     Now = os:system_time(seconds),
-    case ExpSeconds =< Now of
-        true  -> {error, rabbit_misc:format("Provided JWT token has expired at timestamp ~tp (validated at ~tp)", [ExpSeconds, Now])};
+    ExpTs = trunc(Exp),
+    case ExpTs =< Now of
+        true  -> {error, rabbit_misc:format("Provided JWT token has expired at timestamp ~tp (validated at ~tp)", [ExpTs, Now])};
         false -> ok
     end;
+validate_token_expiry(#{<<"exp">> := _}) ->
+    {error, "Provided JWT token has an invalid exp claim: value is not a number"};
 validate_token_expiry(#{}) -> ok.
 
 -spec check_token(raw_jwt_token(), {resource_server(), internal_oauth_provider()}) ->

deps/rabbitmq_auth_backend_oauth2/test/unit_SUITE.erl

@@ -40,7 +40,8 @@ all() ->
         test_restricted_vhost_access_with_a_valid_token,
         test_insufficient_permissions_in_a_valid_token,
         test_token_expiration,
-        test_token_expiration_with_float_exp,
+        test_token_expiry_with_float_exp,
+        test_token_expiry_with_non_numeric_exp,
         test_invalid_signature,
         test_incorrect_kid,
         normalize_token_scope_using_multiple_scopes_key,
@@ -1242,6 +1243,40 @@ test_token_expiration(_) ->
     ?assertMatch({refused, _, _},
                  user_login_authentication(Username, [{password, Token}])).
 
+test_token_expiry_with_float_exp(_) ->
+    Username = <<"username">>,
+    set_env(resource_server_id, <<"rabbitmq">>),
+
+    %% A valid, future float exp must be accepted and expiry_timestamp must
+    %% return the truncated integer — not the atom 'never'.
+    FutureFloatExp = float(os:system_time(seconds) + 600),
+    FutureToken = (?UTIL_MOD:token_with_sub(?UTIL_MOD:expirable_token(), Username))
+                    #{<<"exp">> := FutureFloatExp},
+    {ok, #auth_user{username = Username} = User} =
+        user_login_authentication(Username, [{password, FutureToken}]),
+    ?assertEqual(trunc(FutureFloatExp), rabbit_auth_backend_oauth2:expiry_timestamp(User)),
+
+    %% An already-expired float exp must be refused, not silently accepted.
+    PastFloatExp = float(os:system_time(seconds) - 10),
+    ExpiredToken = (?UTIL_MOD:token_with_sub(?UTIL_MOD:expirable_token(), Username))
+                    #{<<"exp">> := PastFloatExp},
+    ?assertMatch({refused, _, _},
+                 user_login_authentication(Username, [{password, ExpiredToken}])).
+
+test_token_expiry_with_non_numeric_exp(_) ->
+    Username = <<"username">>,
+    set_env(resource_server_id, <<"rabbitmq">>),
+
+    %% A token whose exp claim is not a number (e.g. a string) must be refused,
+    %% not silently accepted because the is_number guard falls through to the
+    %% no-exp-field catch-all clause.
+    lists:foreach(fun(NonNumericExp) ->
+        InvalidToken = (?UTIL_MOD:token_with_sub(?UTIL_MOD:expirable_token(), Username))
+                        #{<<"exp">> := NonNumericExp},
+        ?assertMatch({refused, _, _},
+                     user_login_authentication(Username, [{password, InvalidToken}]))
+    end, [<<"1700000300">>, true, false, null]).
+
 test_incorrect_kid(_) ->
     AltKid   = <<"other-token-key">>,
     Username = <<"username">>,

deps/rabbitmq_management/priv/www/js/dispatcher.js

@@ -308,11 +308,13 @@ dispatcher_add(function(sammy) {
         });
       let datamodel = {
         'limits': '/vhost-limits',
-        'user_limits': '/user-limits',
         'vhosts': '/vhosts'
       }
       if (ac.isAdministratorUser()) {
+        datamodel['user_limits'] = '/user-limits'
         datamodel['users'] = '/users'
+      } else {
+        datamodel['user_limits'] = '/user-limits/' + esc(user_name)
       }
       path('#/limits', datamodel, 'limits');
 

deps/rabbitmq_management/src/rabbit_mgmt_login.erl

@@ -123,7 +123,7 @@ handleAccessToken(Req0, AccessToken, State) ->
 redirect_to_home_with_cookie(CookieName, CookieValue, Req=#{scheme := Scheme}, State) ->
     CookieSettings0 = #{
         http_only => true,
-        path => rabbit_mgmt_util:get_oauth2_bootstrap_cookie_path(),
+        path => rabbit_mgmt_util:prefixed_path(?OAUTH2_BOOTSTRAP_PATH),
         max_age => 30,
         same_site => strict
     },

deps/rabbitmq_management/src/rabbit_mgmt_oauth_bootstrap.erl

@@ -76,7 +76,8 @@ get_auth_mechanism(Req) ->
                                     <<"">>, Req, #{
                                         max_age => 0,
                                         http_only => true,
-                                        path => rabbit_mgmt_util:get_oauth2_bootstrap_cookie_path(),
+                                        path => iolist_to_binary([rabbit_mgmt_util:get_path_prefix(),
+                                                                  ?OAUTH2_BOOTSTRAP_PATH]),
                                         same_site => strict
                                 }),
                                 Auth
@@ -128,7 +129,7 @@ set_token_auth(AuthSettings, Req0) ->
                                     ?OAUTH2_ACCESS_TOKEN, <<"">>, Req0, #{
                                         max_age => 0,
                                         http_only => true,
-                                        path => rabbit_mgmt_util:get_oauth2_bootstrap_cookie_path(),
+                                        path => rabbit_mgmt_util:prefixed_path(?OAUTH2_BOOTSTRAP_PATH),
                                         same_site => strict
                                     }),
                                 ["set_token_auth(", rabbit_json:encode(Token), ");"]

deps/rabbitmq_management/src/rabbit_mgmt_util.erl

@@ -46,7 +46,7 @@
 -export([direct_request/6]).
 -export([qs_val/2]).
 -export([get_path_prefix/0]).
--export([get_oauth2_bootstrap_cookie_path/0]).
+-export([prefixed_path/1]).
 -export([catch_no_such_user_or_vhost/2]).
 -export([method_not_allowed/3]).
 
@@ -467,10 +467,11 @@ fixup_prefix(EnvPrefix) when is_list(EnvPrefix) ->
 fixup_prefix(EnvPrefix) when is_binary(EnvPrefix) ->
     fixup_prefix(rabbit_data_coercion:to_list(EnvPrefix)).
 
-%% Path for the short-lived /login cookies. It must carry the HTTP API path
-%% prefix, or the browser will not send them to the prefixed bootstrap.js.
-get_oauth2_bootstrap_cookie_path() ->
-    iolist_to_binary([get_path_prefix(), ?OAUTH2_BOOTSTRAP_PATH]).
+%% Prepends the configured path prefix to Path, returning a binary.
+%% Used wherever a cookie or redirect path must reflect the management
+%% plugin's configured path_prefix.
+prefixed_path(Path) ->
+    iolist_to_binary([get_path_prefix(), Path]).
 
 %% XXX sort_list_and_paginate/2 is a more proper name for this function, keeping it
 %% with this name for backwards compatibility

deps/rabbitmq_management/src/rabbit_mgmt_wm_user_limits.erl

@@ -24,7 +24,14 @@ allowed_methods(ReqData, Context) ->
     {[<<"GET">>, <<"OPTIONS">>], ReqData, Context}.
 
 is_authorized(ReqData, Context) ->
-    rabbit_mgmt_util:is_authorized_vhost_visible(ReqData, Context).
+    case user(ReqData) of
+        none ->
+            %% listing all users' limits is an administrator-only operation
+            rabbit_mgmt_util:is_authorized_admin(ReqData, Context);
+        Username ->
+            %% reading a specific user's limits: administrator/monitor or own account
+            rabbit_mgmt_util:is_authorized_user(ReqData, Context, [{user, Username}])
+    end.
 
 content_types_provided(ReqData, Context) ->
    {[{<<"application/json">>, to_json}], ReqData, Context}.

deps/rabbitmq_management/test/rabbit_mgmt_http_SUITE.erl

@@ -1838,11 +1838,11 @@ permissions_vhost_test(Config) ->
                 http_get(Config, Path1 ++ "/myvhost1/" ++ Path2, "myuser", "myuser",
                          ?OK),
                 http_get(Config, Path1 ++ "/myvhost2/" ++ Path2, "myuser", "myuser",
-                         ?NOT_AUTHORISED),
+                         ?NOT_FOUND),
                 http_get(Config, Path1 ++ "/myvhost1/" ++ Path2, "mymonitor", "mymonitor",
                          ?OK),
                 http_get(Config, Path1 ++ "/myvhost2/" ++ Path2, "mymonitor", "mymonitor",
-                         ?NOT_AUTHORISED)
+                         ?NOT_FOUND)
         end,
     Test3 =
         fun(Path1) ->
@@ -3393,7 +3393,7 @@ get_fail_test(Config) ->
     http_post(Config, "/queues/%2F/myqueue/get",
               [{ackmode, ack_requeue_false},
                {count,    1},
-               {encoding, auto}], "myuser", "password", ?NOT_AUTHORISED),
+               {encoding, auto}], "myuser", "password", ?NOT_FOUND),
     http_delete(Config, "/queues/%2F/myqueue", {group, '2xx'}),
     http_delete(Config, "/users/myuser", {group, '2xx'}),
     passed.
@@ -3530,7 +3530,7 @@ publish_fail_test(Config) ->
     http_put(Config, "/users/myuser", [{password, <<"password">>},
                                        {tags, <<"management">>}], {group, '2xx'}),
     http_post(Config, "/exchanges/%2F/amq.default/publish", Msg, "myuser", "password",
-              ?NOT_AUTHORISED),
+              ?NOT_FOUND),
     Msg2 = [{exchange,         <<"">>},
             {routing_key,      <<"publish_fail_test">>},
             {properties,       [{user_id, <<"foo">>}]},
@@ -3803,13 +3803,23 @@ policy_permissions_test(Config) ->
                   http_get(Config, "/policies/v/HA",            U, U, ?NOT_AUTHORISED),
                   http_get(Config, "/parameters/test/v/good",   U, U, ?NOT_AUTHORISED)
           end,
+    %% mon and mgmt lack policymaker role entirely, so they get 401.
+    %% policy has policymaker role but no access to the default vhost, so it gets 404
+    %% (the server does not reveal whether the vhost exists to unauthorised users).
     AlwaysNeg =
         fun (U) ->
                 http_put(Config, "/policies/%2F/HA",  Policy, U, U, ?NOT_AUTHORISED),
                 http_put(Config, "/parameters/test/%2F/good", Param, U, U, ?NOT_AUTHORISED),
                 http_get(Config, "/policies/%2F/HA",          U, U, ?NOT_AUTHORISED),
                 http_get(Config, "/parameters/test/%2F/good", U, U, ?NOT_AUTHORISED)
         end,
+    AlwaysNegPolicymaker =
+        fun (U) ->
+                http_put(Config, "/policies/%2F/HA",  Policy, U, U, ?NOT_FOUND),
+                http_put(Config, "/parameters/test/%2F/good", Param, U, U, ?NOT_FOUND),
+                http_get(Config, "/policies/%2F/HA",          U, U, ?NOT_FOUND),
+                http_get(Config, "/parameters/test/%2F/good", U, U, ?NOT_FOUND)
+        end,
     AdminPos =
         fun (U) ->
                 http_put(Config, "/policies/%2F/HA",  Policy, U, U, {group, '2xx'}),
@@ -3820,7 +3830,8 @@ policy_permissions_test(Config) ->
 
     [Neg(U) || U <- ["mon", "mgmt"]],
     [Pos(U) || U <- ["admin", "policy"]],
-    [AlwaysNeg(U) || U <- ["mon", "mgmt", "policy"]],
+    [AlwaysNeg(U) || U <- ["mon", "mgmt"]],
+    AlwaysNegPolicymaker("policy"),
     [AdminPos(U) || U <- ["admin"]],
 
     %% This one is deliberately different between admin and policymaker.
@@ -3954,23 +3965,23 @@ vhost_limits_list_test(Config) ->
     rabbit_ct_broker_helpers:add_user(Config, NoVhostUser),
     rabbit_ct_broker_helpers:set_user_tags(Config, 0, NoVhostUser, [management]),
     [] = http_get(Config, "/vhost-limits", NoVhostUser, NoVhostUser, ?OK),
-    http_get(Config, "/vhost-limits/limit_test_vhost_1", NoVhostUser, NoVhostUser, ?NOT_AUTHORISED),
-    http_get(Config, "/vhost-limits/limit_test_vhost_2", NoVhostUser, NoVhostUser, ?NOT_AUTHORISED),
+    http_get(Config, "/vhost-limits/limit_test_vhost_1", NoVhostUser, NoVhostUser, ?NOT_FOUND),
+    http_get(Config, "/vhost-limits/limit_test_vhost_2", NoVhostUser, NoVhostUser, ?NOT_FOUND),
 
     Vhost1User = <<"limit_test_vhost_1_user">>,
     rabbit_ct_broker_helpers:add_user(Config, Vhost1User),
     rabbit_ct_broker_helpers:set_user_tags(Config, 0, Vhost1User, [management]),
     rabbit_ct_broker_helpers:set_full_permissions(Config, Vhost1User, <<"limit_test_vhost_1">>),
     Limits1 = http_get(Config, "/vhost-limits", Vhost1User, Vhost1User, ?OK),
     Limits1 = http_get(Config, "/vhost-limits/limit_test_vhost_1", Vhost1User, Vhost1User, ?OK),
-    http_get(Config, "/vhost-limits/limit_test_vhost_2", Vhost1User, Vhost1User, ?NOT_AUTHORISED),
+    http_get(Config, "/vhost-limits/limit_test_vhost_2", Vhost1User, Vhost1User, ?NOT_FOUND),
 
     Vhost2User = <<"limit_test_vhost_2_user">>,
     rabbit_ct_broker_helpers:add_user(Config, Vhost2User),
     rabbit_ct_broker_helpers:set_user_tags(Config, 0, Vhost2User, [management]),
     rabbit_ct_broker_helpers:set_full_permissions(Config, Vhost2User, <<"limit_test_vhost_2">>),
     Limits2 = http_get(Config, "/vhost-limits", Vhost2User, Vhost2User, ?OK),
-    http_get(Config, "/vhost-limits/limit_test_vhost_1", Vhost2User, Vhost2User, ?NOT_AUTHORISED),
+    http_get(Config, "/vhost-limits/limit_test_vhost_1", Vhost2User, Vhost2User, ?NOT_FOUND),
     Limits2 = http_get(Config, "/vhost-limits/limit_test_vhost_2", Vhost2User, Vhost2User, ?OK).
 
 vhost_limit_set_test(Config) ->
@@ -4114,7 +4125,17 @@ user_limits_list_test(Config) ->
     },
     rabbit_ct_broker_helpers:set_user_limits(Config, 0, NoVhostUser, maps:get(value, Limits4)),
 
-    ?assertEqual([Limits4], http_get(Config, "/user-limits/no_vhost_user", ?OK)).
+    ?assertEqual([Limits4], http_get(Config, "/user-limits/no_vhost_user", ?OK)),
+
+    %% A management-only user must not be able to enumerate other users' limits.
+    %% Reading own limits is allowed; reading another user's limits or the full
+    %% list must be refused.
+    http_get(Config, "/user-limits", User1, User1, ?NOT_AUTHORISED),
+    http_get(Config, "/user-limits", User2, User2, ?NOT_AUTHORISED),
+    http_get(Config, "/user-limits/" ++ binary_to_list(User1), User1, User1, ?OK),
+    http_get(Config, "/user-limits/" ++ binary_to_list(User2), User1, User1, ?NOT_AUTHORISED),
+    http_get(Config, "/user-limits/" ++ binary_to_list(User2), User2, User2, ?OK),
+    http_get(Config, "/user-limits/" ++ binary_to_list(User1), User2, User2, ?NOT_AUTHORISED).
 
 user_limit_set_test(Config) ->
     ?assertEqual([], http_get(Config, "/user-limits", ?OK)),
@@ -4174,15 +4195,8 @@ user_limit_set_test(Config) ->
     rabbit_ct_broker_helpers:set_user_tags(Config, 0, Vhost1User, [management]),
     rabbit_ct_broker_helpers:set_full_permissions(Config, Vhost1User, Vhost1),
 
-    Limits3 = [
-        #{
-            user => User1,
-            value => #{
-                'max-connections' => 1000,
-                'max-channels'    => 100
-            }
-        }],
-    ?assertEqual(Limits3, http_get(Config, "/user-limits/limit_test_user_1", Vhost1User, Vhost1User, ?OK)),
+    %% A management user cannot read another user's limits.
+    http_get(Config, "/user-limits/limit_test_user_1", Vhost1User, Vhost1User, ?NOT_AUTHORISED),
 
     %% Clear a limit
     http_delete(Config, "/user-limits/limit_test_user_1/max-connections", ?NO_CONTENT),
@@ -4405,7 +4419,7 @@ qq_status_vhost_authorisation_test(Config) ->
     rabbit_ct_broker_helpers:add_user(Config, User, User),
     rabbit_ct_broker_helpers:set_user_tags(Config, 0, User, [management]),
     http_get(Config, "/queues/quorum/qq-status-vh/qq_status_auth/status",
-             User, User, ?NOT_AUTHORISED),
+             User, User, ?NOT_FOUND),
     %% Permissions granted, must succeed.
     rabbit_ct_broker_helpers:set_full_permissions(Config, User, Vhost),
     http_get(Config, "/queues/quorum/qq-status-vh/qq_status_auth/status",

deps/rabbitmq_management/test/rabbit_mgmt_only_http_SUITE.erl

@@ -629,7 +629,7 @@ permissions_vhost_test(Config) ->
                 http_get(Config, Path1 ++ "/myvhost1/" ++ Path2, "myuser", "myuser",
                          ?OK),
                 http_get(Config, Path1 ++ "/myvhost2/" ++ Path2, "myuser", "myuser",
-                         ?NOT_AUTHORISED)
+                         ?NOT_FOUND)
         end,
     Test3 =
         fun(Path1) ->