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
Comments
Attachment: |
Thanks, I'll commit as soon as I resolve the broken webtest... |
(In [3555]) fixes #1929 - thanks halfdan; more refactoring |
These enhancements were undone by r3556. I'm re-opening to continue the discussion from #1919. |
Time permitting, provide some workaround for the multi-domain issue raised in #1376. |
let's create a new ticket when we have optOut improvements |
The original OptOut ticket is already closed. This ticket is for the improvements reverted in r3556. |
I slightly lower scope of this ticket, to allow customizing a few CSS such as
and check each substring that they have only valid characters (eg. numbers/letters/commas) |
Wht is the current status or solution zu this? |
Attachment: |
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? |
Attachment: |
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". |
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
If you like to produce patch for this implementation we would gladly commit it indeed, thanks! |
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? |
I think the most flexible path is a table. (Piwik CMS, anyone?) Piwik_templates
Piwik_template_content
(Defer disabling the widget to ACL implementation.) |
Do you mean a table holding css definitions for each page? |
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. |
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. |
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. |
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.) |
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 :) |
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. |
What styles do you need exactly? Are there really too many? most css styles can fit into a few properties.
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. |
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. |
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? |
At least the font-family and color are a must have. |
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
Do you see a way to do this? |
Have tried the ticket_1929_02.patch with Piwik 1.8.2.
Is there a solution? |
Is this planned for the next release or what is the current blockade to get this rtbc? |
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! |
There is a nice solution posted in: http://forum.piwik.org/read.php?6,102702 to allow customize the opt out iframe CSS. |
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. |
You mean folder plugins/CoreAdminHome/templates/ ? |
Kuddos to Jens Averkamp who released a plugin that lets you customize the CSS!! Please let him know any feedback on https://github.com/Zeichen32/PiwikCustomOptOut/issues Cheers |
Features to add the OptOut widget (now in CoreAdminHome):
The text was updated successfully, but these errors were encountered: