Merge pull request #104 from mharrisb1/87-allow-the-user-to-provide-access-key-via-parameter

feat(auth): allow providing raw private key and email instead of filepath

Author: Michael Harris

CMakeLists.txt

@@ -36,6 +36,7 @@ set(EXTENSION_SOURCES
     src/sheets/range.cpp
     src/sheets/auth_factory.cpp
     src/utils/secret.cpp
+    src/utils/options.cpp
     src/utils/proxy.cpp
     src/utils/version.cpp)
 

docs/pages/index.md

@@ -50,6 +50,14 @@ CREATE SECRET (
     PROVIDER key_file,
     FILEPATH '<path_to_JSON_file_with_private_key>'
 );
+
+-- OR by passing the secret directly
+CREATE SECRET (
+    TYPE gsheet,
+    PROVIDER key_file,
+    EMAIL '<service_account_email>',
+    SECRET '<private_key>'
+);
 ```
 
 ### HTTP Proxy

src/gsheets_auth.cpp

@@ -2,10 +2,11 @@
 #include <cstdlib>
 #include <json.hpp>
 
-#include "duckdb.hpp"
+#include "duckdb/common/exception/binder_exception.hpp"
 
 #include "gsheets_auth.hpp"
 #include "gsheets_utils.hpp"
+#include "utils/options.hpp"
 
 using json = nlohmann::json;
 
@@ -30,7 +31,6 @@ static void RedactCommonKeys(KeyValueSecret &result) {
 	result.redact_keys.insert("proxy_password");
 }
 
-// TODO: Maybe this should be a KeyValueSecret
 static unique_ptr<BaseSecret> CreateGsheetSecretFromAccessToken(ClientContext &context, CreateSecretInput &input) {
 	auto scope = input.scope;
 
@@ -63,23 +63,31 @@ static unique_ptr<BaseSecret> CreateGsheetSecretFromOAuth(ClientContext &context
 	return std::move(result);
 }
 
-// TODO: Maybe this should be a KeyValueSecret
 static unique_ptr<BaseSecret> CreateGsheetSecretFromKeyFile(ClientContext &context, CreateSecretInput &input) {
 	auto scope = input.scope;
 
 	auto result = make_uniq<KeyValueSecret>(scope, input.type, input.provider, input.name);
 
-	// Want to store the private key and email in case the secret is persisted
-	std::string filepath_key = "filepath";
-	auto filepath = (input.options.find(filepath_key)->second).ToString();
-
-	std::ifstream ifs(filepath);
-	if (!ifs.is_open()) {
-		throw IOException("Could not open JSON key file at: " + filepath);
+	std::string email, secret;
+	auto filepath = duckdb::sheets::GetStringOption(input.options, "filepath");
+	if (filepath.empty()) {
+		email = duckdb::sheets::GetStringOption(input.options, "email");
+		if (email.empty()) {
+			throw BinderException("Must provide email if not using filepath");
+		}
+		secret = duckdb::sheets::GetStringOption(input.options, "secret");
+		if (email.empty()) {
+			throw BinderException("Must provide secret value if not using filepath");
+		}
+	} else {
+		std::ifstream ifs(filepath);
+		if (!ifs.is_open()) {
+			throw IOException("Could not open JSON key file at: " + filepath);
+		}
+		json credentials_file = json::parse(ifs);
+		email = credentials_file["client_email"].get<std::string>();
+		secret = credentials_file["private_key"].get<std::string>();
 	}
-	json credentials_file = json::parse(ifs);
-	std::string email = credentials_file["client_email"].get<std::string>();
-	std::string secret = credentials_file["private_key"].get<std::string>();
 
 	// Manage specific secret option
 	(*result).secret_map["email"] = Value(email);
@@ -119,6 +127,8 @@ void CreateGsheetSecretFunctions::Register(ExtensionLoader &loader) {
 	// Register the key_file secret provider
 	CreateSecretFunction key_file_function = {type, "key_file", CreateGsheetSecretFromKeyFile, {}};
 	key_file_function.named_parameters["filepath"] = LogicalType::VARCHAR;
+	key_file_function.named_parameters["email"] = LogicalType::VARCHAR;
+	key_file_function.named_parameters["secret"] = LogicalType::VARCHAR;
 	RegisterCommonSecretParameters(key_file_function);
 
 	loader.RegisterSecretType(secret_type);

src/gsheets_copy.cpp

@@ -1,6 +1,5 @@
 #include <utility>
 
-#include "duckdb/common/case_insensitive_map.hpp"
 #include "duckdb/common/exception.hpp"
 #include "duckdb/common/exception/binder_exception.hpp"
 #include "duckdb/common/file_system.hpp"
@@ -10,6 +9,8 @@
 #include "gsheets_copy.hpp"
 #include "gsheets_utils.hpp"
 
+#include "utils/options.hpp"
+
 #include "sheets/auth_factory.hpp"
 #include "sheets/client.hpp"
 #include "sheets/exception.hpp"
@@ -26,58 +27,20 @@ GSheetCopyFunction::GSheetCopyFunction() : CopyFunction("gsheet") {
 	copy_to_sink = GSheetWriteSink;
 }
 
-static std::string GetStringOption(const case_insensitive_map_t<vector<Value>> &options, const std::string &name,
-                                   const std::string &default_value = "") {
-	const auto it = options.find(name);
-	if (it == options.end()) {
-		return default_value;
-	}
-	std::string err;
-	Value val;
-	if (!it->second.back().DefaultTryCastAs(LogicalType::VARCHAR, val, &err)) {
-		throw BinderException(name + " option must be VARCHAR");
-	}
-	if (val.IsNull()) {
-		throw BinderException(name + " option must not be NULL");
-	}
-	return StringValue::Get(val);
-}
-
-// NOTE: the second value in pair is a flag indicating if the value was set by the user
-static std::pair<bool, bool> GetBoolOption(const case_insensitive_map_t<vector<Value>> &options,
-                                           const std::string &name, bool default_value = false) {
-	const auto it = options.find(name);
-	if (it == options.end()) {
-		return std::make_pair(default_value, false);
-	}
-	if (it->second.size() != 1) {
-		throw BinderException(name + " option must be a single boolean value");
-	}
-	std::string err;
-	Value val;
-	if (!it->second.back().DefaultTryCastAs(LogicalType::BOOLEAN, val, &err)) {
-		throw BinderException(name + " option must be a single boolean value");
-	}
-	if (val.IsNull()) {
-		throw BinderException(name + " option must be a single boolean value");
-	}
-	return std::make_pair(BooleanValue::Get(val), true);
-}
-
 unique_ptr<FunctionData> GSheetCopyFunction::GSheetWriteBind(ClientContext &context, CopyFunctionBindInput &input,
                                                              const vector<string> &names,
                                                              const vector<LogicalType> &sql_types) {
 
 	string file_path = input.info.file_path;
 	auto options = input.info.options;
 
-	auto sheet = GetStringOption(options, "sheet");
-	auto range = GetStringOption(options, "range");
-	bool overwrite_sheet = GetBoolOption(options, "overwrite_sheet", true).first;
-	bool overwrite_range = GetBoolOption(options, "overwrite_range", false).first;
-	bool create_if_not_exists = GetBoolOption(options, "create_if_not_exists", false).first;
+	auto sheet = duckdb::sheets::GetStringOption(options, "sheet");
+	auto range = duckdb::sheets::GetStringOption(options, "range");
+	bool overwrite_sheet = duckdb::sheets::GetBoolOption(options, "overwrite_sheet", true).first;
+	bool overwrite_range = duckdb::sheets::GetBoolOption(options, "overwrite_range", false).first;
+	bool create_if_not_exists = duckdb::sheets::GetBoolOption(options, "create_if_not_exists", false).first;
 
-	auto header_result = GetBoolOption(options, "header", true);
+	auto header_result = duckdb::sheets::GetBoolOption(options, "header", true);
 	bool header = header_result.second ? header_result.first : (overwrite_range || overwrite_sheet);
 
 	if (create_if_not_exists && sheet.empty()) {

src/include/sheets/util/encoding.hpp

@@ -16,5 +16,7 @@ std::string Base64UrlEncode(const unsigned char *data, size_t len);
 
 std::string Base64UrlEncode(const std::string &input);
 
+std::string NormalizePemKey(const std::string &key);
+
 } // namespace sheets
 } // namespace duckdb

src/include/utils/options.hpp

@@ -0,0 +1,24 @@
+#pragma once
+
+#include <string>
+#include <utility>
+
+#include "duckdb/common/types/value.hpp"
+
+namespace duckdb {
+namespace sheets {
+
+std::string GetStringOption(const case_insensitive_map_t<vector<Value>> &options, const std::string &name,
+                            const std::string &default_value = "");
+
+std::string GetStringOption(const case_insensitive_map_t<Value> &options, const std::string &name,
+                            const std::string &default_value = "");
+
+std::pair<bool, bool> GetBoolOption(const case_insensitive_map_t<vector<Value>> &options, const std::string &name,
+                                    bool default_value = false);
+
+std::pair<bool, bool> GetBoolOption(const case_insensitive_map_t<Value> &options, const std::string &name,
+                                    bool default_value = false);
+
+} // namespace sheets
+} // namespace duckdb

src/sheets/auth/service_account_auth.cpp

@@ -66,8 +66,10 @@ std::string ServiceAccountAuth::CreateJwt() {
 	std::string claimsB64 = Base64UrlEncode(claimSet.dump());
 	std::string signInput = headerB64 + "." + claimsB64;
 
+	auto pem = NormalizePemKey(privateKey);
+
 	// Parse PEM private key into EVP_PKEY (RAII handles cleanup)
-	BIOPtr bio(BIO_new_mem_buf(privateKey.c_str(), -1));
+	BIOPtr bio(BIO_new_mem_buf(pem.c_str(), -1));
 	if (!bio) {
 		throw duckdb::IOException("Failed to create BIO for private key");
 	}

src/sheets/util/encoding.cpp

@@ -42,5 +42,15 @@ std::string Base64UrlEncode(const std::string &input) {
 	return Base64UrlEncode(reinterpret_cast<const unsigned char *>(input.c_str()), input.length());
 }
 
+std::string NormalizePemKey(const std::string &key) {
+	std::string pem = key;
+	size_t pos = 0;
+	while ((pos = pem.find("\\n", pos)) != std::string::npos) {
+		pem.replace(pos, 2, "\n");
+		pos += 1;
+	}
+	return pem;
+}
+
 } // namespace sheets
 } // namespace duckdb

src/utils/options.cpp

@@ -0,0 +1,75 @@
+#include "duckdb/common/exception/binder_exception.hpp"
+#include "duckdb/common/types/value.hpp"
+
+#include "utils/options.hpp"
+
+std::string duckdb::sheets::GetStringOption(const case_insensitive_map_t<vector<Value>> &options,
+                                            const std::string &name, const std::string &default_value) {
+	const auto it = options.find(name);
+	if (it == options.end()) {
+		return default_value;
+	}
+	std::string err;
+	Value val;
+	if (!it->second.back().DefaultTryCastAs(LogicalType::VARCHAR, val, &err)) {
+		throw BinderException(name + " option must be VARCHAR");
+	}
+	if (val.IsNull()) {
+		throw BinderException(name + " option must not be NULL");
+	}
+	return StringValue::Get(val);
+}
+
+std::string duckdb::sheets::GetStringOption(const case_insensitive_map_t<Value> &options, const std::string &name,
+                                            const std::string &default_value) {
+	const auto it = options.find(name);
+	if (it == options.end()) {
+		return default_value;
+	}
+	std::string err;
+	Value val;
+	if (!it->second.DefaultTryCastAs(LogicalType::VARCHAR, val, &err)) {
+		throw BinderException(name + " option must be VARCHAR");
+	}
+	if (val.IsNull()) {
+		throw BinderException(name + " option must not be NULL");
+	}
+	return StringValue::Get(val);
+}
+
+std::pair<bool, bool> duckdb::sheets::GetBoolOption(const case_insensitive_map_t<vector<Value>> &options,
+                                                    const std::string &name, bool default_value) {
+	const auto it = options.find(name);
+	if (it == options.end()) {
+		return std::make_pair(default_value, false);
+	}
+	if (it->second.size() != 1) {
+		throw BinderException(name + " option must be a single boolean value");
+	}
+	std::string err;
+	Value val;
+	if (!it->second.back().DefaultTryCastAs(LogicalType::BOOLEAN, val, &err)) {
+		throw BinderException(name + " option must be a single boolean value");
+	}
+	if (val.IsNull()) {
+		throw BinderException(name + " option must be a single boolean value");
+	}
+	return std::make_pair(BooleanValue::Get(val), true);
+}
+
+std::pair<bool, bool> duckdb::sheets::GetBoolOption(const case_insensitive_map_t<Value> &options,
+                                                    const std::string &name, bool default_value) {
+	const auto it = options.find(name);
+	if (it == options.end()) {
+		return std::make_pair(default_value, false);
+	}
+	std::string err;
+	Value val;
+	if (!it->second.DefaultTryCastAs(LogicalType::BOOLEAN, val, &err)) {
+		throw BinderException(name + " option must be a single boolean value");
+	}
+	if (val.IsNull()) {
+		throw BinderException(name + " option must be a single boolean value");
+	}
+	return std::make_pair(BooleanValue::Get(val), true);
+}

test/unit/sheets/util/test_encoding.cpp

@@ -67,3 +67,23 @@ TEST_CASE("Base64UrlEncode JWT header", "[encoding]") {
 	std::string header = R"({"alg":"RS256","typ":"JWT"})";
 	REQUIRE(duckdb::sheets::Base64UrlEncode(header) == "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9");
 }
+
+TEST_CASE("NormalizePemKey Key with literal \\n sequences") {
+	std::string input = "-----BEGIN PRIVATE KEY-----\\nMIIE...\\n-----END PRIVATE KEY-----\\n";
+	std::string expected = "-----BEGIN PRIVATE KEY-----\nMIIE...\n-----END PRIVATE KEY-----\n";
+	REQUIRE(duckdb::sheets::NormalizePemKey(input) == expected);
+}
+
+TEST_CASE("NormalizePemKey Key already has real newlines") {
+	std::string input = "-----BEGIN PRIVATE KEY-----\nMIIE...\n-----END PRIVATE KEY-----\n";
+	REQUIRE(duckdb::sheets::NormalizePemKey(input) == input);
+}
+
+TEST_CASE("NormalizePemKey empty string") {
+	REQUIRE(duckdb::sheets::NormalizePemKey("") == "");
+}
+
+TEST_CASE("NormalizePemKey no newlines at all") {
+	std::string input = "just-a-string-with-no-newlines";
+	REQUIRE(duckdb::sheets::NormalizePemKey(input) == input);
+}