Skip to content

Commit 1547216

Browse files
MarcialRosalesMarcialRosales
committed
Do not propagate none password for http auth backend
1 parent 6e76def commit 1547216

File tree

11 files changed

+87
-33
lines changed

11 files changed

+87
-33
lines changed

‎deps/rabbitmq_auth_backend_http/src/rabbit_auth_backend_http.erl

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,12 @@ is_internal_property(rabbit_auth_backend_http) -> true;
7676
is_internal_property(rabbit_auth_backend_cache) -> true;
7777
is_internal_property(_Other) -> false.
7878

79+
is_internal_none_password(password, none) -> true;
80+
is_internal_none_password(_, _) -> false.
81+
7982
extract_other_credentials(AuthProps) ->
80-
PublicAuthProps = [{K,V} || {K,V} <-AuthProps, not is_internal_property(K)],
83+
PublicAuthProps = [{K,V} || {K,V} <-AuthProps, not is_internal_property(K) and
84+
not is_internal_none_password(K, V)],
8185
case PublicAuthProps of
8286
[] -> resolve_using_persisted_credentials(AuthProps);
8387
_ -> PublicAuthProps

‎deps/rabbitmq_auth_backend_http/test/auth_SUITE.erl

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@
1818
password => <<"Kocur">>,
1919
expected_credentials => [username, password],
2020
tags => [policymaker, monitoring]}).
21+
-define(ALLOWED_USER_2, #{username => <<"Ala3">>,
22+
expected_credentials => [username],
23+
tags => [policymaker, monitoring]}).
2124
-define(ALLOWED_USER_WITH_EXTRA_CREDENTIALS, #{username => <<"Ala2">>,
2225
password => <<"Kocur">>,
2326
client_id => <<"some_id">>,
@@ -46,12 +49,14 @@ shared() ->
4649
grants_access_to_user_passing_additional_required_authprops,
4750
grants_access_to_user_skipping_internal_authprops,
4851
grants_access_to_user_with_credentials_in_rabbit_auth_backend_http,
49-
grants_access_to_user_with_credentials_in_rabbit_auth_backend_cache
52+
grants_access_to_user_with_credentials_in_rabbit_auth_backend_cache,
53+
grants_access_to_ssl_user_with_none_password
5054
].
5155

5256
init_per_suite(Config) ->
5357
rabbit_ct_helpers:run_setup_steps(Config) ++
5458
[{allowed_user, ?ALLOWED_USER},
59+
{allowed_user_2, ?ALLOWED_USER_2},
5560
{allowed_user_with_extra_credentials, ?ALLOWED_USER_WITH_EXTRA_CREDENTIALS},
5661
{denied_user, ?DENIED_USER}].
5762

@@ -65,13 +70,21 @@ init_per_group(over_http, Config) ->
6570
init_per_group(over_https, Config) ->
6671
configure_http_auth_backend("https", Config),
6772
{User1, Tuple1} = extractUserTuple(?ALLOWED_USER),
68-
{User2, Tuple2} = extractUserTuple(?ALLOWED_USER_WITH_EXTRA_CREDENTIALS),
73+
{User2, Tuple2} = extractUserTuple(?ALLOWED_USER_2),
74+
{User3, Tuple3} = extractUserTuple(?ALLOWED_USER_WITH_EXTRA_CREDENTIALS),
6975
CertsDir = ?config(rmq_certsdir, Config),
70-
start_https_auth_server(?AUTH_PORT, CertsDir, ?USER_PATH, #{User1 => Tuple1, User2 => Tuple2}),
71-
Config.
76+
start_https_auth_server(?AUTH_PORT, CertsDir, ?USER_PATH, #{
77+
User1 => Tuple1,
78+
User3 => Tuple3,
79+
User2 => Tuple2}),
80+
Config ++ [{group, over_https}].
7281

7382
extractUserTuple(User) ->
74-
#{username := Username, password := Password, tags := Tags, expected_credentials := ExpectedCredentials} = User,
83+
#{username := Username, tags := Tags, expected_credentials := ExpectedCredentials} = User,
84+
Password = case maps:get(password, User, undefined) of
85+
undefined -> none;
86+
P -> P
87+
end,
7588
{Username, {Password, Tags, ExpectedCredentials}}.
7689

7790
end_per_suite(Config) ->
@@ -91,6 +104,16 @@ grants_access_to_user(Config) ->
91104
?assertMatch({U, T, AuthProps},
92105
{User#auth_user.username, User#auth_user.tags, (User#auth_user.impl)()}).
93106

107+
grants_access_to_ssl_user_with_none_password(Config) ->
108+
case ?config(group, Config) of
109+
over_https ->
110+
#{username := U, tags := T} = ?config(allowed_user_2, Config),
111+
{ok, User} = rabbit_auth_backend_http:user_login_authentication(U, []),
112+
?assertMatch({U, T, []},
113+
{User#auth_user.username, User#auth_user.tags, (User#auth_user.impl)()});
114+
_ ->{skip, "Requires https"}
115+
end.
116+
94117
denies_access_to_user(Config) ->
95118
#{username := U, password := P} = ?config(denied_user, Config),
96119
?assertMatch({refused, "Denied by the backing HTTP service", []},

‎deps/rabbitmq_auth_backend_http/test/auth_http_mock.erl

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@ init(Req = #{method := <<"GET">>}, Users) ->
1414
%%% HELPERS
1515

1616
authenticate(QsVals, Users) ->
17+
ct:log("QsVals: ~p Users: ~p", [QsVals, Users]),
1718
Username = proplists:get_value(<<"username">>, QsVals),
18-
Password = proplists:get_value(<<"password">>, QsVals),
19+
Password = proplists:get_value(<<"password">>, QsVals, none),
1920
case maps:get(Username, Users, undefined) of
2021
{MatchingPassword, Tags, ExpectedCredentials} when Password =:= MatchingPassword ->
2122
case lists:all(fun(C) -> proplists:is_defined(list_to_binary(rabbit_data_coercion:to_list(C)),QsVals) end, ExpectedCredentials) of

‎deps/rabbitmq_mqtt/src/rabbit_mqtt_processor.erl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1096,7 +1096,7 @@ check_vhost_alive(VHost) ->
10961096
check_user_login(VHost, Username, Password, ClientId, PeerIp, ConnName) ->
10971097
AuthProps = [{vhost, VHost},
10981098
{client_id, ClientId},
1099-
{password, Password}],
1099+
{password, Password}],
11001100
case rabbit_access_control:check_user_login(Username, AuthProps) of
11011101
{ok, User = #user{username = Username1}} ->
11021102
notify_auth_result(user_authentication_success, Username1, ConnName),

‎deps/rabbitmq_mqtt/test/auth_SUITE.erl

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ sub_groups() ->
7272
[invalid_client_id_from_cert_san_dns
7373
]},
7474
{ssl_user_with_client_id_in_cert_san_dns, [],
75-
[client_id_from_cert_san_dns
75+
[client_id_from_cert_san_dns
7676
]},
7777
{ssl_user_with_client_id_in_cert_san_dns_1, [],
7878
[client_id_from_cert_san_dns_1
@@ -338,6 +338,7 @@ init_per_testcase(T, Config)
338338
when T =:= client_id_propagation;
339339
T =:= invalid_client_id_from_cert_san_dns;
340340
T =:= client_id_from_cert_san_dns;
341+
T =:= ssl_user_password_not_propagated_if_not_provided;
341342
T =:= client_id_from_cert_san_dns_1;
342343
T =:= client_id_from_cert_san_email;
343344
T =:= client_id_from_cert_dn ->
@@ -477,6 +478,7 @@ end_per_testcase(T, Config)
477478
when T =:= client_id_propagation;
478479
T =:= invalid_client_id_from_cert_san_dns;
479480
T =:= client_id_from_cert_san_dns;
481+
T =:= ssl_user_password_not_propagated_if_not_provided;
480482
T =:= client_id_from_cert_san_dns_1;
481483
T =:= client_id_from_cert_san_email;
482484
T =:= client_id_from_cert_dn ->

‎selenium/full-suite-authnz-messaging

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
authnz-messaging/auth-cache-http-backends.sh
22
authnz-messaging/auth-cache-ldap-backends.sh
3-
authnz-messaging/auth-http-backend.sh
3+
authnz-messaging/auth-http-backend-with-mtls.sh
44
authnz-messaging/auth-http-internal-backends-with-internal.sh
55
authnz-messaging/auth-http-internal-backends.sh
66
authnz-messaging/auth-internal-backend.sh
77
authnz-messaging/auth-internal-mtls-backend.sh
88
authnz-messaging/auth-internal-http-backends.sh
99
authnz-messaging/auth-ldap-backend.sh
10-
authnz-messaging/auth-http-backend.sh

‎selenium/suites/authnz-messaging/auth-http-backend-with-mtls.sh

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#!/usr/bin/env bash
2+
3+
SCRIPT="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
4+
5+
TEST_CASES_PATH=/authnz-msg-protocols
6+
PROFILES="internal-user auth-http auth_backends-http auth-mtls"
7+
# internal-user profile is used because the client certificates to
8+
# access rabbitmq are issued with the alt_name = internal-user
9+
10+
source $SCRIPT/../../bin/suite_template
11+
runWith mock-auth-backend-http

‎selenium/suites/authnz-messaging/auth-http-backend.sh

Lines changed: 0 additions & 9 deletions
This file was deleted.

‎selenium/test/amqp.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ function getAmqpsConnectionOptions() {
2828
}
2929
function getConnectionOptions() {
3030
let scheme = process.env.RABBITMQ_AMQP_SCHEME || 'amqp'
31+
console.log("Using AMQP protocol: " + scheme)
3132
switch(scheme){
3233
case "amqp":
3334
return getAmqpConnectionOptions()

‎selenium/test/authnz-msg-protocols/amqp10.js

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,17 @@ describe('Having AMQP 1.0 protocol enabled and the following auth_backends: ' +
2929
let expectations = []
3030
let username = process.env.RABBITMQ_AMQP_USERNAME
3131
let password = process.env.RABBITMQ_AMQP_PASSWORD
32+
let usemtls = process.env.AMQP_USE_MTLS
3233
let amqp;
3334

34-
before(function () {
35-
if (backends.includes("http") && username.includes("http")) {
35+
before(function () {
36+
if (backends.includes("http") && (username.includes("http") || usemtls)) {
3637
reset()
37-
expectations.push(expectUser({ "username": username, "password": password}, "allow"))
38+
if (!usemtls) {
39+
expectations.push(expectUser({ "username": username, "password": password}, "allow"))
40+
} else {
41+
expectations.push(expectUser({ "username": username}, "allow"))
42+
}
3843
expectations.push(expectVhost({ "username": username, "vhost": "/"}, "allow"))
3944
expectations.push(expectResource({ "username": username, "vhost": "/", "resource": "queue", "name": "my-queue", "permission":"configure", "tags":""}, "allow"))
4045
expectations.push(expectResource({ "username": username, "vhost": "/", "resource": "queue", "name": "my-queue", "permission":"read", "tags":""}, "allow"))

0 commit comments

Comments
 (0)