Skip to content

Commit

Permalink
Merge pull request #80 from nextcloud/getConfigInRequest
Browse files Browse the repository at this point in the history 
get OAuth config in request function and don't pass the data through the different layers
  • Loading branch information
@individual-it
individual-it committed Mar 22, 2022
2 parents 1824266 + 86987d0 commit eeb64c7
Show file tree
Hide file tree
Showing 7 changed files with 135 additions and 231 deletions.
26 changes: 9 additions & 17 deletions lib/Controller/ConfigController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public function setConfig(array $values): DataResponse {

if (isset($values['token'])) {
if ($values['token'] && $values['token'] !== '') {
$result = $this->storeUserInfo($values['token']);
$result = $this->storeUserInfo();
} else {
$this->config->deleteUserValue($this->userId, Application::APP_ID, 'token');
$this->config->deleteUserValue($this->userId, Application::APP_ID, 'login');
Expand Down Expand Up @@ -168,7 +168,7 @@ public function oauthRedirect(string $code = '', string $state = ''): RedirectRe
$this->config->setUserValue($this->userId, Application::APP_ID, 'refresh_token', $refreshToken);
// get user info
// ToDo check response for errors
$this->storeUserInfo($accessToken);
$this->storeUserInfo();
return new RedirectResponse(
$this->urlGenerator->linkToRoute('settings.PersonalSettings.index', ['section' => 'connected-accounts']) .
'?openprojectToken=success'
Expand Down Expand Up @@ -200,20 +200,10 @@ public function oauthRedirect(string $code = '', string $state = ''): RedirectRe
}

/**
* @param string $accessToken
* @return array{error?: string, user_name?: string, errorMesssage?: string}
* @return array{error?: string, user_name?: string, statusCode?: int}
*/
private function storeUserInfo(string $accessToken): array {
$refreshToken = $this->config->getUserValue($this->userId, Application::APP_ID, 'refresh_token');
$clientID = $this->config->getAppValue(Application::APP_ID, 'client_id');
$clientSecret = $this->config->getAppValue(Application::APP_ID, 'client_secret');
$openprojectUrl = $this->config->getAppValue(Application::APP_ID, 'oauth_instance_url');

if (!$openprojectUrl || !OpenProjectAPIService::validateOpenProjectURL($openprojectUrl)) {
return ['error' => 'OpenProject URL is invalid'];
}

$info = $this->openprojectAPIService->request($openprojectUrl, $accessToken, $refreshToken, $clientID, $clientSecret, $this->userId, 'users/me');
private function storeUserInfo(): array {
$info = $this->openprojectAPIService->request($this->userId, 'users/me');
if (isset($info['lastName'], $info['firstName'], $info['id'])) {
$fullName = $info['firstName'] . ' ' . $info['lastName'];
$this->config->setUserValue($this->userId, Application::APP_ID, 'user_id', $info['id']);
Expand All @@ -223,9 +213,11 @@ private function storeUserInfo(string $accessToken): array {
$this->config->deleteUserValue($this->userId, Application::APP_ID, 'user_id');
$this->config->deleteUserValue($this->userId, Application::APP_ID, 'user_name');
if (isset($info['statusCode']) && $info['statusCode'] === 404) {
$info['errorMessage'] = 'Not found';
$info['error'] = 'Not found';
} else {
$info['errorMessage'] = 'Invalid token';
if (!isset($info['error'])) {
$info['error'] = 'Invalid token';
}
}
return $info;
}
Expand Down
18 changes: 3 additions & 15 deletions lib/Controller/OpenProjectAPIController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,7 @@ public function getNotifications(?string $since = null): DataResponse {
if ($this->accessToken === '' || !OpenProjectAPIService::validateOpenProjectURL($this->openprojectUrl)) {
return new DataResponse('', Http::STATUS_BAD_REQUEST);
}
$result = $this->openprojectAPIService->getNotifications(
$this->openprojectUrl, $this->accessToken, $this->refreshToken, $this->clientID, $this->clientSecret, $this->userId, $since, 7
);
$result = $this->openprojectAPIService->getNotifications($this->userId, $since, 7);
if (!isset($result['error'])) {
$response = new DataResponse($result);
} else {
Expand All @@ -153,11 +151,6 @@ public function getSearchedWorkPackages(?string $searchQuery = null, ?int $fileI
);
}
$result = $this->openprojectAPIService->searchWorkPackage(
$this->openprojectUrl,
$this->accessToken,
$this->refreshToken,
$this->clientID,
$this->clientSecret,
$this->userId,
$searchQuery,
$fileId
Expand Down Expand Up @@ -192,11 +185,6 @@ public function linkWorkPackageToFile(int $workpackageId, int $fileId, string $f

try {
$result = $this->openprojectAPIService->linkWorkPackageToFile(
$this->openprojectUrl,
$this->accessToken,
$this->refreshToken,
$this->clientID,
$this->clientSecret,
$workpackageId,
$fileId,
$fileName,
Expand Down Expand Up @@ -227,7 +215,7 @@ public function getOpenProjectWorkPackageStatus(string $id): DataResponse {
return new DataResponse('', Http::STATUS_BAD_REQUEST);
}
$result = $this->openprojectAPIService->getOpenProjectWorkPackageStatus(
$this->openprojectUrl, $this->accessToken, $this->refreshToken, $this->clientID, $this->clientSecret, $this->userId, $id
$this->userId, $id
);
if (!isset($result['error'])) {
$response = new DataResponse($result);
Expand All @@ -251,7 +239,7 @@ public function getOpenProjectWorkPackageType(string $id): DataResponse {
return new DataResponse('', Http::STATUS_BAD_REQUEST);
}
$result = $this->openprojectAPIService->getOpenProjectWorkPackageType(
$this->openprojectUrl, $this->accessToken, $this->refreshToken, $this->clientID, $this->clientSecret, $this->userId, $id
$this->userId, $id
);
if (!isset($result['error'])) {
$response = new DataResponse($result);
Expand Down
5 changes: 1 addition & 4 deletions lib/Search/OpenProjectSearchProvider.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,16 +126,13 @@ public function search(IUser $user, ISearchQuery $query): SearchResult {

$openprojectUrl = $this->config->getAppValue(Application::APP_ID, 'oauth_instance_url');
$accessToken = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'token');
$refreshToken = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'refresh_token');
$clientID = $this->config->getAppValue(Application::APP_ID, 'client_id');
$clientSecret = $this->config->getAppValue(Application::APP_ID, 'client_secret');

$searchEnabled = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'search_enabled', '0') === '1';
if ($accessToken === '' || !$searchEnabled) {
return SearchResult::paginated($this->getName(), [], 0);
}

$searchResults = $this->service->searchWorkPackage($openprojectUrl, $accessToken, $refreshToken, $clientID, $clientSecret, $user->getUID(), $term);
$searchResults = $this->service->searchWorkPackage($user->getUID(), $term);
$searchResults = array_slice($searchResults, $offset, $limit);

if (isset($searchResults['error'])) {
Expand Down
101 changes: 25 additions & 76 deletions lib/Service/OpenProjectAPIService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ private function checkNotificationsForUser(string $userId): void {
$accessToken = $this->config->getUserValue($userId, Application::APP_ID, 'token');
$notificationEnabled = ($this->config->getUserValue($userId, Application::APP_ID, 'notification_enabled', '0') === '1');
if ($accessToken && $notificationEnabled) {
$refreshToken = $this->config->getUserValue($userId, Application::APP_ID, 'refresh_token');
$clientID = $this->config->getAppValue(Application::APP_ID, 'client_id');
$clientSecret = $this->config->getAppValue(Application::APP_ID, 'client_secret');
$openprojectUrl = $this->config->getAppValue(Application::APP_ID, 'oauth_instance_url');
Expand All @@ -127,9 +126,7 @@ private function checkNotificationsForUser(string $userId): void {
$myOPUserId = $this->config->getUserValue($userId, Application::APP_ID, 'user_id');
if ($myOPUserId !== '') {
$myOPUserId = (int) $myOPUserId;
$notifications = $this->getNotifications(
$openprojectUrl, $accessToken, $refreshToken, $clientID, $clientSecret, $userId, $lastNotificationCheck
);
$notifications = $this->getNotifications($userId, $lastNotificationCheck);
if (!isset($notifications['error']) && count($notifications) > 0) {
$newLastNotificationCheck = $notifications[0]['updatedAt'];
$this->config->setUserValue($userId, Application::APP_ID, 'last_notification_check', $newLastNotificationCheck);
Expand Down Expand Up @@ -198,19 +195,14 @@ public function now(): string {
}

/**
* @param string $url
* @param string $accessToken
* @param string $refreshToken
* @param string $clientID
* @param string $clientSecret
* @param string $userId
* @param ?string $since
* @param ?int $limit
* @return array<mixed>
*/
public function getNotifications(string $url, string $accessToken,
string $refreshToken, string $clientID, string $clientSecret, string $userId,
?string $since = null, ?int $limit = null): array {
public function getNotifications(
string $userId, ?string $since = null, ?int $limit = null
): array {
if ($since) {
$filters = '[{"updatedAt":{"operator":"<>d","values":["' . $since . '","' . $this->now() . '"]}},{"status":{"operator":"!","values":["14"]}}]';
} else {
Expand All @@ -226,9 +218,7 @@ public function getNotifications(string $url, string $accessToken,
'sortBy' => '[["updatedAt", "desc"]]',
// 'limit' => $limit,
];
$result = $this->request(
$url, $accessToken, $refreshToken, $clientID, $clientSecret, $userId, 'work_packages', $params
);
$result = $this->request($userId, 'work_packages', $params);
if (isset($result['error'])) {
return $result;
} elseif (!isset($result['_embedded']['elements'])) {
Expand All @@ -243,11 +233,6 @@ public function getNotifications(string $url, string $accessToken,
}

/**
* @param string $url
* @param string $accessToken
* @param string $refreshToken
* @param string $clientID
* @param string $clientSecret
* @param string $userId
* @param string|null $query
* @param int|null $fileId
Expand All @@ -257,9 +242,9 @@ public function getNotifications(string $url, string $accessToken,
* @throws \OCP\PreConditionNotMetException
* @throws \Safe\Exceptions\JsonException
*/
public function searchWorkPackage(string $url, string $accessToken,
string $refreshToken, string $clientID, string $clientSecret, string $userId,
string $query = null, int $fileId = null, int $offset = 0, int $limit = 5): array {
public function searchWorkPackage(
string $userId, string $query = null, int $fileId = null, int $offset = 0, int $limit = 5
): array {
$resultsById = [];
$filters = [];

Expand All @@ -276,9 +261,7 @@ public function searchWorkPackage(string $url, string $accessToken,
'sortBy' => '[["updatedAt", "desc"]]',
// 'limit' => $limit,
];
$searchDescResult = $this->request(
$url, $accessToken, $refreshToken, $clientID, $clientSecret, $userId, 'work_packages', $params
);
$searchDescResult = $this->request($userId, 'work_packages', $params);

if (isset($searchDescResult['error'])) {
return $searchDescResult;
Expand All @@ -300,9 +283,7 @@ public function searchWorkPackage(string $url, string $accessToken,
'sortBy' => '[["updatedAt", "desc"]]',
// 'limit' => $limit,
];
$searchSubjectResult = $this->request(
$url, $accessToken, $refreshToken, $clientID, $clientSecret, $userId, 'work_packages', $params
);
$searchSubjectResult = $this->request($userId, 'work_packages', $params);

if (isset($searchSubjectResult['error'])) {
return $searchSubjectResult;
Expand Down Expand Up @@ -359,21 +340,23 @@ public function getOpenProjectAvatar(string $url,
}

/**
* @param string $openprojectUrl
* @param string $accessToken
* @param string $refreshToken
* @param string $clientID
* @param string $clientSecret
* @param string $userId
* @param string $endPoint
* @param array<mixed> $params
* @param string $method
* @return array<mixed>
* @throws \OCP\PreConditionNotMetException
*/
public function request(string $openprojectUrl, string $accessToken, string $refreshToken,
string $clientID, string $clientSecret, string $userId,
public function request(string $userId,
string $endPoint, array $params = [], string $method = 'GET'): array {
$accessToken = $this->config->getUserValue($userId, Application::APP_ID, 'token');
$refreshToken = $this->config->getUserValue($userId, Application::APP_ID, 'refresh_token');
$clientID = $this->config->getAppValue(Application::APP_ID, 'client_id');
$clientSecret = $this->config->getAppValue(Application::APP_ID, 'client_secret');
$openprojectUrl = $this->config->getAppValue(Application::APP_ID, 'oauth_instance_url');
if (!$openprojectUrl || !OpenProjectAPIService::validateOpenProjectURL($openprojectUrl)) {
return ['error' => 'OpenProject URL is invalid', 'statusCode' => 500];
}
try {
$url = $openprojectUrl . '/api/v3/' . $endPoint;
$options = [
Expand Down Expand Up @@ -433,9 +416,7 @@ public function request(string $openprojectUrl, string $accessToken, string $ref
$accessToken = $result['access_token'];
$this->config->setUserValue($userId, Application::APP_ID, 'token', $accessToken);
// retry the request with new access token
return $this->request(
$openprojectUrl, $accessToken, $refreshToken, $clientID, $clientSecret, $userId, $endPoint, $params, $method
);
return $this->request($userId, $endPoint, $params, $method);
}
}
// try to get the error in the response
Expand Down Expand Up @@ -520,25 +501,12 @@ public static function validateOpenProjectURL(string $openprojectUrl): bool {
/**
* authenticated request to get status of a work package
*
* @param string $url
* @param string $accessToken
* @param string $refreshToken
* @param string $clientID
* @param string $clientSecret
* @param string $userId
* @param string $statusId
* @return string[]
*/
public function getOpenProjectWorkPackageStatus(
string $url,
string $accessToken,
string $refreshToken,
string $clientID,
string $clientSecret,
string $userId,
string $statusId): array {
$result = $this->request(
$url, $accessToken, $refreshToken, $clientID, $clientSecret, $userId, 'statuses/' . $statusId);
public function getOpenProjectWorkPackageStatus(string $userId, string $statusId): array {
$result = $this->request($userId, 'statuses/' . $statusId);
if (!isset($result['id'])) {
return ['error' => 'Malformed response'];
}
Expand All @@ -548,26 +516,12 @@ public function getOpenProjectWorkPackageStatus(
/**
* authenticated request to get status of a work package
*
* @param string $url
* @param string $accessToken
* @param string $refreshToken
* @param string $clientID
* @param string $clientSecret
* @param string $userId
* @param string $typeId
* @return string[]
*/
public function getOpenProjectWorkPackageType(
string $url,
string $accessToken,
string $refreshToken,
string $clientID,
string $clientSecret,
string $userId,
string $typeId
): array {
$result = $this->request(
$url, $accessToken, $refreshToken, $clientID, $clientSecret, $userId, 'types/' . $typeId);
public function getOpenProjectWorkPackageType(string $userId, string $typeId): array {
$result = $this->request($userId, 'types/' . $typeId);
if (!isset($result['id'])) {
return ['error' => 'Malformed response'];
}
Expand Down Expand Up @@ -627,11 +581,6 @@ public function getNode($userId, $fileId) {
* @return int
*/
public function linkWorkPackageToFile(
string $openprojectUrl,
string $accessToken,
string $refreshToken,
string $clientID,
string $clientSecret,
int $workpackageId,
int $fileId,
string $fileName,
Expand Down Expand Up @@ -669,7 +618,7 @@ public function linkWorkPackageToFile(

$params['body'] = \Safe\json_encode($body);
$result = $this->request(
$openprojectUrl, $accessToken, $refreshToken, $clientID, $clientSecret, $userId, 'work_packages/' . $workpackageId. '/file_links', $params, 'POST'
$userId, 'work_packages/' . $workpackageId. '/file_links', $params, 'POST'
);

if (isset($result['error'])) {
Expand Down
6 changes: 0 additions & 6 deletions tests/lib/Controller/ConfigControllerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,6 @@ public function setUpMocks(): void {
$apiServiceMock
->method('request')
->with(
'http://openproject.org',
'oAuthAccessToken',
'oauth',
'oAuthRefreshToken',
'clientID',
'clientSecret',
'testUser',
'users/me'
)
Expand Down
5 changes: 0 additions & 5 deletions tests/lib/Controller/OpenProjectAPIControllerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,11 +213,6 @@ public function testGetSearchedWorkPackages($searchQuery, $fileId, array $expect
$service->expects($this->once())
->method('searchWorkPackage')
->with(
$this->anything(),
$this->anything(),
$this->anything(),
$this->anything(),
$this->anything(),
$this->anything(),
$searchQuery,
$fileId
Expand Down
Loading

0 comments on commit eeb64c7

Please sign in to comment.