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

auth support for paasta APIs #3985

Merged
merged 4 commits into from
Feb 27, 2025
Merged

auth support for paasta APIs #3985

merged 4 commits into from
Feb 27, 2025

Conversation

piax93
Copy link
Contributor

@piax93 piax93 commented Nov 1, 2024

Support for an authn/z tween in the PaaSTA API (mostly inspired by previous work).

There's nothing too surprising here. Maybe the only unusual aspect is that I picked to make "dry-run" the default rather than the other way around. It makes things easier to roll out.

For how the codebase is structured loading the token transparently could be a bit of a pain, especially if we want to maintain a bunch of un-auth endpoints, so I just added the option to set the token header in the API client. From there in the CLI implementations that need auth we can do

from paasta_tools.cli.utils import get_sso_service_auth_token
...
get_paasta_oapi_client(..., get_sso_service_auth_token())

@piax93 piax93 requested a review from Qmando November 1, 2024 15:02
@piax93 piax93 requested a review from a team as a code owner November 1, 2024 15:02
danielpops
danielpops previously approved these changes Nov 7, 2024
@piax93 piax93 changed the title [POC] auth support for paasta APIs auth support for paasta APIs Feb 25, 2025
Copy link
Member

Choose a reason for hiding this comment

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

just curious: if we had an endpoint that was protected by this, but was being called by automation - how would that work? get_sso_service_auth_token() makes it sound like there might be a 2fa challenge/sso login, which wouldn't really be possible for something like jenkins

Copy link
Contributor Author

@piax93 piax93 Feb 26, 2025

Choose a reason for hiding this comment

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

That's an open question, but the current plan is to just apply this to the new remote-run endpoints which are going to be implemented to run prod sandboxes (which are supposed to be used only by human users), so anything else will remain open. That way we can collect telemetry and see if it's worth the effort to lock down more endpoints.

For the specific case of Jenkins, since we run that in k8s, if we decide to enforce authz it's most likely going to be via a service account token mounted into the pods.

* more clear access to auth response
* better mocking in testing
@piax93 piax93 requested a review from nemacysts February 26, 2025 11:00
piax93 added a commit to Yelp/Tron that referenced this pull request Feb 26, 2025
@nemacysts nemacysts merged commit 9856744 into master Feb 27, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants