Repository navigation
feat(auth): sync avatars from OIDC/OAuth providers - #11135
Open
MichaelUray wants to merge 1 commit into
Open
MichaelUray wants to merge 1 commit into
MichaelUray wants to merge 1 commit into
Conversation
The OIDC, Google and GitHub login handlers never read the avatar the identity provider returns (the OIDC picture claim, the passport profile photos), and the global person in the account database had no place to keep it, so workspace persons were always created with a plain color avatar. The provider handler now extracts the avatar URL from the profile and passes it to loginOrSignUpWithProvider/joinWithProvider, which store it on the global person (new nullable person.avatar_url column, account DB migration v28). Only absolute http(s) URLs without credentials and of reasonable length are accepted (normalizeExternalAvatarUrl in core); the URL is never fetched by the server. When a user connects to a workspace, ensureEmployee uses that URL as an external avatar for a newly created person, and refreshes it for an existing person whose avatar is still the default color avatar or a previously synced external one. Uploaded images, Gravatar and an explicitly chosen color are never overwritten. External avatars did not render at all so far: getAvatarProviderId() mapped AvatarType.EXTERNAL to the Image provider, which only reads the stored blob, and the model registered the External provider under the Color provider id. EXTERNAL now has its own provider id (contact.avatarProvider.External) that resolves to GetExternalUrl, which also makes existing GitHub-synced avatars visible. Push notifications use the external URL as sender icon as well. Stored external URLs are re-checked when rendered (getExternalAvatarUrl), so only http(s) URLs are ever used as an image source. The provider avatar URL may contain signed query parameters, so it is not part of the profile returned by getUserProfile; only the person's own client reads it through getPerson to sync the workspace avatar. Fixes hcengineering#8120 Fixes hcengineering#9026 Signed-off-by: Michael Uray <michaeluray@users.noreply.github.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8120
Fixes #9026
Problem and change
The OIDC, Google and GitHub login handlers never read the avatar the identity
provider returns (the OIDC picture claim, the passport profile photos), and the
global person in the account database had no place to keep it, so workspace
persons were always created with a plain color avatar.
The provider handler now extracts the avatar URL from the profile and passes it
to loginOrSignUpWithProvider/joinWithProvider, which store it on the global
person (new nullable person.avatar_url column, account DB migration v28). Only
absolute http(s) URLs without credentials and of reasonable length are
accepted (normalizeExternalAvatarUrl in core); the URL is never fetched by the
server.
When a user connects to a workspace, ensureEmployee uses that URL as an
external avatar for a newly created person, and refreshes it for an existing
person whose avatar is still the default color avatar or a previously synced
external one. Uploaded images, Gravatar and an explicitly chosen color are
never overwritten.
External avatars did not render at all so far: getAvatarProviderId() mapped
AvatarType.EXTERNAL to the Image provider, which only reads the stored blob,
and the model registered the External provider under the Color provider id.
EXTERNAL now has its own provider id (contact.avatarProvider.External) that
resolves to GetExternalUrl, which also makes existing GitHub-synced avatars
visible. Push notifications use the external URL as sender icon as well.
Stored external URLs are re-checked when rendered (getExternalAvatarUrl), so
only http(s) URLs are ever used as an image source.
The provider avatar URL may contain signed query parameters, so it is not part
of the profile returned by getUserProfile; only the person's own client reads
it through getPerson to sync the workspace avatar.
Tests
New unit tests in
foundations/core/packages/core/src/__tests__/externalAvatarUrl.test.ts,pods/authProviders/src/__tests__/providerAvatar.test.ts,plugins/contact/src/__tests__/providerAvatar.test.ts,server/account/src/__tests__/utils.test.tsandserver/account/src/__tests__/operations.test.ts. Checked on a self-hosted instance: a person with an external avatar URL shows that image, a non-http(s) URL falls back to the color avatar, existing avatars render as before. A full OIDC round trip with apictureclaim still deserves a check with a real identity provider.Notes
account_db_v28_add_person_avatar_urladds a nullableperson.avatar_urlcolumn (Postgres/CockroachDB); Mongo needs no migration.getUserProfile; only the person's own client reads it viagetPerson.