Add capability checking to state API handlers

This covers both the entry points from the `storagenode` and
`searchnode` HTTP servers, though the former is mostly in the
name of legacy support.

Ideally, capability checking would exist as a property of the
HTTP server (Portal) bindings, but the abstractions for the
JSON request handling are sufficiently leaky that it ended up
making more sense to push things further down the hierarchy.
It's always a good thing to move away from using strings with
implicit semantics as return types anyway.

The `searchnode` state API handler mapping supports fine grained
capabilities. The legacy `storagenode` state API forwarding does
not; it uses a sledgehammer that expects the union of all possible
API capability requirements.

4 files changed, 25 insertions, 6 deletions

diff --git a/storage/src/tests/storageserver/statereportertest.cpp b/storage/src/tests/storageserver/statereportertest.cpp
index 380604ae77b..f5512fb193d 100644
--- a/storage/src/tests/storageserver/statereportertest.cpp
+++ b/storage/src/tests/storageserver/statereportertest.cpp
@@ -193,7 +193,7 @@ TEST_F(StateReporterTest, report_health) {
     for (int i=0; i<stateCount; i++) {
         _node->getStateUpdater().setCurrentNodeState(nodeStates[i]);
         std::ostringstream ost;
-        _stateReporter->reportStatus(ost, path);
+        ASSERT_TRUE(_stateReporter->reportStatus(ost, path));
         std::string jsonData = ost.str();
         ASSERT_NODE_STATUS(jsonData, codes[i], messages[i]);
     }
diff --git a/storage/src/vespa/storage/storageserver/statereporter.cpp b/storage/src/vespa/storage/storageserver/statereporter.cpp
index 16de56fad22..8548590ea0b 100644
--- a/storage/src/vespa/storage/storageserver/statereporter.cpp
+++ b/storage/src/vespa/storage/storageserver/statereporter.cpp
@@ -6,6 +6,7 @@
 #include <vespa/metrics/metricmanager.h>
 #include <vespa/storage/common/nodestateupdater.h>
 #include <vespa/vdslib/state/nodestate.h>
+#include <vespa/vespalib/net/connection_auth_context.h>
 #include <vespa/vespalib/stllike/asciistream.h>
 
 #include <vespa/log/log.h>
@@ -57,11 +58,18 @@ bool
 StateReporter::reportStatus(std::ostream& out,
                             const framework::HttpUrlPath& path) const
 {
-    vespalib::string status = _stateApi.get(path.getServerSpec(), path.getPath(), getParams(path));
-    if (status.empty()) {
+    // When we get here, capabilities have already been checked at a higher level, so
+    // this will never fail unless a state API handler requires other capabilities than
+    // we require for this reporter (in which case this will silently fail, but not
+    // expose any other information).
+    vespalib::net::ConnectionAuthContext dummy_ctx(vespalib::net::tls::PeerCredentials(), required_capabilities());
+    auto status = _stateApi.get(path.getServerSpec(), path.getPath(), getParams(path), dummy_ctx);
+    if (status.failed()) {
+        LOG(debug, "State API reporting for path '%s' failed with status HTTP %d: %s",
+            path.getPath().c_str(), status.status_code(), vespalib::string(status.status_message()).c_str());
         return false;
     }
-    out << status;
+    out << status.payload();
     return true;
 }
 
diff --git a/storage/src/vespa/storage/storageserver/statereporter.h b/storage/src/vespa/storage/storageserver/statereporter.h
index 25ad6814dad..7edc6dd3aac 100644
--- a/storage/src/vespa/storage/storageserver/statereporter.h
+++ b/storage/src/vespa/storage/storageserver/statereporter.h
@@ -34,10 +34,21 @@ public:
             metrics::MetricManager&,
             ApplicationGenerationFetcher& generationFetcher,
             const std::string& name = "status");
-    ~StateReporter();
+    ~StateReporter() override;
 
     vespalib::string getReportContentType(const framework::HttpUrlPath&) const override;
     bool reportStatus(std::ostream& out, const framework::HttpUrlPath& path) const override;
+
+    // Since we forward to the state API handlers, we require a union of the capabilities
+    // required for the content status pages _as well as_ those needed by the state API handlers.
+    // We only half-heartedly want to support the legacy state v1 mapping via the storagenode
+    // status HTTP server; everyone should use the searchnode HTTP server instead.
+    CapabilitySet required_capabilities() const noexcept override {
+        return StatusReporter::required_capabilities().union_of(CapabilitySet::of({
+            Capability::content_state_api(),
+            Capability::content_metrics_api()
+        }));
+    }
 private:
     metrics::MetricManager &_manager;
     metrics::StateApiAdapter _metricsAdapter;
diff --git a/storage/src/vespa/storageframework/generic/status/statusreportermap.h b/storage/src/vespa/storageframework/generic/status/statusreportermap.h
index 8d72215df49..2388dc0e977 100644
--- a/storage/src/vespa/storageframework/generic/status/statusreportermap.h
+++ b/storage/src/vespa/storageframework/generic/status/statusreportermap.h
@@ -14,7 +14,7 @@ namespace storage::framework {
 struct StatusReporter;
 
 struct StatusReporterMap {
-    virtual ~StatusReporterMap() {}
+    virtual ~StatusReporterMap() = default;
 
     virtual const StatusReporter* getStatusReporter(vespalib::stringref id) = 0;