jaydoane commented on a change in pull request #3599:
URL: https://github.com/apache/couchdb/pull/3599#discussion_r645217968



##########
File path: src/couch_replicator/src/couch_replicator_parse.erl
##########
@@ -565,4 +677,243 @@ t_error_on_local_endpoint(_) ->
     ?assertThrow({bad_rep_doc, Expect}, parse_rep_doc(RepDoc)).
 
 
+remove_basic_auth_from_headers_test_() ->
+    B64 = list_to_binary(b64creds("user", "pass")),
+    [?_assertEqual({{User, Pass}, NoAuthHeaders},
+        remove_basic_auth_from_headers(Headers)) ||
+        {{User, Pass, NoAuthHeaders}, Headers} <- [
+            {
+                {undefined, undefined, #{}},
+                #{}
+            },
+            {
+                {undefined, undefined, #{<<"h">> => <<"v">>}},
+                #{<<"h">> => <<"v">>}
+            },
+            {
+                {undefined, undefined, #{<<"Authorization">> => <<"junk">>}},
+                #{<<"Authorization">> => <<"junk">>}
+            },
+            {
+                {undefined, undefined, #{}},
+                #{<<"Authorization">> => <<"basic X">>}
+            },
+            {
+                {"user", "pass", #{}},
+                #{<<"Authorization">> => <<"Basic ", B64/binary>>}
+            },
+            {
+                {"user", "pass", #{}},
+                #{<<"AuThorization">> => <<"Basic ", B64/binary>>}
+            },
+            {
+                {"user", "pass", #{}},
+                #{<<"Authorization">> => <<"bAsIc ", B64/binary>>}
+            },
+            {
+                {"user", "pass", #{<<"h">> => <<"v">>}},
+                #{
+                    <<"Authorization">> => <<"Basic ", B64/binary>>,
+                    <<"h">> => <<"v">>
+                }
+            }
+        ]
+    ].
+
+
+b64creds(User, Pass) ->
+    base64:encode_to_string(User ++ ":" ++ Pass).
+
+
+set_basic_auth_creds_test() ->
+    Check = fun(User, Pass, Props) ->
+        HttpDb = #{<<"auth_props">> => Props},
+        HttpDb1 = set_basic_auth_creds(User, Pass, HttpDb),
+        maps:get(<<"auth_props">>, HttpDb1)
+    end,
+
+    ?assertEqual(#{}, Check(undefined, undefined, #{})),
+
+    ?assertEqual(#{<<"other">> => #{}}, Check(undefined, undefined,
+        #{<<"other">> => #{}})),
+
+    ?assertEqual(#{
+        <<"basic">> => #{
+            <<"username">> => <<"u">>,
+            <<"password">> => <<"p">>
+        }
+    }, Check("u", "p", #{})),
+
+    ?assertEqual(#{
+        <<"other">> => #{},
+        <<"basic">> => #{
+            <<"username">> => <<"u">>,
+            <<"password">> => <<"p">>
+        }
+    }, Check("u", "p", #{<<"other">> => #{}})).
+
+
+normalize_basic_creds_test_() ->
+    DefaultHeaders = couch_replicator_utils:default_headers_map(),
+    [?_assertEqual(Expect, normalize_basic_auth(Input)) || {Input, Expect} <- [
+        {
+            #{
+                <<"url">> => <<"http://u:[email protected]/db";>>,
+                <<"auth_props">> => #{},
+                <<"headers">> => DefaultHeaders
+            },
+            #{
+                <<"url">> => <<"http://x.y/db";>>,
+                <<"auth_props">> => auth_props("u", "p"),
+                <<"headers">> => DefaultHeaders
+            }
+        },
+        {
+            #{
+                <<"url">> => <<"http://u:p@h:80/db";>>,
+                <<"auth_props">> => #{},
+                <<"headers">> => DefaultHeaders
+            },
+            #{
+                <<"url">> => <<"http://h:80/db";>>,
+                <<"auth_props">> => auth_props("u", "p"),
+                <<"headers">> => DefaultHeaders
+            }
+        },
+        {
+            #{
+                <<"url">> => <<"https://u:p@h/db";>>,
+                <<"auth_props">> => #{},
+                <<"headers">> => DefaultHeaders
+            },
+            #{
+                <<"url">> => <<"https://h/db";>>,
+                <<"auth_props">> => auth_props("u", "p"),
+                <<"headers">> => DefaultHeaders
+            }
+        },
+        {
+            #{
+                <<"url">> => <<"http://u:p@[2001:db8:a1b:12f9::1]/db";>>,
+                <<"auth_props">> => #{},
+                <<"headers">> => DefaultHeaders
+            },
+            #{
+                <<"url">> => <<"http://[2001:db8:a1b:12f9::1]/db";>>,
+                <<"auth_props">> => auth_props("u", "p"),
+                <<"headers">> => DefaultHeaders
+            }
+        },
+        {
+            #{
+                <<"url">> => <<"http://h/db";>>,
+                <<"auth_props">> => #{},
+                <<"headers">> => maps:merge(DefaultHeaders, #{
+                    <<"authorization">> => basic_b64("u", "p")
+                })
+            },
+            #{
+                <<"url">> => <<"http://h/db";>>,
+                <<"auth_props">> => auth_props("u", "p"),
+                <<"headers">> => DefaultHeaders
+            }
+        },
+        {
+            #{
+                <<"url">> => <<"http://h/db";>>,
+                <<"auth_props">> => #{},
+                <<"headers">> => maps:merge(DefaultHeaders, #{
+                    <<"authorization">> => basic_b64("u", "p@")

Review comment:
       Yay I can finally put `@` in my password 🎉 

##########
File path: src/couch_replicator/src/couch_replicator_parse.erl
##########
@@ -565,4 +677,243 @@ t_error_on_local_endpoint(_) ->
     ?assertThrow({bad_rep_doc, Expect}, parse_rep_doc(RepDoc)).
 
 
+remove_basic_auth_from_headers_test_() ->
+    B64 = list_to_binary(b64creds("user", "pass")),
+    [?_assertEqual({{User, Pass}, NoAuthHeaders},
+        remove_basic_auth_from_headers(Headers)) ||
+        {{User, Pass, NoAuthHeaders}, Headers} <- [
+            {
+                {undefined, undefined, #{}},
+                #{}
+            },
+            {
+                {undefined, undefined, #{<<"h">> => <<"v">>}},
+                #{<<"h">> => <<"v">>}
+            },
+            {
+                {undefined, undefined, #{<<"Authorization">> => <<"junk">>}},
+                #{<<"Authorization">> => <<"junk">>}
+            },
+            {
+                {undefined, undefined, #{}},
+                #{<<"Authorization">> => <<"basic X">>}
+            },
+            {
+                {"user", "pass", #{}},
+                #{<<"Authorization">> => <<"Basic ", B64/binary>>}
+            },
+            {
+                {"user", "pass", #{}},
+                #{<<"AuThorization">> => <<"Basic ", B64/binary>>}
+            },
+            {
+                {"user", "pass", #{}},
+                #{<<"Authorization">> => <<"bAsIc ", B64/binary>>}
+            },
+            {
+                {"user", "pass", #{<<"h">> => <<"v">>}},
+                #{
+                    <<"Authorization">> => <<"Basic ", B64/binary>>,
+                    <<"h">> => <<"v">>
+                }
+            }
+        ]
+    ].
+
+
+b64creds(User, Pass) ->
+    base64:encode_to_string(User ++ ":" ++ Pass).
+
+
+set_basic_auth_creds_test() ->
+    Check = fun(User, Pass, Props) ->
+        HttpDb = #{<<"auth_props">> => Props},
+        HttpDb1 = set_basic_auth_creds(User, Pass, HttpDb),
+        maps:get(<<"auth_props">>, HttpDb1)
+    end,
+
+    ?assertEqual(#{}, Check(undefined, undefined, #{})),
+
+    ?assertEqual(#{<<"other">> => #{}}, Check(undefined, undefined,
+        #{<<"other">> => #{}})),
+
+    ?assertEqual(#{
+        <<"basic">> => #{
+            <<"username">> => <<"u">>,
+            <<"password">> => <<"p">>
+        }
+    }, Check("u", "p", #{})),
+
+    ?assertEqual(#{
+        <<"other">> => #{},
+        <<"basic">> => #{
+            <<"username">> => <<"u">>,
+            <<"password">> => <<"p">>
+        }
+    }, Check("u", "p", #{<<"other">> => #{}})).
+
+
+normalize_basic_creds_test_() ->
+    DefaultHeaders = couch_replicator_utils:default_headers_map(),
+    [?_assertEqual(Expect, normalize_basic_auth(Input)) || {Input, Expect} <- [
+        {
+            #{
+                <<"url">> => <<"http://u:[email protected]/db";>>,
+                <<"auth_props">> => #{},
+                <<"headers">> => DefaultHeaders
+            },
+            #{
+                <<"url">> => <<"http://x.y/db";>>,
+                <<"auth_props">> => auth_props("u", "p"),
+                <<"headers">> => DefaultHeaders
+            }
+        },
+        {
+            #{
+                <<"url">> => <<"http://u:p@h:80/db";>>,
+                <<"auth_props">> => #{},
+                <<"headers">> => DefaultHeaders
+            },
+            #{
+                <<"url">> => <<"http://h:80/db";>>,
+                <<"auth_props">> => auth_props("u", "p"),
+                <<"headers">> => DefaultHeaders
+            }
+        },
+        {
+            #{
+                <<"url">> => <<"https://u:p@h/db";>>,
+                <<"auth_props">> => #{},
+                <<"headers">> => DefaultHeaders
+            },
+            #{
+                <<"url">> => <<"https://h/db";>>,
+                <<"auth_props">> => auth_props("u", "p"),
+                <<"headers">> => DefaultHeaders
+            }
+        },
+        {
+            #{
+                <<"url">> => <<"http://u:p@[2001:db8:a1b:12f9::1]/db";>>,
+                <<"auth_props">> => #{},
+                <<"headers">> => DefaultHeaders
+            },
+            #{
+                <<"url">> => <<"http://[2001:db8:a1b:12f9::1]/db";>>,
+                <<"auth_props">> => auth_props("u", "p"),
+                <<"headers">> => DefaultHeaders
+            }
+        },
+        {
+            #{
+                <<"url">> => <<"http://h/db";>>,
+                <<"auth_props">> => #{},
+                <<"headers">> => maps:merge(DefaultHeaders, #{
+                    <<"authorization">> => basic_b64("u", "p")
+                })
+            },
+            #{
+                <<"url">> => <<"http://h/db";>>,
+                <<"auth_props">> => auth_props("u", "p"),
+                <<"headers">> => DefaultHeaders
+            }
+        },
+        {
+            #{
+                <<"url">> => <<"http://h/db";>>,
+                <<"auth_props">> => #{},
+                <<"headers">> => maps:merge(DefaultHeaders, #{
+                    <<"authorization">> => basic_b64("u", "p@")
+                })
+            },
+            #{
+                <<"url">> => <<"http://h/db";>>,
+                <<"auth_props">> => auth_props("u", "p@"),
+                <<"headers">> => DefaultHeaders
+            }
+        },
+        {
+            #{
+                <<"url">> => <<"http://h/db";>>,
+                <<"auth_props">> => #{},
+                <<"headers">> => maps:merge(DefaultHeaders, #{
+                    <<"authorization">> => basic_b64("u", "p@%40")
+                })
+            },
+            #{
+                <<"url">> => <<"http://h/db";>>,
+                <<"auth_props">> => auth_props("u", "p@%40"),
+                <<"headers">> => DefaultHeaders
+            }
+        },
+        {
+            #{
+                <<"url">> => <<"http://h/db";>>,
+                <<"auth_props">> => #{},
+                <<"headers">> => maps:merge(DefaultHeaders, #{
+                    <<"aUthoriZation">> => basic_b64("U", "p")
+                })
+            },
+            #{
+                <<"url">> => <<"http://h/db";>>,
+                <<"auth_props">> => auth_props("U", "p"),
+                <<"headers">> => DefaultHeaders
+            }
+        },
+        {
+            #{
+                <<"url">> => <<"http://u1:p1@h/db";>>,
+                <<"auth_props">> => #{},
+                <<"headers">> => maps:merge(DefaultHeaders, #{
+                    <<"Authorization">> => basic_b64("u2", "p2")
+                })
+            },
+            #{
+                <<"url">> => <<"http://h/db";>>,
+                <<"auth_props">> => auth_props("u1", "p1"),
+                <<"headers">> => DefaultHeaders
+            }
+        },
+        {
+            #{
+                <<"url">> => <<"http://u1:p1@h/db";>>,
+                <<"auth_props">> => auth_props("u2", "p2"),
+                <<"headers">> => DefaultHeaders
+            },
+            #{
+                <<"url">> => <<"http://h/db";>>,
+                <<"auth_props">> => auth_props("u2", "p2"),
+                <<"headers">> => DefaultHeaders
+            }
+        },
+        {
+            #{
+                <<"url">> => <<"http://u1:p1@h/db";>>,

Review comment:
       I wonder if it would be surprising for someone putting different creds 
in multiple locations in the replication doc, that all but one set just gets 
ignored? Would it be better to fail with an error on multiple creds or would 
that not be backwards compatible?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to