Skip to content

Commit

Permalink
refactor and type hints (#1962)
Browse files Browse the repository at this point in the history 
hardening AppConfig against missing or misconfigured values and adding type hints
  • Loading branch information
@dartcafe
dartcafe committed Sep 14, 2021
1 parent 5dfe3a6 commit 1a3f3ee
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 48 deletions.
114 changes: 69 additions & 45 deletions lib/Model/AppSettings.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,110 +54,98 @@ public function __construct() {
}

// Getters
public function getShowLogin(): bool {
return $this->stringToBool($this->config->getAppValue(self::APP_NAME, 'showLogin'), true);
}

public function getAutoArchive(): bool {
return $this->stringToBool($this->config->getAppValue(self::APP_NAME, 'autoArchive'), false);
}

public function getAllowPublicShares(): bool {
return !!$this->config->getAppValue(self::APP_NAME, 'allowPublicShares');
return $this->stringToBool($this->config->getAppValue(self::APP_NAME, 'allowPublicShares'), true);
}

public function getAllowAllAccess(): bool {
return !!$this->config->getAppValue(self::APP_NAME, 'allowAllAccess');
return $this->stringToBool($this->config->getAppValue(self::APP_NAME, 'allowAllAccess'), true);
}

public function getAllowPollCreation(): bool {
return !!$this->config->getAppValue(self::APP_NAME, 'allowPollCreation');
return $this->stringToBool($this->config->getAppValue(self::APP_NAME, 'allowPollCreation'), true);
}

public function getPublicSharesGroups(): array {
$appConfig = $this->config->getAppValue(self::APP_NAME, 'publicSharesGroups');
if ($appConfig) {
return json_decode($appConfig);
}
return [];
return $this->stringToArray($this->config->getAppValue(self::APP_NAME, 'publicSharesGroups'));
}

public function getAllAccessGroups(): array {
$appConfig = $this->config->getAppValue(self::APP_NAME, 'allAccessGroups');
if ($appConfig) {
return json_decode($appConfig);
}
return [];
return $this->stringToArray($this->config->getAppValue(self::APP_NAME, 'allAccessGroups'));
}

public function getPollCreationGroups(): array {
$appConfig = $this->config->getAppValue(self::APP_NAME, 'pollCreationGroups');
if ($appConfig) {
return json_decode($appConfig);
}
return [];
}

public function getShowLogin(): bool {
return !!$this->config->getAppValue(self::APP_NAME, 'showLogin');
}

public function getAutoArchive(): bool {
return !!$this->config->getAppValue(self::APP_NAME, 'autoArchive');
return $this->stringToArray($this->config->getAppValue(self::APP_NAME, 'pollCreationGroups'));
}

public function getAutoArchiveOffset(): int {
return intval($this->config->getAppValue(self::APP_NAME, 'autoArchiveOffset'));
return $this->stringToInteger($this->config->getAppValue(self::APP_NAME, 'autoArchiveOffset'), 30);
}

// Checks
public function getCreationAllowed() {
public function getCreationAllowed(): bool {
if ($this->session->isLoggedIn()) {
return $this->getAllowPollCreation() || $this->isMember($this->getPollCreationGroups());
}
return false;
}

public function getAllAccessAllowed() {
public function getAllAccessAllowed(): bool {
if ($this->session->isLoggedIn()) {
return $this->getAllowAllAccess() || $this->isMember($this->getAllAccessGroups());
}
return false;
}

public function getPublicSharesAllowed() {
public function getPublicSharesAllowed(): bool {
if ($this->session->isLoggedIn()) {
return $this->getAllowPublicShares() || $this->isMember($this->getPublicSharesGroups());
}
return false;
}

// Setters
public function setAllowPublicShares(bool $value) {
$this->config->setAppValue(self::APP_NAME, 'allowPublicShares', strval($value));
public function setAllowPublicShares(bool $value): void {
$this->config->setAppValue(self::APP_NAME, 'allowPublicShares', $this->boolToString($value));
}

public function setShowLogin(bool $value) {
$this->config->setAppValue(self::APP_NAME, 'showLogin', strval($value));
public function setShowLogin(bool $value): void {
$this->config->setAppValue(self::APP_NAME, 'showLogin', $this->boolToString($value));
}

public function setAllowAllAccess(bool $value) {
$this->config->setAppValue(self::APP_NAME, 'allowAllAccess', strval($value));
public function setAllowAllAccess(bool $value): void {
$this->config->setAppValue(self::APP_NAME, 'allowAllAccess', $this->boolToString($value));
}

public function setAllowPollCreation(bool $value) {
$this->config->setAppValue(self::APP_NAME, 'allowPollCreation', strval($value));
public function setAllowPollCreation(bool $value): void {
$this->config->setAppValue(self::APP_NAME, 'allowPollCreation', $this->boolToString($value));
}

public function setPublicSharesGroups(array $value) {
public function setPublicSharesGroups(array $value): void {
$this->config->setAppValue(self::APP_NAME, 'publicSharesGroups', json_encode($value));
}

public function setAllAccessGroups(array $value) {
public function setAllAccessGroups(array $value): void {
$this->config->setAppValue(self::APP_NAME, 'allAccessGroups', json_encode($value));
}

public function setPollCreationGroups(array $value) {
public function setPollCreationGroups(array $value): void {
$this->config->setAppValue(self::APP_NAME, 'pollCreationGroups', json_encode($value));
}

public function setAutoArchive(bool $value) {
$this->config->setAppValue(self::APP_NAME, 'autoArchive', strval($value));
public function setAutoArchive(bool $value): void {
$this->config->setAppValue(self::APP_NAME, 'autoArchive', $this->boolToString($value));
}

public function setAutoArchiveOffset(int $value) {
public function setAutoArchiveOffset(int $value): void {
$this->config->setAppValue(self::APP_NAME, 'autoArchiveOffset', strval($value));
}

Expand All @@ -170,12 +158,15 @@ public function jsonSerialize() {
foreach ($this->getPublicSharesGroups() as $group) {
$publicSharesGroups[] = new Group($group);
}

foreach ($this->getAllAccessGroups() as $group) {
$allAccessGroups[] = new Group($group);
}

foreach ($this->getPollCreationGroups() as $group) {
$pollCreationGroups[] = new Group($group);
}

return [
'allowPublicShares' => $this->getAllowPublicShares(),
'allowAllAccess' => $this->getAllowAllAccess(),
Expand All @@ -189,7 +180,7 @@ public function jsonSerialize() {
];
}

private function isMember(array $groups) {
private function isMember(array $groups): bool {
foreach ($groups as $GID) {
if ($this->groupManager->isInGroup($this->userId, $GID)) {
return true;
Expand All @@ -198,6 +189,39 @@ private function isMember(array $groups) {
return false;
}

private function stringToInteger(string $value, int $default): int {
if ($value !== '') {
return intval($value);
}
return $default;
}

private function stringToArray(string $value): array {
if ($value) {
return json_decode($value);
}
return [];
}

private function stringToBool(string $value, bool $default): bool {
switch ($value) {
case 'yes':
return true;
case 'no':
return false;
default:
return $default;
}
}

private function boolToString(bool $value): string {
if ($value) {
return 'yes';
}
return 'no';
}


protected static function getContainer() {
$app = \OC::$server->query(Application::class);
return $app->getContainer();
Expand Down
6 changes: 5 additions & 1 deletion lib/Service/AnonymizeService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,13 @@ public function getVotes(): array {
}

/**
* * Replaces userIds with displayName to avoid exposing usernames in public polls
* Replaces userIds with displayName to avoid exposing usernames in public polls
*
* @param (Comment|Option|Poll|Vote)[]|Comment|Option|Poll|Vote $arrayOrObject
*
* @return (Comment|Option|Poll|Vote)[]|Comment|Option|Poll|Vote
*
* @psalm-return Comment|Option|Poll|Vote|array<Comment|Option|Poll|Vote>
*/
public static function replaceUserId($arrayOrObject) {
if (is_array($arrayOrObject)) {
Expand Down
2 changes: 1 addition & 1 deletion lib/Service/PreferencesService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public function __construct(
$this->load();
}

public function load() {
public function load(): void {
try {
$this->preferences = $this->preferencesMapper->find($this->userId);
} catch (DoesNotExistException $e) {
Expand Down
2 changes: 1 addition & 1 deletion lib/Service/SettingsService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public function __construct(
/**
* Get app settings with extended group information
*/
public function getAppSettings() {
public function getAppSettings(): AppSettings {
return $this->appSettings;
}

Expand Down

0 comments on commit 1a3f3ee

Please sign in to comment.