You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2021/06/04 01:45:00 UTC
[GitHub] [couchdb] nickva commented on a change in pull request #3599: Improve basic auth credentials handling in replicator
nickva commented on a change in pull request #3599:
URL: https://github.com/apache/couchdb/pull/3599#discussion_r645234779
##########
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:p@x.y/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 thought since we allowed multiple ones previously we'd keep that behavior for compatibility. Next, I was planning on reverting the ibrowse commit and also issue a doc PR indicating that we're deprecating userinfo and if users to have multiple credentials specified which ones get picked.
--
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:
users@infra.apache.org