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

Fix flow #6879

Closed
wants to merge 3 commits into from
Closed

Fix flow #6879

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b19f7b3
Try
mario Oct 19, 2017
276126c
Fix path generation
mario Oct 19, 2017
9064166
Fix bugs
mario Oct 19, 2017
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
3 changes: 2 additions & 1 deletion core/Controller/ClientFlowLoginController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,8 @@ public function generateAppPassword($stateToken,
);
$this->session->remove('oauth.state');
} else {
$redirectUri = 'nc://login/server:' . $this->request->getServerHost() . '&user:' . urlencode($loginName) . '&password:' . urlencode($token);
$serverPath = $this->request->getServerProtocol() . "://" . $this->request->getServerHost() . substr($this->request->getRequestUri(), 0, strpos($this->request->getRequestUri(), "/index.php"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work if you have pretty urls. Since then you don't have index.php

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an alternative suggestion? Even on Android for login flow we always use index.php.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got an idea. Testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added support for pretty urls.

$redirectUri = 'nc://login/server:' . $serverPath . '&user:' . urlencode($loginName) . '&password:' . urlencode($token);
}

return new Http\RedirectResponse($redirectUri);
Expand Down
16 changes: 8 additions & 8 deletions tests/Core/Controller/ClientFlowLoginControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ public function testShowAuthPickerPageWithOcsHeader() {
$this->request
->expects($this->once())
->method('getServerHost')
->willReturn('example.com');
->willReturn('http://example.com');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is cheating? make sure it actually calls getServerProtocol ?!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm actually wouldnt it return example.com? Since it's just getServerHost ... Tests need better thinking xD

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!


$expected = new TemplateResponse(
'core',
Expand All @@ -172,7 +172,7 @@ public function testShowAuthPickerPageWithOcsHeader() {
'instanceName' => 'ExampleCloud',
'urlGenerator' => $this->urlGenerator,
'stateToken' => 'StateToken',
'serverHost' => 'example.com',
'serverHost' => 'http://example.com',
'oauthState' => 'OauthStateToken',
],
'guest'
Expand Down Expand Up @@ -217,7 +217,7 @@ public function testShowAuthPickerPageWithOauth() {
$this->request
->expects($this->once())
->method('getServerHost')
->willReturn('example.com');
->willReturn('http://example.com');

$expected = new TemplateResponse(
'core',
Expand All @@ -228,7 +228,7 @@ public function testShowAuthPickerPageWithOauth() {
'instanceName' => 'ExampleCloud',
'urlGenerator' => $this->urlGenerator,
'stateToken' => 'StateToken',
'serverHost' => 'example.com',
'serverHost' => 'http://example.com',
'oauthState' => 'OauthStateToken',
],
'guest'
Expand Down Expand Up @@ -426,9 +426,9 @@ public function testGeneratePasswordWithPassword() {
$this->request
->expects($this->once())
->method('getServerHost')
->willReturn('example.com');
->willReturn('http://example.com');

$expected = new Http\RedirectResponse('nc://login/server:example.com&user:MyLoginName&password:MyGeneratedToken');
$expected = new Http\RedirectResponse('nc://login/server:http://example.com&user:MyLoginName&password:MyGeneratedToken');
$this->assertEquals($expected, $this->clientFlowLoginController->generateAppPassword('MyStateToken'));
}

Expand Down Expand Up @@ -574,9 +574,9 @@ public function testGeneratePasswordWithoutPassword() {
$this->request
->expects($this->once())
->method('getServerHost')
->willReturn('example.com');
->willReturn('http://example.com');

$expected = new Http\RedirectResponse('nc://login/server:example.com&user:MyLoginName&password:MyGeneratedToken');
$expected = new Http\RedirectResponse('nc://login/server:http://example.com&user:MyLoginName&password:MyGeneratedToken');
$this->assertEquals($expected, $this->clientFlowLoginController->generateAppPassword('MyStateToken'));
}
}