OIDC name for gitlab and authelia

[fflorent]

Context

1. Nor Authelia (see OIDC use username instead of display name for Grists name field #739) or Gitlab do provide given_name or family_name attributes in userinfo
2. Gitlab does not provide an end_session_endpoint, which led to an ugly error when disconnecting

Solutions

1. I propose to use display_name or name as alternative attributes for the given_name + family_name pair;
2. I propose to just redirect to the logout page directly, the user won't be logged out in the IdP;

[dsagal]

Would it be important to fall back to a string (and not, say, undefined)? If none of these options are available, one more reasonable fallback would be email.split("@", 1)[0].

[fflorent]

Isn't it already handled? If I don't provide any name, the left part of the email seem to be used. The magic seems to be operated here:

grist-core/app/gen-server/lib/HomeDBManager.ts

Lines 690 to 696 in 37dc1f8

	if (!user.name) {
	// Set the user's name if our provider knows it. Otherwise use their username
	// from email, for lack of something better. If we don't have a profile at this
	// time, then leave the name blank in the hopes of learning it when the user logs in.
	user.name = (profile && (profile.name || email.split('@')[0])) || '';
	needUpdate = true;
	}

[fflorent]

@dsagal Thanks for your review! I answered to your comments :)

I also changed a bit of strategy, I allow customizing the administrator the user properties to read in userInfo for both name and email. Some other product seem to offer this ability (Hedgedoc through CMD_OAUTH2_USER_PROFILE_DISPLAY_NAME_ATTR or Outline through OIDC_USERNAME_CLAIM)

[dsagal]

With defaultValue, you should be able to change to requireString, and the result would have type string (so you wouldn't need the ? in declaration, or ! in usage).

[dsagal]

Same here: String(userInfo[this._namePropertyKey]) would be safer (in case the actual value happens to be of some other type).

[fflorent]

I rather propose userInfo[ this._namePropertyKey as any ]?.toString() so if the value is undefined/null, it is correctly handled here:

grist-core/app/gen-server/lib/HomeDBManager.ts

Lines 690 to 696 in 37dc1f8

	if (!user.name) {
	// Set the user's name if our provider knows it. Otherwise use their username
	// from email, for lack of something better. If we don't have a profile at this
	// time, then leave the name blank in the hopes of learning it when the user logs in.
	user.name = (profile && (profile.name || email.split('@')[0])) || '';
	needUpdate = true;
	}

[fflorent]

Thanks again for your review @dsagal, I have taken them into account!

[dsagal]

Thanks! One more request please, the rest looks good.

[dsagal]

Thank you!

[dsagal]

@fflorent , there seems to be a merge conflict because of your other change to OIDC. Could you rebase?

[fflorent]

@dsagal @georgegevoian Done :)