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

Include external id in session cache key #56

Closed
wants to merge 1 commit into from

Conversation

mtanda
Copy link

@mtanda mtanda commented Jun 9, 2022

If external id is specified, API call from datasource with invalid external id, should fail.
https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_create_for-user_externalid.html

Currently, Grafana doesn't include external id in cache key.
It may cause issue.

If Grafana has two datasource settings.

  • datasource A with valid external ID
  • datasource B with invalid external ID

Grafana will create one session cache from these two datasource setting.
If Grafana cache datasource A, API call which using datasource B will success (it should fail).

I suggest to include external id in cache key.

Copy link
Contributor

@andresmgot andresmgot left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @mtanda, we are aware of this issue. It happens with more fields, not only the ExternalID (also the SecretKey, for example). We have discussed this in grafana/athena-datasource#144 (comment) but we didn't get to it yet. Our idea is to use a hash of all the settings so we catch any change.

@mtanda
Copy link
Author

mtanda commented Jun 10, 2022

@andresmgot Thanks for reply :-)
The discussed issue seems to be similar issue. By using hash of all the settings will solve my issue too.
So I close my PR.

@mtanda mtanda closed this Jun 10, 2022
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.

None yet

2 participants