Skip to content

Commit 92582a3

Browse files
committed
Use the proper server for the apptoken flow login
If a user can't authenticate normally (because they have 2FA that is not available on their devices for example). The redirect that is generated should be of the proper format. This means 1. Include the protocol 2. Include the possible subfolder Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
1 parent 15ef354 commit 92582a3

File tree

2 files changed

+31
-23
lines changed

2 files changed

+31
-23
lines changed

core/Controller/ClientFlowLoginController.php

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ public function showAuthPickerPage($clientIdentifier = '') {
197197
'instanceName' => $this->defaults->getName(),
198198
'urlGenerator' => $this->urlGenerator,
199199
'stateToken' => $stateToken,
200-
'serverHost' => $this->request->getServerHost(),
200+
'serverHost' => $this->getServerPath(),
201201
'oauthState' => $this->session->get('oauth.state'),
202202
],
203203
'guest'
@@ -235,7 +235,7 @@ public function grantPage($stateToken = '',
235235
'instanceName' => $this->defaults->getName(),
236236
'urlGenerator' => $this->urlGenerator,
237237
'stateToken' => $stateToken,
238-
'serverHost' => $this->request->getServerHost(),
238+
'serverHost' => $this->getServerPath(),
239239
'oauthState' => $this->session->get('oauth.state'),
240240
],
241241
'guest'
@@ -345,32 +345,34 @@ public function generateAppPassword($stateToken,
345345
);
346346
$this->session->remove('oauth.state');
347347
} else {
348-
$serverPostfix = '';
348+
$redirectUri = 'nc://login/server:' . $this->getServerPath() . '&user:' . urlencode($loginName) . '&password:' . urlencode($token);
349349

350-
if (strpos($this->request->getRequestUri(), '/index.php') !== false) {
351-
$serverPostfix = substr($this->request->getRequestUri(), 0, strpos($this->request->getRequestUri(), '/index.php'));
352-
} else if (strpos($this->request->getRequestUri(), '/login/flow') !== false) {
353-
$serverPostfix = substr($this->request->getRequestUri(), 0, strpos($this->request->getRequestUri(), '/login/flow'));
354-
}
350+
// Clear the token from the login here
351+
$this->tokenProvider->invalidateToken($sessionId);
352+
}
355353

356-
$protocol = $this->request->getServerProtocol();
354+
return new Http\RedirectResponse($redirectUri);
355+
}
357356

358-
if ($protocol !== "https") {
359-
$xForwardedProto = $this->request->getHeader('X-Forwarded-Proto');
360-
$xForwardedSSL = $this->request->getHeader('X-Forwarded-Ssl');
361-
if ($xForwardedProto === 'https' || $xForwardedSSL === 'on') {
362-
$protocol = 'https';
363-
}
364-
}
357+
private function getServerPath(): string {
358+
$serverPostfix = '';
365359

360+
if (strpos($this->request->getRequestUri(), '/index.php') !== false) {
361+
$serverPostfix = substr($this->request->getRequestUri(), 0, strpos($this->request->getRequestUri(), '/index.php'));
362+
} else if (strpos($this->request->getRequestUri(), '/login/flow') !== false) {
363+
$serverPostfix = substr($this->request->getRequestUri(), 0, strpos($this->request->getRequestUri(), '/login/flow'));
364+
}
366365

367-
$serverPath = $protocol . "://" . $this->request->getServerHost() . $serverPostfix;
368-
$redirectUri = 'nc://login/server:' . $serverPath . '&user:' . urlencode($loginName) . '&password:' . urlencode($token);
366+
$protocol = $this->request->getServerProtocol();
369367

370-
// Clear the token from the login here
371-
$this->tokenProvider->invalidateToken($sessionId);
368+
if ($protocol !== "https") {
369+
$xForwardedProto = $this->request->getHeader('X-Forwarded-Proto');
370+
$xForwardedSSL = $this->request->getHeader('X-Forwarded-Ssl');
371+
if ($xForwardedProto === 'https' || $xForwardedSSL === 'on') {
372+
$protocol = 'https';
373+
}
372374
}
373375

374-
return new Http\RedirectResponse($redirectUri);
376+
return $protocol . "://" . $this->request->getServerHost() . $serverPostfix;
375377
}
376378
}

tests/Core/Controller/ClientFlowLoginControllerTest.php

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,9 @@ public function testShowAuthPickerPageWithOcsHeader() {
162162
->expects($this->once())
163163
->method('getServerHost')
164164
->willReturn('example.com');
165+
$this->request
166+
->method('getServerProtocol')
167+
->willReturn('https');
165168

166169
$expected = new TemplateResponse(
167170
'core',
@@ -172,7 +175,7 @@ public function testShowAuthPickerPageWithOcsHeader() {
172175
'instanceName' => 'ExampleCloud',
173176
'urlGenerator' => $this->urlGenerator,
174177
'stateToken' => 'StateToken',
175-
'serverHost' => 'example.com',
178+
'serverHost' => 'https://example.com',
176179
'oauthState' => 'OauthStateToken',
177180
],
178181
'guest'
@@ -218,6 +221,9 @@ public function testShowAuthPickerPageWithOauth() {
218221
->expects($this->once())
219222
->method('getServerHost')
220223
->willReturn('example.com');
224+
$this->request
225+
->method('getServerProtocol')
226+
->willReturn('https');
221227

222228
$expected = new TemplateResponse(
223229
'core',
@@ -228,7 +234,7 @@ public function testShowAuthPickerPageWithOauth() {
228234
'instanceName' => 'ExampleCloud',
229235
'urlGenerator' => $this->urlGenerator,
230236
'stateToken' => 'StateToken',
231-
'serverHost' => 'example.com',
237+
'serverHost' => 'https://example.com',
232238
'oauthState' => 'OauthStateToken',
233239
],
234240
'guest'

0 commit comments

Comments
 (0)