Switch LDAP avatar to store raw JPEG instead of base64-encoded string #157

New Issue

raucao · 2024-01-22T10:58:40Z

raucao commented

2024-01-22 10:58:40 +00:00

I'm not sure where we found the information for it being generally stored as base64 string, but I just found out that Gitea only supports the binary format, and the RFC for inetOrgPerson also doesn't mention encoded strings.

Since I only just deployed the version with base64 today, I'll change it asap so we don't announce before it works with Gitea and ejabberd at least.

I'm not sure where we found the information for it being generally stored as base64 string, but I just found out that Gitea only supports the binary format, and the [RFC for inetOrgPerson](https://www.rfc-editor.org/rfc/rfc2798#section-2.6) also doesn't mention encoded strings. Since I only just deployed the version with base64 today, I'll change it asap so we don't announce before it works with Gitea and ejabberd at least.

raucao added the

bug

enhancement

labels 2024-01-22 10:58:45 +00:00

raucao self-assigned this 2024-01-22 10:58:55 +00:00

raucao changed title from ~~Switch LDAP avatar to store raw JPEG/PNG instead of base64-encoded string~~ to Switch LDAP avatar to store raw JPEG instead of base64-encoded string

2024-01-22 10:59:14 +00:00

raucao commented

2024-01-22 17:04:09 +00:00

This one turns out to be more difficult than I expected. Could use another set of eyes to look at what I have, which isn't working as it should...

raucao added this to the 1.0.0 Release milestone 2024-02-08 12:21:14 +00:00

raucao added the

integration

ldap

label 2024-02-25 11:23:43 +00:00

raucao referenced this issue from a commit

2024-03-01 10:14:32 +00:00

Hide avatar settings behind feature flag

raucao commented

2024-03-07 13:20:26 +00:00

I have temporarily hidden the feature by default, so we don't have to care about migrating existing avatars at some point.

raucao commented

2025-05-11 10:00:56 +00:00

I have a working implementation of this in 9e2210c now, but now I'm not sure it makes sense anymore. There are a few issues with the idea of storing this in LDAP in general, which I'll document here for posterity:

If we're strict about it, the file type would have to be a JPEG, which means when converting it, we lose e.g. PNG transparency for images that use it.
We can't store multiple versions (e.g. sizes) this way
A public HTTP endpoint would rely on LDAP as backing file store, but it would be more efficient and robust to just use our public S3 endpoints directly (which also have proxy caching built in already)
Not all services are able to use the avatar from LDAP directly, so if we want to have options for updating an avatar for multiple services, we need to store it in other places anyway. And we should probably make it possible to not overwrite all avatars in all places via UI options
I'm not sure how widely supported this is, but for XMPP, the protocol makes it possible to also use HTTP URIs instead of image data when publishing avatar updates to PEP

So, considering all these points, I think it makes more sense to just use ActiveStorage to store avatars in S3/Garage (or filesystem for smaller akkounts deployments), and then have multiple jobs to update them in services where needed or desired. One of them can be the LDAP one that from abovementioned commit, so it's possible to use it from there also.

I have a working implementation of this in 9e2210c now, but now I'm not sure it makes sense anymore. There are a few issues with the idea of storing this in LDAP in general, which I'll document here for posterity: * If we're strict about it, the file type would have to be a JPEG, which means when converting it, we lose e.g. PNG transparency for images that use it. * We can't store multiple versions (e.g. sizes) this way * A public HTTP endpoint would rely on LDAP as backing file store, but it would be more efficient and robust to just use our public S3 endpoints directly (which also have proxy caching built in already) * Not all services are able to use the avatar from LDAP directly, so if we want to have options for updating an avatar for multiple services, we need to store it in other places anyway. And we should probably make it possible to not overwrite all avatars in all places via UI options * I'm not sure how widely supported this is, but for XMPP, [the protocol](https://xmpp.org/extensions/xep-0084.html#examples) makes it possible to also use HTTP URIs instead of image data when publishing avatar updates to PEP So, considering all these points, I think it makes more sense to just use ActiveStorage to store avatars in S3/Garage (or filesystem for smaller akkounts deployments), and then have multiple jobs to update them in services where needed or desired. One of them can be the LDAP one that from abovementioned commit, so it's possible to use it from there also.

raucao referenced a pull request that will close this issue

2025-05-11 14:44:56 +00:00

User avatars #223

raucao closed this issue

2025-05-14 14:58:15 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: kosmos/akkounts#157