Problem/Motivation

Currently we cannot set the email, as we're not guaranteed to have an email attribute with a value in our SAML responses. This can potentially be problematic, as that would mean multiple users would end up with a email value being null.

I think a workaround here would be to construct an "fake" email based on some other data we do have, but to do that we need some entrypoint to be able to modify the attributes data before SamlService runs the login logic.

Steps to reproduce

Proposed resolution

Comments

johnwebdev created an issue. See original summary.

roderik’s picture

You should be able to use an EventSubscriber that subscribes to SamlauthEvents::USER_SYNC events, for things like this. See UserSyncEventSubscriber for an example.

(I'm using the word "example" lightly because you'll want to completely re-do onUserSync() (and the constructor/class variables) in your own copy... but it gets you into the direction. I think what you really want to do is not "change the attributes" but if ( ? ) $event->getAccount()->setEmail( ? );

roderik’s picture

Hm. I see the issue: UserSyncEventSubscriber will throw an exception if the email isn't set in the attribute.

I think (if I understand your use case correctly and this is an issue you are looking to solve for new users only) what should happen is

1) you make your event subscriber and register it with a higher priority as UserSyncEventSubscriber, e.g. (if I'm not mixing up numbers) 75:

classs yourSubscriber
  public static function getSubscribedEvents() {
    $events[SamlauthEvents::USER_SYNC][] = ['onUserSync', 75];
    return $events;
  }

...and set the email in there for new accounts

2) I should fix UserSyncEventSubscriber line 205:

- elseif ($account->isNew()) {
+ elseif ($account->isNew() && !$account->getEmail()) {

Can you test that?

johnwebdev’s picture

Category: Feature request » Support request
Status: Active » Fixed

Just implementing the subscriber seems to work. Thanks!

  • roderik committed 7664941 on 8.x-3.x
    Issue #3216515 by roderik: enable populating new account e-mail address...
roderik’s picture

That's slightly surprising to me because it still feels like the UserSyncEventSubscriber should have been fixed. So I just pushed the change anyway.

(I'm a bit late; had the commit locally for a few days.)

  • roderik committed 7664941 on 4.x
    Issue #3216515 by roderik: enable populating new account e-mail address...

Status: Fixed » Closed (fixed)

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