Skip to content

Commit ba55395

Browse files
julien-ncjulien-nc
authored
Merge pull request #1184 from nextcloud/fix/1183/fix-backchannel-sid-sub-logic
Fix backchannel logout when no SID is set in the logout token
2 parents 0ae3e34 + 260d9d7 commit ba55395

File tree

2 files changed

+105
-46
lines changed

2 files changed

+105
-46
lines changed

‎lib/Controller/LoginController.php‎

Lines changed: 64 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -780,61 +780,83 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok
780780
);
781781
}
782782

783-
// get the auth token ID associated with the logout token's sid attr
784-
$sid = $logoutTokenPayload->sid;
785-
try {
786-
$oidcSession = $this->sessionMapper->findSessionBySid($sid);
787-
} catch (DoesNotExistException $e) {
783+
if (!isset($logoutTokenPayload->iss)) {
788784
return $this->getBackchannelLogoutErrorResponse(
789-
'invalid SID',
790-
'The sid of the logout token was not found',
791-
['session_sid_not_found' => $sid]
785+
'invalid iss',
786+
'The logout token should contain an iss attribute',
787+
['iss_should_be_set' => true]
792788
);
793-
} catch (MultipleObjectsReturnedException $e) {
789+
}
790+
$iss = $logoutTokenPayload->iss;
791+
792+
if (!isset($logoutTokenPayload->sid) && !isset($logoutTokenPayload->sub)) {
794793
return $this->getBackchannelLogoutErrorResponse(
795-
'invalid SID',
796-
'The sid of the logout token was found multiple times',
797-
['multiple_logout_tokens_found' => $sid]
794+
'invalid sid+sub',
795+
'The logout token should contain sid or sub or both',
796+
['no_sid_no_sub' => true]
798797
);
799798
}
800799

801-
if (isset($logoutTokenPayload->sub)) {
800+
$oidcSessionsToKill = [];
801+
802+
// if SID is set, we look for this specific session (with or without using the sub, depending on if the sub is set)
803+
if (isset($logoutTokenPayload->sid)) {
804+
$sid = $logoutTokenPayload->sid;
805+
$sub = $logoutTokenPayload->sub ?? null;
806+
try {
807+
$oidcSession = $this->sessionMapper->findSessionBySid($sid, $sub, $iss);
808+
} catch (DoesNotExistException $e) {
809+
return $this->getBackchannelLogoutErrorResponse(
810+
$sub === null ? 'invalid SID or ISS' : 'invalid SID, SUB or ISS',
811+
$sub === null ? 'No session was found for this (sid,iss)' : 'No session was found for this (sid,sub,iss)',
812+
['session_not_found' => $sid]
813+
);
814+
} catch (MultipleObjectsReturnedException $e) {
815+
return $this->getBackchannelLogoutErrorResponse(
816+
$sub === null ? 'invalid SID or ISS' : 'invalid SID, SUB or ISS',
817+
$sub === null ? 'Multiple sessions were found with this (sid,iss)' : 'Multiple sessions were found with this (sid,sub,iss)',
818+
['multiple_sessions_found' => $sid]
819+
);
820+
}
821+
$oidcSessionsToKill[] = $oidcSession;
822+
} else {
823+
// here we know the sid is not set so the sub is set
802824
$sub = $logoutTokenPayload->sub;
803-
if ($oidcSession->getSub() !== $sub) {
825+
try {
826+
$oidcSessionsToKill = $this->sessionMapper->findSessionsBySubAndIss($sub, $iss);
827+
} catch (\OCP\Db\Exception $e) {
804828
return $this->getBackchannelLogoutErrorResponse(
805-
'invalid SUB',
806-
'The sub does not match the one from the login ID token',
807-
['invalid_sub' => $sub]
829+
'error with sub+iss',
830+
'Failed to retrieve session with sub+iss',
831+
['sub_iss_error' => true]
808832
);
809833
}
810-
}
811-
$iss = $logoutTokenPayload->iss;
812-
if ($oidcSession->getIss() !== $iss) {
813-
return $this->getBackchannelLogoutErrorResponse(
814-
'invalid ISS',
815-
'The iss does not match the one from the login ID token',
816-
['invalid_iss' => $iss]
817-
);
818-
}
819834

820-
// i don't know why but the cast is necessary
821-
$authTokenId = (int)$oidcSession->getAuthtokenId();
822-
try {
823-
$authToken = $this->authTokenProvider->getTokenById($authTokenId);
824-
// we could also get the auth token by nc session ID
825-
// $authToken = $this->authTokenProvider->getToken($oidcSession->getNcSessionId());
826-
$userId = $authToken->getUID();
827-
$this->authTokenProvider->invalidateTokenById($userId, $authToken->getId());
828-
} catch (InvalidTokenException $e) {
829-
return $this->getBackchannelLogoutErrorResponse(
830-
'nc session not found',
831-
'The authentication session was not found in Nextcloud',
832-
['nc_auth_session_not_found' => $authTokenId]
833-
);
835+
if (empty($oidcSessionsToKill)) {
836+
return $this->getBackchannelLogoutErrorResponse(
837+
'nothing found with sub+iss',
838+
'No session found with sub+iss',
839+
['sub_iss_no_session_found' => true]
840+
);
841+
}
834842
}
835843

836-
// cleanup
837-
$this->sessionMapper->delete($oidcSession);
844+
foreach ($oidcSessionsToKill as $oidcSession) {
845+
// i don't know why but the cast is necessary
846+
$authTokenId = (int)$oidcSession->getAuthtokenId();
847+
try {
848+
$authToken = $this->authTokenProvider->getTokenById($authTokenId);
849+
// we could also get the auth token by nc session ID
850+
// $authToken = $this->authTokenProvider->getToken($oidcSession->getNcSessionId());
851+
$userId = $authToken->getUID();
852+
$this->authTokenProvider->invalidateTokenById($userId, $authToken->getId());
853+
} catch (InvalidTokenException $e) {
854+
$this->logger->warning('[BackchannelLogout] Nextcloud session not found', ['authtoken_id' => $authTokenId]);
855+
}
856+
857+
// cleanup
858+
$this->sessionMapper->delete($oidcSession);
859+
}
838860

839861
return new JSONResponse([], Http::STATUS_OK);
840862
}

‎lib/Db/SessionMapper.php‎

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use OCP\AppFramework\Db\DoesNotExistException;
1313
use OCP\AppFramework\Db\MultipleObjectsReturnedException;
1414
use OCP\AppFramework\Db\QBMapper;
15+
use OCP\DB\Exception;
1516
use OCP\DB\QueryBuilder\IQueryBuilder;
1617

1718
use OCP\IDBConnection;
@@ -43,21 +44,57 @@ public function getSession(int $id): Session {
4344
}
4445

4546
/**
46-
* Find session by sid (from the OIDC id token)
47+
* Find sessions by sub and iss
48+
*
49+
* @param string $sub
50+
* @param string $iss
51+
* @return Session[]
52+
* @throws Exception
53+
*/
54+
public function findSessionsBySubAndIss(string $sub, string $iss): array {
55+
$qb = $this->db->getQueryBuilder();
56+
57+
$qb->select('*')
58+
->from($this->getTableName())
59+
->where(
60+
$qb->expr()->eq('sub', $qb->createNamedParameter($sub, IQueryBuilder::PARAM_STR))
61+
)
62+
->andWhere(
63+
$qb->expr()->eq('iss', $qb->createNamedParameter($iss, IQueryBuilder::PARAM_STR))
64+
);
65+
66+
return $this->findEntities($qb);
67+
}
68+
69+
/**
70+
* Find session by sid and optionally sub and iss
4771
*
4872
* @param string $sid
73+
* @param string|null $sub
74+
* @param string|null $iss
4975
* @return Session
50-
* @throws \OCP\AppFramework\Db\DoesNotExistException
51-
* @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
76+
* @throws DoesNotExistException
77+
* @throws Exception
78+
* @throws MultipleObjectsReturnedException
5279
*/
53-
public function findSessionBySid(string $sid): Session {
80+
public function findSessionBySid(string $sid, ?string $sub = null, ?string $iss = null): Session {
5481
$qb = $this->db->getQueryBuilder();
5582

5683
$qb->select('*')
5784
->from($this->getTableName())
5885
->where(
5986
$qb->expr()->eq('sid', $qb->createNamedParameter($sid, IQueryBuilder::PARAM_STR))
6087
);
88+
if ($sub !== null) {
89+
$qb->andWhere(
90+
$qb->expr()->eq('sub', $qb->createNamedParameter($sub, IQueryBuilder::PARAM_STR))
91+
);
92+
}
93+
if ($iss !== null) {
94+
$qb->andWhere(
95+
$qb->expr()->eq('iss', $qb->createNamedParameter($iss, IQueryBuilder::PARAM_STR))
96+
);
97+
}
6198

6299
return $this->findEntity($qb);
63100
}

0 commit comments

Comments
 (0)