search

djc / gcp_auth

Minimal authentication library for Google Cloud Platform (GCP)
Other
63 stars 38 forks source link

Add support for providing subject for JWT claim when gettng token #92

Closed zhrebicek closed 5 months ago

zhrebicek commented 8 months ago

This is useful when you want to access stuff like Google Workspace API, where you can't just use service account but need to impersonate use with correct rights in google workspace.

Tested it with my code and was able to access google workspace alerts.

If needed I can tweak naming of get_token_with_subject and comment, but otherwise looks it fits.

zhrebicek commented 8 months ago

@djc @hrvolapeter Hi, what do I need to do to get this merged?

As I added get_token_with_subject and left get_token as is, it should not be breaking change.

hrvolapeter commented 8 months ago

Hi @zhrebicek I had a look and I think it looks good. But I'd like to get review from @djc as well let's give him a few more days he might be on vacation somewhere

djc commented 8 months ago

Most of this seems fine. The thing I'm somewhat concerned is about is that this offers the subject API on all kinds of ServiceAccount implementations, even though only the CustomServiceAccount really supports it. Maybe this should be offered as an API that's only available directly from a CustomServiceAccount instance instead? That will require a bit of refactoring...

zhrebicek commented 8 months ago

@djc yeah it makes sense, how would you imagine it refactored?

Should AuthenticationManager become trait with some special case for CustomServiceAccount? With something like DefaultAuthenticationManager and CustomAuthenticationManager?

Maybe I need to revisit our usage in code, and check this library some more to know better place as you say on CustomServiceAccount.

djc commented 5 months ago

I've submitted a different approach in #106, all feedback welcome!

djc commented 5 months ago

Merged #106 instead.