Skip to content

Commit c6adc7f

Browse files
julien-ncjulien-nc
authored
Merge branch 'main' into main
2 parents 8e9495f + 8a12878 commit c6adc7f

13 files changed

Lines changed: 674 additions & 426 deletions

‎composer.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
"nextcloud/coding-standard": "^1.0.0",
4141
"symfony/event-dispatcher": "^4",
4242
"phpunit/phpunit": "^9.5",
43-
"nextcloud/ocp": "dev-stable25",
43+
"nextcloud/ocp": "dev-stable26",
4444
"psalm/phar": "^5.20"
4545
},
4646
"extra": {

‎composer.lock‎

Lines changed: 8 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/Command/UpsertProvider.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ class UpsertProvider extends Base {
5959
'shortcut' => null, 'mode' => InputOption::VALUE_REQUIRED, 'setting_key' => ProviderService::SETTING_EXTRA_CLAIMS,
6060
'description' => 'Extra claims to request when getting tokens',
6161
],
62+
'mapping-language' => [
63+
'shortcut' => null, 'mode' => InputOption::VALUE_REQUIRED, 'setting_key' => ProviderService::SETTING_MAPPING_LANGUAGE,
64+
'description' => 'Attribute mapping of the account language',
65+
],
6266
'mapping-website' => [
6367
'shortcut' => null, 'mode' => InputOption::VALUE_REQUIRED, 'setting_key' => ProviderService::SETTING_MAPPING_WEBSITE,
6468
'description' => 'Attribute mapping of the website',

‎lib/Controller/LoginController.php‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,13 @@
5252
use OCP\User\Events\BeforeUserLoggedInEvent;
5353
use OCP\User\Events\UserLoggedInEvent;
5454
use Psr\Log\LoggerInterface;
55+
use UnexpectedValueException;
5556

5657
class LoginController extends BaseOidcController {
5758
private const STATE = 'oidc.state';
5859
private const NONCE = 'oidc.nonce';
5960
public const PROVIDERID = 'oidc.providerid';
60-
private const REDIRECT_AFTER_LOGIN = 'oidc.redirect';
61+
public const REDIRECT_AFTER_LOGIN = 'oidc.redirect';
6162
private const ID_TOKEN = 'oidc.id_token';
6263
private const CODE_VERIFIER = 'oidc.code_verifier';
6364

@@ -398,7 +399,13 @@ public function code(string $state = '', string $code = '', string $scope = '',
398399
$idTokenRaw = $data['id_token'];
399400
$jwks = $this->discoveryService->obtainJWK($provider, $idTokenRaw);
400401
JWT::$leeway = 60;
401-
$idTokenPayload = JWT::decode($idTokenRaw, $jwks);
402+
try {
403+
$idTokenPayload = JWT::decode($idTokenRaw, $jwks);
404+
} catch (UnexpectedValueException $e) {
405+
$this->logger->debug('Failed to decode the JWT token, retrying with fresh JWK');
406+
$jwks = $this->discoveryService->obtainJWK($provider, $idTokenRaw, false);
407+
$idTokenPayload = JWT::decode($idTokenRaw, $jwks);
408+
}
402409

403410
$this->logger->debug('Parsed the JWT payload: ' . json_encode($idTokenPayload, JSON_THROW_ON_ERROR));
404411

@@ -514,6 +521,8 @@ public function code(string $state = '', string $code = '', string $scope = '',
514521
$this->userSession->createSessionToken($this->request, $user->getUID(), $user->getUID());
515522
$this->userSession->createRememberMeToken($user);
516523
// TODO server should/could be refactored so we don't need to manually create the user session and dispatch the login-related events
524+
// Warning! If GSS is used, it reacts to the BeforeUserLoggedInEvent and handles the redirection itself
525+
// So nothing after dispatching this event will be executed
517526
$this->eventDispatcher->dispatchTyped(new BeforeUserLoggedInEvent($user->getUID(), null, \OC::$server->get(Backend::class)));
518527
$this->eventDispatcher->dispatchTyped(new UserLoggedInEvent($user, $user->getUID(), null, false));
519528
}

‎lib/Service/DiscoveryService.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,13 @@ public function obtainDiscovery(Provider $provider): array {
6565
/**
6666
* @param Provider $provider
6767
* @param string $tokenToDecode This is used to potentially fix the missing alg in
68+
* @param bool $useCache
6869
* @return array
6970
* @throws \JsonException
7071
*/
71-
public function obtainJWK(Provider $provider, string $tokenToDecode): array {
72+
public function obtainJWK(Provider $provider, string $tokenToDecode, bool $useCache = true): array {
7273
$lastJwksRefresh = $this->providerService->getSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE_TIMESTAMP);
73-
if ($lastJwksRefresh !== '' && (int)$lastJwksRefresh > time() - self::INVALIDATE_JWKS_CACHE_AFTER_SECONDS) {
74+
if ($lastJwksRefresh !== '' && $useCache && (int)$lastJwksRefresh > time() - self::INVALIDATE_JWKS_CACHE_AFTER_SECONDS) {
7475
$rawJwks = $this->providerService->getSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE);
7576
$rawJwks = json_decode($rawJwks, true);
7677
} else {

‎lib/Service/ProviderService.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ class ProviderService {
2626
public const SETTING_MAPPING_EMAIL = 'mappingEmail';
2727
public const SETTING_MAPPING_QUOTA = 'mappingQuota';
2828
public const SETTING_MAPPING_GROUPS = 'mappingGroups';
29+
public const SETTING_MAPPING_LANGUAGE = 'mappingLanguage';
2930
public const SETTING_MAPPING_ADDRESS = 'mappingAddress';
3031
public const SETTING_MAPPING_STREETADDRESS = 'mappingStreetaddress';
3132
public const SETTING_MAPPING_POSTALCODE = 'mappingPostalcode';
@@ -140,6 +141,7 @@ private function getSupportedSettings(): array {
140141
self::SETTING_MAPPING_QUOTA,
141142
self::SETTING_MAPPING_UID,
142143
self::SETTING_MAPPING_GROUPS,
144+
self::SETTING_MAPPING_LANGUAGE,
143145
self::SETTING_MAPPING_ADDRESS,
144146
self::SETTING_MAPPING_STREETADDRESS,
145147
self::SETTING_MAPPING_POSTALCODE,

‎lib/Service/ProvisioningService.php‎

Lines changed: 53 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
namespace OCA\UserOIDC\Service;
99

10+
use InvalidArgumentException;
11+
use OC\Accounts\AccountManager;
1012
use OCA\UserOIDC\AppInfo\Application;
1113
use OCA\UserOIDC\Db\UserMapper;
1214
use OCA\UserOIDC\Event\AttributeMappedEvent;
@@ -23,6 +25,7 @@
2325
use OCP\ISession;
2426
use OCP\IUser;
2527
use OCP\IUserManager;
28+
use OCP\L10N\IFactory;
2629
use OCP\User\Events\UserChangedEvent;
2730
use Psr\Log\LoggerInterface;
2831
use Throwable;
@@ -42,6 +45,7 @@ public function __construct(
4245
private IAvatarManager $avatarManager,
4346
private IConfig $config,
4447
private ISession $session,
48+
private IFactory $l10nFactory,
4549
) {
4650
}
4751

@@ -76,6 +80,9 @@ public function provisionUser(string $tokenUserId, int $providerId, object $idTo
7680
$quotaAttribute = $this->providerService->getSetting($providerId, ProviderService::SETTING_MAPPING_QUOTA, 'quota');
7781
$quota = $idTokenPayload->{$quotaAttribute} ?? null;
7882

83+
$languageAttribute = $this->providerService->getSetting($providerId, ProviderService::SETTING_MAPPING_LANGUAGE, 'language');
84+
$language = $idTokenPayload->{$languageAttribute} ?? null;
85+
7986
$genderAttribute = $this->providerService->getSetting($providerId, ProviderService::SETTING_MAPPING_GENDER, 'gender');
8087
$gender = $idTokenPayload->{$genderAttribute} ?? null;
8188

@@ -151,7 +158,11 @@ public function provisionUser(string $tokenUserId, int $providerId, object $idTo
151158
}
152159

153160
$account = $this->accountManager->getAccount($user);
154-
$scope = 'v2-local';
161+
$fallbackScope = 'v2-local';
162+
$defaultScopes = array_merge(
163+
AccountManager::DEFAULT_SCOPES,
164+
$this->config->getSystemValue('account_manager.default_property_scope', []) ?? []
165+
);
155166

156167
// Update displayname
157168
if (isset($userName)) {
@@ -227,7 +238,20 @@ public function provisionUser(string $tokenUserId, int $providerId, object $idTo
227238
$this->eventDispatcher->dispatchTyped($event);
228239
$this->logger->debug('Phone mapping event dispatched');
229240
if ($event->hasValue()) {
230-
$account->setProperty('phone', $event->getValue(), $scope, '1', '');
241+
$account->setProperty('phone', $event->getValue(), $defaultScopes[IAccountManager::PROPERTY_PHONE] ?? $fallbackScope, '1', '');
242+
}
243+
244+
$event = new AttributeMappedEvent(ProviderService::SETTING_MAPPING_LANGUAGE, $idTokenPayload, $language);
245+
$this->eventDispatcher->dispatchTyped($event);
246+
$this->logger->debug('Language mapping event dispatched');
247+
if ($event->hasValue()) {
248+
$language = $event->getValue();
249+
$languagesCodes = $this->l10nFactory->findAvailableLanguages();
250+
if (in_array($language, $languagesCodes, true) || $language === 'en') {
251+
$this->config->setUserValue($user->getUID(), 'core', 'lang', $language);
252+
} else {
253+
$this->logger->debug('Invalid language in ID token', ['language' => $language]);
254+
}
231255
}
232256

233257
$addressParts = null;
@@ -266,15 +290,15 @@ public function provisionUser(string $tokenUserId, int $providerId, object $idTo
266290
$this->eventDispatcher->dispatchTyped($event);
267291
$this->logger->debug('Address mapping event dispatched');
268292
if ($event->hasValue() && $event->getValue() !== null && $event->getValue() !== '') {
269-
$account->setProperty('address', $event->getValue(), $scope, '1', '');
293+
$account->setProperty('address', $event->getValue(), $defaultScopes[IAccountManager::PROPERTY_ADDRESS] ?? $fallbackScope, '1', '');
270294
}
271295

272296
// Update the website
273297
$event = new AttributeMappedEvent(ProviderService::SETTING_MAPPING_WEBSITE, $idTokenPayload, $website);
274298
$this->eventDispatcher->dispatchTyped($event);
275299
$this->logger->debug('Website mapping event dispatched');
276300
if ($event->hasValue() && $event->getValue() !== null && $event->getValue() !== '') {
277-
$account->setProperty('website', $event->getValue(), $scope, '1', '');
301+
$account->setProperty('website', $event->getValue(), $defaultScopes[IAccountManager::PROPERTY_WEBSITE] ?? $fallbackScope, '1', '');
278302
}
279303

280304
// Update the avatar
@@ -290,23 +314,23 @@ public function provisionUser(string $tokenUserId, int $providerId, object $idTo
290314
$this->eventDispatcher->dispatchTyped($event);
291315
$this->logger->debug('Twitter mapping event dispatched');
292316
if ($event->hasValue() && $event->getValue() !== null && $event->getValue() !== '') {
293-
$account->setProperty('twitter', $event->getValue(), $scope, '1', '');
317+
$account->setProperty('twitter', $event->getValue(), $defaultScopes[IAccountManager::PROPERTY_TWITTER] ?? $fallbackScope, '1', '');
294318
}
295319

296320
// Update fediverse
297321
$event = new AttributeMappedEvent(ProviderService::SETTING_MAPPING_FEDIVERSE, $idTokenPayload, $fediverse);
298322
$this->eventDispatcher->dispatchTyped($event);
299323
$this->logger->debug('Fediverse mapping event dispatched');
300324
if ($event->hasValue() && $event->getValue() !== null && $event->getValue() !== '') {
301-
$account->setProperty('fediverse', $event->getValue(), $scope, '1', '');
325+
$account->setProperty('fediverse', $event->getValue(), $defaultScopes[IAccountManager::PROPERTY_FEDIVERSE] ?? $fallbackScope, '1', '');
302326
}
303327

304328
// Update the organisation
305329
$event = new AttributeMappedEvent(ProviderService::SETTING_MAPPING_ORGANISATION, $idTokenPayload, $organisation);
306330
$this->eventDispatcher->dispatchTyped($event);
307331
$this->logger->debug('Organisation mapping event dispatched');
308332
if ($event->hasValue() && $event->getValue() !== null && $event->getValue() !== '') {
309-
$account->setProperty('organisation', $event->getValue(), $scope, '1', '');
333+
$account->setProperty('organisation', $event->getValue(), $defaultScopes[IAccountManager::PROPERTY_ORGANISATION] ?? $fallbackScope, '1', '');
310334
}
311335

312336
// Update role
@@ -322,28 +346,46 @@ public function provisionUser(string $tokenUserId, int $providerId, object $idTo
322346
$this->eventDispatcher->dispatchTyped($event);
323347
$this->logger->debug('Headline mapping event dispatched');
324348
if ($event->hasValue() && $event->getValue() !== null && $event->getValue() !== '') {
325-
$account->setProperty('headline', $event->getValue(), $scope, '1', '');
349+
$account->setProperty('headline', $event->getValue(), $defaultScopes[IAccountManager::PROPERTY_HEADLINE] ?? $fallbackScope, '1', '');
326350
}
327351

328352
// Update the biography
329353
$event = new AttributeMappedEvent(ProviderService::SETTING_MAPPING_BIOGRAPHY, $idTokenPayload, $biography);
330354
$this->eventDispatcher->dispatchTyped($event);
331355
$this->logger->debug('Biography mapping event dispatched');
332356
if ($event->hasValue() && $event->getValue() !== null && $event->getValue() !== '') {
333-
$account->setProperty('biography', $event->getValue(), $scope, '1', '');
357+
$account->setProperty('biography', $event->getValue(), $defaultScopes[IAccountManager::PROPERTY_BIOGRAPHY] ?? $fallbackScope, '1', '');
334358
}
335359

336360
// Update the gender
361+
// Since until now there is no default for property for gender we have to use default
362+
// In v31 there will be introduced PRONOUNS, which could be of better use
337363
$event = new AttributeMappedEvent(ProviderService::SETTING_MAPPING_GENDER, $idTokenPayload, $gender);
338364
$this->eventDispatcher->dispatchTyped($event);
339365
$this->logger->debug('Gender mapping event dispatched');
340366
if ($event->hasValue() && $event->getValue() !== null && $event->getValue() !== '') {
341-
$account->setProperty('gender', $event->getValue(), $scope, '1', '');
367+
$account->setProperty('gender', $event->getValue(), $fallbackScope, '1', '');
342368
}
343369

344370
$this->session->set('user_oidc.oidcUserData', $oidcGssUserData);
345371

346-
$this->accountManager->updateAccount($account);
372+
while (true) {
373+
try {
374+
$this->accountManager->updateAccount($account);
375+
break;
376+
} catch (InvalidArgumentException $e) {
377+
// If the message is a property name, then this was throws because of an invalid property value
378+
if (in_array($e->getMessage(), IAccountManager::ALLOWED_PROPERTIES)) {
379+
$property = $account->getProperty($e->getMessage());
380+
// Remove the property from account
381+
$account->setProperty($property->getName(), '', $property->getScope(), IAccountManager::NOT_VERIFIED);
382+
$this->logger->info('Invalid account property provisioned', ['account' => $user->getUID(), 'property' => $property->getName()]);
383+
continue;
384+
}
385+
// unrelated error - rethrow
386+
throw $e;
387+
}
388+
}
347389
return $user;
348390
}
349391

0 commit comments

Comments
 (0)