Skip to content

Commit dc34928

Browse files
authored
refactor: Improve REST catalog's authenticate method (#1712)
1 parent 6abace7 commit dc34928

File tree

1 file changed

+19
-23
lines changed

1 file changed

+19
-23
lines changed

crates/catalog/rest/src/client.rs

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -193,15 +193,17 @@ impl HttpClient {
193193
Ok(())
194194
}
195195

196-
/// Authenticate the request by filling token.
196+
/// Authenticates the request by adding a bearer token to the authorization header.
197197
///
198-
/// - If neither token nor credential is provided, this method will do nothing.
199-
/// - If only credential is provided, this method will try to fetch token from the server.
200-
/// - If token is provided, this method will use the token directly.
198+
/// This method supports three authentication modes:
201199
///
202-
/// # TODO
200+
/// 1. **No authentication** - Skip authentication when both `credential` and `token` are missing.
201+
/// 2. **Token authentication** - Use the provided `token` directly for authentication.
202+
/// 3. **OAuth authentication** - Exchange `credential` for a token, cache it, then use it for authentication.
203203
///
204-
/// Support refreshing token while needed.
204+
/// When both `credential` and `token` are present, `token` takes precedence.
205+
///
206+
/// # TODO: Support automatic token refreshing.
205207
async fn authenticate(&self, req: &mut Request) -> Result<()> {
206208
// Clone the token from lock without holding the lock for entire function.
207209
let token = self.token.lock().await.clone();
@@ -210,24 +212,18 @@ impl HttpClient {
210212
return Ok(());
211213
}
212214

213-
// Use token if provided.
214-
if let Some(token) = &token {
215-
req.headers_mut().insert(
216-
http::header::AUTHORIZATION,
217-
format!("Bearer {token}").parse().map_err(|e| {
218-
Error::new(
219-
ErrorKind::DataInvalid,
220-
"Invalid token received from catalog server!",
221-
)
222-
.with_source(e)
223-
})?,
224-
);
225-
return Ok(());
226-
}
215+
// Either use the provided token or exchange credential for token, cache and use that
216+
let token = match token {
217+
Some(token) => token,
218+
None => {
219+
let token = self.exchange_credential_for_token().await?;
220+
// Update token so that we use it for next request instead of
221+
// exchanging credential for token from the server again
222+
*self.token.lock().await = Some(token.clone());
223+
token
224+
}
225+
};
227226

228-
let token = self.exchange_credential_for_token().await?;
229-
// Update token.
230-
*self.token.lock().await = Some(token.clone());
231227
// Insert token in request.
232228
req.headers_mut().insert(
233229
http::header::AUTHORIZATION,

0 commit comments

Comments
 (0)