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 restricting of app password permissions #719

Merged
merged 18 commits into from Nov 16, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/dav/lib/Connector/Sabre/Auth.php
Expand Up @@ -159,6 +159,7 @@ function check(RequestInterface $request, ResponseInterface $response) {
} catch (Exception $e) {
$class = get_class($e);
$msg = $e->getMessage();
\OC::$server->getLogger()->logException($e);
throw new ServiceUnavailable("$class: $msg");
}
}
Expand Down
7 changes: 7 additions & 0 deletions db_structure.xml
Expand Up @@ -1152,6 +1152,13 @@
<length>4</length>
</field>

<field>
<name>scope</name>
<type>clob</type>
<default></default>
<notnull>false</notnull>
</field>

<index>
<name>authtoken_token_index</name>
<unique>true</unique>
Expand Down
4 changes: 4 additions & 0 deletions lib/composer/composer/autoload_classmap.php
Expand Up @@ -189,6 +189,7 @@
'OCP\\LDAP\\ILDAPProviderFactory' => $baseDir . '/lib/public/LDAP/ILDAPProviderFactory.php',
'OCP\\Lock\\ILockingProvider' => $baseDir . '/lib/public/Lock/ILockingProvider.php',
'OCP\\Lock\\LockedException' => $baseDir . '/lib/public/Lock/LockedException.php',
'OCP\\Lockdown\\ILockdownManager' => $baseDir . '/lib/public/Lockdown/ILockdownManager.php',
'OCP\\Mail\\IMailer' => $baseDir . '/lib/public/Mail/IMailer.php',
'OCP\\Migration\\IOutput' => $baseDir . '/lib/public/Migration/IOutput.php',
'OCP\\Migration\\IRepairStep' => $baseDir . '/lib/public/Migration/IRepairStep.php',
Expand Down Expand Up @@ -580,6 +581,9 @@
'OC\\Lock\\DBLockingProvider' => $baseDir . '/lib/private/Lock/DBLockingProvider.php',
'OC\\Lock\\MemcacheLockingProvider' => $baseDir . '/lib/private/Lock/MemcacheLockingProvider.php',
'OC\\Lock\\NoopLockingProvider' => $baseDir . '/lib/private/Lock/NoopLockingProvider.php',
'OC\\Lockdown\\Filesystem\\NullCache' => $baseDir . '/lib/private/Lockdown/Filesystem/NullCache.php',
'OC\\Lockdown\\Filesystem\\NullStorage' => $baseDir . '/lib/private/Lockdown/Filesystem/NullStorage.php',
'OC\\Lockdown\\LockdownManager' => $baseDir . '/lib/private/Lockdown/LockdownManager.php',
'OC\\Log' => $baseDir . '/lib/private/Log.php',
'OC\\Log\\ErrorHandler' => $baseDir . '/lib/private/Log/ErrorHandler.php',
'OC\\Log\\Errorlog' => $baseDir . '/lib/private/Log/Errorlog.php',
Expand Down
4 changes: 4 additions & 0 deletions lib/composer/composer/autoload_static.php
Expand Up @@ -219,6 +219,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
'OCP\\LDAP\\ILDAPProviderFactory' => __DIR__ . '/../../..' . '/lib/public/LDAP/ILDAPProviderFactory.php',
'OCP\\Lock\\ILockingProvider' => __DIR__ . '/../../..' . '/lib/public/Lock/ILockingProvider.php',
'OCP\\Lock\\LockedException' => __DIR__ . '/../../..' . '/lib/public/Lock/LockedException.php',
'OCP\\Lockdown\\ILockdownManager' => __DIR__ . '/../../..' . '/lib/public/Lockdown/ILockdownManager.php',
'OCP\\Mail\\IMailer' => __DIR__ . '/../../..' . '/lib/public/Mail/IMailer.php',
'OCP\\Migration\\IOutput' => __DIR__ . '/../../..' . '/lib/public/Migration/IOutput.php',
'OCP\\Migration\\IRepairStep' => __DIR__ . '/../../..' . '/lib/public/Migration/IRepairStep.php',
Expand Down Expand Up @@ -610,6 +611,9 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
'OC\\Lock\\DBLockingProvider' => __DIR__ . '/../../..' . '/lib/private/Lock/DBLockingProvider.php',
'OC\\Lock\\MemcacheLockingProvider' => __DIR__ . '/../../..' . '/lib/private/Lock/MemcacheLockingProvider.php',
'OC\\Lock\\NoopLockingProvider' => __DIR__ . '/../../..' . '/lib/private/Lock/NoopLockingProvider.php',
'OC\\Lockdown\\Filesystem\\NullCache' => __DIR__ . '/../../..' . '/lib/private/Lockdown/Filesystem/NullCache.php',
'OC\\Lockdown\\Filesystem\\NullStorage' => __DIR__ . '/../../..' . '/lib/private/Lockdown/Filesystem/NullStorage.php',
'OC\\Lockdown\\LockdownManager' => __DIR__ . '/../../..' . '/lib/private/Lockdown/LockdownManager.php',
'OC\\Log' => __DIR__ . '/../../..' . '/lib/private/Log.php',
'OC\\Log\\ErrorHandler' => __DIR__ . '/../../..' . '/lib/private/Log/ErrorHandler.php',
'OC\\Log\\Errorlog' => __DIR__ . '/../../..' . '/lib/private/Log/Errorlog.php',
Expand Down
33 changes: 33 additions & 0 deletions lib/private/Authentication/Token/DefaultToken.php
Expand Up @@ -87,6 +87,17 @@ class DefaultToken extends Entity implements IToken {
*/
protected $lastCheck;

/**
* @var string
*/
protected $scope;

public function __construct() {
$this->addType('type', 'int');
$this->addType('lastActivity', 'int');
$this->addType('lastCheck', 'int');
}

public function getId() {
return $this->id;
}
Expand Down Expand Up @@ -119,6 +130,7 @@ public function jsonSerialize() {
'name' => $this->name,
'lastActivity' => $this->lastActivity,
'type' => $this->type,
'scope' => $this->getScopeAsArray()
];
}

Expand All @@ -140,4 +152,25 @@ public function setLastCheck($time) {
return parent::setLastCheck($time);
}

public function getScope() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a little redundant right.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

required to implement the interface

return parent::getScope();
}

public function getScopeAsArray() {
$scope = json_decode($this->getScope(), true);
if (!$scope) {
return [
'filesystem'=> true
];
}
return $scope;
}

public function setScope($scope) {
if (is_array($scope)) {
parent::setScope(json_encode($scope));
} else {
parent::setScope((string)$scope);
}
}
}
31 changes: 27 additions & 4 deletions lib/private/Authentication/Token/DefaultTokenMapper.php
Expand Up @@ -72,17 +72,40 @@ public function invalidateOld($olderThan, $remember = IToken::DO_NOT_REMEMBER) {
public function getToken($token) {
/* @var $qb IQueryBuilder */
$qb = $this->db->getQueryBuilder();
$result = $qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'remember', 'token', 'last_activity', 'last_check')
$result = $qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'remember', 'token', 'last_activity', 'last_check', 'scope')
->from('authtoken')
->where($qb->expr()->eq('token', $qb->createParameter('token')))
->setParameter('token', $token)
->where($qb->expr()->eq('token', $qb->createNamedParameter($token)))
->execute();

$data = $result->fetch();
$result->closeCursor();
if ($data === false) {
throw new DoesNotExistException('token does not exist');
}
;
return DefaultToken::fromRow($data);
}

/**
* Get the token for $id
*
* @param string $id
* @throws DoesNotExistException
* @return DefaultToken
*/
public function getTokenById($id) {
/* @var $qb IQueryBuilder */
$qb = $this->db->getQueryBuilder();
$result = $qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'token', 'last_activity', 'last_check', 'scope')
->from('authtoken')
->where($qb->expr()->eq('id', $qb->createNamedParameter($id)))
->execute();

$data = $result->fetch();
$result->closeCursor();
if ($data === false) {
throw new DoesNotExistException('token does not exist');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is not tested.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add tests later.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added test

};
return DefaultToken::fromRow($data);
}

Expand All @@ -98,7 +121,7 @@ public function getToken($token) {
public function getTokenByUser(IUser $user) {
/* @var $qb IQueryBuilder */
$qb = $this->db->getQueryBuilder();
$qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'remember', 'token', 'last_activity', 'last_check')
$qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'remember', 'token', 'last_activity', 'last_check', 'scope')
->from('authtoken')
->where($qb->expr()->eq('uid', $qb->createNamedParameter($user->getUID())))
->setMaxResults(1000);
Expand Down
17 changes: 16 additions & 1 deletion lib/private/Authentication/Token/DefaultTokenProvider.php
Expand Up @@ -145,7 +145,7 @@ public function getTokenByUser(IUser $user) {
}

/**
* Get a token by token id
* Get a token by token
*
* @param string $tokenId
* @throws InvalidTokenException
Expand All @@ -159,6 +159,21 @@ public function getToken($tokenId) {
}
}

/**
* Get a token by token id
*
* @param string $tokenId
* @throws InvalidTokenException
* @return DefaultToken
*/
public function getTokenById($tokenId) {
try {
return $this->mapper->getTokenById($tokenId);
} catch (DoesNotExistException $ex) {
throw new InvalidTokenException();
}
}

/**
* @param string $oldSessionId
* @param string $sessionId
Expand Down
11 changes: 10 additions & 1 deletion lib/private/Authentication/Token/IProvider.php
Expand Up @@ -50,7 +50,16 @@ public function generateToken($token, $uid, $loginName, $password, $name, $type
* @throws InvalidTokenException
* @return IToken
*/
public function getToken($tokenId) ;
public function getToken($tokenId);

/**
* Get a token by token id
*
* @param string $tokenId
* @throws InvalidTokenException
* @return DefaultToken
*/
public function getTokenById($tokenId);

/**
* Duplicate an existing session token
Expand Down
23 changes: 22 additions & 1 deletion lib/private/Authentication/Token/IToken.php
Expand Up @@ -67,9 +67,30 @@ public function getPassword();
public function getLastCheck();

/**
* Get the timestamp of the last password check
* Set the timestamp of the last password check
*
* @param int $time
*/
public function setLastCheck($time);

/**
* Get the authentication scope for this token
*
* @return string
*/
public function getScope();

/**
* Get the authentication scope for this token
*
* @return array
*/
public function getScopeAsArray();

/**
* Set the authentication scope for this token
*
* @param array $scope
*/
public function setScope($scope);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typehint here

}
42 changes: 27 additions & 15 deletions lib/private/Files/Filesystem.php
Expand Up @@ -62,6 +62,7 @@
use OC\Files\Config\MountProviderCollection;
use OC\Files\Mount\MountPoint;
use OC\Files\Storage\StorageFactory;
use OC\Lockdown\Filesystem\NullStorage;
use OCP\Files\Config\IMountProvider;
use OCP\Files\Mount\IMountPoint;
use OCP\Files\NotFoundException;
Expand Down Expand Up @@ -216,7 +217,7 @@ class Filesystem {
* @internal
*/
public static function logWarningWhenAddingStorageWrapper($shouldLog) {
self::$logWarningWhenAddingStorageWrapper = (bool) $shouldLog;
self::$logWarningWhenAddingStorageWrapper = (bool)$shouldLog;
}

/**
Expand Down Expand Up @@ -426,25 +427,36 @@ public static function initMountPoints($user = '') {
self::$usersSetup[$user] = true;
}

/** @var \OC\Files\Config\MountProviderCollection $mountConfigManager */
$mountConfigManager = \OC::$server->getMountProviderCollection();
if (\OC::$server->getLockdownManager()->canAccessFilesystem()) {
/** @var \OC\Files\Config\MountProviderCollection $mountConfigManager */
$mountConfigManager = \OC::$server->getMountProviderCollection();

// home mounts are handled seperate since we need to ensure this is mounted before we call the other mount providers
$homeMount = $mountConfigManager->getHomeMountForUser($userObject);
// home mounts are handled seperate since we need to ensure this is mounted before we call the other mount providers
$homeMount = $mountConfigManager->getHomeMountForUser($userObject);

self::getMountManager()->addMount($homeMount);
self::getMountManager()->addMount($homeMount);

\OC\Files\Filesystem::getStorage($user);
\OC\Files\Filesystem::getStorage($user);

// Chance to mount for other storages
if ($userObject) {
$mounts = $mountConfigManager->getMountsForUser($userObject);
array_walk($mounts, array(self::$mounts, 'addMount'));
$mounts[] = $homeMount;
$mountConfigManager->registerMounts($userObject, $mounts);
}
// Chance to mount for other storages
if ($userObject) {
$mounts = $mountConfigManager->getMountsForUser($userObject);
array_walk($mounts, array(self::$mounts, 'addMount'));
$mounts[] = $homeMount;
$mountConfigManager->registerMounts($userObject, $mounts);
}

self::listenForNewMountProviders($mountConfigManager, $userManager);
self::listenForNewMountProviders($mountConfigManager, $userManager);
} else {
self::$mounts->addMount(new MountPoint(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This else statement is not covered by tests.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:sadpandaface:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

new NullStorage([]),
'/' . $user
));
self::$mounts->addMount(new MountPoint(
new NullStorage([]),
'/' . $user . '/files'
));
}
\OC_Hook::emit('OC_Filesystem', 'post_initMountPoints', array('user' => $user));
}

Expand Down