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

Allow to customize some CSS of optOut widget #1929

Closed
halfdan opened this issue Jan 2, 2011 · 36 comments
Closed

Allow to customize some CSS of optOut widget #1929

halfdan opened this issue Jan 2, 2011 · 36 comments
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. worksforme The issue cannot be reproduced and things work as intended.
Milestone

Comments

@halfdan
Copy link
Member

halfdan commented Jan 2, 2011

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
Copy link
Member Author

halfdan commented Jan 2, 2011

Attachment:
OptOut.patch

@robocoder
Copy link
Contributor

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

@robocoder
Copy link
Contributor

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

@robocoder
Copy link
Contributor

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

@robocoder
Copy link
Contributor

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

@robocoder
Copy link
Contributor

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

@mattab
Copy link
Member

mattab commented Feb 17, 2011

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

@robocoder
Copy link
Contributor

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

@mattab
Copy link
Member

mattab commented Apr 28, 2011

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-matomo-user
Copy link

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-matomo-user
Copy link

Attachment:
ticket_1929.patch

@anonymous-matomo-user
Copy link

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-matomo-user
Copy link

Attachment:
ticket_1929_02.patch

@anonymous-matomo-user
Copy link

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
Copy link
Member

mattab commented Jan 8, 2012

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-matomo-user
Copy link

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
Copy link
Contributor

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-matomo-user
Copy link

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

@robocoder
Copy link
Contributor

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-matomo-user
Copy link

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-matomo-user
Copy link

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
Copy link
Contributor

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

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

@mattab
Copy link
Member

mattab commented Jan 25, 2012

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-matomo-user
Copy link

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
Copy link
Member

mattab commented Jan 26, 2012

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-matomo-user
Copy link

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
Copy link
Member

mattab commented Jan 26, 2012

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-matomo-user
Copy link

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

@anonymous-matomo-user
Copy link

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-matomo-user
Copy link

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-matomo-user
Copy link

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

@mattab
Copy link
Member

mattab commented Feb 15, 2013

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
Copy link
Member

mattab commented Apr 9, 2013

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

@gka
Copy link
Contributor

gka commented May 6, 2013

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

https://github.com/gka/piwik/compare/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-matomo-user
Copy link

You mean folder plugins/CoreAdminHome/templates/ ?

@mattab
Copy link
Member

mattab commented Jan 10, 2014

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

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. worksforme The issue cannot be reproduced and things work as intended.
Projects
None yet
Development

No branches or pull requests

5 participants