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

403 in viz.json for private/password-protected visualizations #2306

Closed
rochoa opened this issue Feb 18, 2015 · 14 comments
Closed

403 in viz.json for private/password-protected visualizations #2306

rochoa opened this issue Feb 18, 2015 · 14 comments
Assignees
Milestone

Comments

@rochoa
Copy link
Contributor

rochoa commented Feb 18, 2015

STR

  1. Create a new private visualization
  2. Click on share
  3. Access the URL from CartoDB.js option

Current result

Even being authenticated it returns a 403 error

Expected result

Being authenticated it should return the viz.json
Non-authenticated and non-authorized (if the visualization is shared in an organization) requests should keep raising a 403 error

Although I tagged this as a bug I'm not sure if this was intentional from the very beginning.

@rochoa
Copy link
Contributor Author

rochoa commented Feb 18, 2015

@Kartones let's discuss this here

cc'ing @javierarce and @xavijam as this affects new dashboard previews

@rochoa
Copy link
Contributor Author

rochoa commented Feb 18, 2015

As @javierarce mentioned in #2297 this also happens in password protected visualizations.

@rochoa rochoa changed the title 403 in viz.json for private visualizations 403 in viz.json for private/password-protected visualizations Feb 18, 2015
@rochoa
Copy link
Contributor Author

rochoa commented Feb 18, 2015

@Kartones
Copy link
Contributor

Ok, it was done on purpose but can be easily changed:

def allow_vizjson_v2_for?(visualization)
  visualization && (visualization.public? || visualization.public_with_link?)
end

I can easily change it to allow to pass always if authenticated

@rochoa
Copy link
Contributor Author

rochoa commented Feb 18, 2015

I will go with that approach... but remember we may cache the viz.json

@Kartones
Copy link
Contributor

in Varnish? no prob, I'll check that as IIRC on privacy change of a visualization we invalidate Varnish, else implement it.

@rochoa
Copy link
Contributor Author

rochoa commented Feb 18, 2015

It's not only about privacy change. I mean:

Let's say the visualization is "private" and I'm the owner. I authenticate and request the viz.json and it gets cached in varnish. Now I open an incognito window and request the viz.json, varnish will attend the request because it's cached and won't check privacy in rails/backend.

@Kartones
Copy link
Contributor

uh, ok, then private/password protected need to be NOT cached, right? (and invalidated upon privacy change).

Is the only safe & easy way I see... either what options do we have?

@rochoa
Copy link
Contributor Author

rochoa commented Feb 18, 2015

The easier way is not caching private/password-protected viz.json

@rochoa
Copy link
Contributor Author

rochoa commented Feb 18, 2015

Have in mind that it will troublesome in performance terms

@Kartones
Copy link
Contributor

Yep, but against that we can only cover with lower level caching... in the backend as you suggested. /cc @rafatower

@Kartones Kartones added this to the Sanabria milestone Feb 18, 2015
@Kartones Kartones self-assigned this Feb 18, 2015
Kartones pushed a commit that referenced this issue Feb 18, 2015
@rafatower
Copy link
Contributor

We can run the privacy checks in the controller and return the full viz.json cached. Invalidation should be orthogonal with respect to privacy checks (and other before checks).

Anyways the big performance penalty is on hitting the rails stack.

BTW, I have some stats I'll share with you guys.

(sampled over a 24h period on the 16th)

Performance-wise, we need profiling.

@rafatower
Copy link
Contributor

Related to #2194

Invalidations shall be different for redis and varnish:

  • redis: no invalidation on privacy changes
  • varnish: always

@Kartones
Copy link
Contributor

Issue of not detecting cookie based sessions moved to #2394

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants