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

Fix STS SessionToken generation #8169

Merged
merged 2 commits into from
Jun 27, 2024
Merged

Conversation

Neon-White
Copy link
Contributor

@Neon-White Neon-White commented Jun 26, 2024

Explain the changes

  1. This PR fixes STS functionality by separating jwt_utils.make_auth_token into two functions - make_auth_token which always signs the request, and make_internal_auth_token that's used for RPC calls and only falls back to make_auth_token in case NOOBAA_AUTH_TOKEN isn't provided. Now, STS requests use make_auth_token directly, while internal callers were changed to use make_internal_auth_token

Issues: Fixed #xxx / Gap #xxx

  1. https://bugzilla.redhat.com/show_bug.cgi?id=2009627

Testing Instructions:

  1. See the following comment
  • Doc added/updated
  • Tests added

Copy link
Member

@guymguym guymguym left a comment

Choose a reason for hiding this comment

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

I prefer to separate it to two different functions, one for simply make_auth_token that is simply signing the input, and a separate one to be used for rpc tokens which can be called make_internal_auth_token than can check the config for remote endpoints and if not then fallback to call the other function. This means replacing the callers to use the right variant, which will make it more explicit and safer

@shirady
Copy link
Contributor

shirady commented Jun 27, 2024

@Neon-White I deleted my comment because after sending it I saw @guymguym's comment.

@Neon-White
Copy link
Contributor Author

Neon-White commented Jun 27, 2024

Please note that I use Github's Squash and merge feature which merges a single squashed commit without modifying the branch - so the number next to Commits on top will remain 2, but the merge is squashed.

@Neon-White Neon-White merged commit e76dbcd into noobaa:master Jun 27, 2024
10 checks passed
@Neon-White
Copy link
Contributor Author

image
Squashed and merged

@Neon-White Neon-White deleted the fix-sts branch June 27, 2024 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants