Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Allow to customize some CSS of optOut widget #1929

Closed
halfdan opened this Issue · 36 comments

5 participants

@halfdan
Collaborator

Features to add the OptOut widget (now in CoreAdminHome):

  • option to disable the widget

  • using &css=.. as a parameter allows the user to style the content of the iframe

    • should guard against XSS; blacklist url(), expression(), and @import
  • using &control=.. the user can choose between 3 different control mechanisms (button, checkbox, textlink)

  • parameter for more terse text (i.e., as originally implemented)

@halfdan
Collaborator

Attachment:
OptOut.patch

@robocoder
Collaborator

Thanks, I'll commit as soon as I resolve the broken webtest...

@robocoder
Collaborator

(In [3555]) fixes #1929 - thanks halfdan; more refactoring

@robocoder
Collaborator

These enhancements were undone by r3556. I'm re-opening to continue the discussion from #1919.

@robocoder
Collaborator

(In [3816]) refs #1919, refs #1929, refs #1982 - delete tracking cookie when opting out (i.e., ignore cookie is set)

@robocoder
Collaborator

Time permitting, provide some workaround for the multi-domain issue raised in #1376.

@mattab
Owner

let's create a new ticket when we have optOut improvements

@robocoder
Collaborator

The original OptOut ticket is already closed. This ticket is for the improvements reverted in r3556.

@mattab
Owner

I slightly lower scope of this ticket, to allow customizing a few CSS such as

  • font size
  • font family
  • font color
  • background color

and check each substring that they have only valid characters (eg. numbers/letters/commas)

@anonymous-piwik-user

Wht is the current status or solution zu this?
None of our websites use a serif-like font, I would only need a minimal solution to set a system wide style for all websites. And I don't want to mess around in the code and rework after each update.

@anonymous-piwik-user

I had another idea to resolve the css problem of the optout iframe. I decided to add a new field to the site record called "css_path". It is shown in the SitesManager. The url of the iframe gets a new parameter "idsite" containing the site's id. If there is a css path set for that site a <link> is inserted in the optout, containing the path to the "main_url" appended with the "css_path".

I fairly new to piwik development so I may have missed something, e.g. the field must still be included in the database updates, I haven't found this, as well as the information about appending the "idsite" Parameter to the <iframe> url.

I created a patch based on the current trunk. May anyone take a look at it?

@anonymous-piwik-user

I found a little mistake that led to a template error because of a missing variable if the "idsite" parameter was not set or not css path was set for this site.

Therefore I attached "ticket_1929_0.patch".

@mattab
Owner

thommyhh thanks for the patch, but probably a easier way to implement this feature would be to add GET parameters to the iframe, for example

  • fontSize integer
  • fontColor hex
  • backgroundColor hex
  • etc. for the styles we want to allow overriding to allow better iframe look and feel when integrated in websites.

If you like to produce patch for this implementation we would gladly commit it indeed, thanks!

@anonymous-piwik-user

I noticed the idea of changing some css parameters, but I have set also properties like padding and/or margin. So I thought it is a better idea to have the possibility to add a complete css file to include instead of changing some hard coded properties. Also, with the way you mentioned above you can only set the properties for a single element, e.g. the <body> tag, instead of styleing the <body> or the <p>.

What do you mean with the last point? What to override?

@robocoder
Collaborator

I think the most flexible path is a table. (Piwik CMS, anyone?)

Piwik_templates

  • template_id - auto inc int
  • path - text (controller name + action name)

Piwik_template_content

  • template_id - int
  • lang - char(2)
  • css - text
  • html body - text

(Defer disabling the widget to ACL implementation.)

@anonymous-piwik-user

Do you mean a table holding css definitions for each page?

@robocoder
Collaborator

A table holding css and body text that could be localized.

If css is non-empty, a <style> section is appended to the <head>.

If body is empty, then the default template is used.

If lang is empty, this is the default if the user's locale doesn't exist.

@anonymous-piwik-user

The problem here is, that you want the css under control of the editors/admins of the website not the piwik user/admin. It may be the same, but doesn't need to. That's why we want to <link> a css file from the webserver.

In our case the piwik admins and website admins are the same. But it can easily be two different companies not giving each other permission to access their system.

@anonymous-piwik-user

Another problem you seem not to address is the different css for each site configured in piwik. Thats the problem we have here. We are starting to use one piwik installation for several websites and every website has of course its own styles, that we want to use in the opt out iframe as well.

@robocoder
Collaborator

This is just a brain dump while I'm on the train:

  • expose lightweight API to get / set cookie state
  • provide a javascript client to set / delete first party cookie (#1376)
  • refactor widget to use API; serves as a reference/example implementation

Customizing css and/or text becomes external to Piwik. (In the absence of theming plugin.)

@mattab
Owner

We need to keep this easy and simple: css provided as GET parameters is enough. What are the parameters we wish to customize?

Then we only allow characters a-Z0-9 for example, and a few others like #. etc.

Because security wise, we cannot allow to insert Xss in this iframe, and is why we must restrict css that can be overridden. Please submit a simple patch if you can :)

@anonymous-piwik-user

Overriding some css properties for the whole document is definitely not enough. You can not style the document like this. You can only style one element. I would then suggest to use to originally intended version with adding the css file via get parameter (&css=foo).

I think there is no xss vulnerability because if you are at the point that you can change the "&css" Parameter in the iframe you must have already got into the website itself and the owner has a much bigger problem than the manipulated Parameter. And as the file is only <link>ed, I don't see the problem here.

@mattab
Owner

Overriding some css properties for the whole document is definitely not enough. You can not style the document like this. You can only style one element. I would then suggest to use to originally intended version with adding the css file via get parameter (&css=foo).

What styles do you need exactly? Are there really too many? most css styles can fit into a few properties.

I think there is no xss vulnerability because if you are at the point that you can change the "&css" Parameter in the iframe you must have already got into the website itself and the owner has a much bigger problem than the manipulated Parameter.

What we are concerned here is not website vuln, but Piwik vuln, where a XSS in Piwik itself is of course considered a big issue for us. See http://ha.ckers.org/xss.html for more info and why it is not acceptable to load a user submitted css.

@anonymous-piwik-user

First to the xss stuff. I tried in in IE6, it works but I still don't get the point why this is critical to Piwik. There is nothing stored at this point so the xss vulnerability is only non-persistent and I don't see the attack vector here.

The problem about the css properties is, that I want to use several css properties like padding, margin, font-style, color, may be line-height, text-decoration... There are to much css properties to make this hardcoded what properties can be used. And additionally you can e.g. then only style the <body> tag and not use the normal css rules.

Another important point is, that the control should be at the website editors/admins not the Piwik admins.

@mattab
Owner

The xss could allow to steal a piwik user cookie, session, etc.

If we have to use GET parameters, what CSS properties would be required?

@anonymous-piwik-user

At least the font-family and color are a must have.

@anonymous-piwik-user

Well, as css inclusion is not an option because of xss and css properties set by get params isn't either because of missing flexibility I think I will go back to my first idea to set a css path in the sitesmanager.

As I said before adding style properties to the <body> tag does not make sense. There must be the possibility to add complete css definitions like

* {
  margin: 0;
  padding: 0;
}

body {
  color: ...;
  font-family:....;
  font-size: 12px;
}

p {
  margin-top: 10px;
}

....

Do you see a way to do this?

@anonymous-piwik-user

Have tried the ticket_1929_02.patch with Piwik 1.8.2.
Result in Settings > Websites > "CSS Path":

Notice: Undefined index: css_path in .../piwik/tmp/templates_c/%%EE^EE8^EE8EF7B4%%SitesManager.tpl.php on line 215

Backtrace -->
#0 Piwik_ErrorHandler(...) called at [.../piwik/tmp/templates_c/%%EE^EE8^EE8EF7B4%%SitesManager.tpl.php:215]#1 include(...) called at [.../piwik/libs/Smarty/Smarty.class.php:1263]#2 Smarty->fetch(...) called at [.../piwik/core/View.php:133]#3 Piwik_View->render(...) called at [.../piwik/plugins/SitesManager/Controller.php:66]#4 Piwik_SitesManager_Controller->index(...) called at [:]#5 call_user_func_array(...) called at [.../piwik/core/FrontController.php:138]#6 Piwik_FrontController->dispatch(...) called at [.../piwik/index.php:53]

Is there a solution?

@anonymous-piwik-user

Is this planned for the next release or what is the current blockade to get this rtbc?

@mattab
Owner

I guess a nice and powerful solution would be to ask users to put a custom CSS file in eg. piwik/custom-optout.css and, if this file exists, will be output in the opt out HTML.

It's not planned but we could add this Im sure!

@mattab
Owner

There is a nice solution posted in: http://forum.piwik.org/read.php?6,102702 to allow customize the opt out iframe CSS.

@gka
Collaborator
gka commented

Here's a very simple fix for custom out-out styling:

gka@piwik:2.x-twig...2.x-optout-custom-style

It works as proposed by Matt before, but it also allows for different styles. The user can put custom CSS files in the /themes/ folder. The file names must start with optout_, e.g. optout_foo.css.

When embedding the iFrame the user must add a GET parameter style, e.g. &style=foo. Piwik then checks if the file exists and includes it.

The characters :, ., and / are replaced to prevent XSS attacks.

Simple solution, I think we should add it to Core.

@anonymous-piwik-user

You mean folder plugins/CoreAdminHome/templates/ ?

@mattab
Owner

Kuddos to Jens Averkamp who released a plugin that lets you customize the CSS!!
Check it out at http://plugins.piwik.org/CustomOptOut

Please let him know any feedback on https://github.com/Zeichen32/PiwikCustomOptOut/issues

Cheers

@mattab mattab added the R: worksforme label
@nerdoc nerdoc referenced this issue in medworx/kindisch
Closed

PIWIK Opt-Out lässt sich kaum anpassen #146

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.