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.

Sign up for GitHub

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

Jump to bottom

get OAuth config in request function and don't pass the data through the different layers #80

Merged
merged 1 commit into from
Mar 22, 2022
Merged

get OAuth config in request function and don't pass the data through the different layers #80

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 9 additions & 17 deletions lib/Controller/ConfigController.php
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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 @@ -623,11 +577,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 @@ -665,7 +614,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
Open in desktop
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
Open in desktop
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