occ web executor by VicDeo · Pull Request #24957 · owncloud/core

VicDeo · 2016-06-01T17:00:20Z

As per #21985

DeepDiver1975 · 2016-06-01T21:09:56Z

Nice! Go for it!

mmattel · 2016-06-02T07:08:03Z

Reminder:
We must not forget to check Apache/nginx rules to allow web access for the occ command (according to #21985 https://domain.tld/owncloud/index.php/occ/app:list?format=json).

Maybe also a check in core if web access to occ is possible and log this - else we never find why it is not working...

Possible changes needed:

Apache (in core)

 RewriteRule ^(\.|autotest|occ|issue|indie|db_|console).* - [R=404,L]

nginx (in the documentation):

location ~ ^/(?:\.|autotest|occ|issue|indie|db_|console) {
        deny all;
    }

nickvergessen · 2016-06-02T09:03:41Z

Woot index.php/occ is called, not the occ file directly?

PVince81 · 2016-06-02T13:38:27Z

Woot index.php/occ is called, not the occ file directly?

Probably because the ajax request needs to be translated to actual arguments.
Might work as well to call the "occ" one directly but then you lose the advantage of routing / parameter mapping.

nickvergessen · 2016-06-02T13:46:43Z

Probably because the ajax request needs to be translated to actual arguments.
Might work as well to call the "occ" one directly but then you lose the advantage of routing / parameter mapping.

Well you shouldn't call it directly, because it's a bash file ;) But that is exactly what I wanted to point out. The occ file is still not needed to be accessible, so why should any rewrite rule require changing?

mmattel · 2016-06-02T14:34:11Z

@nickvergessen you are right.
I have overseen that https://domain.tld/owncloud/index.php/occ/app:list?format=json does not access occ via ownclouds root as a shell script. Root access is covered and secured by the rewrite rules. Sorry for the confusion. 😄

DeepDiver1975 · 2016-06-06T15:07:36Z

@VicDeo what's the status with this? THX

PVince81 · 2016-06-10T14:22:31Z

Code already looks good so far

nickvergessen · 2016-06-13T09:45:59Z

core/Controller/OccController.php

+		'app:getpath',
+		'app:list',
+		'check',
+		'config:list',


can we check on the parameters? --private should not be allowed, because otherwise you can get the plaintext database, mail and other passwords.

@nickvergessen --private is required to read updater.secret see owncloud/updater#233

VicDeo · 2016-06-13T14:38:16Z

@PVince81 The last step is

Bypass maintenance mode

PVince81 · 2016-06-15T09:38:37Z

@VicDeo when you change labels please post a comment. Changing labels doesn't trigger a github notification.

So this is ready for review ?

nickvergessen · 2016-06-15T10:10:14Z

core/Controller/OccController.php

+		'upgrade'
+	];
+
+	public function __construct($appName, IRequest $request) {


just leave this out?

VicDeo · 2016-06-15T16:10:25Z

@nickvergessen thanks, everything's addressed

DeepDiver1975 · 2016-06-20T09:07:02Z

OccController has no unit tests - this has to be added please

VicDeo · 2016-06-21T00:54:10Z

@DeepDiver1975 done

DeepDiver1975 · 2016-06-21T06:55:18Z

@VicDeo integration tests look fishy - please have a look

21:23:59 [Mon Jun 20 21:24:55 2016] Argument 1 passed to OC::checkMaintenanceMode() must implement interface OCP\IRequest, none given, called in /ssd/jenkins/workspace/ocs-api-integration-tests-ci/public.php on line 38 and defined at /ssd/jenkins/workspace/ocs-api-integration-tests-ci/lib/base.php#280
21:23:59 [Mon Jun 20 21:24:55 2016] Undefined variable: request at /ssd/jenkins/workspace/ocs-api-integration-tests-ci/lib/base.php#282
21:23:59 [Mon Jun 20 21:24:55 2016] PHP Fatal error:  Call to a member function getScriptName() on null in /ssd/jenkins/workspace/ocs-api-integration-tests-ci/lib/base.php on line 282
21:23:59 [Mon Jun 20 21:24:55 2016] PHP Stack trace:
21:23:59 [Mon Jun 20 21:24:55 2016] PHP   1. {main}() /ssd/jenkins/workspace/ocs-api-integration-tests-ci/public.php:0
21:23:59 [Mon Jun 20 21:24:55 2016] PHP   2. OC::checkMaintenanceMode() /ssd/jenkins/workspace/ocs-api-integration-tests-ci/public.php:38
21:23:59 [Mon Jun 20 21:24:55 2016] ::1:56378 [500]: /public.php/webdav/ - Call to a member function getScriptName() on null in /ssd/jenkins/workspace/ocs-api-integration-tests-ci/lib/base.php on line 282
21:23:59 [Mon Jun 20 21:24:55 2016] Call to a member function getScriptName() on null at /ssd/jenkins/workspace/ocs-api-integration-tests-ci/lib/base.php#282
21:23:59 [Mon Jun 20 21:24:55 2016] Internal Server Error

DeepDiver1975 · 2016-06-21T07:00:15Z

tests/Core/Controller/OccControllerTest.php

+	private $controller;
+
+	public static function setUpBeforeClass(){
+		self::$oldSecret = \OC::$server->getConfig()->setSystemValue('updater.secret', '');


@VicDeo please inject an instance of IConfig into the OccController and mock this onject for testing. THX

Fix broken integration test OccControllerTests do not require database access - moch them all! Kill unused sprintf

VicDeo · 2016-06-21T15:46:54Z

tests/Core/Controller/OccControllerTest.php

+			->willReturnCallback(
+				function ($input, $output) {
+					/** @var Output $output */
+					$output->writeln('{"installed":true,"version":"9.1.0.8","versionstring":"9.1.0 beta 2","edition":""}');


@DeepDiver1975 I didn't go this way because it requires adjustment with any version bump.

Not really relevant because we don't assert on the version.

DeepDiver1975 · 2016-06-22T11:12:16Z

👍

* Initial web executor * Fix PHPDoc Fix broken integration test OccControllerTests do not require database access - moch them all! Kill unused sprintf

PVince81 · 2016-06-24T15:27:45Z

stable9: #25222

* Initial web executor * Fix PHPDoc Fix broken integration test OccControllerTests do not require database access - moch them all! Kill unused sprintf

lock · 2019-08-05T09:01:32Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

VicDeo added the 2 - Developing label Jun 1, 2016

VicDeo added this to the 9.1-current milestone Jun 1, 2016

PVince81 added the feature:update label Jun 8, 2016

VicDeo force-pushed the web-executor branch from 212fb0c to 4337610 Compare June 9, 2016 19:36

nickvergessen reviewed Jun 13, 2016
View reviewed changes

VicDeo changed the title ~~[WIP] occ web executor~~ occ web executor Jun 13, 2016

VicDeo added 3 - To Review sev2-high and removed 2 - Developing labels Jun 13, 2016

nickvergessen reviewed Jun 15, 2016
View reviewed changes

nickvergessen added 2 - Developing and removed 3 - To Review labels Jun 15, 2016

VicDeo mentioned this pull request Jun 15, 2016

Switching to web-executor owncloud/updater#355

Merged

6 tasks

VicDeo force-pushed the web-executor branch from aae91eb to 84a883b Compare June 15, 2016 16:07

VicDeo added 3 - To Review and removed 2 - Developing labels Jun 15, 2016

PVince81 added the backport-request label Jun 17, 2016

DeepDiver1975 self-assigned this Jun 20, 2016

DeepDiver1975 force-pushed the web-executor branch from 84a883b to 8f73e7b Compare June 20, 2016 09:05

VicDeo force-pushed the web-executor branch from 8f73e7b to 9954493 Compare June 21, 2016 00:48

DeepDiver1975 reviewed Jun 21, 2016
View reviewed changes

DeepDiver1975 force-pushed the web-executor branch from 1b4de3b to c93a38e Compare June 21, 2016 09:59

VicDeo and others added 2 commits June 21, 2016 14:41

Initial web executor

8b9b515

Fix PHPDoc

ab477f9

Fix broken integration test OccControllerTests do not require database access - moch them all! Kill unused sprintf

DeepDiver1975 force-pushed the web-executor branch from c93a38e to ab477f9 Compare June 21, 2016 12:41

VicDeo reviewed Jun 21, 2016
View reviewed changes

DeepDiver1975 merged commit 854352d into master Jun 22, 2016

DeepDiver1975 deleted the web-executor branch June 22, 2016 11:12

DeepDiver1975 pushed a commit that referenced this pull request Jun 22, 2016

occ web executor (#24957)

b027cd6

* Initial web executor * Fix PHPDoc Fix broken integration test OccControllerTests do not require database access - moch them all! Kill unused sprintf

DeepDiver1975 mentioned this pull request Jun 22, 2016

[stable9] occ web executor #25222

Merged

PVince81 removed the backport-request label Jun 24, 2016

DeepDiver1975 pushed a commit that referenced this pull request Jun 27, 2016

occ web executor (#24957)

e42c088

* Initial web executor * Fix PHPDoc Fix broken integration test OccControllerTests do not require database access - moch them all! Kill unused sprintf

DeepDiver1975 pushed a commit that referenced this pull request Jul 1, 2016

occ web executor (#24957)

9fcb269

* Initial web executor * Fix PHPDoc Fix broken integration test OccControllerTests do not require database access - moch them all! Kill unused sprintf

MorrisJobke mentioned this pull request Jul 7, 2016

Sync master nextcloud/server#333

Merged

lock bot locked as resolved and limited conversation to collaborators Aug 5, 2019

Comments

Conversation

VicDeo commented Jun 1, 2016

Uh oh!

DeepDiver1975 commented Jun 1, 2016

Uh oh!

mmattel commented Jun 2, 2016

Uh oh!

nickvergessen commented Jun 2, 2016

Uh oh!

PVince81 commented Jun 2, 2016

Uh oh!

nickvergessen commented Jun 2, 2016

Uh oh!

mmattel commented Jun 2, 2016

Uh oh!

DeepDiver1975 commented Jun 6, 2016

Uh oh!

PVince81 commented Jun 10, 2016

Uh oh!

nickvergessen Jun 13, 2016

Choose a reason for hiding this comment

Uh oh!

VicDeo Jun 13, 2016

Choose a reason for hiding this comment

Uh oh!

VicDeo commented Jun 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PVince81 commented Jun 15, 2016

Uh oh!

nickvergessen Jun 15, 2016

Choose a reason for hiding this comment

Uh oh!

VicDeo commented Jun 15, 2016

Uh oh!

DeepDiver1975 commented Jun 20, 2016

Uh oh!

VicDeo commented Jun 21, 2016

Uh oh!

DeepDiver1975 commented Jun 21, 2016

Uh oh!

DeepDiver1975 Jun 21, 2016

Choose a reason for hiding this comment

Uh oh!

VicDeo Jun 21, 2016

Choose a reason for hiding this comment

Uh oh!

DeepDiver1975 Jun 21, 2016

Choose a reason for hiding this comment

Uh oh!

DeepDiver1975 commented Jun 22, 2016

Uh oh!

PVince81 commented Jun 24, 2016

Uh oh!

lock bot commented Aug 5, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

VicDeo commented Jun 13, 2016 •

edited

Loading