Skip to content

Commit 1e5d043

Browse files
Rigbyfab4claude
andcommitted
feat(bq_driver): add AllowedProjects DSN option to scope SQLTables
Power Query's Navigator opens a connection and immediately calls SQLTables with wildcard catalog/schema arguments. GetFilteredProjectIds answered that by enumerating every project the authenticated principal can reach via the projects.list REST API, then listing datasets for each one. In a large enterprise organization the principal can typically *see* far more projects than it holds BigQuery permissions on. Listing datasets for such a project fails with a permission error, and because GetResultSetForDatasets surfaces the first error from its parallel fan-out, one inaccessible project fails the whole SQLTables call. The result is that the Navigator tree cannot be built at all, even though the user only wanted the handful of projects they do have access to. Enumeration is also slow and quota-heavy at that scale, but the permission failure is the blocking issue. Add an AllowedProjects DSN option naming the projects to expose. When set, GetFilteredProjectIds filters that list and skips projects.list entirely, so no inaccessible project is ever contacted. The existing filter semantics are preserved: entries are matched against the catalog argument with the same LIKE pattern / SQL_ATTR_METADATA_ID rules used elsewhere. AllowedProjects and AdditionalProjects stay orthogonal and composable: if both are set, enumeration starts from the allowlist and still appends the AdditionalProjects ids on top. Also add a pick list to the Advanced Options dialog so non-technical users need not hand-type project ids. "Load Projects" reuses the existing DriverForm::GetCatalogAndDataset path to populate a checkbox list view, preserving ticks across reloads and keeping saved ids the account no longer reports. The DSN key remains usable headlessly for automation. Tests: unit coverage for FilterAllowedProjects and ALLOWEDPROJECTS parsing, a dialog round-trip test, and an integration test asserting that a project the principal can reach but did not allow-list is absent from SQLTables. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 59fdbb8 commit 1e5d043

17 files changed

Lines changed: 439 additions & 18 deletions

google/cloud/odbc/bq_driver/internal/driver_adv_opt_form.cc

Lines changed: 161 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,10 @@
1313
// limitations under the License.
1414

1515
#include "google/cloud/odbc/bq_driver/internal/driver_adv_opt_form.h"
16+
#include "google/cloud/odbc/bq_driver/internal/driver_form.h"
1617
#include "google/cloud/odbc/bq_driver/internal/odbc_internal_commons.h"
1718
#include "google/cloud/odbc/bq_driver/internal/trace_utils.h"
19+
#include <set>
1820
#include <shellapi.h>
1921

2022
namespace google::cloud::odbc_bq_driver_internal {
@@ -53,6 +55,7 @@ std::string AdvanceOptions::rows_per_block_ = kDefaultRowsPerBlock;
5355
std::string AdvanceOptions::default_string_length_ = kDefaultStringLength;
5456
std::string AdvanceOptions::session_location_;
5557
std::string AdvanceOptions::additional_projects_;
58+
std::string AdvanceOptions::allowed_projects_;
5659
std::string AdvanceOptions::query_properties_;
5760
std::string AdvanceOptions::use_wchar_;
5861
std::string AdvanceOptions::enable_session_;
@@ -75,6 +78,7 @@ std::string const kLargeResultsTempTableExpirationTime =
7578
"LargeResultsTempTableExpirationTime";
7679
std::string const kSessionLocation = "SessionLocation";
7780
std::string const kAdditionalProjects = "AdditionalProjects";
81+
std::string const kAllowedProjects = "AllowedProjects";
7882
std::string const kQueryProperties = "QueryProperties";
7983
std::string const kUseWChar = "UseWVarChar";
8084
std::string const kEnableSession = "EnableSession";
@@ -97,7 +101,11 @@ int const kButtonWidth = 68;
97101
int const kXAxis = 10;
98102
int const kOkButtonX = 330;
99103
int const kCancelButtonX = 410;
100-
int const kButtonY = 613;
104+
// The allowed-projects pick list and its "Load Projects" button occupy the
105+
// space below the query-properties box, so the OK/Cancel row sits lower than
106+
// the other control offsets in this file would suggest.
107+
int const kButtonY = 753;
108+
int const kAllowedProjectsListHeight = 100;
101109
int const kYAxis = 20;
102110
int const kEditBoxWidth = 260;
103111
int const kEditBoxHeight = 17;
@@ -428,6 +436,25 @@ void AdvanceOptions::CreateAdditionalControls(HFONT h_font) {
428436
SetWindowSubclass(GetDlgItem(adv_hwnd, kIdcQueryPropertiesEdit),
429437
InputSubclassProc, 0, 0);
430438

439+
HWND h_allowed_projects_label = CreateLabel(
440+
adv_hwnd, "Allowed projects (all accessible if none checked):", kXAxis,
441+
kYAxis + 635, kWidth * 6, kHeight, WS_VISIBLE | SS_LEFT);
442+
SendMessage(h_allowed_projects_label, WM_SETFONT, (WPARAM)h_font, TRUE);
443+
444+
HWND h_load_projects_button =
445+
CreateButton(adv_hwnd, "Load Projects", kXAxis + 400, kYAxis + 633,
446+
kButtonWidth + 22, kButtonHeight, kIdcLoadProjectsButton);
447+
SendMessage(h_load_projects_button, WM_SETFONT, (WPARAM)h_font, TRUE);
448+
449+
HWND h_allowed_projects_list_view =
450+
CreateListView(adv_hwnd, kXAxis, kYAxis + 655, kWidth + 445,
451+
kAllowedProjectsListHeight, kIdcAllowedProjectsListView);
452+
SendMessage(h_allowed_projects_list_view, WM_SETFONT, (WPARAM)h_font, TRUE);
453+
// Without contacting the account, the ids saved in the DSN are all we know.
454+
// Show them ticked so OK round-trips the value even if the user never
455+
// presses "Load Projects".
456+
PopulateAllowedProjectsListView(h_allowed_projects_list_view, {});
457+
431458
// This feature is turned off for the private release. It will be restored for
432459
// the public release with an accompanying documentation link.
433460
// TODO(b/461668255):Restore BigQuery documentation URL
@@ -443,6 +470,77 @@ void AdvanceOptions::CreateAdditionalControls(HFONT h_font) {
443470
// SendMessage(h_hyperlink, WM_SETFONT, (WPARAM)h_font, TRUE);
444471
}
445472

473+
void AdvanceOptions::PopulateAllowedProjectsListView(
474+
HWND h_list_view, std::vector<std::string> const& project_ids) {
475+
if (!h_list_view) {
476+
return;
477+
}
478+
479+
// Ticked ids must survive a reload, whether they came from the saved DSN or
480+
// from ticks the user made before pressing "Load Projects".
481+
std::set<std::string> checked;
482+
for (auto& project_id : Split(allowed_projects_, ",")) {
483+
Trim(project_id);
484+
if (!project_id.empty()) {
485+
checked.insert(project_id);
486+
}
487+
}
488+
int const existing_count = ListView_GetItemCount(h_list_view);
489+
for (int i = 0; i < existing_count; ++i) {
490+
char buffer[256] = {0};
491+
ListView_GetItemText(h_list_view, i, 0, buffer,
492+
static_cast<int>(sizeof(buffer)));
493+
if (buffer[0] != '\0' && ListView_GetCheckState(h_list_view, i)) {
494+
checked.insert(buffer);
495+
}
496+
}
497+
498+
// Keep ticked ids the account no longer reports, so reloading the list never
499+
// silently drops a value the user had already saved.
500+
std::vector<std::string> rows = project_ids;
501+
std::set<std::string> const listed(project_ids.begin(), project_ids.end());
502+
for (auto const& project_id : checked) {
503+
if (listed.find(project_id) == listed.end()) {
504+
rows.push_back(project_id);
505+
}
506+
}
507+
508+
ListView_DeleteAllItems(h_list_view);
509+
for (size_t i = 0; i < rows.size(); ++i) {
510+
LVITEM item = {};
511+
item.mask = LVIF_TEXT;
512+
item.iItem = static_cast<int>(i);
513+
item.iSubItem = 0;
514+
item.pszText = const_cast<char*>(rows[i].c_str());
515+
int const index = ListView_InsertItem(h_list_view, &item);
516+
if (index >= 0 && checked.find(rows[i]) != checked.end()) {
517+
ListView_SetCheckState(h_list_view, index, TRUE);
518+
}
519+
}
520+
}
521+
522+
std::string AdvanceOptions::CollectCheckedProjects(HWND h_list_view) {
523+
// Without the control there is nothing to read; keep the value already held
524+
// rather than clearing it.
525+
if (!h_list_view) {
526+
return allowed_projects_;
527+
}
528+
std::vector<std::string> checked;
529+
int const count = ListView_GetItemCount(h_list_view);
530+
for (int i = 0; i < count; ++i) {
531+
if (!ListView_GetCheckState(h_list_view, i)) {
532+
continue;
533+
}
534+
char buffer[256] = {0};
535+
ListView_GetItemText(h_list_view, i, 0, buffer,
536+
static_cast<int>(sizeof(buffer)));
537+
if (buffer[0] != '\0') {
538+
checked.push_back(buffer);
539+
}
540+
}
541+
return Join(checked, ",");
542+
}
543+
446544
void AdvanceOptions::CreateButtons(HFONT h_font) {
447545
HWND h_ok_button =
448546
CreateButton(adv_hwnd, "OK", kOkButtonX + 12, kButtonY + 44, kButtonWidth,
@@ -639,6 +737,9 @@ LRESULT CALLBACK AdvanceOptions::AdvanceOptProc(HWND hwnd, UINT u_msg,
639737
sizeof(additional_projects_buffer));
640738
additional_projects_ = additional_projects_buffer;
641739

740+
allowed_projects_ = CollectCheckedProjects(
741+
GetDlgItem(hwnd, kIdcAllowedProjectsListView));
742+
642743
HWND h_query_properties_edit =
643744
GetDlgItem(hwnd, kIdcQueryPropertiesEdit);
644745
char query_properties_buffer[1024] = {0};
@@ -770,6 +871,59 @@ LRESULT CALLBACK AdvanceOptions::AdvanceOptProc(HWND hwnd, UINT u_msg,
770871
break;
771872
}
772873

874+
case kIdcLoadProjectsButton: {
875+
if (HIWORD(w_param) != BN_CLICKED) {
876+
break;
877+
}
878+
// The credentials live on the parent DSN dialog; this dialog holds no
879+
// copy of them.
880+
HWND h_parent = GetParent(hwnd);
881+
char key_file_buffer[1024] = {0};
882+
char auth_buffer[256] = {0};
883+
if (h_parent) {
884+
GetWindowText(GetDlgItem(h_parent, kIdcKeyfileEdit),
885+
key_file_buffer, sizeof(key_file_buffer));
886+
GetWindowText(GetDlgItem(h_parent, kIdcAuthBox), auth_buffer,
887+
sizeof(auth_buffer));
888+
}
889+
if (auth_buffer[0] == '\0') {
890+
ShowErrorWindow(hwnd,
891+
"Select an OAuth mechanism on the main dialog "
892+
"before loading projects.");
893+
break;
894+
}
895+
bool const is_adc =
896+
(strcmp(auth_buffer, "Application Default Credentials") == 0);
897+
if (!is_adc && key_file_buffer[0] == '\0') {
898+
ShowErrorWindow(hwnd,
899+
"Enter a key file path on the main dialog before "
900+
"loading projects.");
901+
break;
902+
}
903+
904+
auto projects_or = DriverForm::GetCatalogAndDataset(
905+
"Catalog", is_adc ? "" : key_file_buffer, auth_buffer, "");
906+
if (!projects_or.Ok()) {
907+
LOG(ERROR) << "AdvanceOptions::AdvanceOptProc::GetCatalogAndDataset"
908+
":: "
909+
<< projects_or.GetStatusRecord().message;
910+
MessageBox(hwnd, projects_or.GetStatusRecord().message.c_str(),
911+
"Error", MB_OK | MB_ICONERROR);
912+
break;
913+
}
914+
915+
std::vector<std::string> project_ids;
916+
for (auto& project_id : Split(projects_or.GetValue(), ";")) {
917+
Trim(project_id);
918+
if (!project_id.empty()) {
919+
project_ids.push_back(std::move(project_id));
920+
}
921+
}
922+
PopulateAllowedProjectsListView(
923+
GetDlgItem(hwnd, kIdcAllowedProjectsListView), project_ids);
924+
break;
925+
}
926+
773927
case kIdcCancelButton:
774928
DestroyWindow(hwnd); // Close the window
775929
break;
@@ -831,6 +985,7 @@ void AdvanceOptions::SetValues(Section const& attribute_map) {
831985
std::to_string(kDefaultMaxRetries));
832986
session_location_ = GetValueOrDefault(attribute_map, kSessionLocation);
833987
additional_projects_ = GetValueOrDefault(attribute_map, kAdditionalProjects);
988+
allowed_projects_ = GetValueOrDefault(attribute_map, kAllowedProjects);
834989
query_properties_ = GetValueOrDefault(attribute_map, kQueryProperties);
835990
// TODO(b/497725655): Enable UI feature after public release
836991
// use_wchar_ = GetValueOrDefault(attribute_map, kUseWChar);
@@ -858,6 +1013,7 @@ void AdvanceOptions::ResetToDefaults() {
8581013
max_retries_ = std::to_string(kDefaultMaxRetries);
8591014
session_location_.clear();
8601015
additional_projects_.clear();
1016+
allowed_projects_.clear();
8611017
query_properties_.clear();
8621018
// use_wchar_.clear();
8631019
enable_session_.clear();
@@ -884,13 +1040,15 @@ void AdvanceOptions::Show(HWND hwnd) {
8841040
(HBRUSH)(COLOR_WINDOW + 1); // Sets background to white
8851041
INITCOMMONCONTROLSEX icc;
8861042
icc.dwSize = sizeof(INITCOMMONCONTROLSEX);
887-
icc.dwICC = ICC_STANDARD_CLASSES;
1043+
// ICC_LISTVIEW_CLASSES registers WC_LISTVIEW, which the allowed-projects pick
1044+
// list needs; the other controls on this dialog are standard classes.
1045+
icc.dwICC = ICC_STANDARD_CLASSES | ICC_LISTVIEW_CLASSES;
8881046
InitCommonControlsEx(&icc);
8891047

8901048
RegisterClass(&wc_adv);
8911049

8921050
int window_width = 525;
893-
int window_height = 720;
1051+
int window_height = 860;
8941052
int screen_width = GetSystemMetrics(SM_CXSCREEN);
8951053
int screen_height = GetSystemMetrics(SM_CYSCREEN);
8961054
int x_pos = (screen_width - window_width) / 2;

google/cloud/odbc/bq_driver/internal/driver_adv_opt_form.h

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
#pragma comment(lib, "Comctl32.lib") // Link with Comctl32.lib
2121

2222
namespace google::cloud::odbc_bq_driver_internal {
23-
// NEXTID:153
23+
// NEXTID:155
2424
static int const kIdcUseDefaultCheckbox = 128;
2525
static int const kIdcDatasetNameEdit = 129;
2626
static int const kIdcTempExpirationEdit = 130;
@@ -45,6 +45,8 @@ static int const kIdcMaxRetriesEdit = 149;
4545
static int const kIdcEnablePscGcdCheckbox = 150;
4646
static int const kIdcPrivateServiceNameEdit = 151;
4747
static int const kIdcUniverseDomainEdit = 152;
48+
static int const kIdcAllowedProjectsListView = 153;
49+
static int const kIdcLoadProjectsButton = 154;
4850

4951
class AdvanceOptions {
5052
public:
@@ -75,6 +77,9 @@ class AdvanceOptions {
7577
inline std::string const& GetAdditionalProjects() const {
7678
return additional_projects_;
7779
}
80+
inline std::string const& GetAllowedProjects() const {
81+
return allowed_projects_;
82+
}
7883
inline std::string const& GetQueryProperties() const {
7984
return query_properties_;
8085
}
@@ -124,6 +129,7 @@ class AdvanceOptions {
124129
static std::string default_string_length_;
125130
static std::string session_location_;
126131
static std::string additional_projects_;
132+
static std::string allowed_projects_;
127133
static std::string query_properties_;
128134
static std::string use_wchar_;
129135
static std::string enable_session_;
@@ -137,6 +143,16 @@ class AdvanceOptions {
137143
static std::string enable_gcd_;
138144
static std::string universe_domain_;
139145

146+
// Fill the allowed-projects pick list with 'project_ids', keeping ticked any
147+
// id that is currently ticked as well as any id already in
148+
// 'allowed_projects_'. Used both when the dialog opens and when the user
149+
// reloads the list from the account.
150+
static void PopulateAllowedProjectsListView(
151+
HWND h_list_view, std::vector<std::string> const& project_ids);
152+
153+
// Comma-join the ticked rows of the allowed-projects pick list.
154+
static std::string CollectCheckedProjects(HWND h_list_view);
155+
140156
static LRESULT CALLBACK AdvanceOptProc(HWND hwnd, UINT uMsg, WPARAM w_param,
141157
LPARAM l_param);
142158
static char const CLASS_NAME[];

google/cloud/odbc/bq_driver/internal/driver_adv_opt_form_test.cc

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,31 @@ TEST_F(AdvanceOptionsTest, ShowWindow) {
7272
<< "Window should be visible after calling Show.";
7373
}
7474

75+
TEST_F(AdvanceOptionsTest, AllowedProjectsPickListRoundTripsThroughOk) {
76+
// Opening the dialog with a saved value should tick those ids without
77+
// contacting the account, and OK should save back exactly what is ticked.
78+
advance_options->SetValues({{"AllowedProjects", "project-a,project-b"}});
79+
80+
// Show() runs a modal GetMessage loop. Queue WM_QUIT first so it builds the
81+
// controls and returns immediately instead of blocking this test; the window
82+
// itself stays alive for the assertions below.
83+
PostQuitMessage(0);
84+
advance_options->Show(nullptr);
85+
HWND hwnd = advance_options->GetHwnd();
86+
ASSERT_NE(hwnd, nullptr) << "Window should be created and displayed.";
87+
88+
HWND h_list = GetDlgItem(hwnd, kIdcAllowedProjectsListView);
89+
ASSERT_NE(h_list, nullptr) << "Allowed projects list should be created.";
90+
ASSERT_EQ(ListView_GetItemCount(h_list), 2);
91+
EXPECT_TRUE(ListView_GetCheckState(h_list, 0));
92+
EXPECT_TRUE(ListView_GetCheckState(h_list, 1));
93+
94+
ListView_SetCheckState(h_list, 1, FALSE);
95+
ClickButton(hwnd, kIdcOKButton);
96+
97+
EXPECT_EQ(advance_options->GetAllowedProjects(), "project-a");
98+
}
99+
75100
TEST_F(AdvanceOptionsTest, SetValuesValidinput) {
76101
Section attribute_map = {{"SQLDialect", "1"},
77102
{"LargeResultsDatasetId", "dataset1"},
@@ -81,6 +106,7 @@ TEST_F(AdvanceOptionsTest, SetValuesValidinput) {
81106
{"LargeResultsTempTableExpirationTime", "3600000"},
82107
{"SessionLocation", "USA"},
83108
{"AdditionalProjects", "projectA,projectB"},
109+
{"AllowedProjects", "projectC,projectD"},
84110
{"QueryProperties", "property1=value1"},
85111
{"MaxThreads", "10"},
86112
{"MaxRetries", "9"}};
@@ -96,6 +122,7 @@ TEST_F(AdvanceOptionsTest, SetValuesValidinput) {
96122
EXPECT_EQ(options.GetTempTableExpiration(), "3600000");
97123
EXPECT_EQ(options.GetSessionLocation(), "USA");
98124
EXPECT_EQ(options.GetAdditionalProjects(), "projectA,projectB");
125+
EXPECT_EQ(options.GetAllowedProjects(), "projectC,projectD");
99126
EXPECT_EQ(options.GetQueryProperties(), "property1=value1");
100127
EXPECT_EQ(options.GetMaxThreads(), "10");
101128
EXPECT_EQ(options.GetMaxRetries(), "9");
@@ -118,8 +145,19 @@ TEST_F(AdvanceOptionsTest, SetValuesMissingkeys) {
118145
EXPECT_EQ(options.GetTempTableExpiration(), "3600000");
119146
EXPECT_EQ(options.GetSessionLocation(), "");
120147
EXPECT_EQ(options.GetAdditionalProjects(), "");
148+
EXPECT_EQ(options.GetAllowedProjects(), "");
121149
EXPECT_EQ(options.GetQueryProperties(), "");
122150
EXPECT_EQ(options.GetAllowHtapiForLargeResultsCheckbox(), "");
123151
}
124152

153+
TEST_F(AdvanceOptionsTest, ResetToDefaultsClearsAllowedProjects) {
154+
AdvanceOptions options;
155+
options.SetValues({{"AllowedProjects", "projectC,projectD"}});
156+
ASSERT_EQ(options.GetAllowedProjects(), "projectC,projectD");
157+
158+
options.ResetToDefaults();
159+
160+
EXPECT_EQ(options.GetAllowedProjects(), "");
161+
}
162+
125163
} // namespace google::cloud::odbc_bq_driver_internal

google/cloud/odbc/bq_driver/internal/driver_form.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ static Section BuildTestConnectionAttributes(
223223
attributes_map["SessionLocation"] = adv_form.GetSessionLocation();
224224
attributes_map["MaxThreads"] = adv_form.GetMaxThreads();
225225
attributes_map["AdditionalProjects"] = adv_form.GetAdditionalProjects();
226+
attributes_map["AllowedProjects"] = adv_form.GetAllowedProjects();
226227
attributes_map["QueryProperties"] = adv_form.GetQueryProperties();
227228
attributes_map["UseWVarChar"] = adv_form.GetUseWchar();
228229
attributes_map["EnableSession"] = adv_form.GetEnableSession();

google/cloud/odbc/bq_driver/internal/odbc_conn_handle.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ void ConnectionHandle::SetUp(Section& dsn_section,
153153
dsn_.kms_key_name = dsn_section["KMSKEYNAME"];
154154
dsn_.session_location = dsn_section["SESSIONLOCATION"];
155155
dsn_.additional_projects = dsn_section["ADDITIONALPROJECTS"];
156+
dsn_.allowed_projects = dsn_section["ALLOWEDPROJECTS"];
156157
dsn_.psc = dsn_section["PRIVATESERVICECONNECTURIS"];
157158
dsn_.enable_gcd =
158159
dsn_section["ENABLEGCD"] == "1" || dsn_section["ENABLEGCD"] == "true";

google/cloud/odbc/bq_driver/internal/odbc_conn_handle.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ struct Dsn {
103103
// Proxy options fields
104104
google::cloud::odbc_bigquery_client_interface::ProxyOptions proxy_options;
105105
std::string additional_projects;
106+
std::string allowed_projects;
106107
std::string psc;
107108
bool enable_gcd;
108109
std::string universe_domain;

0 commit comments

Comments
 (0)