Skip to content

OIDC: Support picture claim for use as user avatar #4271

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Ghost-chu opened this issue May 26, 2023 · 5 comments · Fixed by #5429
Closed

OIDC: Support picture claim for use as user avatar #4271

Ghost-chu opened this issue May 26, 2023 · 5 comments · Fixed by #5429

Comments

@Ghost-chu
Copy link

Describe the Bug

Although the OIDC response contains the picture field, the Bookstack still use default user avatar.

{
  "sub": "<censored>",
  "iss": "<censored>",
  "aud": "7acd8e81792f80dc48e9",
  "preferred_username": "<censored>",
  "name": "<censored>",
  "email": "<censored>",
  "picture": "https://cdn.<censored>/casdoor/avatar/<censored>/Ghost_chu.png?t=1685018195637388715"
}

Steps to Reproduce

  1. Setup the OIDC for Bookstack
  2. Create a new user and upload a avatar from your OIDC provider management
  3. Login to Bookstack
  4. Bookstack use default blue avatar as new user default avatar

Expected Behaviour

Bookstack should use the avatar from OIDC response instead the default avatar

Screenshots or Additional Context

No response

Browser Details

Brave 1.51.118 Chromium: 113.0.5672.126(Release) (64 bit)

Exact BookStack Version

v23.05.2

PHP Version

No response

Hosting Environment

debian-11.7 - Bookstack Docker Image by LinuxServer

      - AUTH_METHOD=oidc
      - AUTH_AUTO_INITIATE=true
      - OIDC_NAME=<censored>
      - OIDC_DISPLAY_NAME_CLAIMS=name
      - OIDC_CLIENT_ID=<censored>
      - OIDC_CLIENT_SECRET=<censored>
      - OIDC_ISSUER=<censored>
      - OIDC_ISSUER_DISCOVER=true
@ssddanbrown ssddanbrown changed the title The OIDC authencation doesn't use picture field as user avatar OIDC: Support picture claim for use as user avatar May 26, 2023
@ssddanbrown
Copy link
Member

Thanks for raising, but I have recategorised this as a feature request, and updated the title to suit, since this is not a break in existing logic. We've just never specifically supported user avatars via the picture claim.

@cal940
Copy link

cal940 commented Feb 3, 2024

hello, is there any new progress on this issue?

would be nice to see this feature in the following releases.

@ssddanbrown ssddanbrown mentioned this issue Jun 12, 2024
1 task
@jasonpincin
Copy link

Plus one on this one, fwiw.

@rubentalstra
Copy link

rubentalstra commented Jan 20, 2025

I’ve opened a merge request (#5429) adding optional support for fetching user avatars from the OIDC picture claim. It reuses our existing avatar logic so there’s no major code duplication. A new oidc.fetch_avatars config option must be enabled for this feature to take effect. Feedback and testing are welcome!

@ssddanbrown
Copy link
Member

This has now been added via #5429 and #5626.
Thanks to @rubentalstra for providing an implementation for this.
Thanks @Ghost-chu for the original request.
This will be part of the next feature release.

It will be disabled by default, but enabled with a .env option.
Just to confirm though, this does not assure it will support all auth providers. The exact details of using the picture claim are not too detailed in the spec, and it looks like auth providers like to do awkward things.
BookStack will fetch an image at the picture claim, following up to 3 GET redirects, and the image provided will need to be one of BookStack's accepted image formats (png, webp, avif, gif, bmp).
Any platforms acting outside of that (For example Azure which seems to need credentials) will be outside the scope of what we support, at least in this revision, although it may be possible to use the logical theme system to implement custom workarounds.
Also, this will fetch and assign the avatar image at every login, where the user does not have an avatar already assigned (either manually configured in platform or via a prior fetch). This aligns with our logic for LDAP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

5 participants