Skip to content

Commit 013be30

Browse files
committed
Avoid updating password hash when request with simple password scheme
When using the `simple` password scheme, the number of iterations is `undefined`, so the user's password hash is updated every time when a new request is made using user's authentication credentials. Add a case statement to avoid this situation.
1 parent 40ae312 commit 013be30

File tree

2 files changed

+209
-3
lines changed

2 files changed

+209
-3
lines changed

src/couch/src/couch_password_hasher.erl

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,6 @@ is_doc(UserProps) ->
113113
couch_util:get_value(<<"_rev">>, UserProps) /= undefined.
114114

115115
needs_upgrade(UserProps) ->
116-
CurrentScheme = couch_util:get_value(<<"password_scheme">>, UserProps),
117116
TargetScheme = ?l2b(chttpd_util:get_chttpd_auth_config("password_scheme", "pbkdf2")),
118117
CurrentPRF = couch_util:get_value(<<"pbkdf2_prf">>, UserProps),
119118
TargetPRF = ?l2b(chttpd_util:get_chttpd_auth_config("pbkdf2_prf", "sha256")),
@@ -122,9 +121,9 @@ needs_upgrade(UserProps) ->
122121
"iterations", 600000
123122
),
124123
case {TargetScheme, TargetIterations, TargetPRF} of
125-
{CurrentScheme, CurrentIterations, _} when CurrentScheme == <<"simple">> ->
124+
{<<"simple">>, _, _} when CurrentIterations =:= undefined ->
126125
false;
127-
{CurrentScheme, CurrentIterations, CurrentPRF} when CurrentScheme == <<"pbkdf2">> ->
126+
{<<"pbkdf2">>, CurrentIterations, CurrentPRF} ->
128127
false;
129128
{_, _, _} ->
130129
true
Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
% Licensed under the Apache License, Version 2.0 (the "License"); you may not
2+
% use this file except in compliance with the License. You may obtain a copy of
3+
% the License at
4+
%
5+
% http://www.apache.org/licenses/LICENSE-2.0
6+
%
7+
% Unless required by applicable law or agreed to in writing, software
8+
% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
9+
% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
10+
% License for the specific language governing permissions and limitations under
11+
% the License.
12+
13+
-module(couch_passwords_hasher_tests).
14+
15+
-include_lib("couch/include/couch_db.hrl").
16+
-include_lib("couch/include/couch_eunit.hrl").
17+
18+
-define(USER, "couch_passowrds_hash_test_admin").
19+
-define(PASS, "pass").
20+
-define(AUTH, {basic_auth, {?USER, ?PASS}}).
21+
-define(CONTENT_JSON, {"Content-Type", "application/json"}).
22+
-define(RANDOM_USER, "user-" ++ ?b2l(couch_uuids:random())).
23+
-define(MOD, couch_password_hasher).
24+
25+
setup(Scheme) ->
26+
Hashed = couch_passwords:hash_admin_password(?PASS),
27+
config:set("admins", ?USER, ?b2l(Hashed), false),
28+
Db = ?b2l(?tempdb()),
29+
create_db(Db),
30+
config:set("chttpd_auth", "authentication_db", Db, false),
31+
config:set("chttpd_auth", "password_scheme", Scheme, false),
32+
meck:new(?MOD, [passthrough]),
33+
Db.
34+
35+
teardown(_, Db) ->
36+
delete_db(Db),
37+
config:delete("admins", ?USER, false),
38+
config:delete("chttpd_auth", "authentication_db", false),
39+
config:delete("chttpd_auth", "password_scheme", false),
40+
meck:unload().
41+
42+
couch_password_hasher_test_() ->
43+
{
44+
"couch_password_hasher tests",
45+
{
46+
setup,
47+
fun() -> test_util:start_couch([chttpd]) end,
48+
fun test_util:stop_couch/1,
49+
[
50+
upgrade_password_hash_tests("simple"),
51+
upgrade_password_hash_tests("pbkdf2")
52+
]
53+
}
54+
}.
55+
56+
upgrade_password_hash_tests(Scheme) ->
57+
{
58+
"password scheme " ++ Scheme ++ " tests",
59+
foreachx,
60+
fun setup/1,
61+
fun teardown/2,
62+
[
63+
{Scheme, Test}
64+
|| Test <-
65+
[
66+
fun create_user_by_admin_should_not_upgrade_password_hash/2,
67+
fun request_by_user_should_not_upgrade_password_hash/2,
68+
fun update_user_password_by_user_should_not_upgrade_password_hash/2
69+
]
70+
]
71+
}.
72+
73+
create_user_by_admin_should_not_upgrade_password_hash(_, Db) ->
74+
?_test(begin
75+
meck:reset(?MOD),
76+
User = ?RANDOM_USER,
77+
create_user(Db, User, ?PASS),
78+
?assertNot(
79+
meck:called(?MOD, handle_cast, [
80+
{upgrade_password_hash, '_', ?l2b(User), '_', '_', '_', '_'}, '_'
81+
])
82+
)
83+
end).
84+
85+
request_by_user_should_not_upgrade_password_hash(_, Db) ->
86+
?_test(begin
87+
User = ?RANDOM_USER,
88+
create_user(Db, User, ?PASS),
89+
{200, _} = req(get, url(Db, "org.couchdb.user:" ++ User)),
90+
?assertNot(
91+
meck:called(?MOD, handle_cast, [
92+
{upgrade_password_hash, '_', ?l2b(User), '_', '_', '_', '_'}, '_'
93+
])
94+
),
95+
96+
meck:reset(?MOD),
97+
Headers = [{basic_auth, {User, ?PASS}}],
98+
{200, _} = req(get, url(), Headers, []),
99+
?assertNot(
100+
meck:called(?MOD, handle_cast, [
101+
{upgrade_password_hash, '_', ?l2b(User), '_', '_', '_', '_'}, '_'
102+
])
103+
)
104+
end).
105+
106+
update_user_password_by_user_should_not_upgrade_password_hash(_, Db) ->
107+
?_test(begin
108+
User = ?RANDOM_USER,
109+
?debugVal(User),
110+
create_user(Db, User, ?PASS),
111+
{200, #{<<"_rev">> := Rev}} = req(get, url(Db, "org.couchdb.user:" ++ User)),
112+
?assertNot(
113+
meck:called(?MOD, handle_cast, [
114+
{upgrade_password_hash, '_', ?l2b(User), '_', '_', '_', '_'}, '_'
115+
])
116+
),
117+
118+
meck:reset(?MOD),
119+
NewPass = "new_password",
120+
update_password(Db, User, NewPass, ?b2l(Rev)),
121+
?assertNot(
122+
meck:called(?MOD, handle_cast, [
123+
{upgrade_password_hash, '_', ?l2b(User), '_', '_', '_', '_'}, '_'
124+
])
125+
),
126+
127+
OldAuth = [{basic_auth, {User, ?PASS}}],
128+
{401, _} = req(get, url(), OldAuth, []),
129+
?assertNot(
130+
meck:called(?MOD, handle_cast, [
131+
{upgrade_password_hash, '_', ?l2b(User), '_', '_', '_', '_'}, '_'
132+
])
133+
),
134+
NewAuth = [{basic_auth, {User, NewPass}}],
135+
{200, _} = req(get, url(), NewAuth, []),
136+
?assertNot(
137+
meck:called(?MOD, handle_cast, [
138+
{upgrade_password_hash, '_', ?l2b(User), '_', '_', '_', '_'}, '_'
139+
])
140+
)
141+
end).
142+
143+
%%%%%%%%%%%%%%%%%%%% Utility Functions %%%%%%%%%%%%%%%%%%%%
144+
url() ->
145+
Addr = config:get("chttpd", "bind_address", "127.0.0.1"),
146+
Port = mochiweb_socket_server:get(chttpd, port),
147+
lists:concat(["http://", Addr, ":", Port]).
148+
149+
url(Db) ->
150+
url() ++ "/" ++ Db.
151+
152+
url(Db, Path) ->
153+
url(Db) ++ "/" ++ Path.
154+
155+
create_db(Db) ->
156+
case req(put, url(Db)) of
157+
{201, #{}} -> ok;
158+
Error -> error({failed_to_create_test_db, Db, Error})
159+
end.
160+
161+
delete_db(Db) ->
162+
case req(delete, url(Db)) of
163+
{200, #{}} -> ok;
164+
Error -> error({failed_to_delete_test_db, Db, Error})
165+
end.
166+
167+
create_user(Db, UserName, Password) ->
168+
ok = couch_auth_cache:ensure_users_db_exists(),
169+
User = ?l2b(UserName),
170+
Pass = ?l2b(Password),
171+
case req(put, url(Db, "org.couchdb.user:" ++ UserName), user_doc(User, Pass)) of
172+
{201, #{}} -> ok;
173+
Error -> error({failed_to_create_user, UserName, Error})
174+
end.
175+
176+
update_password(Db, UserName, NewPassword, Rev) ->
177+
User = ?l2b(UserName),
178+
NewPass = ?l2b(NewPassword),
179+
Headers = [?AUTH, {"If-Match", Rev}],
180+
case req(put, url(Db, "org.couchdb.user:" ++ UserName), Headers, user_doc(User, NewPass)) of
181+
{201, #{}} -> ok;
182+
Error -> error({failed_to_update_password, UserName, Error})
183+
end.
184+
185+
user_doc(User, Pass) ->
186+
jiffy:encode(
187+
{[
188+
{<<"name">>, User},
189+
{<<"password">>, Pass},
190+
{<<"roles">>, []},
191+
{<<"type">>, <<"user">>}
192+
]}
193+
).
194+
195+
req(Method, Url) ->
196+
Headers = [?CONTENT_JSON, ?AUTH],
197+
{ok, Code, _, Res} = test_request:request(Method, Url, Headers),
198+
{Code, jiffy:decode(Res, [return_maps])}.
199+
200+
req(Method, Url, Body) ->
201+
Headers = [?CONTENT_JSON, ?AUTH],
202+
{ok, Code, _, Res} = test_request:request(Method, Url, Headers, Body),
203+
{Code, jiffy:decode(Res, [return_maps])}.
204+
205+
req(Method, Url, Headers, Body) ->
206+
{ok, Code, _, Res} = test_request:request(Method, Url, Headers, Body),
207+
{Code, jiffy:decode(Res, [return_maps])}.

0 commit comments

Comments
 (0)