[WIP] Allow * wildcard in trusted_domains to support unstable DNS names and unstable IP addresses

[jernst]

I'm not quite understanding how ownCloud releases are produced from git, nor how to run the tests, and so I don't quite know how to test this. I'm hoping that a friendly soul will advise and/or merge. Thanks.
Corresponding pull requests to docs coming.

[ghost]

Thanks a lot for your contribution! Contributions to the core repo require a signed contributors agreement http://owncloud.org/contribute/agreement/ Alternatively you can add a comment here stating that this contribution is MIT licensed. Some more details about out pull request workflow can be found here: http://owncloud.org/code-reviews-on-github/

[scrutinizer-notifier]

The inspection completed: 2 new issues, 1 updated code elements

[karlitschek]

@LukasReschke

[LukasReschke]

No space here after the (

[LukasReschke]

Thanks for your contribution @jernst.

We're using PHPUnit for our PHP unit tests, running a single test-suite is pretty straight forward:

1. Download PHPUnit (https://phpunit.de/getting-started.html)
2. Open a terminal
3. cd into the owncloud root directory
4. cd into the tests directory within that
5. run the unit test on the file that contains the test such as phpunit lib/request.php

The output will look like following if everything worked:

➜  tests git:(master) ✗ phpunit lib/request.php
PHPUnit 4.3.4 by Sebastian Bergmann.

Configuration read from /Users/lreschke/Programming/core/tests/phpunit.xml.dist

.............................................

Time: 366 ms, Memory: 12.00Mb

OK (45 tests, 60 assertions)

If something failed you will pretty much notice it as it will complain - I guess that should be enough to get you started for now to adjust the unit tests? I'm kinda reluctant to add new features without having them covered by tests.

Then we also have that fancy autotest.sh script which you can invoke with bash autotest.sh sqlite which runs ALL unit-tests and takes about 15 minutes. You normally don't need to run that.

By the way, don't worry about the "Failed" sign here on GitHub our Continuous Integration (CI) system is currently somewhat buggy, that system is running our autotest.sh script for every contribution to ensure that nothing broke. We hope to get that sorted out over christmas.

Let me know if you have any further question or require further assistance. I'd be glad to help you! :-)

[DeepDiver1975]

unit tests are required -> moving to OC8.1

[BernhardPosselt]

I feel like this is really bad from a security POV and basically circumvents the host header poisoning prevention. Use a free service like http://www.noip.com/ to handle dynamic ips or use a VPN 👎

[jernst]

@Raydiation: either you enforce security in the code base to the extent that it is "impossible" to run ownCloud with anything other than maximum security (per your view, whatever that is), in which case you make it impossible for users who know what they are doing (or are willing to take the risk) to do what they want. Or you set the right secure defaults, but let people override them. I'm in favor of #2, thus this proposed patch. I do not believe that it reduces security at all, unless the user decides they want that.

[LukasReschke]

Agreed with @jernst - if this change is done correctly I don't mind it.

There is always a compromise between usability and security. This one is fine and has certainly some reasonability behind it. (for example auto-provisoning in a fixed IP block / subdomain etc...)

[LukasReschke]

Requires unit tests and rebase. Otherwise we can't merge this :-(

[uboslinux]

We currently do our own ownCloud patch in UBOS, it's here: https://github.com/indiebox/ubos-owncloud/blob/master/owncloud/allow-wildcard-trusted.patch. This replaces this merge request.

Corresponding test is here: https://github.com/indiebox/ubos-owncloud/blob/master/owncloud/tests/OwnCloud1Test.pm but that uses the UBOS webapptest framework, not your's. I don't currently have time to get up to speed with your test infrastructure, so I don't know how to proceed.

[ghost]

Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/12057/
💣 Test FAILed. 💣

[LukasReschke]

Closing due to inactivity. Feel free to reopen once unit tests have been added, see

core/tests/lib/appframework/http/RequestTest.php

Lines 824 to 848 in 525745c

	public function testGetServerHostWithUntrustedDomain() {
	$this->config
	->expects($this->at(3))
	->method('getSystemValue')
	->with('trusted_domains')
	->will($this->returnValue(['my.trusted.host']));
	$this->config
	->expects($this->at(4))
	->method('getSystemValue')
	->with('trusted_domains')
	->will($this->returnValue(['my.trusted.host']));
	$request = new Request(
	[
	'server' => [
	'HTTP_X_FORWARDED_HOST' => 'my.untrusted.host',
	],
	],
	$this->secureRandom,
	$this->config,
	$this->stream
	);
	$this->assertEquals('my.trusted.host', $request->getServerHost());
	}

and Co. for examples.