Skip to content

Commit 48541c7

Browse files
romangeromange
committed
fix: authorize the http connection to call commands (#2863)
fix: authorize the http connection to call DF commands The assumption is that basic-auth already covers the authentication part. And thanks to @sunneydev for finding the bug and providing the tests. The tests actually uncovered another bug where we may parse partial http requests. This one is handled by romange/helio#243 Signed-off-by: Roman Gershman <[email protected]>
1 parent 5b7f315 commit 48541c7

File tree

5 files changed

+121
-92
lines changed

5 files changed

+121
-92
lines changed

‎helio

‎src/facade/dragonfly_connection.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -659,6 +659,9 @@ void Connection::HandleRequests() {
659659
HttpConnection http_conn{http_listener_};
660660
http_conn.SetSocket(peer);
661661
http_conn.set_user_data(cc_.get());
662+
663+
// We validate the http request using basic-auth inside HttpConnection::HandleSingleRequest.
664+
cc_->authenticated = true;
662665
auto ec = http_conn.ParseFromBuffer(io_buf_.InputBuffer());
663666
io_buf_.ConsumeInput(io_buf_.InputLen());
664667
if (!ec) {

‎src/server/http_api.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,9 @@ void HttpAPI(const http::QueryArgs& args, HttpRequest&& req, Service* service,
204204
}
205205
}
206206

207+
// TODO: to add a content-type/json check.
207208
if (!success) {
209+
VLOG(1) << "Invalid body " << body;
208210
auto response = http::MakeStringResponse(h2::status::bad_request);
209211
http::SetMime(http::kTextMime, &response);
210212
response.body() = "Failed to parse json\r\n";

‎tests/dragonfly/connection_test.py

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -746,31 +746,3 @@ async def client_pause():
746746

747747
assert not all.done()
748748
await all
749-
750-
751-
@dfly_args({"proactor_threads": "1", "expose_http_api": "true"})
752-
async def test_http(df_server: DflyInstance):
753-
client = df_server.client()
754-
async with ClientSession() as session:
755-
async with session.get(f"http://localhost:{df_server.port}") as resp:
756-
assert resp.status == 200
757-
758-
body = '["set", "foo", "МайяХилли", "ex", "100"]'
759-
async with session.post(f"http://localhost:{df_server.port}/api", data=body) as resp:
760-
assert resp.status == 200
761-
text = await resp.text()
762-
assert text.strip() == '{"result":"OK"}'
763-
764-
body = '["get", "foo"]'
765-
async with session.post(f"http://localhost:{df_server.port}/api", data=body) as resp:
766-
assert resp.status == 200
767-
text = await resp.text()
768-
assert text.strip() == '{"result":"МайяХилли"}'
769-
770-
body = '["foo", "bar"]'
771-
async with session.post(f"http://localhost:{df_server.port}/api", data=body) as resp:
772-
assert resp.status == 200
773-
text = await resp.text()
774-
assert text.strip() == '{"error": "unknown command `FOO`"}'
775-
776-
assert await client.ttl("foo") > 0

‎tests/dragonfly/http_conf_test.py

Lines changed: 115 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,77 +1,129 @@
11
import aiohttp
2+
from . import dfly_args
3+
from .instance import DflyInstance
24

35

4-
async def test_password(df_factory):
5-
with df_factory.create(port=1112, requirepass="XXX") as server:
6-
async with aiohttp.ClientSession() as session:
7-
resp = await session.get(f"http://localhost:{server.port}/")
8-
assert resp.status == 401
9-
async with aiohttp.ClientSession(
10-
auth=aiohttp.BasicAuth("default", "wrongpassword")
11-
) as session:
12-
resp = await session.get(f"http://localhost:{server.port}/")
13-
assert resp.status == 401
14-
async with aiohttp.ClientSession(auth=aiohttp.BasicAuth("default", "XXX")) as session:
15-
resp = await session.get(f"http://localhost:{server.port}/")
16-
assert resp.status == 200
6+
def get_http_session(*args):
7+
if args:
8+
return aiohttp.ClientSession(auth=aiohttp.BasicAuth(*args))
9+
return aiohttp.ClientSession()
1710

1811

19-
async def test_skip_metrics(df_factory):
20-
with df_factory.create(port=1112, admin_port=1113, requirepass="XXX") as server:
21-
async with aiohttp.ClientSession(auth=aiohttp.BasicAuth("whoops", "whoops")) as session:
22-
resp = await session.get(f"http://localhost:{server.port}/metrics")
23-
assert resp.status == 200
24-
async with aiohttp.ClientSession(auth=aiohttp.BasicAuth("whoops", "whoops")) as session:
25-
resp = await session.get(f"http://localhost:{server.admin_port}/metrics")
26-
assert resp.status == 200
12+
@dfly_args({"proactor_threads": "1", "requirepass": "XXX"})
13+
async def test_password(df_server: DflyInstance):
14+
async with get_http_session() as session:
15+
resp = await session.get(f"http://localhost:{df_server.port}/")
16+
assert resp.status == 401
17+
async with get_http_session("default", "wrongpassword") as session:
18+
resp = await session.get(f"http://localhost:{df_server.port}/")
19+
assert resp.status == 401
20+
async with get_http_session("default", "XXX") as session:
21+
resp = await session.get(f"http://localhost:{df_server.port}/")
22+
assert resp.status == 200
2723

2824

29-
async def test_no_password_main_port(df_factory):
30-
with df_factory.create(
31-
port=1112,
32-
) as server:
33-
async with aiohttp.ClientSession(auth=aiohttp.BasicAuth("default", "XXX")) as session:
34-
resp = await session.get(f"http://localhost:{server.port}/")
35-
assert resp.status == 200
36-
async with aiohttp.ClientSession(auth=aiohttp.BasicAuth("random")) as session:
37-
resp = await session.get(f"http://localhost:{server.port}/")
38-
assert resp.status == 200
39-
async with aiohttp.ClientSession() as session:
40-
resp = await session.get(f"http://localhost:{server.port}/")
41-
assert resp.status == 200
25+
@dfly_args({"proactor_threads": "1", "requirepass": "XXX", "admin_port": 1113})
26+
async def test_skip_metrics(df_server: DflyInstance):
27+
async with get_http_session("whoops", "whoops") as session:
28+
resp = await session.get(f"http://localhost:{df_server.port}/metrics")
29+
assert resp.status == 200
30+
async with get_http_session("whoops", "whoops") as session:
31+
resp = await session.get(f"http://localhost:{df_server.admin_port}/metrics")
32+
assert resp.status == 200
33+
34+
35+
async def test_no_password_main_port(df_server: DflyInstance):
36+
async with get_http_session("default", "XXX") as session:
37+
resp = await session.get(f"http://localhost:{df_server.port}/")
38+
assert resp.status == 200
39+
async with get_http_session("random") as session:
40+
resp = await session.get(f"http://localhost:{df_server.port}/")
41+
assert resp.status == 200
42+
async with get_http_session() as session:
43+
resp = await session.get(f"http://localhost:{df_server.port}/")
44+
assert resp.status == 200
45+
4246

47+
@dfly_args(
48+
{
49+
"proactor_threads": "1",
50+
"requirepass": "XXX",
51+
"admin_port": 1113,
52+
"primary_port_http_enabled": True,
53+
"admin_nopass": True,
54+
}
55+
)
56+
async def test_no_password_on_admin(df_server: DflyInstance):
57+
async with get_http_session("default", "XXX") as session:
58+
resp = await session.get(f"http://localhost:{df_server.admin_port}/")
59+
assert resp.status == 200
60+
async with get_http_session("random") as session:
61+
resp = await session.get(f"http://localhost:{df_server.admin_port}/")
62+
assert resp.status == 200
63+
async with get_http_session() as session:
64+
resp = await session.get(f"http://localhost:{df_server.admin_port}/")
65+
assert resp.status == 200
4366

44-
async def test_no_password_on_admin(df_factory):
45-
with df_factory.create(
46-
port=1112,
47-
admin_port=1113,
48-
requirepass="XXX",
49-
primary_port_http_enabled=True,
50-
admin_nopass=True,
51-
) as server:
52-
async with aiohttp.ClientSession(auth=aiohttp.BasicAuth("default", "XXX")) as session:
53-
resp = await session.get(f"http://localhost:{server.admin_port}/")
67+
68+
@dfly_args({"proactor_threads": "1", "requirepass": "XXX", "admin_port": 1113})
69+
async def test_password_on_admin(df_server: DflyInstance):
70+
async with get_http_session("default", "badpass") as session:
71+
resp = await session.get(f"http://localhost:{df_server.admin_port}/")
72+
assert resp.status == 401
73+
async with get_http_session() as session:
74+
resp = await session.get(f"http://localhost:{df_server.admin_port}/")
75+
assert resp.status == 401
76+
async with get_http_session("default", "XXX") as session:
77+
resp = await session.get(f"http://localhost:{df_server.admin_port}/")
78+
assert resp.status == 200
79+
80+
81+
@dfly_args({"proactor_threads": "1", "expose_http_api": "true"})
82+
async def test_no_password_on_http_api(df_server: DflyInstance):
83+
async with get_http_session("default", "XXX") as session:
84+
resp = await session.post(f"http://localhost:{df_server.port}/api", json=["ping"])
85+
assert resp.status == 200
86+
async with get_http_session("random") as session:
87+
resp = await session.post(f"http://localhost:{df_server.port}/api", json=["ping"])
88+
assert resp.status == 200
89+
async with get_http_session() as session:
90+
resp = await session.post(f"http://localhost:{df_server.port}/api", json=["ping"])
91+
assert resp.status == 200
92+
93+
94+
@dfly_args({"proactor_threads": "1", "expose_http_api": "true"})
95+
async def test_http_api(df_server: DflyInstance):
96+
client = df_server.client()
97+
async with get_http_session() as session:
98+
body = '["set", "foo", "МайяХилли", "ex", "100"]'
99+
async with session.post(f"http://localhost:{df_server.port}/api", data=body) as resp:
54100
assert resp.status == 200
55-
async with aiohttp.ClientSession(auth=aiohttp.BasicAuth("random")) as session:
56-
resp = await session.get(f"http://localhost:{server.admin_port}/")
101+
text = await resp.text()
102+
assert text.strip() == '{"result":"OK"}'
103+
104+
body = '["get", "foo"]'
105+
async with session.post(f"http://localhost:{df_server.port}/api", data=body) as resp:
57106
assert resp.status == 200
58-
async with aiohttp.ClientSession() as session:
59-
resp = await session.get(f"http://localhost:{server.admin_port}/")
107+
text = await resp.text()
108+
assert text.strip() == '{"result":"МайяХилли"}'
109+
110+
body = '["foo", "bar"]'
111+
async with session.post(f"http://localhost:{df_server.port}/api", data=body) as resp:
60112
assert resp.status == 200
113+
text = await resp.text()
114+
assert text.strip() == '{"error": "unknown command `FOO`"}'
61115

116+
assert await client.ttl("foo") > 0
62117

63-
async def test_password_on_admin(df_factory):
64-
with df_factory.create(
65-
port=1112,
66-
admin_port=1113,
67-
requirepass="XXX",
68-
) as server:
69-
async with aiohttp.ClientSession(auth=aiohttp.BasicAuth("default", "badpass")) as session:
70-
resp = await session.get(f"http://localhost:{server.admin_port}/")
71-
assert resp.status == 401
72-
async with aiohttp.ClientSession() as session:
73-
resp = await session.get(f"http://localhost:{server.admin_port}/")
74-
assert resp.status == 401
75-
async with aiohttp.ClientSession(auth=aiohttp.BasicAuth("default", "XXX")) as session:
76-
resp = await session.get(f"http://localhost:{server.admin_port}/")
77-
assert resp.status == 200
118+
119+
@dfly_args({"proactor_threads": "1", "expose_http_api": "true", "requirepass": "XXX"})
120+
async def test_password_on_http_api(df_server: DflyInstance):
121+
async with get_http_session("default", "badpass") as session:
122+
resp = await session.post(f"http://localhost:{df_server.port}/api", json=["ping"])
123+
assert resp.status == 401
124+
async with get_http_session() as session:
125+
resp = await session.post(f"http://localhost:{df_server.port}/api", json=["ping"])
126+
assert resp.status == 401
127+
async with get_http_session("default", "XXX") as session:
128+
resp = await session.post(f"http://localhost:{df_server.port}/api", json=["ping"])
129+
assert resp.status == 200

0 commit comments

Comments
 (0)