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

Generic OAuth2 tokens provider declaration #4694

Open
kratkyzobak opened this issue Jun 16, 2023 · 15 comments
Open

Generic OAuth2 tokens provider declaration #4694

kratkyzobak opened this issue Jun 16, 2023 · 15 comments
Assignees
Labels
feature-request All issues for new features that have not been committed to help wanted Looking for support from community needs-discussion stale-bot-ignore All issues that should not be automatically closed by our stale bot

Comments

@kratkyzobak
Copy link
Contributor

Proposal

Add new TriggerAuthentification property like oAuth2token with target's clientID / resource / scope (based on various oauth integrations) with helpers for scalers developers.

Example TriggerAuthentification

- spec:
     podIdentity:
          source: azure-workload
     oAuth2token:
          source: podIdentity  # or hashiCorpVault, keycloakInstance etc
          targetResource: "xxxxxx" # resource to ask access token for        

Example "helper" interface for scalers:

tokenProvider, err := NewOAuth2TokenProvider(config); // returns sthg like OAuthTokenProvider from azidentity, but generic

if err != nil {
    token, err := tokenProvider.GetValidToken(); // would request token / refresh if needed
   
    // here would go scaler specific token usage (Bearer auth via HTTP / use as password for MySQL / ....)
}

Use-Case

Based on trying to implement #4657.

Short-term JWT tokens are introduced in many resources now. There are lot of OIDC providers in current Kubernetes world (Kubernetes itself OIDC, two OIDC flows in AKS, Identity providers within EKS and GCK.... self managed Keycloacks, Hashicorp Vaults....).

If every scaler would have to implement each, there would be lot of garbage code with different quality and different configuration style. Just as in #4657 I have implemented OIDC only for AAD Workload Identity since it's only one I'm interested in (and only one I understand enough to be able to test it as "after hours job").

There is allways 3 parts of job:

  • obtain secrets to communicate with identity server (you need speicfic configuration for each provider)
  • obtain access token for target resource from identity server (you usually need only resourceId/clientId/applicationId of resource)
  • use access token - scaler's specific way

I think, there is opportunity to implement first 2 parts in some standardized way to be shared between scalers and then provide scalers mantainers simple interface to support multiple identity servers by one implementation with reusing same configuration for users.

Is this a feature you are interested in implementing yourself?

No

Anything else?

I'm able to provide some hands-on with implementation, but I'm not storng enough in GO to implement this whole by myself.
Also, there would be needed some triage before implementation (mainly to cross-check various identity providers requirements). Also, same as implementation itself, there would be need for some abstractions in test (some sort of parametrized test to test "all" identity providers?)

@kratkyzobak kratkyzobak added feature-request All issues for new features that have not been committed to needs-discussion labels Jun 16, 2023
@tomkerkhove tomkerkhove moved this from To Triage to To Do in Roadmap - KEDA Core Jul 5, 2023
@JorTurFer JorTurFer added the help wanted Looking for support from community label Jul 6, 2023
@JorTurFer
Copy link
Member

I think that this could be a good addition, WDYT @kedacore/keda-core-contributors ?

@zroubalik
Copy link
Member

Yeah, I like it too.

@mingmcb
Copy link
Contributor

mingmcb commented Aug 7, 2023

is there any update on this one? any ETA when this will be complete?

@JorTurFer
Copy link
Member

Summer has been complex, I think that in September we will continue at 100%.
Nobody is working on this at this moment, are you willing to start with this?

@stale
Copy link

stale bot commented Oct 24, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale All issues that are marked as stale due to inactivity label Oct 24, 2023
@JoshuaCuevasUofT
Copy link

Hi, me and a team at University of Toronto would like to work on this issue and contribute to its implementation. We're mostly comprised of computer science students, and this would be the first open source contribution for many of us. If there's any that has worked on this or is still working on this please let us know.

@zroubalik
Copy link
Member

@JoshuaCuevasUofT hi, this is great! I am not aware of any work on this.

@kedacore/keda-maintainers wdyt?

Copy link

stale bot commented Nov 3, 2023

This issue has been automatically closed due to inactivity.

@stale stale bot closed this as completed Nov 3, 2023
@github-project-automation github-project-automation bot moved this from To Do to Ready To Ship in Roadmap - KEDA Core Nov 3, 2023
@zroubalik zroubalik reopened this Nov 3, 2023
@github-project-automation github-project-automation bot moved this from Ready To Ship to Proposed in Roadmap - KEDA Core Nov 3, 2023
@stale stale bot removed the stale All issues that are marked as stale due to inactivity label Nov 3, 2023
@JoshuaCuevasUofT
Copy link

Hi, I have some clarifying questions about this proposal's scope.

A) I wanted to clarify whether or not this proposal is only for Oauth2 tokens.

B) Including something like the property “oAuth2token” into the TriggerAuthentication passing clientID / resource / scope sounds good. It should be straightforward (though a little tedious) to create a dictionary that refers the resource (for example Microsoft AAD) to a certain JWKS url (for example https://login.microsoftonline.com/%s/discovery/keys). And the scope to some specifications.

The dictionary would then contain many functions like these, varying with IDP. And be inherited by every sub-scaler.

func WithAzureADOAuth(tenantID string, clientID string) RabbitOAuthConfig {
	return RabbitOAuthConfig{
		Enable:    true,
		ClientID:  clientID,
		ScopesKey: "roles",
		JwksURI:   fmt.Sprintf("https://login.microsoftonline.com/%s/discovery/keys", tenantID),
	}
}

C) When a scaler parses this information from the ScaledObject yaml, and a triggerauth yaml, the parse function seems to be unique for every scaler. 1) It would be hard to implement a new property in triggerauth and change every scaler’s parse function. 2) Is this necessary if app-to-app authentication between a scaler and some Oauth2 token provider should require the same information/parameters across all scalers (given the dictionary above)? It makes sense to add some inherited function for all sub-scalers to parse the new “oauth2token” property from triggerauthentication yaml. But I can’t find an example of that. If this makes sense, could you point to something I could take a look at?

Am I missing anything to this proposal?

@JoshuaCuevasUofT
Copy link

Also @whyismynamerudy @VigneshNk and @kathy23980 are confirmed to be working with me on this issue over the school semester so tediousness shouldn't be too bad when split between 4 people. Again, apologies for the inconsistency. Depending on the scope, I think it's a fair timeline to say we should have this done within a month.

@JoshuaCuevasUofT
Copy link

@zroubalik do you have any comments sir?

Copy link

stale bot commented Jan 15, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale All issues that are marked as stale due to inactivity label Jan 15, 2024
@zroubalik zroubalik removed the stale All issues that are marked as stale due to inactivity label Jan 16, 2024
@JorTurFer
Copy link
Member

I'm not an expert with OIDC stuff, but maybe we could implement this as a new CRD to handle all the OIDC information as reference it int he TriggerAuthentication., I see this as a service that we could inject into the scalers during the creation, configured for the given OIDC.
In this case, the scaler would just need to adapt their logic to use the injected item to generate the tokens (we could use a pointer to know if it's configured or not, but maybe this is just an implementation detail).

I say about another service injected into the scalers, because I'm assuming that probably we won't be able to standarize all the OIDC provider. Sadly I'm quite sure that Azure/AWS/GCP include their **** flavour of the things, but maybe I'm wrong, making our lives easier xD.

This new CRD could allow us to have multiple "well known providers" with their own config and another generic one for generic OIDC providers. This is also the approach that Grafana does (but it's also true that they do more than just requesting a token).

But I fully agree that we need to standardize the process somehow, creating a mechanism to include OIDC support on scalers because nowadays almost any service can be protected behind an OIDC, and this could be extensible to almost all the scalers (except the cloud provider services)

@jkremser
Copy link
Contributor

+1 using the same approach as Grafana folks. They require client_id and client_secret to be provided for each type of OAuth provider. Also

auth_url = https://foo/auth
token_url = https://foo/token
api_url = https://foo/userinfo 

seems to be common for all of them. Grafana also provides a simple way for authorization by running JMESPath queries over the json returned from /userinfo (api_url) endpoint. This part, together with the call to the /userinfo endpoint can be probably omitted, because all keda needs, is probably the jwt token and authN, but I am not OAuth expert.

Also not sure if the token refreshing logic is needed here or not. I guess it depends on the frequency of usage, maybe asking for a new token over and over again can be easier.

Maybe some code could be reused:

Copy link

stale bot commented Apr 14, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale All issues that are marked as stale due to inactivity label Apr 14, 2024
@JorTurFer JorTurFer added stale-bot-ignore All issues that should not be automatically closed by our stale bot and removed stale All issues that are marked as stale due to inactivity labels Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request All issues for new features that have not been committed to help wanted Looking for support from community needs-discussion stale-bot-ignore All issues that should not be automatically closed by our stale bot
Projects
Status: Proposed
Development

No branches or pull requests

6 participants