Skip to content

Commit

Permalink
[OP#49621] fix app password form becomes inactive (#456)
Browse files Browse the repository at this point in the history 
* Check and remove the app password with name OpenProject

Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>

* add phpunit test

Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>

* add api test to cover the bug

Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>

* add unit test for multiple token

Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>

* add unit test for delete token

Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>

---------

Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
  • Loading branch information
@SwikritiT
SwikritiT committed Aug 16, 2023
1 parent ef993b5 commit 0398905
Show file tree
Hide file tree
Showing 4 changed files with 253 additions and 9 deletions.
16 changes: 13 additions & 3 deletions lib/Service/OpenProjectAPIService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1129,8 +1129,12 @@ public function generateAppPasswordTokenForUser(): string {
*/
public function deleteAppPassword(): void {
if ($this->hasAppPassword()) {
$tokenId = $this->tokenProvider->getTokenByUser(Application::OPEN_PROJECT_ENTITIES_NAME)[0]->getId();
$this->tokenProvider->invalidateTokenById(Application::OPEN_PROJECT_ENTITIES_NAME, $tokenId);
$tokens = $this->tokenProvider->getTokenByUser(Application::OPEN_PROJECT_ENTITIES_NAME);
foreach ($tokens as $token) {
if ($token->getName() === Application::OPEN_PROJECT_ENTITIES_NAME) {
$this->tokenProvider->invalidateTokenById(Application::OPEN_PROJECT_ENTITIES_NAME, $token->getId());
}
}
}
}

Expand All @@ -1140,7 +1144,13 @@ public function deleteAppPassword(): void {
* @return bool
*/
public function hasAppPassword(): bool {
return sizeof($this->tokenProvider->getTokenByUser(Application::OPEN_PROJECT_ENTITIES_NAME)) === 1;
$tokens = $this->tokenProvider->getTokenByUser(Application::OPEN_PROJECT_ENTITIES_NAME);
foreach ($tokens as $token) {
if ($token->getName() === Application::OPEN_PROJECT_ENTITIES_NAME) {
return true;
}
}
return false;
}

/**
Expand Down
10 changes: 6 additions & 4 deletions tests/acceptance/features/api/setup.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -538,10 +538,9 @@ Feature: setup the integration through an API
And groupfolder "OpenProject" should be managed by the user "OpenProject"
# the next step is only for the tests, because that user has a random password
Given the administrator has changed the password of "OpenProject" to the default testing password
Then user "OpenProject" should have a folder called "OpenProject"

And user "OpenProject" should have a folder called "OpenProject"
# folders inside the OpenProject folder can only be deleted/renamed by the OpenProject user
Given user "Carol" has been created
And user "Carol" has been created
And user "Carol" has been added to the group "OpenProject"
And user "OpenProject" has created folder "/OpenProject/project-abc"
Then user "Carol" should have a folder called "OpenProject/project-abc"
Expand Down Expand Up @@ -635,9 +634,12 @@ Feature: setup the integration through an API
When user "OpenProject" sends a "PROPFIND" request to "/remote.php/webdav" using current app password
Then the HTTP status code should be "207"

# this is to provide test coverage for issues like this
# https://community.openproject.org/projects/nextcloud-integration/work_packages/49621
When a new browser session for "Openproject" starts
# but other values can be updated by sending a PATCH request
# also we can replace old app password by sending PATCH request to get new user app password
When the administrator sends a PATCH request to the "setup" endpoint with this data:
And the administrator sends a PATCH request to the "setup" endpoint with this data:
"""
{
"values" : {
Expand Down
83 changes: 83 additions & 0 deletions tests/acceptance/features/bootstrap/FeatureContext.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use GuzzleHttp\Client;
use GuzzleHttp\Exception\RequestException;
use GuzzleHttp\Psr7\Request;
use GuzzleHttp\Cookie\CookieJar;
use PHPUnit\Framework\Assert;
use Psr\Http\Message\ResponseInterface;

Expand Down Expand Up @@ -39,6 +40,9 @@ class FeatureContext implements Context {

private ?ResponseInterface $response = null;

private CookieJar $cookieJar;
private string $requestToken;

public function getAdminUsername(): string {
return $this->adminUsername;
}
Expand All @@ -55,6 +59,20 @@ public function getBaseUrl(): string {
return $this->baseUrl;
}

/**
* @return string
*/
public function getRequestToken():string {
return $this->requestToken;
}

/**
* @return CookieJar
*/
public function getCookieJar():CookieJar {
return $this->cookieJar;
}

/**
* @return ResponseInterface|null
*/
Expand Down Expand Up @@ -83,6 +101,7 @@ public function __construct(
$this->adminUsername = $adminUsername;
$this->adminPassword = $adminPassword;
$this->regularUserPassword = $regularUserPassword;
$this->cookieJar = new CookieJar();
}

/**
Expand Down Expand Up @@ -909,6 +928,70 @@ public function theContentOfFileAtForUserShouldBe(
Assert::assertSame($content, $this->response->getBody()->getContents());
}

/**
* @When a new browser session for :user starts
*
* @param string $user
*
* @return void
*/
public function aNewBrowserSessionForUserStarts(string $user):void {
$loginUrl = $this->getBaseUrl() . '/index.php/login';
$options['cookies'] = $this->getCookieJar();
// Request a new session and extract CSRF token
$this->setResponse(
$this->sendHttpRequest(
$loginUrl,
null,
null,
'GET',
null,
null,
$options
)
);
$this->theHttpStatusCodeShouldBe(200);
$this->extractRequestTokenFromResponse($this->getResponse());

// Login and extract new token
$body = [
'user' => $user,
'password' => $this->getRegularUserPassword(),
'requesttoken' => $this->getRequestToken()
];
$options['cookies'] = $this->getCookieJar();
$this->setResponse(
$this->sendHttpRequest(
$loginUrl,
null,
null,
'POST',
null,
$body,
$options
)
);
$this->theHttpStatusCodeShouldBe(200);
$this->extractRequestTokenFromResponse($this->getResponse());
}

/**
* @param ResponseInterface $response
*
* @return void
*/
public function extractRequestTokenFromResponse(ResponseInterface $response):void {
$this->requestToken = \substr(
\preg_replace(
'/(.*)data-requesttoken="(.*)">(.*)/sm',
'\2',
$response->getBody()->getContents()
),
0,
89
);
}

/**
* @param string|null $username
* @param string|null $password
Expand Down
153 changes: 151 additions & 2 deletions tests/lib/Service/OpenProjectAPIServiceTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@ private function getOpenProjectAPIService(
* @param ISubAdmin|null $subAdminManagerMock
* @param ISecureRandom|null $iSecureRandomMock
* @param IConfig|null $configMock
* @param IProvider|null $tokenProviderMock
* @return OpenProjectAPIService|MockObject
*/
private function getServiceMock(
Expand All @@ -411,7 +412,8 @@ private function getServiceMock(
$appManagerMock = null,
$subAdminManagerMock = null,
$iSecureRandomMock = null,
$configMock = null
$configMock = null,
$tokenProviderMock = null
): OpenProjectAPIService {
$onlyMethods[] = 'getBaseUrl';
if ($rootMock === null) {
Expand All @@ -438,6 +440,9 @@ private function getServiceMock(
if ($configMock === null) {
$configMock = $this->createMock(IConfig::class);
}
if ($tokenProviderMock === null) {
$tokenProviderMock = $this->createMock(IProvider::class);
}
$mock = $this->getMockBuilder(OpenProjectAPIService::class)
->setConstructorArgs(
[
Expand All @@ -454,7 +459,7 @@ private function getServiceMock(
$groupManagerMock,
$appManagerMock,
$this->createMock(IDBConnection::class),
$this->createMock(IProvider::class),
$tokenProviderMock,
$iSecureRandomMock,
$this->createMock(IEventDispatcher::class),
$subAdminManagerMock,
Expand Down Expand Up @@ -1739,6 +1744,150 @@ public function testIsSystemReadyForGroupFolderSetUpUserOrGroupExistsException(
$service->isSystemReadyForProjectFolderSetUp();
}

public function testProjectFolderHasAppPassword(): void {
$tokenProviderMock = $this->getMockBuilder(IProvider::class)->disableOriginalConstructor()
->getMock();
$tokenMock = $this->getMockBuilder(IToken::class)->getMock();
$tokenMock
->method('getName')
->willReturn('OpenProject');
$tokenProviderMock
->method('getTokenByUser')
->with(Application::OPEN_PROJECT_ENTITIES_NAME)
->willReturn([$tokenMock]);
$service = $this->getServiceMock([],
null,
null,
null,
null,
null,
null,
null,
null,
$tokenProviderMock);
$this->assertTrue($service->hasAppPassword());
}

public function testProjectFolderHasMultipleAppPassword(): void {
$tokenProviderMock = $this->getMockBuilder(IProvider::class)->disableOriginalConstructor()
->getMock();
$tokenMock1 = $this->getMockBuilder(IToken::class)->getMock();
$tokenMock1
->method('getName')
->willReturn('session');
$tokenMock2 = $this->getMockBuilder(IToken::class)->getMock();
$tokenMock2
->method('getName')
->willReturn('test');
$tokenMock3 = $this->getMockBuilder(IToken::class)->getMock();
$tokenMock3
->method('getName')
->willReturn('new-token');
$tokenMock4 = $this->getMockBuilder(IToken::class)->getMock();
$tokenMock4
->method('getName')
->willReturn('OpenProject');
$tokenProviderMock
->method('getTokenByUser')
->with(Application::OPEN_PROJECT_ENTITIES_NAME)
->willReturn([$tokenMock1,$tokenMock2,$tokenMock3,$tokenMock4]);
$service = $this->getServiceMock([],
null,
null,
null,
null,
null,
null,
null,
null,
$tokenProviderMock);
$this->assertTrue($service->hasAppPassword());
}

public function testProjectFolderHasAppPasswordNegativeCondition(): void {
$tokenProviderMock = $this->getMockBuilder(IProvider::class)->disableOriginalConstructor()
->getMock();
$tokenMock = $this->getMockBuilder(IToken::class)->getMock();
$tokenMock
->method('getName')
->willReturn('session');
$tokenProviderMock
->method('getTokenByUser')
->with(Application::OPEN_PROJECT_ENTITIES_NAME)
->willReturn([$tokenMock]);
$service = $this->getServiceMock([],
null,
null,
null,
null,
null,
null,
null,
null,
$tokenProviderMock);
$this->assertFalse($service->hasAppPassword());
}

public function testProjectFolderDeleteAppPassword(): void {
$tokenProviderMock = $this->getMockBuilder(IProvider::class)->disableOriginalConstructor()
->getMock();
$tokenMock1 = $this->getMockBuilder(IToken::class)->getMock();
$tokenMock1
->method('getName')
->willReturn('session');
$tokenMock1
->method('getId')
->willReturn(1);
$tokenMock2 = $this->getMockBuilder(IToken::class)->getMock();
$tokenMock2
->method('getName')
->willReturn('test');
$tokenMock2
->method('getId')
->willReturn(2);
$tokenMock3 = $this->getMockBuilder(IToken::class)->getMock();
$tokenMock3
->method('getName')
->willReturn('new-token');
$tokenMock3
->method('getId')
->willReturn(3);
$tokenMock4 = $this->getMockBuilder(IToken::class)->getMock();
$tokenMock4
->method('getName')
->willReturn('OpenProject');
$tokenMock4
->method('getId')
->willReturn(4);
$tokenMock5 = $this->getMockBuilder(IToken::class)->getMock();
$tokenMock5
->method('getName')
->willReturn('OpenProject');
$tokenMock5
->method('getId')
->willReturn(5);
$tokenProviderMock
->method('getTokenByUser')
->with(Application::OPEN_PROJECT_ENTITIES_NAME)
->willReturn([$tokenMock1,$tokenMock2,$tokenMock3,$tokenMock4,$tokenMock5]);
$service = $this->getServiceMock(['hasAppPassword'],
null,
null,
null,
null,
null,
null,
null,
null,
$tokenProviderMock)
;
$service->method('hasAppPassword')->willReturn(true);
$tokenProviderMock->expects($this->exactly(2))
->method('invalidateTokenById')
->withConsecutive([Application::OPEN_PROJECT_ENTITIES_NAME, 4], [Application::OPEN_PROJECT_ENTITIES_NAME, 5]);
$service->deleteAppPassword();
}

public function testLinkWorkPackageToFilePact(): void {
$consumerRequest = new ConsumerRequest();
$consumerRequest
Expand Down

0 comments on commit 0398905

Please sign in to comment.