-
Notifications
You must be signed in to change notification settings - Fork 231
add api-key parameter #3690
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add api-key parameter #3690
Changes from 1 commit
4e9df48
9bb4cac
197109f
137a7bd
c335a5a
d9d22b0
3af935a
2342e80
f8e18d9
8168727
48e4013
5962b1f
5127127
d8af40b
ae6fe1a
66a9815
9e76259
7a69454
cf5eef9
e3c4e18
cbabe42
6607d66
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -124,6 +124,7 @@ const std::string HttpRestApiHandler::v3_RegexExp = | |||||
| const std::string HttpRestApiHandler::metricsRegexExp = R"((.?)\/metrics(\?(.*))?)"; | ||||||
|
|
||||||
| HttpRestApiHandler::HttpRestApiHandler(ovms::Server& ovmsServer, int timeout_in_ms) : | ||||||
| api_key(ovmsServer.getAPIKey()), | ||||||
| predictionRegex(predictionRegexExp), | ||||||
| modelstatusRegex(modelstatusRegexExp), | ||||||
| configReloadRegex(configReloadRegexExp), | ||||||
|
|
@@ -140,6 +141,7 @@ HttpRestApiHandler::HttpRestApiHandler(ovms::Server& ovmsServer, int timeout_in_ | |||||
| metricsRegex(metricsRegexExp), | ||||||
| timeout_in_ms(timeout_in_ms), | ||||||
| ovmsServer(ovmsServer), | ||||||
|
|
||||||
|
|
||||||
| kfsGrpcImpl(dynamic_cast<const GRPCServerModule*>(this->ovmsServer.getModule(GRPC_SERVER_MODULE_NAME))->getKFSGrpcImpl()), | ||||||
| grpcGetModelMetadataImpl(dynamic_cast<const GRPCServerModule*>(this->ovmsServer.getModule(GRPC_SERVER_MODULE_NAME))->getTFSModelMetadataImpl()), | ||||||
|
|
@@ -221,7 +223,7 @@ void HttpRestApiHandler::registerAll() { | |||||
| }); | ||||||
| registerHandler(V3, [this](const std::string_view uri, const HttpRequestComponents& request_components, std::string& response, const std::string& request_body, HttpResponseComponents& response_components, std::shared_ptr<HttpAsyncWriter> serverReaderWriter, std::shared_ptr<MultiPartParser> multiPartParser) -> Status { | ||||||
| OVMS_PROFILE_FUNCTION(); | ||||||
| return processV3(uri, request_components, response, request_body, std::move(serverReaderWriter), std::move(multiPartParser)); | ||||||
| return processV3(uri, request_components, response, request_body, std::move(serverReaderWriter), std::move(multiPartParser), api_key); | ||||||
| }); | ||||||
| registerHandler(Metrics, [this](const std::string_view uri, const HttpRequestComponents& request_components, std::string& response, const std::string& request_body, HttpResponseComponents& response_components, std::shared_ptr<HttpAsyncWriter> serverReaderWriter, std::shared_ptr<MultiPartParser> multiPartParser) -> Status { | ||||||
| return processMetrics(request_components, response, request_body); | ||||||
|
|
@@ -668,14 +670,31 @@ Status HttpRestApiHandler::processListModelsRequest(std::string& response) { | |||||
| return StatusCode::OK; | ||||||
| } | ||||||
|
|
||||||
| Status HttpRestApiHandler::processV3(const std::string_view uri, const HttpRequestComponents& request_components, std::string& response, const std::string& request_body, std::shared_ptr<HttpAsyncWriter> serverReaderWriter, std::shared_ptr<MultiPartParser> multiPartParser) { | ||||||
| Status HttpRestApiHandler::processV3(const std::string_view uri, const HttpRequestComponents& request_components, std::string& response, const std::string& request_body, std::shared_ptr<HttpAsyncWriter> serverReaderWriter, std::shared_ptr<MultiPartParser> multiPartParser, const std::string& api_key) { | ||||||
dtrawins marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| #if (MEDIAPIPE_DISABLE == 0) | ||||||
| OVMS_PROFILE_FUNCTION(); | ||||||
|
|
||||||
| HttpPayload request; | ||||||
| std::string modelName; | ||||||
| bool streamFieldVal = false; | ||||||
|
|
||||||
| // convert headers to lowercase because http headers are case insensitive | ||||||
| for (const auto& [key, value] : request_components.headers) { | ||||||
| std::string lowercaseKey = key; | ||||||
| std::transform(lowercaseKey.begin(), lowercaseKey.end(), lowercaseKey.begin(), | ||||||
| [](unsigned char c){ return std::tolower(c); }); | ||||||
| } | ||||||
| if (!api_key.empty()) { | ||||||
dtrawins marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| if (request_components.headers.count("authorization")) { | ||||||
| if (request_components.headers.at("authorization") != "Bearer " + api_key) { | ||||||
|
||||||
| if (request_components.headers.at("authorization") != "Bearer " + api_key) { | |
| if (request_components.headers.at("authorization") != "Bearer " + apiKey) { |
Is exact match safe here? No risk of some additional whitespaces?
Outdated
Copilot
AI
Oct 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Header lookup uses exact case 'authorization' but HTTP headers are case-insensitive. This will fail if the client sends 'Authorization' (capitalized). Use case-insensitive header lookup or convert the headers map to lowercase keys.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| SPDLOG_DEBUG("Unauthorized request - invalid API key {} instead of {}", request_components.headers.at("authorization"), api_key); | |
| SPDLOG_DEBUG("Unauthorized request - invalid API key {} instead of {}", request_components.headers.at("authorization"), apiKey); |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -302,6 +302,7 @@ Status Server::startModules(ovms::Config& config) { | |||||
| // that's why we delay starting the servable until the very end while we need to create it before | ||||||
| // GRPC & REST | ||||||
| Status status; | ||||||
| apiKey = config.apiKey(); | ||||||
| bool inserted = false; | ||||||
| auto it = modules.end(); | ||||||
| if (config.getServerSettings().serverMode == UNKNOWN_MODE) { | ||||||
|
|
@@ -371,6 +372,10 @@ void Server::ensureModuleShutdown(const std::string& name) { | |||||
| it->second->shutdown(); | ||||||
| } | ||||||
|
|
||||||
| std::string Server::getAPIKey() const { | ||||||
| return apiKey; | ||||||
| } | ||||||
|
||||||
| } | |
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -51,8 +51,11 @@ class Server { | |||||||||||||||
| virtual ~Server(); | ||||||||||||||||
| Status startModules(ovms::Config& config); | ||||||||||||||||
| void shutdownModules(); | ||||||||||||||||
| std::string getAPIKey() const; | ||||||||||||||||
| std::string setAPIKey(const std::string& newApiKey); | ||||||||||||||||
|
||||||||||||||||
| std::string setAPIKey(const std::string& newApiKey); | |
| std::string setAPIKey(const std::string& newApiKey) { | |
| std::unique_lock<std::shared_mutex> lock(modulesMtx); | |
| std::string oldApiKey = apiKey; | |
| apiKey = newApiKey; | |
| return oldApiKey; | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HttpServerModule has access to ovms::config in HTTPServerModule::start(ovms::Config)
Either extend ovms::createAndStartDrogonHttpServer() with additional parameter or wrap
apiKey together with restBindAddress,restPort, workersCount in helper struct nested inside server settings (HTTPServerConfig?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bump
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -133,6 +133,7 @@ const std::unordered_map<StatusCode, std::string> Status::statusMessageMap = { | |||||
| {StatusCode::UNKNOWN_REQUEST_COMPONENTS_TYPE, "Request components type not recognized"}, | ||||||
| {StatusCode::FAILED_TO_PARSE_MULTIPART_CONTENT_TYPE, "Request of multipart type but failed to parse"}, | ||||||
| {StatusCode::FAILED_TO_DEDUCE_MODEL_NAME_FROM_URI, "Failed to deduce model name from all possible ways"}, | ||||||
| {StatusCode::UNAUTHORIZED, "Unauthorized request due to invalid api-key"}, | ||||||
|
||||||
| {StatusCode::UNAUTHORIZED, "Unauthorized request due to invalid api-key"}, | |
| {StatusCode::UNAUTHORIZED, "Unauthorized request due to invalid or missing api-key"}, |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -176,6 +176,7 @@ enum class StatusCode { | |||||
| UNKNOWN_REQUEST_COMPONENTS_TYPE, /*!< Components type not recognized */ | ||||||
| FAILED_TO_PARSE_MULTIPART_CONTENT_TYPE, /*!< Request of multipart type but failed to parse */ | ||||||
| FAILED_TO_DEDUCE_MODEL_NAME_FROM_URI, /*!< Failed to deduce model name from all possible ways */ | ||||||
| UNAUTHORIZED, /*!< Unauthorized request due to invalid api-key*/ | ||||||
|
||||||
| UNAUTHORIZED, /*!< Unauthorized request due to invalid api-key*/ | |
| UNAUTHORIZED, /*!< Unauthorized request due to invalid or missing api-key*/ |
Uh oh!
There was an error while loading. Please reload this page.