Skip to content

improved login flow #190

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

Merged
ArtificialOwl merged 1 commit into from
Apr 17, 2026
Merged

improved login flow #190

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
17 changes: 8 additions & 9 deletions lib/AppInfo/Application.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,32 +158,31 @@ private function redirectToMasterLogin() {
$userSession = Server::get(IUserSession::class);
if ($userSession->isLoggedIn()) {
$this->logger->debug('already logged in, we stay on slave', ['app' => self::APP_ID]);

return;
}

/** @var IRequest $request */
$request = Server::get(IRequest::class);
if ($request->getPathInfo() !== '/login') {
$pathInfo = (string)$request->getPathInfo();
if (!in_array($pathInfo, ['/login', '/login/v2', '/login/flow', '/login/v2/flow'], true)) {
$this->logger->debug('login page not called, we stay on slave', ['app' => self::APP_ID]);

return;
}

$params = $request->getParams();
if (isset($params['direct'])) {
$this->logger->debug('direct login page requested, we stay on slave', ['app' => self::APP_ID]
);

$this->logger->debug('direct login page requested, we stay on slave', ['app' => self::APP_ID]);
return;
}

if (isset($params['redirect_url'])) {
$masterUrl = rtrim($masterUrl, '/') . '/index.php/login?redirect_url=' . urlencode($params['redirect_url']);
$redirectUrl = $params['redirect_url'] ?? null;
if ($redirectUrl === null) {
$masterUrl = rtrim($masterUrl, '/') . '/index.php' . $pathInfo;
} else {
$masterUrl = rtrim($masterUrl, '/') . '/index.php/login?redirect_url=' . urlencode($redirectUrl);
}

$this->logger->debug('Redirecting client to ' . $masterUrl, ['app' => self::APP_ID]);

header('Location: ' . $masterUrl);
exit();
} catch (Exception|ContainerExceptionInterface|NotFoundExceptionInterface $e) {
Expand Down
7 changes: 6 additions & 1 deletion lib/Controller/SlaveController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,12 @@ public function autoLogin(string $jwt): RedirectResponse {
$this->slaveService->updateUserById($uid);
$this->logger->debug('userdata updated on lus');

$home = $this->urlGenerator->getAbsoluteURL($target);
if (str_starts_with($target, 'http://') || str_starts_with($target, 'https://')) {
$home = $target;
} else {
$home = $this->urlGenerator->getAbsoluteURL($target);
}

if (!empty($params)) {
$home .= '?' . http_build_query($params);
}
Expand Down
107 changes: 79 additions & 28 deletions lib/Master.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,13 @@
namespace OCA\GlobalSiteSelector;

use Exception;
use OC\Core\Controller\ClientFlowLoginV2Controller;
use OC\Core\Service\LoginFlowV2Service;
use OCA\GlobalSiteSelector\AppInfo\Application;
use OCA\GlobalSiteSelector\UserDiscoveryModules\IUserDiscoveryModule;
use OCA\GlobalSiteSelector\Vendor\Firebase\JWT\JWT;
use OCA\GlobalSiteSelector\Vendor\Firebase\JWT\Key;
use OCP\AppFramework\Http\StandaloneTemplateResponse;
use OCP\Authentication\IApacheBackend;
use OCP\HintException;
use OCP\Http\Client\IClientService;
Expand All @@ -22,6 +25,8 @@
use OCP\ISession;
use OCP\Security\ICrypto;
use OCP\Server;
use OCP\ServerVersion;
use OCP\Util;
use Psr\Container\ContainerExceptionInterface;
use Psr\Container\NotFoundExceptionInterface;
use Psr\Log\LoggerInterface;
Expand All @@ -34,36 +39,20 @@
* @package OCA\GlobalSiteSelector
*/
class Master {
private ISession $session;
private GlobalSiteSelector $gss;
private ICrypto $crypto;
private Lookup $lookup;
private IRequest $request;
private IClientService $clientService;
private IConfig $config;
private LoggerInterface $logger;

public function __construct(
ISession $session,
GlobalSiteSelector $gss,
ICrypto $crypto,
Lookup $lookup,
IRequest $request,
IClientService $clientService,
IConfig $config,
LoggerInterface $logger,
private readonly ISession $session,
private readonly GlobalSiteSelector $gss,
private readonly ICrypto $crypto,
private readonly LoginFlowV2Service $loginFlowV2Service,
private readonly ServerVersion $serverVersion,
private readonly Lookup $lookup,
private readonly IRequest $request,
private readonly IClientService $clientService,
private readonly IConfig $config,
private readonly LoggerInterface $logger,
) {
$this->session = $session;
$this->gss = $gss;
$this->crypto = $crypto;
$this->lookup = $lookup;
$this->request = $request;
$this->clientService = $clientService;
$this->config = $config;
$this->logger = $logger;
}


/**
* find users location and redirect them to the right server
*
Expand Down Expand Up @@ -108,6 +97,8 @@ public function handleLoginRequest(
$userDiscoveryModule = $this->config->getSystemValueString('gss.user.discovery.module', '');
$this->logger->debug('handleLoginRequest: discovery module is: ' . $userDiscoveryModule);

$redirectUrl = $this->request->getParam('redirect_url', '');

$isSamlOrOidc = false;
if (class_exists('\OCA\User_SAML\UserBackend')
&& $backend instanceof \OCA\User_SAML\UserBackend) {
Expand Down Expand Up @@ -148,11 +139,33 @@ public function handleLoginRequest(
// TODO: switch 'oidc.redirect' to \OCA\UserOIDC\Controller\LoginController::REDIRECT_AFTER_LOGIN once switched to public
$options['target'] = $this->forceRelativeUrl($this->session->get('oidc.redirect') ?? '/');

// Fix: restore the slave flow path into options.target after all backend blocks.
//
// Application.php passes the slave flow path as redirect_url in the /login
// query string, e.g. redirect_url=%2Findex.php%2Flogin%2Fv2%2Fflow%2FF
//
// By the time handleLoginRequest() fires, the current request is the OIDC
// callback (/apps/user_oidc/code?state=...&code=...) with no redirect_url.
// We recover it from oidc.redirect (stored in the session by UserOIDC before
// the OIDC redirect). oidc.redirect is the full pre-OIDC request URL:
// https://master/index.php/login?redirect_url=%2Findex.php%2Flogin%2Fv2%2Fflow%2FF
// We parse its query string to extract redirect_url = /index.php/login/v2/flow/F
$oidcRedirect = (string)($this->session->get('oidc.redirect') ?? '');
if ($oidcRedirect !== '') {
parse_str(parse_url($oidcRedirect, PHP_URL_QUERY) ?? '', $oidcRedirectParams);
$redirectUrl = $oidcRedirectParams['redirect_url'] ?? $redirectUrl;
}

$this->logger->debug('handleLoginRequest: backend is OIDC.', ['options' => $options]);
} else {
$this->logger->debug('handleLoginRequest: backend is not SAML or OIDC');
}

if ($this->isPath(['/login/flow', '/login/v2/flow'], $redirectUrl ?? '')) {
$options['target'] = $redirectUrl;
$this->logger->debug('handleLoginRequest: overriding target with slave flow path: ' . $options['target']);
}

$this->logger->debug('handleLoginRequest: uid is: ' . $uid);

// let local account login, everyone else will redirected to a client
Expand Down Expand Up @@ -247,9 +260,10 @@ protected function redirectUser($uid, $password, $location, array $options = [])
IRequest::USER_AGENT_CLIENT_IOS,
IRequest::USER_AGENT_CLIENT_ANDROID,
IRequest::USER_AGENT_CLIENT_DESKTOP,
'/mirall|csyncoC/', // <-- Support also not compliant Desktop Clients
'/^.*\(Android\)$/'
]
);
) || $this->isPath(['/login/flow/grant', '/login/v2/grant'], $options['target'] ?? '');

$requestUri = $this->request->getRequestUri();
// check for both possible direct webdav end-points
Expand All @@ -262,7 +276,16 @@ protected function redirectUser($uid, $password, $location, array $options = [])
} elseif ($isClient && !$isDirectWebDavAccess) {
$this->logger->debug('redirectUser: client request generating apptoken');
$appToken = $this->getAppToken($location, $uid, $password, $options);
$redirectUrl = 'nc://login/server:' . $location . '&user:' . urlencode($uid) . '&password:' . urlencode($appToken);

$loginV2Token = $this->session->get(ClientFlowLoginV2Controller::TOKEN_NAME);
if ($loginV2Token !== null && $location !== '') {
$result = $this->loginFlowV2Service->flowDoneWithAppPassword($loginV2Token, $location, $uid, $appToken);
echo $this->handleFlowDone($result)->render();
die();
} else {
// fallback to v1
$redirectUrl = 'nc://login/server:' . $location . '&user:' . urlencode($uid) . '&password:' . urlencode($appToken);
}
} else {
$this->logger->debug('redirectUser: direct login so forward to target node');
$jwt = $this->createJwt($uid, $password, $options);
Expand Down Expand Up @@ -396,4 +419,32 @@ private function forceRelativeUrl(string $url): string {

return $url;
}

private function isPath(array $search, string $path): bool {
if ($path === '') {
return false;
}

foreach ($search as $entry) {
if (str_starts_with($path, $entry) || str_starts_with($path, '/index.php' . $entry)) {
return true;
}
}

return false;
}

private function handleFlowDone(bool $result): StandaloneTemplateResponse {
if ($result) {
// login flow v2 templates were moved in NC33
if ($this->serverVersion->getMajorVersion() >= 33) {
Util::addScript('core', 'login_flow');
return new StandaloneTemplateResponse('core', 'loginflow', renderAs: 'guest');
}

return new StandaloneTemplateResponse('core', 'loginflowv2/done', renderAs: 'guest');
}

return new StandaloneTemplateResponse('core', '403', ['message' => 'Could not complete login'], 'guest');
}
}
2 changes: 1 addition & 1 deletion lib/PublicCapabilities.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public function getCapabilities(): array {
return [
'globalscale' => [
'enabled' => true,
'desktoplogin' => 1,
'desktoplogin' => 2,
'token' => $this->globalScaleService->getLocalToken(),
]
];
Expand Down
10 changes: 10 additions & 0 deletions tests/unit/lib/MasterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

namespace OCA\GlobalSiteSelector\Tests\Unit;

use OC\Core\Service\LoginFlowV2Service;
use OCA\GlobalSiteSelector\AppInfo\Application;
use OCA\GlobalSiteSelector\GlobalSiteSelector;
use OCA\GlobalSiteSelector\Lookup;
Expand All @@ -21,6 +22,9 @@
use OCP\IRequest;
use OCP\ISession;
use OCP\Security\ICrypto;
use OCP\Server;
use OCP\ServerVersion;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;
use Test\TestCase;

Expand Down Expand Up @@ -51,6 +55,8 @@ class MasterTest extends TestCase {

/** @var ISession | \PHPUnit_Framework_MockObject_MockObject */
private $session;
private LoginFlowV2Service&MockObject $loginflow;
private ServerVersion $serverVersion;

public function setUp(): void {
parent::setUp();
Expand All @@ -60,6 +66,8 @@ public function setUp(): void {
$this->crypto = $this->createMock(ICrypto::class);
$this->lookup = $this->getMockBuilder(Lookup::class)
->disableOriginalConstructor()->getMock();
$this->loginflow = $this->createMock(LoginFlowV2Service::class);
$this->serverVersion = Server::get(ServerVersion::class);
$this->request = $this->createMock(IRequest::class);
$this->clientService = $this->createMock(IClientService::class);
$this->config = $this->createMock(IConfig::class);
Expand All @@ -79,6 +87,8 @@ private function getInstance(array $mockMethods = []) {
$this->session,
$this->gss,
$this->crypto,
$this->loginflow,
$this->serverVersion,
$this->lookup,
$this->request,
$this->clientService,
Expand Down
Loading