Adds a permission check for app directories #10538

weeman1337 · 2018-08-04T16:49:43Z

This PR adds a setup check for the permissions of the app root directories.

There is a test for the case the permissions are fine. It's hard to add a test for the wrong permissions, because an ordinary user can't do a chown.

Closes #10154

rullzer · 2018-08-09T14:23:53Z

I think it would be good to get this into 14 as well. As it fixes potential update issues with the update

weeman1337 · 2018-08-09T16:36:44Z

Here is a question: I messed around with my git config. For a while the sign off was broken. It didn't complain and added the "signed off" but actually didn't sign it at all o_O

I managed to fix it. Does anyone know a way how to sign off commits in GitHub afterwards?

Signed-off-by: Michael Weimann <mail@michael-weimann.eu>

juliusknorr · 2018-08-09T16:43:47Z

@weeman1337 There is a difference between signing your commits and adding a sign off message. The signoff message is fine for all those commits. The gpg signing seems to be missing, however you would need to do an interactive rebase and edit each commit. You can then just call git commit --amend --no-edit -S to sign your commits with gpg.

juliusknorr · 2018-08-09T16:44:12Z

@weeman1337 😁 Seems you already found it 👍

weeman1337 · 2018-08-09T16:46:18Z

@juliushaertl jep found it - this time it worked. Sometimes it seems that GitHub doesn't recognize updates to already existing commits... ¯\_(ツ)_/¯

juliusknorr · 2018-08-09T16:48:19Z

settings/Controller/CheckSetupController.php

+			$appsDir = new DirectoryIterator($appRoot['path']);
+			foreach ($appsDir as $fileInfo) {
+				if ($fileInfo->isDir() && !$fileInfo->isDot()) {
+					$absAppPath = $appsPath . DIRECTORY_SEPARATOR . $fileInfo->getFilename();


We should check from the config if the apps directory is marked as writable. If not, there is no reason to report it imho.

Signed-off-by: Joas Schilling <coding@schilljs.com>

only update the encrypted version after the write operation is finished and the stream is closed Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>

Signed-off-by: Julius Härtl <jus@bitgrid.net>

In some cases, the DeletedShareAPIController requires explicit handling of each type of share (for example, to format a share for a DataResponse). Room shares are implemented in an external app (Nextcloud Talk), so in order to keep the controller as isolated as possible from room share specifics all that explicit handling is done in a helper class provided by the Talk app. In other cases it is just enough to call the share manager specifying a room share type; note that the share manager is guarded against share types for which there is no provider, so it is not necessary to explicitly check that before passing room shares to the share manager. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

The test just ensures that the controller will gracefully reject the creation instead of failing miserably; the integration tests when Talk is enabled are in the Talk repository. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

The MountProvider for shares creates mount points for the files shared with the user, which makes possible to use the received shared files and folders as regular files and folders. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

A user can move her own shares into a received share. When that happens she is effectively handing over the ownership of the file, so the share needs to be updated to reflect the new owner. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Like done for other types of shares, room shares are now explicitly described as such in the UI. The avatar used is the image provided in the "shareWithAvatar" property of the share. If none is given then the avatar is the first letter of the display name of the room share with a coloured background seeded from the room token. If the display name of the room is empty then no letter is shown in the avatar; no special handling is done in that case. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Like done with group shares, received room shares are described as such in the UI. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

The DeletedShareAPIController and ShareAPIController helpers for room shares are defined in Talk, so the classes do not exist when Talk is not installed. Due to this when the object returned by "getRoomShareHelper" is used Phan complains that the class is not declared. This is not a problem, though, because when the class is not available "getRoomShareHelper" throws an exception, which is then caught where that method was called. Therefore now those warnings from Phan are suppressed (it would be better to use "@phan-suppress-next-line" instead, but it is not yet available in our Phan version). Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Tokens will be used to give access to a share to guests in public rooms. Although the token itself is created in the provider of room shares and no changes are needed for that, due to the code structure it is necessary to explicitly call the provider from the manager when getting a room share by token. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Fixes #10500. Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

weeman1337 · 2018-08-09T17:39:10Z

Ok - I messed it up :( I'll do a new branch/PR including the changes.

juliusknorr · 2018-08-09T18:01:03Z

@weeman1337 seems you tried to pull in recent master changes? A rebase on master and a force push is totally fine inside branches. 😉

weeman1337 · 2018-08-09T18:18:37Z

@juliushaertl yeah good to know. So far my prime directive was "never fiddle around in the git history, once it's public". :)

juliusknorr · 2018-08-09T18:59:02Z

Yeah, that is only problematic if multiple people work on the same branch. Otherwise we have very little issues with that. ;)

weeman1337 added the feature: apps management label Aug 4, 2018

weeman1337 requested review from MorrisJobke and rullzer and removed request for MorrisJobke August 4, 2018 16:56

rullzer requested review from nickvergessen, ChristophWurst and juliusknorr August 9, 2018 14:22

rullzer added this to the Nextcloud 14 milestone Aug 9, 2018

weeman1337 added the 3. to review Waiting for reviews label Aug 9, 2018

weeman1337 added 3 commits August 9, 2018 18:43

Adds a setup check for app directory permissions.

56a4779

Signed-off-by: Michael Weimann <mail@michael-weimann.eu>

Adds a test for the app directory permission check.

ec8c74f

Signed-off-by: Michael Weimann <mail@michael-weimann.eu>

Extends the setup check js tests

e092d8e

Signed-off-by: Michael Weimann <mail@michael-weimann.eu>

weeman1337 force-pushed the feature-10154-app-directory-permission-check branch from f8cd5f5 to e092d8e Compare August 9, 2018 16:43

juliusknorr requested changes Aug 9, 2018

View reviewed changes

nickvergessen and others added 10 commits August 9, 2018 19:33

Fix code errors

e1c4baa

Signed-off-by: Joas Schilling <coding@schilljs.com>

Add an option for mail addons

66a9ec9

Signed-off-by: Joas Schilling <coding@schilljs.com>

make file cache updates more robust

5cc8398

only update the encrypted version after the write operation is finished and the stream is closed Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>

Move locale preview to two lines to reduce grid height

57ca99d

Signed-off-by: Julius Härtl <jus@bitgrid.net>

Make user management heading font size lighter

30e7346

Signed-off-by: Julius Härtl <jus@bitgrid.net>

Rename CalDAV to Calendar settings

526e9e4

Signed-off-by: Julius Härtl <jus@bitgrid.net>

files_external: Move delete option to 3-dots menu

df361e6

Signed-off-by: Julius Härtl <jus@bitgrid.net>

files_external: Add description to settings

c639499

Signed-off-by: Julius Härtl <jus@bitgrid.net>

Move encryption settings to security

676337a

Signed-off-by: Julius Härtl <jus@bitgrid.net>

systemtags: Submit input on enter

24ae8e5

Signed-off-by: Julius Härtl <jus@bitgrid.net>

danxuliu and others added 21 commits August 9, 2018 19:33

Add room shares to DAV and recent files "share-types" property

1488f84

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Transfer room shares too with the "files:transfer-ownership" command

d9d28aa

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Log sharing and unsharing with a room in the auditing app

b4d5277

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Add custom handling for room shares to the resharer information

d5ef4db

Like done with group shares, received room shares are described as such in the UI. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Fix login redirection if only one 2FA provider is active

8db15e2

Fixes #10500. Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

Rename providerset method to get primary providers

6375718

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

Restores the guest icons

2d61496

Removed unused variable isFile

36d4481

Fix app code checker to ignore build directory

76292d9

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

Fix info.xsd to sync with the appstore one

cb9f3ec

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

Nextcloud 14.0.0 beta3

3895f11

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

Add integration/unit test for the double-insert of same values

f4a7be4

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

Fix double-inserts of the same provider state

114dbd5

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

Excludes not writable app roots from the directory permission check

b8c247a

weeman1337 closed this Aug 9, 2018

weeman1337 mentioned this pull request Aug 9, 2018

Adds a permission check for app directories #10628

Merged

weeman1337 deleted the feature-10154-app-directory-permission-check branch August 9, 2018 17:53

nextcloud-bot mentioned this pull request Sep 22, 2018

Add app rating counter next to star visualization #11318

Closed

nextcloud-bot mentioned this pull request Nov 12, 2018

Add CI job to check for fixup commits #12414

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds a permission check for app directories #10538

Adds a permission check for app directories #10538

weeman1337 commented Aug 4, 2018 •

edited

Loading

rullzer commented Aug 9, 2018

weeman1337 commented Aug 9, 2018

juliusknorr commented Aug 9, 2018

juliusknorr commented Aug 9, 2018

weeman1337 commented Aug 9, 2018

juliusknorr Aug 9, 2018

weeman1337 commented Aug 9, 2018

juliusknorr commented Aug 9, 2018

weeman1337 commented Aug 9, 2018

juliusknorr commented Aug 9, 2018

Adds a permission check for app directories #10538

Adds a permission check for app directories #10538

Conversation

weeman1337 commented Aug 4, 2018 • edited Loading

rullzer commented Aug 9, 2018

weeman1337 commented Aug 9, 2018

juliusknorr commented Aug 9, 2018

juliusknorr commented Aug 9, 2018

weeman1337 commented Aug 9, 2018

juliusknorr Aug 9, 2018

Choose a reason for hiding this comment

weeman1337 commented Aug 9, 2018

juliusknorr commented Aug 9, 2018

weeman1337 commented Aug 9, 2018

juliusknorr commented Aug 9, 2018

weeman1337 commented Aug 4, 2018 •

edited

Loading