Skip to content

Commit

Permalink
Merge pull request #89 from nextcloud/refactor/config
Browse files Browse the repository at this point in the history 
refactor(config): Centralize config handling and use strong typing
  • Loading branch information
@kyteinsky
kyteinsky authored Jul 25, 2024
2 parents ef1b60c + caf8267 commit 3030c1a
Show file tree
Hide file tree
Showing 19 changed files with 661 additions and 234 deletions.
5 changes: 5 additions & 0 deletions composer.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,10 @@
"target-directory": "vendor-bin",
"forward-command": true
}
},
"autoload": {
"psr-4": {
"OCA\\Gitlab\\": "lib/"
}
}
}
8 changes: 0 additions & 8 deletions lib/AppInfo/Application.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,17 @@
use OCA\Gitlab\Search\GitlabSearchReposProvider;
use OCP\AppFramework\App;
use OCP\AppFramework\Bootstrap\IBootContext;

use OCP\AppFramework\Bootstrap\IBootstrap;
use OCP\AppFramework\Bootstrap\IRegistrationContext;
use OCP\Collaboration\Reference\RenderReferenceEvent;
use OCP\IConfig;

use OCP\Util;

class Application extends App implements IBootstrap {
public const APP_ID = 'integration_gitlab';
public const DEFAULT_GITLAB_URL = 'https://gitlab.com';

private IConfig $config;

public function __construct(array $urlParams = []) {
parent::__construct(self::APP_ID, $urlParams);

$container = $this->getContainer();
$this->config = $container->get(IConfig::class);
}

public function register(IRegistrationContext $context): void {
Expand Down
111 changes: 55 additions & 56 deletions lib/Controller/ConfigController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,31 +13,35 @@

use DateTime;
use OCA\Gitlab\AppInfo\Application;
use OCA\Gitlab\Model\AdminConfig;
use OCA\Gitlab\Model\UserConfig;
use OCA\Gitlab\Reference\GitlabReferenceProvider;
use OCA\Gitlab\Service\ConfigService;
use OCA\Gitlab\Service\GitlabAPIService;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\Http\RedirectResponse;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\AppFramework\Services\IInitialState;
use OCP\IConfig;
use OCP\IL10N;
use OCP\IRequest;
use OCP\IURLGenerator;
use OCP\PreConditionNotMetException;

class ConfigController extends Controller {

public function __construct(string $appName,
public function __construct(
string $appName,
IRequest $request,
private IConfig $config,
private ConfigService $config,
private IURLGenerator $urlGenerator,
private IL10N $l,
private IInitialState $initialStateService,
private GitlabAPIService $gitlabAPIService,
private GitlabReferenceProvider $gitlabReferenceProvider,
private ?string $userId) {
private string $userId,
) {
parent::__construct($appName, $request);
}

Expand All @@ -48,14 +52,13 @@ public function __construct(string $appName,
* @throws PreConditionNotMetException
*/
public function setConfig(array $values): DataResponse {
foreach ($values as $key => $value) {
if ($key === 'url' || $key === 'token') {
return new DataResponse([], Http::STATUS_BAD_REQUEST);
}

$this->config->setUserValue($this->userId, Application::APP_ID, $key, $value);
$userConfig = UserConfig::fromArray($values);
if ($userConfig->url !== null || $userConfig->token !== null) {
return new DataResponse([], Http::STATUS_BAD_REQUEST);
}

$userConfig->saveConfig($this->userId, $this->config);

return new DataResponse([]);
}

Expand All @@ -66,28 +69,27 @@ public function setConfig(array $values): DataResponse {
* @throws PreConditionNotMetException
*/
public function setSensitiveConfig(array $values): DataResponse {
$userConfig = UserConfig::fromArray($values);
$userConfig->saveConfig($this->userId, $this->config);

// revoke the oauth token if needed
if (isset($values['token']) && $values['token'] === '') {
$tokenType = $this->config->getUserValue($this->userId, Application::APP_ID, 'token_type');
if ($userConfig->token === '') {
$tokenType = $this->config->getUserTokenType($this->userId);
if ($tokenType === 'oauth') {
$this->gitlabAPIService->revokeOauthToken($this->userId);
}
}

foreach ($values as $key => $value) {
$this->config->setUserValue($this->userId, Application::APP_ID, $key, $value);
}

$result = [];

if (isset($values['token'])) {
if ($userConfig->token !== null) {
// if the token is set, cleanup refresh token and expiration date
$this->config->deleteUserValue($this->userId, Application::APP_ID, 'token_type');
$this->config->deleteUserValue($this->userId, Application::APP_ID, 'refresh_token');
$this->config->deleteUserValue($this->userId, Application::APP_ID, 'token_expires_at');
$this->config->deleteUserTokenType($this->userId);
$this->config->deleteUserRefreshToken($this->userId);
$this->config->deleteUserTokenExpiresAt($this->userId);
$this->gitlabReferenceProvider->invalidateUserCache($this->userId);

if ($values['token'] && $values['token'] !== '') {
if ($userConfig->token !== '') {
$info = $this->storeUserInfo();
if (isset($info['error'])) {
return new DataResponse(['error' => $info['error']], Http::STATUS_BAD_REQUEST);
Expand All @@ -96,13 +98,13 @@ public function setSensitiveConfig(array $values): DataResponse {
$result['user_displayname'] = $info['userdisplayname'] ?? '';
// store token type if it's valid (so we have a user name)
if ($result['user_name'] !== '') {
$this->config->setUserValue($this->userId, Application::APP_ID, 'token_type', 'personal');
$this->config->setUserTokenType($this->userId, 'personal');
}
} else {
$this->config->deleteUserValue($this->userId, Application::APP_ID, 'user_id');
$this->config->deleteUserValue($this->userId, Application::APP_ID, 'user_name');
$this->config->deleteUserValue($this->userId, Application::APP_ID, 'user_displayname');
$this->config->deleteUserValue($this->userId, Application::APP_ID, 'token');
$this->config->deleteUserId($this->userId);
$this->config->deleteUserName($this->userId);
$this->config->deleteUserDisplayName($this->userId);
$this->config->deleteUserToken($this->userId);
$result['user_name'] = '';
}
}
Expand All @@ -116,23 +118,22 @@ public function setSensitiveConfig(array $values): DataResponse {
* @return DataResponse
*/
public function setAdminConfig(array $values): DataResponse {
foreach ($values as $key => $value) {
if ($key === 'client_id' || $key === 'client_secret' || $key === 'oauth_instance_url') {
return new DataResponse([], Http::STATUS_BAD_REQUEST);
}

$this->config->setAppValue(Application::APP_ID, $key, $value);
$adminConfig = AdminConfig::fromArray($values);
if ($adminConfig->client_id !== null || $adminConfig->client_secret !== null || $adminConfig->oauth_instance_url !== null) {
return new DataResponse([], Http::STATUS_BAD_REQUEST);
}

$adminConfig->saveConfig($this->config);
return new DataResponse(1);
}

/**
* @PasswordConfirmationRequired
*/
public function setSensitiveAdminConfig(array $values): DataResponse {
foreach ($values as $key => $value) {
$this->config->setAppValue(Application::APP_ID, $key, $value);
}
$adminConfig = AdminConfig::fromArray($values);
$adminConfig->saveConfig($this->config);

return new DataResponse(1);
}

Expand Down Expand Up @@ -160,21 +161,20 @@ public function popupSuccessPage(string $user_name, string $user_displayname): T
* @throws PreConditionNotMetException
*/
public function oauthRedirect(string $code = '', string $state = ''): RedirectResponse {
$configState = $this->config->getUserValue($this->userId, Application::APP_ID, 'oauth_state');
$clientID = $this->config->getAppValue(Application::APP_ID, 'client_id');
$clientSecret = $this->config->getAppValue(Application::APP_ID, 'client_secret');
$configState = $this->config->getUserOauthState($this->userId);
$clientID = $this->config->getAdminClientId();
$clientSecret = $this->config->getAdminClientSecret();

// anyway, reset state
$this->config->deleteUserValue($this->userId, Application::APP_ID, 'oauth_state');
$this->config->deleteUserOauthState($this->userId);

if ($clientID and $clientSecret and $configState !== '' and $configState === $state) {
$redirect_uri = $this->config->getUserValue($this->userId, Application::APP_ID, 'redirect_uri');
$adminOauthUrl = $this->config->getAppValue(Application::APP_ID, 'oauth_instance_url', Application::DEFAULT_GITLAB_URL) ?: Application::DEFAULT_GITLAB_URL;
$adminOauthUrl = $this->config->getAdminOauthUrl();
$result = $this->gitlabAPIService->requestOAuthAccessToken($adminOauthUrl, [
'client_id' => $clientID,
'client_secret' => $clientSecret,
'code' => $code,
'redirect_uri' => $redirect_uri,
'redirect_uri' => $this->config->getUserRedirectUri($this->userId),
'grant_type' => 'authorization_code'
], 'POST');
if (isset($result['access_token'])) {
Expand All @@ -184,25 +184,24 @@ public function oauthRedirect(string $code = '', string $state = ''): RedirectRe
if (isset($result['expires_in'])) {
$nowTs = (new Datetime())->getTimestamp();
$expiresAt = $nowTs + (int)$result['expires_in'];
$this->config->setUserValue($this->userId, Application::APP_ID, 'token_expires_at', strval($expiresAt));
$this->config->setUserTokenExpiresAt($this->userId, $expiresAt);
}
$this->config->setUserValue($this->userId, Application::APP_ID, 'url', $adminOauthUrl);
$this->config->setUserValue($this->userId, Application::APP_ID, 'token', $accessToken);
$this->config->setUserValue($this->userId, Application::APP_ID, 'refresh_token', $refreshToken);
$this->config->setUserValue($this->userId, Application::APP_ID, 'token_type', 'oauth');
$this->config->setUserUrl($this->userId, $adminOauthUrl);
$this->config->setUserToken($this->userId, $accessToken);
$this->config->setUserRefreshToken($this->userId, $refreshToken);
$this->config->setUserTokenType($this->userId, 'oauth');
$userInfo = $this->storeUserInfo();

$usePopup = $this->config->getAppValue(Application::APP_ID, 'use_popup', '0') === '1';
if ($usePopup) {
if ($this->config->getAdminUsePopup()) {
return new RedirectResponse(
$this->urlGenerator->linkToRoute('integration_gitlab.config.popupSuccessPage', [
'user_name' => $userInfo['username'] ?? '',
'user_displayname' => $userInfo['userdisplayname'] ?? '',
])
);
} else {
$oauthOrigin = $this->config->getUserValue($this->userId, Application::APP_ID, 'oauth_origin');
$this->config->deleteUserValue($this->userId, Application::APP_ID, 'oauth_origin');
$oauthOrigin = $this->config->getUserOauthOrigin($this->userId);
$this->config->deleteUserOauthOrigin($this->userId);
if ($oauthOrigin === 'settings') {
return new RedirectResponse(
$this->urlGenerator->linkToRoute('settings.PersonalSettings.index', ['section' => 'connected-accounts']) .
Expand Down Expand Up @@ -236,17 +235,17 @@ public function oauthRedirect(string $code = '', string $state = ''): RedirectRe
private function storeUserInfo(): array {
$info = $this->gitlabAPIService->request($this->userId, 'user');
if (isset($info['username']) && isset($info['id'])) {
$this->config->setUserValue($this->userId, Application::APP_ID, 'user_id', $info['id']);
$this->config->setUserValue($this->userId, Application::APP_ID, 'user_name', $info['username']);
$this->config->setUserValue($this->userId, Application::APP_ID, 'user_displayname', $info['name']);
$this->config->setUserId($this->userId, $info['id']);
$this->config->setUserName($this->userId, $info['username']);
$this->config->setUserDisplayName($this->userId, $info['name']);
return [
'username' => $info['username'],
'userid' => $info['id'],
'userdisplayname' => $info['name'],
];
} else {
$this->config->setUserValue($this->userId, Application::APP_ID, 'user_id', '');
$this->config->setUserValue($this->userId, Application::APP_ID, 'user_name', '');
$this->config->deleteUserId($this->userId);
$this->config->deleteUserName($this->userId);
return $info;
}
}
Expand Down
29 changes: 15 additions & 14 deletions lib/Controller/GitlabAPIController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,30 @@

namespace OCA\Gitlab\Controller;

use OCA\Gitlab\AppInfo\Application;
use Exception;
use OCA\Gitlab\Service\ConfigService;
use OCA\Gitlab\Service\GitlabAPIService;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\DataDisplayResponse;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\Http\RedirectResponse;

use OCP\IConfig;
use OCP\IRequest;
use OCP\IURLGenerator;

class GitlabAPIController extends Controller {

private string $accessToken;

public function __construct(string $appName,
IRequest $request,
private IConfig $config,
private IURLGenerator $urlGenerator,
public function __construct(
string $appName,
IRequest $request,
private ConfigService $config,
private IURLGenerator $urlGenerator,
private GitlabAPIService $gitlabAPIService,
private ?string $userId) {
private string $userId,
) {
parent::__construct($appName, $request);
$this->accessToken = $this->config->getUserValue($this->userId, Application::APP_ID, 'token');
$this->accessToken = $this->config->getUserToken($userId);
}

/**
Expand All @@ -43,7 +44,7 @@ public function __construct(string $appName,
*
* @param int $userId
* @return DataDisplayResponse|RedirectResponse
* @throws \Exception
* @throws Exception
*/
public function getUserAvatar(int $userId) {
$result = $this->gitlabAPIService->getUserAvatar($this->userId, $userId);
Expand All @@ -65,7 +66,7 @@ public function getUserAvatar(int $userId) {
*
* @param int $projectId
* @return DataDisplayResponse|RedirectResponse
* @throws \Exception
* @throws Exception
*/
public function getProjectAvatar(int $projectId) {
$result = $this->gitlabAPIService->getProjectAvatar($this->userId, $projectId);
Expand All @@ -86,7 +87,7 @@ public function getProjectAvatar(int $projectId) {
*
* @param string|null $since
* @return DataResponse
* @throws \Exception
* @throws Exception
*/
public function getEvents(?string $since = null): DataResponse {
if ($this->accessToken === '') {
Expand All @@ -107,7 +108,7 @@ public function getEvents(?string $since = null): DataResponse {
*
* @param string|null $since
* @return DataResponse
* @throws \Exception
* @throws Exception
*/
public function getTodos(?string $since = null): DataResponse {
if ($this->accessToken === '') {
Expand All @@ -127,7 +128,7 @@ public function getTodos(?string $since = null): DataResponse {
*
* @param int $id
* @return DataResponse
* @throws \Exception
* @throws Exception
*/
public function markTodoAsDone(int $id): DataResponse {
if ($this->accessToken === '') {
Expand Down
Loading

0 comments on commit 3030c1a

Please sign in to comment.