Merge pull request rabbitmq#16584 from rabbitmq/mk-minor-management-plugin-refinements

Minor refactoring of two HTTP API areas

Author: michaelklishin

deps/rabbitmq_management/include/rabbit_mgmt.hrl

@@ -18,3 +18,13 @@
 -define(OAUTH2_BOOTSTRAP_PATH,                      <<"js/oidc-oauth/bootstrap.js">>).
 -define(MANAGEMENT_LOGIN_STRICT_AUTH_MECHANISM,     <<"strict_auth_mechanism">>).
 -define(MANAGEMENT_LOGIN_PREFERRED_AUTH_MECHANISM,  <<"preferred_auth_mechanism">>).
+
+-define(DEFAULT_LOGIN_ERROR_CODE, <<"not_authorised">>).
+
+-define(LOGIN_ERROR_CODES,
+        [<<"Not_Authorized">>,
+         ?DEFAULT_LOGIN_ERROR_CODE,
+         <<"invalid_credentials">>,
+         <<"token_invalid">>,
+         <<"token_expired">>,
+         <<"unsupported_mechanism">>]).

deps/rabbitmq_management/src/rabbit_mgmt_login.erl

@@ -8,11 +8,14 @@
 -module(rabbit_mgmt_login).
 
 -export([init/2]).
+-export([canonical_login_error/1, sanitise_location/1]).
 
 -include_lib("rabbitmq_management_agent/include/rabbit_mgmt_records.hrl").
 -include("rabbit_mgmt.hrl").
 -include_lib("kernel/include/logger.hrl").
 
+-define(MAX_LOCATION_LEN, 1024).
+
 %%--------------------------------------------------------------------
 %% /api/login endpoint
 %%
@@ -118,7 +121,7 @@ handleAccessToken(Req0, AccessToken, State) ->
                                          AccessToken,
                                          Req1, State);
         {false, ReqData1, Reason} ->
-            replyWithError(Reason, ReqData1, State)
+            reply_with_error(Reason, ReqData1, State)
     end.
 
 redirect_to_home_with_cookie(CookiePath, CookieName, CookieValue, Req=#{scheme := Scheme}, State) ->
@@ -143,14 +146,40 @@ get_home_uri(Req0) ->
     cowboy_req:uri(Req0, #{path => rabbit_mgmt_util:get_path_prefix() ++ "/", qs => undefined}).
 
 
-replyWithError(Reason, Req, State) ->
-    Home = cowboy_req:uri(Req, #{
-        path => rabbit_mgmt_util:get_path_prefix() ++ "/", 
-        qs => "error=" ++ Reason
+reply_with_error(Reason, Req, State) ->
+    Code     = canonical_login_error(Reason),
+    QS       = cow_qs:qs([{<<"error">>, Code}]),
+    Home     = cowboy_req:uri(Req, #{
+        path => rabbit_mgmt_util:get_path_prefix() ++ "/",
+        qs   => QS
     }),
-    Req2 = cowboy_req:reply(302, #{
-        <<"Location">> => iolist_to_binary(Home) 
-    }, <<>>, Req),
+    Location = sanitise_location(iolist_to_binary(Home)),
+    Req2 = cowboy_req:reply(302, #{<<"Location">> => Location}, <<>>, Req),
     {ok, Req2, State}.
 
-
+-spec canonical_login_error(term()) -> binary().
+canonical_login_error(Reason) when is_binary(Reason) ->
+    case lists:member(Reason, ?LOGIN_ERROR_CODES) of
+        true  -> Reason;
+        false -> ?DEFAULT_LOGIN_ERROR_CODE
+    end;
+canonical_login_error(Reason) when is_atom(Reason) ->
+    canonical_login_error(atom_to_binary(Reason, utf8));
+canonical_login_error(Reason) when is_list(Reason) ->
+    try unicode:characters_to_binary(Reason) of
+        B when is_binary(B) -> canonical_login_error(B);
+        _                   -> ?DEFAULT_LOGIN_ERROR_CODE
+    catch
+        error:_ -> ?DEFAULT_LOGIN_ERROR_CODE
+    end;
+canonical_login_error(_) ->
+    ?DEFAULT_LOGIN_ERROR_CODE.
+
+-spec sanitise_location(binary()) -> binary().
+sanitise_location(Loc) when byte_size(Loc) > ?MAX_LOCATION_LEN ->
+    <<"/">>;
+sanitise_location(Loc) ->
+    case binary:match(Loc, [<<"\r">>, <<"\n">>]) of
+        nomatch -> Loc;
+        _       -> <<"/">>
+    end.

deps/rabbitmq_management/src/rabbit_mgmt_util.erl

@@ -31,6 +31,7 @@
 -export([filter_tracked_conn_list/3]).
 -export([with_decode/5, decode/1, decode/2, set_resp_header/3,
          args/1, read_complete_body/1, read_complete_body_with_limit/2]).
+-export([content_disposition_attachment/1]).
 -export([reply_list/3, reply_list/5, reply_list/4,
          sort_list/2, destination_type/1, reply_list_or_paginate/3
          ]).
@@ -68,6 +69,7 @@
 
 -define(MAX_RANGE, 500).
 -define(MAX_FILTER_LENGTH, 1024).
+-define(MAX_ATTACHMENT_FILENAME_LEN, 500).
 
 -record(pagination, {page = undefined, page_size = undefined,
                      name = undefined, use_regex = undefined}).
@@ -1138,7 +1140,54 @@ filter_tracked_conn_list(List, ReqData, #context{user = #user{username = FilterU
 set_resp_header(K, V, ReqData) ->
     cowboy_req:set_resp_header(K, strip_crlf(V), ReqData).
 
-strip_crlf(Str) -> lists:append(string:tokens(Str, "\r\n")).
+strip_crlf(Str) when is_list(Str) ->
+    lists:append(string:tokens(Str, "\r\n"));
+strip_crlf(Str) when is_binary(Str) ->
+    binary:replace(Str, [<<"\r">>, <<"\n">>], <<>>, [global]).
+
+-spec content_disposition_attachment(binary()) -> binary().
+content_disposition_attachment(Filename0) when is_binary(Filename0) ->
+    Filename = case byte_size(Filename0) > ?MAX_ATTACHMENT_FILENAME_LEN of
+                   true  -> binary:part(Filename0, 0, ?MAX_ATTACHMENT_FILENAME_LEN);
+                   false -> Filename0
+               end,
+    Ascii   = ascii_fallback(Filename),
+    Encoded = rfc5987_encode(Filename),
+    <<"attachment; filename=\"", Ascii/binary,
+      "\"; filename*=UTF-8''", Encoded/binary>>.
+
+-spec ascii_fallback(binary()) -> binary().
+ascii_fallback(B) ->
+    << <<(safe_byte(C))>> || <<C>> <= B >>.
+
+-spec safe_byte(byte()) -> byte().
+safe_byte(C) when C >= $A, C =< $Z -> C;
+safe_byte(C) when C >= $a, C =< $z -> C;
+safe_byte(C) when C >= $0, C =< $9 -> C;
+safe_byte($.) -> $.;
+safe_byte($_) -> $_;
+safe_byte($-) -> $-;
+safe_byte(_)  -> $_.
+
+%% RFC 5987 percent-encoding. Shares the unreserved alphabet with the
+%% ASCII fallback above so both forms decode to the same character set.
+-spec rfc5987_encode(binary()) -> binary().
+rfc5987_encode(B) ->
+    << <<(rfc5987_byte(C))/binary>> || <<C>> <= B >>.
+
+-spec rfc5987_byte(byte()) -> binary().
+rfc5987_byte(C) when C >= $A, C =< $Z -> <<C>>;
+rfc5987_byte(C) when C >= $a, C =< $z -> <<C>>;
+rfc5987_byte(C) when C >= $0, C =< $9 -> <<C>>;
+rfc5987_byte($.) -> <<".">>;
+rfc5987_byte($_) -> <<"_">>;
+rfc5987_byte($-) -> <<"-">>;
+rfc5987_byte(C) ->
+    <<"%", (hex_nibble(C bsr 4)), (hex_nibble(C band 16#0F))>>.
+
+-spec hex_nibble(0..15) -> byte().
+hex_nibble(N) when N < 10 -> $0 + N;
+hex_nibble(N)             -> $A + N - 10.
 
 args([]) -> args(#{});
 args(L)  -> rabbit_mgmt_format:to_amqp_table(L).

deps/rabbitmq_management/src/rabbit_mgmt_wm_definitions.erl

@@ -85,10 +85,10 @@ all_definitions(ReqData, Context) ->
     Result = TopLevelDefsAndMetadata ++ retain_whitelisted(Contents),
     ReqData1 = case rabbit_mgmt_util:qs_val(<<"download">>, ReqData) of
                    undefined -> ReqData;
-                   Filename  -> rabbit_mgmt_util:set_resp_header(
-                       <<"Content-Disposition">>,
-                       "attachment; filename=" ++
-                       binary_to_list(Filename), ReqData)
+                   Filename  ->
+                       Header = rabbit_mgmt_util:content_disposition_attachment(Filename),
+                       rabbit_mgmt_util:set_resp_header(
+                         <<"Content-Disposition">>, Header, ReqData)
                end,
 
     rabbit_mgmt_util:reply(Result, ReqData1, Context).
@@ -152,8 +152,9 @@ vhost_definitions(ReqData, VHostName, Context) ->
     ReqData1 = case rabbit_mgmt_util:qs_val(<<"download">>, ReqData) of
                    undefined -> ReqData;
                    Filename  ->
-                       HeaderVal = "attachment; filename=" ++ binary_to_list(Filename),
-                       rabbit_mgmt_util:set_resp_header(<<"Content-Disposition">>, HeaderVal, ReqData)
+                       Header = rabbit_mgmt_util:content_disposition_attachment(Filename),
+                       rabbit_mgmt_util:set_resp_header(
+                         <<"Content-Disposition">>, Header, ReqData)
                end,
     rabbit_mgmt_util:reply(Result, ReqData1, Context).