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: Add external credentials support #80

Conversation

kirillmakhonin-brt
Copy link
Contributor

  • Adds support for OAuth2 Client Credentials Grant - can be used to use external HTTP endpoint for pulling Databricks credentials (instead of using fixed PAT token).

@kirillmakhonin-brt
Copy link
Contributor Author

Hello @mullerpeter, could you please review this Pull Request?

@mullerpeter
Copy link
Owner

Hi @kirillmakhonin-brt

Thanks for opening the PR, but I have a few questions.

  • What is the specific use case why this is needed?
  • Is there any documentation on how the external HTTP endpoint to get the token should be setup? Is this something provided by databricks?
  • What happens if the token returned via OAuth expires?
  • I see the need for allowing other authentication then PAT tokens (especially one not tied to a user account). As mentioned here Databricks OAuth machine-to-machine (M2M) authentication #79 the Databricks OAuth M2M Auth could be the way to go. Would this be something you could also use for your use case? Because then I would rather go with this way, as this is supported natively by the databricks-sql-golang SDK. Support for this would just need to added to this plugin, which has been on my todo list.

@mullerpeter
Copy link
Owner

Hi @kirillmakhonin-brt

Support for OAuth machine-to-machine (M2M) authentication is now added in v1.2.6
Let me know if that also works for your use case or if this PR is still needed.

@kirillmakhonin-brt
Copy link
Contributor Author

@mullerpeter unfortunately this (M2M) requires you to still have your databricks credentials (in this case client id and secret) stored in Grafana, so you still need to go through credentials rotation protocol in secured envinronments.
If you OK - I'll just add an extra drop down with "OAuth2 Client Credentials Grant" to cover cases where credentials are stored outside of Grafana and just pulled in when they are needed

@kirillmakhonin-brt kirillmakhonin-brt force-pushed the feat/add-support-rfc6749-client-credentials-grant branch from 2fc3233 to 7a2aef0 Compare June 19, 2024 03:36
@kirillmakhonin-brt
Copy link
Contributor Author

kirillmakhonin-brt commented Jun 19, 2024

@mullerpeter updated, please review

What is the specific use case why this is needed?

This covers use cases when token provider is deployed on a side (e.g. in sidecar) and reads token from secure place (e.g. Hashicorp vault etc) where it is rotated on shedule

Is there any documentation on how the external HTTP endpoint to get the token should be setup? Is this something provided by databricks?

This is standard OAuth2 Client Credentials Grant

What happens if the token returned via OAuth expires?

Oauth2 library automatically pulls new token

I see the need for allowing other authentication then PAT tokens (especially one not tied to a user account). As mentioned here #79 the Databricks OAuth M2M Auth could be the way to go. Would this be something you could also use for your use case? Because then I would rather go with this way, as this is supported natively by the databricks-sql-golang SDK. Support for this would just need to added to this plugin, which has been on my todo list.

This is a special case where users levelrage any external OAuth2 compatible provider to store Databricks access tokens

@mullerpeter
Copy link
Owner

In general this PR looks good to me, I just think it needs a bit more documentation so that other people understand how to use this.

  • Is there any open-source token provider (or Managed Service) you could link to and with which this would work out of the box?
  • If not, just some basic information on how this token provider should be setup would be nice, incase this only support self-developed solutions. But then I also think this might be a bit too niche.
  • Do you know of other Grafana Plugins allowing this kind of OAuth authentication flow (or external secrets)? Because I could not find any.
  • Supporting for external secret providers like HashiCorp Vault or Azure Keyvault directly would be nice in general, but maybe a bit overkill for such a plugin...

@mullerpeter
Copy link
Owner

Another work around would be a sidecar to rotate the secret in grafana directly via update-an-existing-data-source API Endpoint, have you thought of that?

@kirillmakhonin-brt
Copy link
Contributor Author

Thank you!

  • Is there any open-source token provider (or Managed Service) you could link to and with which this would work out of the box?

I believe any OAuth2 provider should support this, e.g. keycloak, but problem is that Databricks token should be returned (which no one of public systems support afaik), so this is more a usage of open-source industry standard protocol for ability to extend authentication for Databricks

  • If not, just some basic information on how this token provider should be setup would be nice, incase this only support self-developed solutions. But then I also think this might be a bit too niche.

This is really well documented in RFC6749

  • Do you know of other Grafana Plugins allowing this kind of OAuth authentication flow (or external secrets)? Because I could not find any.

I dont know such details

  • Supporting for external secret providers like HashiCorp Vault or Azure Keyvault directly would be nice in general, but maybe a bit overkill for such a plugin...

Yes, that's why we've tried to find some open source standard protocol which can be easily implemented for secret storage providers (as a proxy)

@kirillmakhonin-brt
Copy link
Contributor Author

Another work around would be a sidecar to rotate the secret in grafana directly via update-an-existing-data-source API Endpoint, have you thought of that?

Yes, we are considering this as a backup, but it requires Grafana specific solution (which we try to avoid)

@mullerpeter
Copy link
Owner

Ok thanks for your reply.

Don't think this is a heavily requested feature for this plugin, especially since no other plugins seems to support this kind of auth flow. But as it's not introducing any downsides, I'm fine with merging it, I mean you need it apparently and maybe someone else will also make use of it.

Thanks a lot for the contribution, will create a new release.

@mullerpeter mullerpeter merged commit ab6a898 into mullerpeter:main Jun 20, 2024
3 checks passed
@kirillmakhonin-brt kirillmakhonin-brt deleted the feat/add-support-rfc6749-client-credentials-grant branch June 21, 2024 16:45
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.

2 participants