diff --git a/dDashboardServer/auth/ApiKeyService.cpp b/dDashboardServer/auth/ApiKeyService.cpp index 758e2eb6b..e7268b30d 100644 --- a/dDashboardServer/auth/ApiKeyService.cpp +++ b/dDashboardServer/auth/ApiKeyService.cpp @@ -206,6 +206,9 @@ namespace ApiKeyService { std::lock_guard lock(state.mutex); const auto& cached = Lookup(state, DashboardAuthService::Sha256Hex(token)); 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); if (!owner) return std::nullopt; return Verified{ cached.key->accountId, owner->username, owner->gmLevel, diff --git a/dDashboardServer/auth/ApiKeyService.h b/dDashboardServer/auth/ApiKeyService.h index 2a2e4ff6a..472126924 100644 --- a/dDashboardServer/auth/ApiKeyService.h +++ b/dDashboardServer/auth/ApiKeyService.h @@ -44,7 +44,8 @@ namespace ApiKeyService { }; 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 { uint32_t accountId{}; std::string username; diff --git a/dDashboardServer/routes/APIRoutes.cpp b/dDashboardServer/routes/APIRoutes.cpp index ac3a9e535..508cc61ac 100644 --- a/dDashboardServer/routes/APIRoutes.cpp +++ b/dDashboardServer/routes/APIRoutes.cpp @@ -109,14 +109,13 @@ namespace { if (!context.apiKey) return true; const auto& key = *context.apiKey; 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); } // Register a DataTables endpoint. The fetcher returns the DB layer's JSON string. Access: a GM level or a Perm. template 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) { const auto request = ParseDataTablesRequest(context.body); const auto body = ParseBody(context); diff --git a/dDashboardServer/routes/ApiKeyRoutes.cpp b/dDashboardServer/routes/ApiKeyRoutes.cpp index 1f153e413..d2ee652a9 100644 --- a/dDashboardServer/routes/ApiKeyRoutes.cpp +++ b/dDashboardServer/routes/ApiKeyRoutes.cpp @@ -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 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); }); ApiKeyService::SetClientAddress([](const HTTPContext& context) { return ClientAddress(context); }); diff --git a/dDashboardServer/routes/RouteUtils.cpp b/dDashboardServer/routes/RouteUtils.cpp index 64a5daf72..f5f613240 100644 --- a/dDashboardServer/routes/RouteUtils.cpp +++ b/dDashboardServer/routes/RouteUtils.cpp @@ -35,8 +35,12 @@ namespace RouteUtils { std::vector g_RouteDocs; int g_ReadRoutes = 0; - std::shared_ptr MakeRequireAuth(std::shared_ptr middleware) { - if (g_ReadRoutes > 0) middleware->SetReadsOnly(); + bool Reads(eHTTPMethod method, const std::string& path) { + return method == eHTTPMethod::GET || g_ReadRoutes > 0 || (method == eHTTPMethod::POST && path.starts_with("/api/tables/")); + } + + std::shared_ptr MakeRequireAuth(std::shared_ptr middleware, eHTTPMethod method, const std::string& path) { + if (Reads(method, path)) middleware->SetReadsOnly(); 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) { std::vector middleware; - if (minGmLevel >= 0) middleware.push_back(MakeRequireAuth(std::make_shared(static_cast(minGmLevel)))); - g_RouteDocs.push_back({ std::string(magic_enum::enum_name(method)), path, minGmLevel, description, "" }); + if (minGmLevel >= 0) middleware.push_back(MakeRequireAuth(std::make_shared(static_cast(minGmLevel)), method, path)); + 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)); } 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()); std::vector middleware; - middleware.push_back(MakeRequireAuth(std::make_shared(std::function([key = permission.key] { return Permissions::Level(key); }), permission.key))); - g_RouteDocs.push_back({ std::string(magic_enum::enum_name(method)), path, Permissions::Level(permission.key), description, permission.key }); + middleware.push_back(MakeRequireAuth(std::make_shared(std::function([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, Reads(method, path) }); Register(method, path, std::move(middleware), std::move(handler)); } diff --git a/dDashboardServer/routes/RouteUtils.h b/dDashboardServer/routes/RouteUtils.h index a13ac5f25..f0d242e09 100644 --- a/dDashboardServer/routes/RouteUtils.h +++ b/dDashboardServer/routes/RouteUtils.h @@ -243,6 +243,7 @@ namespace RouteUtils { int16_t minGmLevel; std::string description; 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 @@ -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, 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 - // body), so read-only API keys may use them + // Routes registered while one of these lives only read, even POSTs (lookups that send their query as a body), so + // read-only API keys may use them. POST /api/tables/... (DataTables) always counts as a read. struct ReadRoutes { ReadRoutes(); ~ReadRoutes();