I'm in the habit of removing unnecessary fields from Drupal entities, and this usually includes the user picture field from the User entity. However doing that breaks Discourse SSO with the error:

NOTICE: PHP message: Error: Call to a member function isEmpty() on null in modules/contrib/discourse_sso/src/Controller/DiscourseSsoController.php on line 59

which refers to the code:

    $picture = (!$account->user_picture->isEmpty())
             ? $account->get('user_picture')->entity->url()
             : '';

I've made a quick patch to fix the issue by checking if $account->user_picture is set before checking if it is empty:

    $picture = (isset($account->user_picture) && !$account->user_picture->isEmpty())
             ? $account->get('user_picture')->entity->url()
             : '';
CommentFileSizeAuthor
#11 3019548-11.discourse_sso.SSO-breaks-if-User-Picture-field-is-removed-from-User-entity.patch699 bytesugolek
#7 3019548-6.discourse_sso.SSO-breaks-if-User-Picture-field-is-removed-from-User-entity.patch606 bytesjurgenhaas
#4 interdiff.3019548.4.txt638 bytesjurgenhaas
#4 3019548-4.discourse_sso.SSO-breaks-if-User-Picture-field-is-removed-from-User-entity.patch605 bytesjurgenhaas
discourse_sso_no_user_picture_field.patch626 bytesscott.whittaker

Comments

scott.whittaker created an issue. See original summary.

Pls’s picture

Pls commented

Hey Scott,

Nice patch. Just tested and it works perfectly! Cheers!

Pls’s picture

Pls commented
Status: Active » Reviewed & tested by the community
jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 3019548-4.discourse_sso.SSO-breaks-if-User-Picture-field-is-removed-from-User-entity.patch605 bytes
new interdiff.3019548.4.txt638 bytes

We have got the same issue and the patch does not solve the problem, we're getting this:

LogicException: The controller result claims to be providing relevant cache metadata, but leaked metadata was detected. Please ensure you are not rendering content too early. Returned object class: Drupal\Core\Routing\TrustedRedirectResponse. in Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (line 154 of /mnt/storage/jails/live/var/www/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php).

Reworked the patch and would be glad to get it reviewed.

  • boromino committed b46b3cf on 8.x-1.x authored by jurgenhaas 
    Issue #3019548 by jurgenhaas, scott.whittaker: SSO breaks if User...

jurgenhaas credited boromino.

jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
commented
StatusFileSize
new 3019548-6.discourse_sso.SSO-breaks-if-User-Picture-field-is-removed-from-User-entity.patch606 bytes

Sorry, the condition needs to be negated.

  • boromino committed 5464946 on 8.x-1.x authored by jurgenhaas 
    Issue #3019548 by jurgenhaas, scott.whittaker, boromino: SSO breaks if...

  • jurgenhaas committed fc1ea4f on 8.x-1.x 
    Issue #3019548 by jurgenhaas, scott.whittaker, boromino: Replace call to...
jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
commented

There has been another issue in the latest code. Calling url() on an entity is running into an early render scenario because generating urls needs to be aware of context (e.g. language) which may use caching. To avoid this we need to call toUrl()-toString(TRUE) instead which by the same time also avoids the usage of long deprecated functions.

ugolek’s picture

ugolek commented
StatusFileSize
new 3019548-11.discourse_sso.SSO-breaks-if-User-Picture-field-is-removed-from-User-entity.patch699 bytes

I think the better to check if $account has field user_picture before trying to get the field value. I have attached updated patch.

jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
commented
Assigned: Unassigned » jurgenhaas
Issue tags: +Drupal 9 porting weekend

  • jurgenhaas committed 13ddd26 on 2.0.x 
    Issue #3019548 by jurgenhaas, scott.whittaker, lebster, boromino: SSO...
jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
commented
Version: 8.x-1.x-dev » 2.0.x-dev
Status: Needs review » Fixed

Slightly changed the patch from @lebster and committed to latest dev release.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.