Repository: directus/directus
File: api/src/auth/drivers/openid.ts (TypeScript)

Code before:

		const userPayload = {
			provider,
			first_name: userInfo['given_name'],
			last_name: userInfo['family_name'],
			email: email,
			external_identifier: identifier,
			role: role,
			auth_data: tokenSet.refresh_token && JSON.stringify({ refreshToken: tokenSet.refresh_token }),
		};

		const userId = await this.fetchUserId(identifier);

		if (userId) {
			// Run hook so the end user has the chance to augment the
			// user that is about to be updated
			let emitPayload: Record<string, unknown> = {
				auth_data: userPayload.auth_data,
				// Make sure a user's role gets updated if his openid group or role mapping changes
				role: role,
			};

			if (syncUserInfo) {
				emitPayload = {
					...emitPayload,
					first_name: userPayload.first_name,
					last_name: userPayload.last_name,
					email: userPayload.email,
				};
			}

			const updatedUserPayload = await emitter.emitFilter(
				`auth.update`,
				emitPayload,
				{
					identifier,
					provider: this.config['provider'],
					providerPayload: { accessToken: tokenSet.access_token, idToken: tokenSet.id_token, userInfo },
				},
				{ database: getDatabase(), schema: this.schema, accountability: null },
			);

			// Update user to update refresh_token and other properties that might have changed
			if (Object.values(updatedUserPayload).some((value) => value !== undefined)) {
				await this.usersService.updateOne(userId, updatedUserPayload);
			}

			return userId;
		}

		const isEmailVerified = !requireVerifiedEmail || userInfo['email_verified'];

Review comment (suggestion):
Here's a potentially tricky question:

What would one expect to happen when you manually change somebody's role in Directus, but then the OpenID role overrides that again back to what is configured in the env? 🤔 Is there a case where you'd use the env var as a "first grouping" with the intention to later change the role in Directus itself outside of the OpenID mapping?

I _think_ I'd personally expect the OID mapping to be the source of truth, but that's something we should make very clear in the docs if that's the case 🤔 

Code after:
		const identifier = userInfo[identifierKey ?? 'sub'] ? String(userInfo[identifierKey ?? 'sub']) : email;

		if (!identifier) {
			logger.warn(`[OpenID] Failed to find user identifier for provider "${provider}"`);
			throw new InvalidCredentialsError();
		}

		const userPayload = {
			provider,
			first_name: userInfo['given_name'],
			last_name: userInfo['family_name'],
			email: email,
			external_identifier: identifier,
			role: role,
			auth_data: tokenSet.refresh_token && JSON.stringify({ refreshToken: tokenSet.refresh_token }),
		};

		const userId = await this.fetchUserId(identifier);

		if (userId) {
			// Run hook so the end user has the chance to augment the
			// user that is about to be updated
			let emitPayload: Record<string, unknown> = {
				auth_data: userPayload.auth_data,
				// Make sure a user's role gets updated if his openid group or role mapping changes
				role: role,
			};

			if (syncUserInfo) {
				emitPayload = {
					...emitPayload,
					first_name: userPayload.first_name,
					last_name: userPayload.last_name,
					email: userPayload.email,
				};
			}

			const updatedUserPayload = await emitter.emitFilter(
				`auth.update`,
				emitPayload,
				{
					identifier,
					provider: this.config['provider'],
					providerPayload: { accessToken: tokenSet.access_token, idToken: tokenSet.id_token, userInfo },
				},
				{ database: getDatabase(), schema: this.schema, accountability: null },
			);

			// Update user to update refresh_token and other properties that might have changed
			if (Object.values(updatedUserPayload).some((value) => value !== undefined)) {
				await this.usersService.updateOne(userId, updatedUserPayload);