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 wildcard domain for 3rd party cookies to be set explicitly in config.ini.php #150

Conversation

wThomas84
Copy link

Pull request for the enhancements discussed in this thread: http://forum.piwik.org/read.php?2,107159,page=1#msg-107379

Please note that in order that this feature can work propperly an entry in the config.ini.php must be set. The key for that entry is "cookie_domain". I was not sure whether there must be a default value set somewhere else for that config key. Means: I do not know whether there would be a runtime error in line 418 of Request.php in case that value is not set in config.ini.php.

In order to be able to set a 3rd party cookie's domain explicitly (so
that the browser accepts it and sets it to the correct domain), the
constructor was adapted to accept a cookie domain parameter. The
concerning usages of that constructor were adapted as well.
Now it is possible to put a line 'cookie_domain = ",mydomain.tld" ' into
the config.ini.php that will be used by piwik.
Extend Cookie constructor to accept domain. Merged accidentially commited master into topic branch
The changes make sure that cookie wildcard domains always follow the
pattern ".<domain_name>", independent of the domain that was configured
in the config.ini.php for the key "cookie_domain".
@halfdan
Copy link
Member

halfdan commented Nov 20, 2013

Can you please rebase/squash these commits into a single one?

You can add the key to global.ini.php. Also make sure that the tests still pass.

@wThomas84
Copy link
Author

Hi,
that was a fast feedback :)

When I put that key into global.ini.php how is it actually interpreted in case I leave the value empty. From my understanding it should be null and not an empty string.

Please, can you give me a short hint / link about how or where I can run or let the tests run?

@wThomas84 wThomas84 closed this Nov 21, 2013
@wThomas84
Copy link
Author

Closed in order to be replaced by request with squashed commits.

@halfdan
Copy link
Member

halfdan commented Nov 21, 2013

No need to close - you can squash and then push -f to the same branch. The PR will automatically update.

@wThomas84
Copy link
Author

Ah, sorry, was looking for that solution. New request already open. Now I know for the next time.

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

Successfully merging this pull request may close these issues.

None yet

2 participants