mirror of
https://github.com/DarkflameUniverse/DarkflameServer.git
synced 2026-10-02 02:43:44 +00:00
fix(dashboard): DataTables queries count as reads for read-only API keys
POST /api/tables/... only reads, so read-only keys may use it (and the API docs list it for them). Keys limited to some addresses can't open the WebSocket, whose address isn't checked, nor keys whose allowed paths leave out /ws. Refusals are audited without an account target. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -206,6 +206,9 @@ namespace ApiKeyService {
|
|||||||
std::lock_guard lock(state.mutex);
|
std::lock_guard lock(state.mutex);
|
||||||
const auto& cached = Lookup(state, DashboardAuthService::Sha256Hex(token));
|
const auto& cached = Lookup(state, DashboardAuthService::Sha256Hex(token));
|
||||||
if (!cached.key) return std::nullopt;
|
if (!cached.key) return std::nullopt;
|
||||||
|
// The WebSocket doesn't tell us the address, so a key limited to some addresses can't use it; one limited to
|
||||||
|
// some paths only if /ws is one of them
|
||||||
|
if (!cached.allowedIps.empty() || !ApiKeys::PathAllowed(cached.allowedPaths, "/ws")) return std::nullopt;
|
||||||
const auto owner = CheckKeyAndOwner(*cached.key);
|
const auto owner = CheckKeyAndOwner(*cached.key);
|
||||||
if (!owner) return std::nullopt;
|
if (!owner) return std::nullopt;
|
||||||
return Verified{ cached.key->accountId, owner->username, owner->gmLevel,
|
return Verified{ cached.key->accountId, owner->username, owner->gmLevel,
|
||||||
|
|||||||
@@ -44,7 +44,8 @@ namespace ApiKeyService {
|
|||||||
};
|
};
|
||||||
eResult Authenticate(const std::string& token, HTTPContext& context, HTTPReply& reply);
|
eResult Authenticate(const std::string& token, HTTPContext& context, HTTPReply& reply);
|
||||||
|
|
||||||
// For WebSocket connections: the owner and scope, without counting against the limits
|
// For WebSocket connections: the owner and scope, without counting against the limits. Keys limited to some
|
||||||
|
// addresses can't be used there (the address isn't known), nor keys whose allowed paths leave out /ws.
|
||||||
struct Verified {
|
struct Verified {
|
||||||
uint32_t accountId{};
|
uint32_t accountId{};
|
||||||
std::string username;
|
std::string username;
|
||||||
|
|||||||
@@ -109,14 +109,13 @@ namespace {
|
|||||||
if (!context.apiKey) return true;
|
if (!context.apiKey) return true;
|
||||||
const auto& key = *context.apiKey;
|
const auto& key = *context.apiKey;
|
||||||
if (ApiKeyService::SessionOnlyPath(doc.path)) return false;
|
if (ApiKeyService::SessionOnlyPath(doc.path)) return false;
|
||||||
if (key.readOnly && doc.method != "GET") return false;
|
if (key.readOnly && !doc.reads) return false;
|
||||||
return doc.permission.empty() ? (doc.minGmLevel <= 0 || key.allPermissions) : key.Has(doc.permission);
|
return doc.permission.empty() ? (doc.minGmLevel <= 0 || key.allPermissions) : key.Has(doc.permission);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Register a DataTables endpoint. The fetcher returns the DB layer's JSON string. Access: a GM level or a Perm.
|
// Register a DataTables endpoint. The fetcher returns the DB layer's JSON string. Access: a GM level or a Perm.
|
||||||
template<typename Access>
|
template<typename Access>
|
||||||
void TableRoute(const std::string& path, const Access& access, const std::string& description, TableFetcher fetcher) {
|
void TableRoute(const std::string& path, const Access& access, const std::string& description, TableFetcher fetcher) {
|
||||||
ReadRoutes reads; // the query is a POST body, but it only reads
|
|
||||||
Route(eHTTPMethod::POST, path, access, description, [fetcher = std::move(fetcher)](HTTPReply& reply, const HTTPContext& context) {
|
Route(eHTTPMethod::POST, path, access, description, [fetcher = std::move(fetcher)](HTTPReply& reply, const HTTPContext& context) {
|
||||||
const auto request = ParseDataTablesRequest(context.body);
|
const auto request = ParseDataTablesRequest(context.body);
|
||||||
const auto body = ParseBody(context);
|
const auto body = ParseBody(context);
|
||||||
|
|||||||
@@ -279,7 +279,7 @@ namespace ApiKeyRoutes {
|
|||||||
|
|
||||||
// Refusals of keys (their scope, read-only, addresses, paths, limits) go to the audit log, a minute apart at most
|
// Refusals of keys (their scope, read-only, addresses, paths, limits) go to the audit log, a minute apart at most
|
||||||
ApiKeyService::SetDeniedHook([](const HTTPContext& context, const std::string& reason) {
|
ApiKeyService::SetDeniedHook([](const HTTPContext& context, const std::string& reason) {
|
||||||
Audit(context, "api_key_denied", reason, AuditTarget::Account(context.accountId));
|
Audit(context, "api_key_denied", reason);
|
||||||
});
|
});
|
||||||
RequireAuthMiddleware::SetApiKeyDeniedHook([](const HTTPContext& context, const std::string& reason) { ApiKeyService::NoteDenied(context, reason); });
|
RequireAuthMiddleware::SetApiKeyDeniedHook([](const HTTPContext& context, const std::string& reason) { ApiKeyService::NoteDenied(context, reason); });
|
||||||
ApiKeyService::SetClientAddress([](const HTTPContext& context) { return ClientAddress(context); });
|
ApiKeyService::SetClientAddress([](const HTTPContext& context) { return ClientAddress(context); });
|
||||||
|
|||||||
@@ -35,8 +35,12 @@ namespace RouteUtils {
|
|||||||
std::vector<RouteDoc> g_RouteDocs;
|
std::vector<RouteDoc> g_RouteDocs;
|
||||||
int g_ReadRoutes = 0;
|
int g_ReadRoutes = 0;
|
||||||
|
|
||||||
std::shared_ptr<RequireAuthMiddleware> MakeRequireAuth(std::shared_ptr<RequireAuthMiddleware> middleware) {
|
bool Reads(eHTTPMethod method, const std::string& path) {
|
||||||
if (g_ReadRoutes > 0) middleware->SetReadsOnly();
|
return method == eHTTPMethod::GET || g_ReadRoutes > 0 || (method == eHTTPMethod::POST && path.starts_with("/api/tables/"));
|
||||||
|
}
|
||||||
|
|
||||||
|
std::shared_ptr<RequireAuthMiddleware> MakeRequireAuth(std::shared_ptr<RequireAuthMiddleware> middleware, eHTTPMethod method, const std::string& path) {
|
||||||
|
if (Reads(method, path)) middleware->SetReadsOnly();
|
||||||
return middleware;
|
return middleware;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -63,16 +67,16 @@ namespace RouteUtils {
|
|||||||
|
|
||||||
void Route(eHTTPMethod method, const std::string& path, int16_t minGmLevel, const std::string& description, Handler handler) {
|
void Route(eHTTPMethod method, const std::string& path, int16_t minGmLevel, const std::string& description, Handler handler) {
|
||||||
std::vector<MiddlewarePtr> middleware;
|
std::vector<MiddlewarePtr> middleware;
|
||||||
if (minGmLevel >= 0) middleware.push_back(MakeRequireAuth(std::make_shared<RequireAuthMiddleware>(static_cast<uint8_t>(minGmLevel))));
|
if (minGmLevel >= 0) middleware.push_back(MakeRequireAuth(std::make_shared<RequireAuthMiddleware>(static_cast<uint8_t>(minGmLevel)), method, path));
|
||||||
g_RouteDocs.push_back({ std::string(magic_enum::enum_name(method)), path, minGmLevel, description, "" });
|
g_RouteDocs.push_back({ std::string(magic_enum::enum_name(method)), path, minGmLevel, description, "", Reads(method, path) });
|
||||||
Register(method, path, std::move(middleware), std::move(handler));
|
Register(method, path, std::move(middleware), std::move(handler));
|
||||||
}
|
}
|
||||||
|
|
||||||
void Route(eHTTPMethod method, const std::string& path, const Perm& permission, const std::string& description, Handler handler) {
|
void Route(eHTTPMethod method, const std::string& path, const Perm& permission, const std::string& description, Handler handler) {
|
||||||
if (!Permissions::Find(permission.key)) LOG("Route %s uses unknown permission %s; nobody can use it", path.c_str(), permission.key.c_str());
|
if (!Permissions::Find(permission.key)) LOG("Route %s uses unknown permission %s; nobody can use it", path.c_str(), permission.key.c_str());
|
||||||
std::vector<MiddlewarePtr> middleware;
|
std::vector<MiddlewarePtr> middleware;
|
||||||
middleware.push_back(MakeRequireAuth(std::make_shared<RequireAuthMiddleware>(std::function<uint8_t()>([key = permission.key] { return Permissions::Level(key); }), permission.key)));
|
middleware.push_back(MakeRequireAuth(std::make_shared<RequireAuthMiddleware>(std::function<uint8_t()>([key = permission.key] { return Permissions::Level(key); }), permission.key), method, path));
|
||||||
g_RouteDocs.push_back({ std::string(magic_enum::enum_name(method)), path, Permissions::Level(permission.key), description, permission.key });
|
g_RouteDocs.push_back({ std::string(magic_enum::enum_name(method)), path, Permissions::Level(permission.key), description, permission.key, Reads(method, path) });
|
||||||
Register(method, path, std::move(middleware), std::move(handler));
|
Register(method, path, std::move(middleware), std::move(handler));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -243,6 +243,7 @@ namespace RouteUtils {
|
|||||||
int16_t minGmLevel;
|
int16_t minGmLevel;
|
||||||
std::string description;
|
std::string description;
|
||||||
std::string permission; // set for routes guarded by a permission; minGmLevel is then its level at the time
|
std::string permission; // set for routes guarded by a permission; minGmLevel is then its level at the time
|
||||||
|
bool reads{}; // makes no changes (a GET, or a POST that only reads): read-only API keys may use it
|
||||||
};
|
};
|
||||||
|
|
||||||
// A named permission (Permissions.h) guarding a route; its GM level can be changed while the server runs
|
// A named permission (Permissions.h) guarding a route; its GM level can be changed while the server runs
|
||||||
@@ -259,8 +260,8 @@ namespace RouteUtils {
|
|||||||
void Route(eHTTPMethod method, const std::string& path, int16_t minGmLevel, const std::string& description, Handler handler);
|
void Route(eHTTPMethod method, const std::string& path, int16_t minGmLevel, const std::string& description, Handler handler);
|
||||||
void Route(eHTTPMethod method, const std::string& path, const Perm& permission, const std::string& description, Handler handler);
|
void Route(eHTTPMethod method, const std::string& path, const Perm& permission, const std::string& description, Handler handler);
|
||||||
|
|
||||||
// Routes registered while one of these lives only read, even POSTs (DataTables and lookups send their query as a
|
// Routes registered while one of these lives only read, even POSTs (lookups that send their query as a body), so
|
||||||
// body), so read-only API keys may use them
|
// read-only API keys may use them. POST /api/tables/... (DataTables) always counts as a read.
|
||||||
struct ReadRoutes {
|
struct ReadRoutes {
|
||||||
ReadRoutes();
|
ReadRoutes();
|
||||||
~ReadRoutes();
|
~ReadRoutes();
|
||||||
|
|||||||
Reference in New Issue
Block a user