Skip to content

Commit f87df69

Browse files
Run full test suite and fix any failures\n\nTask ID: task-5.1-final-validation
1 parent 4e111ed commit f87df69

File tree

7 files changed

+70
-51
lines changed

7 files changed

+70
-51
lines changed

rust/src/auth/config.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,10 +107,9 @@ impl AuthConfig {
107107
match mechanism {
108108
AuthMechanism::Pat => {
109109
if access_token.is_none() {
110-
return Err(DatabricksErrorHelper::invalid_argument()
111-
.message(
112-
"databricks.access_token is required when auth mechanism is 0 (PAT)",
113-
));
110+
return Err(DatabricksErrorHelper::invalid_argument().message(
111+
"databricks.access_token is required when auth mechanism is 0 (PAT)",
112+
));
114113
}
115114
}
116115
AuthMechanism::OAuth => {

rust/src/auth/oauth/cache.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ struct CacheKey {
3535
scopes: Vec<String>,
3636
}
3737

38-
3938
impl CacheKey {
4039
/// Creates a new cache key from the given parameters.
4140
fn new(host: &str, client_id: &str, scopes: &[String]) -> Self {
@@ -71,7 +70,6 @@ impl CacheKey {
7170
/// Cache I/O errors are logged as warnings and never block authentication.
7271
pub struct TokenCache;
7372

74-
7573
impl TokenCache {
7674
/// Returns the cache directory path.
7775
/// Location: `~/.config/databricks-adbc/oauth/`

rust/src/auth/oauth/callback.rs

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,10 @@ impl CallbackServer {
152152
/// Returns the redirect URI for this callback server.
153153
///
154154
/// This URI should be used when constructing the OAuth authorization URL.
155-
/// The format is `http://localhost:{port}/callback`.
155+
/// The format is `http://localhost:{port}` (matching the redirect URI
156+
/// registered for the `databricks-cli` OAuth application).
156157
pub fn redirect_uri(&self) -> String {
157-
format!("http://localhost:{}/callback", self.port)
158+
format!("http://localhost:{}", self.port)
158159
}
159160

160161
/// Waits for the OAuth callback with the authorization code.
@@ -276,7 +277,7 @@ impl CallbackServer {
276277

277278
let request = String::from_utf8_lossy(&buffer[..n]);
278279

279-
// Parse the request line (e.g., "GET /callback?code=...&state=... HTTP/1.1")
280+
// Parse the request line (e.g., "GET /?code=...&state=... HTTP/1.1")
280281
let first_line = request.lines().next().unwrap_or("");
281282
let parts: Vec<&str> = first_line.split_whitespace().collect();
282283

@@ -424,7 +425,7 @@ mod tests {
424425
.expect("Failed to connect to callback server");
425426

426427
let request = format!(
427-
"GET /callback?code=test-auth-code-123&state={} HTTP/1.1\r\nHost: localhost:{}\r\n\r\n",
428+
"GET /?code=test-auth-code-123&state={} HTTP/1.1\r\nHost: localhost:{}\r\n\r\n",
428429
expected_state, port
429430
);
430431

@@ -465,7 +466,7 @@ mod tests {
465466
.expect("Failed to connect");
466467

467468
let request = format!(
468-
"GET /callback?code=auth-code&state={} HTTP/1.1\r\nHost: localhost:{}\r\n\r\n",
469+
"GET /?code=auth-code&state={} HTTP/1.1\r\nHost: localhost:{}\r\n\r\n",
469470
wrong_state, port
470471
);
471472

@@ -529,7 +530,7 @@ mod tests {
529530

530531
// Send callback without 'code' parameter
531532
let request = format!(
532-
"GET /callback?state=test-state HTTP/1.1\r\nHost: localhost:{}\r\n\r\n",
533+
"GET /?state=test-state HTTP/1.1\r\nHost: localhost:{}\r\n\r\n",
533534
port
534535
);
535536

@@ -560,7 +561,7 @@ mod tests {
560561

561562
// Simulate authorization server error response
562563
let request = format!(
563-
"GET /callback?error=access_denied&error_description=User%20denied%20consent HTTP/1.1\r\nHost: localhost:{}\r\n\r\n",
564+
"GET /?error=access_denied&error_description=User%20denied%20consent HTTP/1.1\r\nHost: localhost:{}\r\n\r\n",
564565
port
565566
);
566567

@@ -619,7 +620,13 @@ mod tests {
619620
uri
620621
);
621622
// Verify the port is a valid non-zero number
622-
let port: u16 = uri.strip_prefix("http://localhost:").unwrap().strip_suffix("/callback").unwrap_or_else(|| uri.strip_prefix("http://localhost:").unwrap()).parse().unwrap();
623+
let port: u16 = uri
624+
.strip_prefix("http://localhost:")
625+
.unwrap()
626+
.strip_suffix("/callback")
627+
.unwrap_or_else(|| uri.strip_prefix("http://localhost:").unwrap())
628+
.parse()
629+
.unwrap();
623630
assert!(port > 0, "Expected a non-zero port, got: {}", port);
624631
}
625632
}

rust/src/auth/oauth/m2m.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,8 @@ impl ClientCredentialsProvider {
188188
token_endpoint_override: Option<String>,
189189
) -> Result<Self> {
190190
// Discover OIDC endpoints or use override
191-
let (token_endpoint, auth_endpoint) = if let Some(token_endpoint) = token_endpoint_override {
191+
let (token_endpoint, auth_endpoint) = if let Some(token_endpoint) = token_endpoint_override
192+
{
192193
// Use override for token endpoint, still discover auth endpoint
193194
// (auth endpoint is not typically overridden for M2M since it's not used)
194195
let endpoints = OidcEndpoints::discover(host, &http_client).await?;
@@ -209,7 +210,6 @@ impl ClientCredentialsProvider {
209210
scopes,
210211
})
211212
}
212-
213213
}
214214

215215
impl AuthProvider for ClientCredentialsProvider {

rust/src/auth/oauth/token_store.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ pub(crate) struct TokenStore {
8383
refreshing: Arc<AtomicBool>,
8484
}
8585

86-
8786
impl TokenStore {
8887
/// Creates a new empty token store.
8988
pub fn new() -> Self {

rust/src/database.rs

Lines changed: 48 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -406,13 +406,11 @@ impl Optionable for Database {
406406
.to_adbc()
407407
})
408408
}
409-
"databricks.auth.scopes" => {
410-
self.auth_config.scopes.clone().ok_or_else(|| {
411-
DatabricksErrorHelper::invalid_state()
412-
.message("option 'databricks.auth.scopes' is not set")
413-
.to_adbc()
414-
})
415-
}
409+
"databricks.auth.scopes" => self.auth_config.scopes.clone().ok_or_else(|| {
410+
DatabricksErrorHelper::invalid_state()
411+
.message("option 'databricks.auth.scopes' is not set")
412+
.to_adbc()
413+
}),
416414
"databricks.auth.token_endpoint" => {
417415
self.auth_config.token_endpoint.clone().ok_or_else(|| {
418416
DatabricksErrorHelper::invalid_state()
@@ -511,7 +509,8 @@ impl adbc_core::Database for Database {
511509
})?;
512510

513511
// Validate auth configuration
514-
let mechanism = self.auth_config
512+
let mechanism = self
513+
.auth_config
515514
.validate(&self.access_token)
516515
.map_err(|e| e.to_adbc())?;
517516

@@ -540,7 +539,10 @@ impl adbc_core::Database for Database {
540539
))
541540
}
542541
AuthMechanism::OAuth => {
543-
let flow = self.auth_config.flow.expect("flow should be validated above");
542+
let flow = self
543+
.auth_config
544+
.flow
545+
.expect("flow should be validated above");
544546
match flow {
545547
AuthFlow::TokenPassthrough => {
546548
// Token passthrough - use PersonalAccessToken provider with the provided token
@@ -552,25 +554,34 @@ impl adbc_core::Database for Database {
552554
}
553555
AuthFlow::ClientCredentials => {
554556
// Client credentials flow (M2M) - create ClientCredentialsProvider
555-
let client_id = self.auth_config.client_id.as_ref()
557+
let client_id = self
558+
.auth_config
559+
.client_id
560+
.as_ref()
556561
.expect("client_id should be validated above");
557-
let client_secret = self.auth_config.client_secret.as_ref()
562+
let client_secret = self
563+
.auth_config
564+
.client_secret
565+
.as_ref()
558566
.expect("client_secret should be validated above");
559567

560568
// Default scope for M2M is "all-apis" (no offline_access since M2M has no refresh token)
561569
let scopes_str = self.auth_config.scopes.as_deref().unwrap_or("all-apis");
562-
let scopes: Vec<String> = scopes_str.split_whitespace().map(String::from).collect();
570+
let scopes: Vec<String> =
571+
scopes_str.split_whitespace().map(String::from).collect();
563572

564573
// Create the provider (async operation - needs runtime)
565574
let provider = runtime
566-
.block_on(crate::auth::ClientCredentialsProvider::new_with_full_config(
567-
host,
568-
client_id,
569-
client_secret,
570-
http_client.clone(),
571-
scopes,
572-
self.auth_config.token_endpoint.clone(),
573-
))
575+
.block_on(
576+
crate::auth::ClientCredentialsProvider::new_with_full_config(
577+
host,
578+
client_id,
579+
client_secret,
580+
http_client.clone(),
581+
scopes,
582+
self.auth_config.token_endpoint.clone(),
583+
),
584+
)
574585
.map_err(|e| e.to_adbc())?;
575586

576587
Arc::new(provider)
@@ -932,7 +943,10 @@ mod tests {
932943
)
933944
.unwrap();
934945

935-
assert_eq!(db.auth_config.client_secret, Some("test-secret".to_string()));
946+
assert_eq!(
947+
db.auth_config.client_secret,
948+
Some("test-secret".to_string())
949+
);
936950
assert_eq!(
937951
db.get_option_string(OptionDatabase::Other(
938952
"databricks.auth.client_secret".into()
@@ -951,7 +965,10 @@ mod tests {
951965
)
952966
.unwrap();
953967

954-
assert_eq!(db.auth_config.scopes, Some("all-apis offline_access".to_string()));
968+
assert_eq!(
969+
db.auth_config.scopes,
970+
Some("all-apis offline_access".to_string())
971+
);
955972
assert_eq!(
956973
db.get_option_string(OptionDatabase::Other("databricks.auth.scopes".into()))
957974
.unwrap(),
@@ -1564,10 +1581,13 @@ mod tests {
15641581
.unwrap();
15651582

15661583
// Verify config is stored correctly
1567-
assert_eq!(db.auth_mechanism, Some(AuthMechanism::OAuth));
1568-
assert_eq!(db.auth_flow, Some(AuthFlow::ClientCredentials));
1569-
assert_eq!(db.auth_client_id, Some("test-client-id".to_string()));
1570-
assert_eq!(db.auth_client_secret, Some("test-secret".to_string()));
1584+
assert_eq!(db.auth_config.mechanism, Some(AuthMechanism::OAuth));
1585+
assert_eq!(db.auth_config.flow, Some(AuthFlow::ClientCredentials));
1586+
assert_eq!(db.auth_config.client_id, Some("test-client-id".to_string()));
1587+
assert_eq!(
1588+
db.auth_config.client_secret,
1589+
Some("test-secret".to_string())
1590+
);
15711591
}
15721592

15731593
#[test]
@@ -1612,7 +1632,7 @@ mod tests {
16121632

16131633
// Verify custom scopes are stored
16141634
assert_eq!(
1615-
db.auth_scopes,
1635+
db.auth_config.scopes,
16161636
Some("custom-scope-1 custom-scope-2".to_string())
16171637
);
16181638
}
@@ -1659,7 +1679,7 @@ mod tests {
16591679

16601680
// Verify token_endpoint override is stored
16611681
assert_eq!(
1662-
db.auth_token_endpoint,
1682+
db.auth_config.token_endpoint,
16631683
Some("https://custom.endpoint/token".to_string())
16641684
);
16651685
}

rust/tests/oauth_e2e.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -129,9 +129,7 @@ fn test_m2m_end_to_end() {
129129

130130
// Create driver and database
131131
let mut driver = Driver::new();
132-
let mut database = driver
133-
.new_database()
134-
.expect("Failed to create database");
132+
let mut database = driver.new_database().expect("Failed to create database");
135133

136134
// Configure database with M2M OAuth
137135
database
@@ -286,9 +284,7 @@ fn test_u2m_end_to_end() {
286284

287285
// Create driver and database
288286
let mut driver = Driver::new();
289-
let mut database = driver
290-
.new_database()
291-
.expect("Failed to create database");
287+
let mut database = driver.new_database().expect("Failed to create database");
292288

293289
// Configure database with U2M OAuth
294290
database

0 commit comments

Comments
 (0)