Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: oauth for builder [ENG-493] #44

Merged
merged 1 commit into from
Sep 23, 2024
Merged

Conversation

rswanson
Copy link
Member

@rswanson rswanson commented Sep 19, 2024

TL;DR

Added OAuth2 authentication for the builder's communication with Quincey.

What changed?

  • Added OAuth2 dependencies to the builder crate
  • Introduced new OAuth-related configuration parameters
  • Implemented OAuth token fetching in the submit task
  • Updated the Quincey API call to include the OAuth bearer token

How to test?

  1. Set the new OAuth environment variables:
    • OAUTH_CLIENT_ID
    • OAUTH_CLIENT_SECRET
    • OAUTH_AUTHENTICATE_URL
    • OAUTH_TOKEN_URL
  2. Ensure the Quincey service is configured to accept OAuth tokens
  3. Run the builder and verify it can successfully authenticate and communicate with Quincey

Why make this change?

This change enhances the security of the communication between the builder and Quincey by implementing OAuth2 authentication. It ensures that only authorized builders can interact with Quincey, improving the overall security posture of the system.

Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @rswanson and the rest of your teammates on Graphite Graphite

@rswanson rswanson requested a review from a team September 19, 2024 16:01
@rswanson rswanson added the enhancement New feature or request label Sep 19, 2024 — with Graphite App
@rswanson rswanson force-pushed the swanny/feat_oauth_for_builder branch 2 times, most recently from d8f31ad to 2c6e1d6 Compare September 19, 2024 16:05
@rswanson rswanson self-assigned this Sep 19, 2024
Copy link
Contributor

@prestwich prestwich left a comment

Choose a reason for hiding this comment

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

style

crates/builder/src/tasks/submit.rs Outdated Show resolved Hide resolved
crates/builder/src/tasks/submit.rs Outdated Show resolved Hide resolved
crates/builder/src/tasks/submit.rs Outdated Show resolved Hide resolved
let resp: reqwest::Response = self
.client
.post(self.config.quincey_url.as_ref())
.json(sig_request)
.bearer_auth(token.access_token().secret())
Copy link
Contributor

Choose a reason for hiding this comment

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

should we also address tx pool oath in the same PR?

we don't necessarily need to worry about efficiency of streamlining to one oath call for both tx pool & quincey within the example builder but would be a nice-to-have if it were simple

Copy link
Member Author

Choose a reason for hiding this comment

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

currently we don't call the protected endpoints (/get-order(s)) so there isn't anything to update. Adding the functionality itself would be outside the scope of this

@rswanson rswanson marked this pull request as ready for review September 20, 2024 14:02
@rswanson rswanson force-pushed the swanny/feat_oauth_for_builder branch 2 times, most recently from d257ea1 to 7265e3a Compare September 20, 2024 14:04
@rswanson rswanson force-pushed the swanny/feat_oauth_for_builder branch from 7265e3a to a267157 Compare September 20, 2024 15:58
Copy link
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

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

seems tests are failing?

@rswanson rswanson force-pushed the swanny/feat_oauth_for_builder branch from a267157 to b3147ac Compare September 23, 2024 14:05
@rswanson rswanson force-pushed the swanny/feat_oauth_for_builder branch from b3147ac to 8e68614 Compare September 23, 2024 14:06
@rswanson rswanson merged commit fc0be3d into main Sep 23, 2024
5 checks passed
@rswanson rswanson changed the title feat: oauth for builder feat: oauth for builder [ENG-493] Sep 23, 2024
@Evalir Evalir deleted the swanny/feat_oauth_for_builder branch September 24, 2024 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants