Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: Make OAuth token server configurable #305

Merged
merged 1 commit into from
Mar 26, 2024
Merged

feat: Make OAuth token server configurable #305

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 43 additions & 2 deletions crates/catalog/rest/src/catalog.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ impl RestCatalogConfig {
}

fn get_token_endpoint(&self) -> String {
[&self.uri, PATH_V1, "oauth", "tokens"].join("/")
if let Some(auth_url) = self.props.get("rest.authorization-url") {
auth_url.to_string()
} else {
[&self.uri, PATH_V1, "oauth", "tokens"].join("/")
}
}

fn namespaces_endpoint(&self) -> String {
Expand Down Expand Up @@ -865,8 +869,12 @@ mod tests {
}

async fn create_oauth_mock(server: &mut ServerGuard) -> Mock {
create_oauth_mock_with_path(server, "/v1/oauth/tokens").await
}

async fn create_oauth_mock_with_path(server: &mut ServerGuard, path: &str) -> Mock {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor to reuse code in create_oauth_mock().

server
.mock("POST", "/v1/oauth/tokens")
.mock("POST", path)
.with_status(200)
.with_body(
r#"{
Expand Down Expand Up @@ -955,6 +963,39 @@ mod tests {
);
}

#[tokio::test]
async fn test_oauth_with_auth_url() {
let mut server = Server::new_async().await;
let config_mock = create_config_mock(&mut server).await;

let mut auth_server = Server::new_async().await;
let auth_server_path = "/some/path";
let oauth_mock = create_oauth_mock_with_path(&mut auth_server, auth_server_path).await;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To explain the unit test, we first create a mock response for our OAuth Server. Then in line 978 - 981, set rest.authorization-url to use the OAuth Server (instead of default server). Finally, we verify the token is correct returned from our OAuth Server.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to assert the rest.authorization-url using .match?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because we already assert rest.authorization-url implicitly. If rest.authorization-url is not honored, we won't be able to get token from the mocked OAuth server (using same url) 🙂.


let mut props = HashMap::new();
props.insert("credential".to_string(), "client1:secret1".to_string());
props.insert(
"rest.authorization-url".to_string(),
format!("{}{}", auth_server.url(), auth_server_path).to_string(),
);

let catalog = RestCatalog::new(
RestCatalogConfig::builder()
.uri(server.url())
.props(props)
.build(),
)
.await
.unwrap();

oauth_mock.assert_async().await;
config_mock.assert_async().await;
assert_eq!(
catalog.config.props.get("token"),
Some(&"ey000000000000".to_string())
);
}

#[tokio::test]
async fn test_config_override_prefix() {
let mut server = Server::new_async().await;
Expand Down
Loading