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

External plugins requiring auth do not work in 5.2.0+ #12551

Closed
bobrik opened this Issue Jul 9, 2018 · 9 comments

Comments

Projects
None yet
3 participants
@bobrik
Copy link
Contributor

commented Jul 9, 2018

What Grafana version are you using?

5.2.1

What datasource are you using?

Prometheus

What OS are you running grafana on?

Irrelevant

What did you do?

Open a dashboard with jdbranham-diagram-panel plugin.

What was the expected result?

Things work as they did before.

What happened instead?

Panel fails to load, because request is redirected to auth. The reason is missing cookies for /public/vendor/plugin-css/css.js. If I open css.js in a separate tab to cache it, next dashboard load works.

I suspect it broke in 8bcd55d after #12096. cc @davkal

It's very similar to systemjs/systemjs#1731.

@torkelo

This comment has been minimized.

Copy link
Member

commented Jul 10, 2018

thanks for identifying the commit that caused this, someone else reported it yesterday as well.

@davkal

This comment has been minimized.

Copy link
Contributor

commented Jul 10, 2018

Hi @bobrik, could you please help me reproducing this?

What I found so far:

  • I'm testing this on 30c882c which has 8bcd55d included
  • the installation i'm using is hosted by grafana.com, which has Google auth
  • I added the plugin panel to a dashboard with no issues
  • In a production deployment /public/vendor/plugin-css/css.js is part of a the bundle, so should not be loaded separately. Could you attach a screenshot of dev tools showing that the file is loaded independently?
  • the css files required by css.js from the plugin are requested with cookies (grafana_sess, grafana_user, grafana_remember)

Where does your setup differ?

@bobrik

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2018

We are using Grafana 5.2.1 docker image with plugin installed, running behind reverse proxy that does Google auth, similar to nginx-google-oauth. When I click to add Diagram panel:

image

The cookies we need are not grafana_ cookies, they are cookies set by out reverse proxy that does the auth and passes X-Webauth-User to Grafana if cookies are valid.

@torkelo

This comment has been minimized.

Copy link
Member

commented Jul 10, 2018

@davkal somehow the plugin css is not bundled and since it does not start with plugin it does not get the authorization config flag after you added the plugin prefix requirement. Maybe it’s the fact that this is systemjs plugin that makes the bundling not works. Was there a reason you added the plugin prefix , or was that in a PR that you merged?

@torkelo

This comment has been minimized.

Copy link
Member

commented Jul 11, 2018

So basically I think it's the adding of the plugin* prefix here that is the root of the problem as it makes the authorization: true flag not apply to css loader when it is being fetched. 8bcd55d#diff-7d38adb6eb3fcc661871ef8d0968ffcfR50

@davkal

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2018

There was no reason other than trying to limit the scope to plugins. But if that css.js import is still being done per file, then we should remove it. I'll make a PR.

@davkal

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2018

Noticed the problem now too, css.js was excluded from the plugin loader mechanism. PR opened.

@bobrik

This comment has been minimized.

Copy link
Contributor Author

commented Jul 11, 2018

Is there any ETA on 5.2.2 to have this in a public release?

@torkelo torkelo modified the milestones: 5.3, 5.2.2 Jul 12, 2018

@torkelo

This comment has been minimized.

Copy link
Member

commented Jul 12, 2018

No eta yet, maybe in 2 weeks?

marefr added a commit that referenced this issue Jul 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.