undefined - Contrib.social

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

Sounds plausible. It's kind-of the opposite of old issue #2848809: Login/logout fails with non-default language being active with the URL prefix negotiation → though.

I'll need to read it carefully again, and check that the proposed change doesn't reintroduce any bug / unwanted behavior.

It may well be possible that path_processing should be TRUE for /saml/login and /saml/logout, while staying FALSE for the other /saml/* routes. (In other words: that that old issue changed too much, without carefully thinking about this situation.)

📌 | SAML Authentication | Handle invalid destination parameters better

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

📌 | SAML Authentication | Handle invalid destination parameters better

💬 | SAML Authentication | Passing $return_to parameter to SamlService->login() doesn't redirect as expected

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

(1AM typo in commit message. Commit is here.)
.
Thanks for the report. Yes, this is a bug... which I never consciously registered.

There's supposedly-always an exception handler active, which takes care of returning a (redirect) response to an error page. But the 'destination' parameter should always be removed in that case. In other words: the 'destination' wasn't removed early enough.

This makes DOMAIN/saml/login?destination=/:88/favicon.ico at DOMAIN/:88/favicon.ico, instead of the error page... which is a bug.

Bug fixed by removing the 'destination' properly/earlier. "The error page" by default is the homepage (with the standard message saying there was an error which has been logged). If that isn't good for your specific case, you can set an "Error redirect URL" in config.

(I briefly considered (additionally) checking/removing invalid 'destination' values, but that seems to be the RequestSanitizer::checkDestination()'s job, so I won't duplicate that.)

📌 | Asymmetric (private/public) Keys | Automated Drupal 11 compatibility fixes for key_asymmetric

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

My previous comment was quite wrong.

The module is indeed, since v2.0.0-alpha2, unable to deal with relative paths as $return_to - because I introduced a UrlHelper::isValid(... TRUE) call. There is no documentation that says why, and the documentation of the $return_to parameter (which existed in v1, but I think it never worked at all) has not been updated to reflect this.

I can't remember whether or why I assumed that the RelayState should be an absolute URL. (According to the internet, the SAML spec says) it's an opaque value.

Because I don't like the fact that $return_to value worked differently than the "Redirect URL" config settings, I added the ability to be 'just a path' in addition to a URL. Some testing suggests this should be fine.

(Oh, if only we had functional tests for the actual login-IdP-ACS-redirect path...)

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

You're right. This shows how low-maintained this module is. 1.2.0 released now.

✨ | SAML Authentication | Suport mapping multi-value fields

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

I've added a nots to the bottom of the project README.

💬 | SAML Authentication | Login not possible if user has authenticated with another system.

💬 | SAML Authentication | Possible to check for IdP authentication status and retrieve user attributes?

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

Thank you for the report. This feedback (confirms my thoughts from the previous comment and) makes me think about necessary next steps.

I'll close this issue though, because (though the last comment does correctly answer my previous comment) it's not clear from the initial issue description, what should be done here.

I've added a section to the bottom of the README that describes the issue and current situation / possible followup. If anyone does want to implement the followup, that wlll likely be better off as a new issue.

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

I'm setting this to postponed:

Still a good idea, and my last large comment is still relevant if anyone ever wants to implement this.

But I'm not going to experiment with it myself, so I also won't add things to the README myself yet. (People who are adventurous enough to look into this, can find this issue.)

Also... if I ever get to do the externalauth rewrite that I'm dreaming about... it would actually be nice to have support for "not logging a user in, but only starting a session" in externalauth. That makes the things to do in samlauth (per my last large comment) easier and less hacky.

✨ | SAML Authentication | Suport mapping multi-value fields

💬 | SAML Authentication | "Error encountered during processing SAML single-logout response" with Microsoft ADFS, Azure AD, Entra IdP

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

Unfortunately, `UserFieldsEventSubscriber` has proven to be obtuse and IMHO perhaps overengineered. So I'll most likely be writing a one-off data sync using the SamlauthEvents::USER_SYNC event for my purposes.

That's fair. If there are any others who have needed this multi-value functionality, they have apparently also done that.

This is true, but isn't it possible to know if the destination supports multiple values?

Very possibly yes. With some hardcoding for field types probably, but that's OK. That is why I said [anyone is] welcome to contribute or sponsor adding this functionality.

So this issue stays open for that.

Despite the "obtuse and perhaps overengineered" comment, I do think it is clear enough where to add code for this.

💬 | SAML Authentication | "Error encountered during processing SAML single-logout response" with Microsoft ADFS, Azure AD, Entra IdP

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

Thank you! This kind of 'maintenance from users' is exactly what this module needs.

(Per the giant comment above the code line that you changed: I think that actually, the option should not exist / should be "on with automatic fallback" in the PHP-SAML library, so that Microsoft IdPs work by default. But it seems I won't get to discussing that and also actually convincing the maintainers that this is true...)

Comment about 1 month ago →

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

roderik → made their first commit to this issue’s fork.

✨ | SAML Authentication | Suport mapping multi-value fields

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

Multi-value fields are easily supportable in principle, as you indicate; it just needs some thought and testing. And you are the first one to bring it up in all these years, so that thought/testing hasn't been invested yet.

The challenge for generic support is two fold:

1. On the 'source' side (the SAML message), we get all values as arrays, so there is no way to distinguish between a single value and an array containing a single value. (Feel free to set me straight, if e.g. the attribute data in your SAML responses is somehow marked as "an actual array".)

2. On the 'destination' side, not all fields actually support multi-value. (Case in point: user name, email, language.) And some may be configured/defined to be only single-value, even though the Drupal data model supports multiple. We don't want to just set a multi-value array into these fields, at least not without testing / logging a message.

You're welcome to contribute or sponsor adding this functionality. Even a only-partly-generic patch or extra subscriber helps get it into the general module in time. Failing that: it should be fairly easy to roll your own event subscriber with just a few lines of actual logic. Something like

$new = $event->getAttributes()[YOURATTRIBUTE];
$old = $event->getAccount()->get(YOURFIELD);
if ($new != $old) {
  $event->getAccount()->set(YOURFIELD, $new);
  $event->markAccountChanged();
}

I would also like clarity on why samlauth_user_fields does user linking as well as syncing.

I'm not sure whether you're talking about the boolean map_users config value (which didn't initially exist at all / had its reasons for being introduced, but I'm not sure that's relevant here). Bottom line is: linking is optional.samlauth_user_fields.field_mapping is a per-field mapping, and no field is linked unless you explicitly set the optional 'link_user_order' sub-value for the specific field.

(From just reviewing the config schema, I'm not sure from the top of my head if saving the samlauth_user_fields.field_mapping value always saves a 'link_user_order' with a default value of 0. It wasn't supposed to. If it does: that's a bug. If it doesn't: I don't see any issue with this.)

💬 | SAML Authentication | Passing $return_to parameter to SamlService->login() doesn't redirect as expected

Comment about 1 month ago →

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

This seems like an oversight / wrong documentation on my part.

A relative path as RelayState parameter (as a request parameter attached to the POST request that also contains the SAML response) should work fine. **

So I think your IdP strips it (i.e. it does not propagate the RelayState to that POST request) if it's a relative path. Maybe most/all IdPs do that. (I should doublecheck SAML docs.)

I guess part of the reason this has not been reported before, is that the destination parameter (if one visits /saml/login?desination=PATH) is converted to an absolute URL, and not many people pass an argument to the $return_to parameter.

I should at least amend the function parameter docs/comments for $return_to.

---

** Unnecessary detail -- I was checking / wrote the below down, before my conclusion above:

Is checked for 'being safe' in https://git.drupalcode.org/project/samlauth/-/blob/8.x-3.x/src/Controlle...
Then is checked for access in https://git.drupalcode.org/project/samlauth/-/blob/8.x-3.x/src/Controlle...
If the path is ignored in either case, there should be a watchdog warning.

I don't see any other code influencing the RelayState parameter.

💬 | SAML Authentication | Possible to check for IdP authentication status and retrieve user attributes?

💬 | SAML Authentication | Possible to check for IdP authentication status and retrieve user attributes?

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

I'm going to turn this into a reminder for me to, long-term, add something to the README and/or add a config option to skip user login (that might not be exposed in the UI).

It would be good to know if the current saveSamlSession() code is adequate for implementing such a use case.

💬 | SAML Authentication | Possible to check for IdP authentication status and retrieve user attributes?

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

We don't, however, map these users to individual Drupal accounts as there's no need (or desire) for the users to be registered locally

Oof! That makes things harder. I haven't heard yet, of a use case like this.

To summarize my previous response: samlauth intrinsically links "Idp authentication" to "Drupal user login". That's easier for the common use case.

But in your case,

you need to peel those things apart again
you likely need to store some (extra) status/data in a 'user session' that is separate from a logged-in Drupal user (which simpleSAMLphp did, and which samlauth doesn't do), in order to do this access check on those paths.

I think it's possible, though, to use just the "SAML authentication" part of this module and to remove the "Drupal user login" part, without making things too complicated. (I'm going to assume, for simplicity, that you never want to have a Drupal user login after someone returns from the IdP.)

You'll need to overwrite the samlauth.saml service with a custom service that extends SamlService
Your extending class can 'empty out' doLogin() (or change acs(), which is the only caller of doLogin(). But I assume leaving acs() is easier).
This is assuming that you don't want anything registered in the authmap table. (Also, I am not going into the horribble piece of spaghetti that is the doLogin() execution path... because you won't need it / will remove it.)
Then I advise experimenting with saveSamlSession():
- There is a concept of a "Saml session", separate from the Drupal login. But ss you can see from the horribly large comments, it's an afterthought / code that has just stuck around since v1.0 and not (to my knowledge) been used well.
- Decide what data you need, in an access check that you do on the other page loads.
- Then store that data somewhere you can get to it. (getAttributes() is available here if you need it)
- As you can see, currently the 'session data' is stored in PrivateTempStore (and you can just add your data next to it). I'm personally not convinced that this is better than just storing this data in $_SESSION -- because as soon as you store something in PrivateTempStore, you automatically/necessarily get a session anyway. I guess it depends on the nature of your data.

I.... think that works without further complications?

If you discover things that need to be patched along the way, I'm open to it. Although, until other people come along who want the same thing, I can imagine this is not going to turn into a configurable option to "skip user creation/login", but instead just a note in the README that this can be achieved with the above.

💬 | SAML Authentication | Possible to check for IdP authentication status and retrieve user attributes?

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

isAuthenticated() does not exist, because it addresses an issue that is specific to SimpleSAMLphp / does not exist with SamlAuth.

See the two images at https://www.drupal.org/docs/contributed-modules/saml-authentication/usin... → if necessary:

with SimpleSAMLphp, SAML auth happens outside of Drupal. Then the user is redirected to Drupal, with a cookie set -- and Drupal still needs to figure out whether the at-that-moment-anonymous user is allowed to be logged in. (using isAuthenticated(), which -from my memory- checks the cookie.)
with SamlAuth, SAML auth happens inside Drupal, during the POST request to /saml/acs. And the SamlAuth module immediately logs the user in, during the same POST request. Either the user is logged in, or the user is presented with a login error. There is no other 'vague in-between state' like with SimpleSAMLphp auth.

Is it possible that the array is empty due to the IdP configuration? Or, am I perhaps misunderstanding at which point the array is actually populated given the current state of the SAML authentication flow?

getAttributes() only returns data while processing the /saml/acs POST request, not on any other page load -- and only after SAML authentication has already happened and was successful.

Without knowing the specifics of what you are solving, I am guessing:

you're trying to do something during some event very early in the page load -- which is not a concept that works anymore.
- you should be doing the same thing immediately after successful SAML authentication. There's an event for that: SamlAuthEvents::USER_SYNC. See the comments there. See UserSyncEventSubscriber for an example implementation.
Note on this event, SAML authentication has already happened and your user ($event->getAccount()) is going to be logged in (except if you or some other event subscriber throws an exception or something horrible happens while saving the user account). So you don't need to do an isAuthenticated() check.

Or, alternatively, if you want to do something after authentication and Drupal user login has happened, and you cannot cancel it in any way anymore: check the ExternalAuthEvents::LOGIN event. SamlService->getAttributes() should work there always, if you need the attribute values (though the service isn't injected so it's slightly ugly).

This means you need to do some rewrite work, for this different concept. But the result will probably be (conceptually) simpler, I'm guessing.

✨ | Lupus Decoupled Drupal | Views - Add a CE block display

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

Mustapha did the tracing for this, and made a first change. I picked it up to check how exactly the change should be implemented (and to confuse myself on the way).

I've tried to improve the code so the block provides a proper CE response. When done so the block rendering should pick up the CE wrapped in the render-array. But it seems the views/block rendering pipeline wraps the render array somewhere, so it ends up markup instead of json still.

Yes, the block rendering does pick up the CE wrapped in some render array:

The call tree is:
1. LupusDecoupledBlockRenderer::getBlocks()
2. Drupal\views\Plugin\Block\ViewsBlock::build()
3. Drupal\views\Element\View::prerenderViewElement()
4. Drupal\views\ViewExecutable::executeDisplay()

4. returns the simple '#theme = custom_element' wrapper, and 2 + 3 add all the other stuff around it, to 'complete' the render array for a view.

(...) the block rendering should pick up the CE wrapped in the render-array.

I assume this means that you want to do a combination of two things:

A. pick the CE out of above ViewsBlock::build() structure;

B. set the CE instead of the rendered HTML, into $output[$region], in getBlocks(). (Until now, this is never done.)

About "B": OK, no further remarks.

re. "A": this feels to me like, at least in theory, 1) we could be throwing away part of the rendered output (produced by e.g. theme functions set by the block plugin; 2) we could be breaking compatibility by changing HTML output to something else. So I felt like I should put some thought into when exactly we are 'allowed' to do this:

If the CE is the only render-element-child, like this specific example? Sure. We can throw away the theme-wrapper function.
If there are several render-element-children, all CEs? Maybe, but...
...what if there are several render-element-children, some of which are not CEs? We can probably renderRoot() the non-CE render-element-children individually, and add them as drupal-markup CEs (like is mentioned in 📌 Improve CustomElement::createFromRenderArray() to better handle entites that render to custom elements Active )?
...and what if they're not all first-level children? Do we need to recurse, then?

I've also seen 📌 Improve CustomElement::createFromRenderArray() to better handle entites that render to custom elements Active , which proposes to do something like this inside CustomElement::createFromRenderArray(). But thinking about the above points just confused me.

So, in the end, I added only the first bullet point, directly into getBlocks(). That's what we need right now. And I can't justify the testing time for making this more generic / decide how 'backward compatible' the other cases should be... unless I'm explicitly told that we should address those cases now.

@fago see MR. Is this the code change you want? Or am I misinterpreting some things?

🐛 | Configuration Override Warn | Failing phpunit tests

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

Might be fixed by https://git.drupalcode.org/project/config_override_warn/-/merge_requests... ( 📌 Automated Drupal 11 compatibility fixes for config_override_warn Needs review ), specifically the extra installEntitySchema() call. So apparently that fix is not only for D11?

Note - I didn't check, just leaving a quick note.

💬 | SAML Authentication | Database error when new users try to log in

📌 | Configuration Override Warn | Automated Drupal 11 compatibility fixes for config_override_warn

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

Thank you for the detailed report.

First - the less important thing, but important for myself:

Per "2)" in my response #9: I still have no clue why you're getting this ugly error and there is not an earlier 'real' error message about the name being empty. But you confirmed your configuration, so I have something to manually reproduce and test, some time.

This issue is "Needs Work" for at least that point: reproducing and testing, and then seeing whether/where I should add a real error ("Your user name is not being populated from SAML data, your 'User name attribute' setting is likely misconfigured" or something like that).

---

Now, to the details of your setup:

You only sent the <saml2:NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified">correct@emailaddress.com</saml2:NameID> part of your response.

I am going to assume that your SAMLResponse contains no attributes at all. Attributes look something like (real example at https://www.samltool.com/generic_sso_res.php) :

      <saml:Attribute Name="uid">
        <saml:AttributeValue xsi:type="xs:string">test</saml:AttributeValue>
      </saml:Attribute>

Therefore I am also going to assume that you want to have Drupal users created whose name == the e-mail address == the NameID that you are getting back in the response.

If my assumption is correct, then this is a combination of
1. Your configuration is wrong
2. The module is not giving you any hints about how your configuration could be "fixed". So the UI is wrong/incomplete.

Per 1: If you indeed have no attributes in your response, then the configured "NameID" attribute (for user name) does not exist, and the configured "urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified" attribute (for email) does not exist... because no atrributes exist at all.

Note again that "The NameID" and "SAML Attributes" are completely separate things. And (I assume from your comment) you only get the NameID, and zero SAML attributes, in your response.

Per 2: this module only recently implemented support for "NameID" (as opposed to an atrribute) for its "Unique ID" setting.

I failed to, at the same time, adjust the UI for the "User name attribute" and "Email attribute" fields. (And you're the first to semi-explicitly tell me that your responses have zero attributes.)

Solution: this 'trick' should work: set !nameid for the value of both "User name attribute" and "Email attribute" settings. Note: must be lowercase, starts with !.

This issue is also "Needs work" for fixing the UI to make that clear.

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

Maybe there's a prior question of whether this module is still needed?

Just for the people only reading this issue:

Yes, it's been established that this is still needed, in #3462541-4: Difference with what is now in core? → and further.

So a merge + release is appreciated.

📌 | Configuration Override Warn | Difference with what is now in core?

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

This module can be useful until the mentioned render array attribute gets widely used in the contrib space.

Yes.
In addition: this module also shows overrides for config entities. (It's perfectly possible to override config entities' properties using $config in settings.php.)

We have somewhat simple entities, in a form that extends EntityForm, and this module correctly reports (I changed the names):

In the config file MODULE-NAME.ENTITY-TYPE.ENTITY-ID
- The value for PROPERTY-NAME has been overridden.

I don't see a plan to support this in Core. (And I don't know enough about overriding config entities, to know if changing the code to support this, would be simple. But at this moment, all the code is in ConfigFormBase, which != EntityForm.)

🐛 | Configuration Inspector | Crashes on leftovover themeconfig

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

I came here from 🐛 Expected argument of type "array", "string" given Active and this MR inspired me to make a MR there. (The code is different: this one catches a crash in checkValues(); #3444730 catches a crash in validateValues().)

While I have not reproduced this... I think it you can probably argue that

this does not need tests at all, because we can/should just assume that
- checkConfigSchema() can throw any old exception
- and checkValues() just needs to take care of those and never choke
...so all you need to do is check the phpcs error and then set to review without tests?

I admit that this in my validateValues() case, that is clearer to me than for this MR. (Because this calls checkConfigSchema(), which I don't know, whereas my case calls Typed Data stuff, which is "urgh, scary black box, we must be prepared to handle any exception that could throw".)

🐛 | Configuration Inspector | Expected argument of type "array", "string" given

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

🐛 | Configuration Inspector | Expected argument of type "array", "string" given

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

I was inspired by the recent 🐛 Crashes on leftovover themeconfig Active PR to look at the code again and could make some sense of it (because I now understand/think that I can ignore the impenetrable TypedData code). #3359846 does (almost) the same thing to different code, btw.

Proposed fix included in new MR. And...

...do we actually need tests (and steps to reproduce) here?
Or can we just assume that
- TypedData stuff can throw any old exception
- and we just need to deal with it (i.e. catch it and turn it into a violation)?

I think it's the second. I think this just needs a try-catch + return error, and I don't need to bother about writing tests that cause any exception.

Manual testing done

UI:
- Before: fatal error
- After: the row for the offending config item reports "1 error"
- The actual error is reported in the list/detail screen. This does not change / did not suffer from the fatal error.
drush config:inspect:
- the fatal error is never reached, because inspector->validateValues() is called before inspector->checkValues(), which is (I think) therefore never called and doesn't get a chance to choke.
- however, if it was called, this try/catch gives better output. (I tried. Before the fix, you only get the fatal error reported by drush.)

Reproducing the error (though I'm arguing it's unnecessary)

1) we have #19

2) Putting comments #12 and #20 in context:

If you have a firmatted text field, and you change the default value to

default_value:
  format: restricted_html

and import that: this will cause the fatal error in the issue title.

Random useless info

This is of course a completely bogus value: the actual default value would be:

default_value:
  -
    value: '<p>this is my default text</p>'
    format: restricted_html

...and Core does not allow an empty "value".

Our config did, however, have only a default format in the config, like:

default_value:
  -
    format: restricted_html

...which, I assume, was somehow necessary to work around a bug / select the correct default format, when allowed formats was still implemented in contrib (allowed_formats module). But:

Core does not allow to save a default format without a value, through the UI
a drush config:export will re-export this format-without-value in the above format which causes the fatal error when you re-import the exported file.

💬 | SAML Authentication | Support multiple environments for DEV/TEST/LIVE (other?)

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

I rely on other people to specify their 'config sync' policy / best practices. So I'll rebrand this a task.

People must be doing this. But so far, I don't remember anyone really commenting on it.

Are they using config_ignore? If so, what are they ignoring? Just the entity ID (provided the certs are in files)?

Do they want to use multiple IdP configs for this? ✨ Multiple IDP Support Needs work

When there's a somewhat authoritative reply from someone, we can determine if this needs documentation, or other work.

💬 | SAML Authentication | Disallow login based on attributes

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

Thanks for the re-explanation, I'm not always fast picking everything up :)

Regardless whether this solution would be suitable for your needs: the fact that a UserVisibleException thrown in a USER_SYNC event subscriber is a bug that should be fixed. (And noone's reported it so far + I somehow failed to spot it + there are no tests for this.)

I "plan to soon" test and extend SamlController::handleExceptionInRenderContext() to also recognize the wrapped EntityStorageException, then. (and/or any exception that wraps a UserVisibleException...)

That's arguably a hack to counter a hacky situation (the fact that the event is dispatched during hook_user_presave). But it is a recognizable specific hack without further issues. And for the moment I'm still sticking to the hacky situation and living in hope I'll tweak externalauth someday.

(Because - better summary: otherwise I'd need to give up on either dispatching the same event for new and re-logging-in users, or having protection against saving half-constructed user entities with bogus data.)

💬 | SAML Authentication | Disallow login based on attributes

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

(Aaaaaaaactually re. "done during saving":

There are complications. The gist is: I would like to do separate the user sync out from the eventual user_save(), but that requires a rewrite of the externalauth module if you never want to run the risk of ending up with a saved user with half-incorrect data.

I think. It's been a while since I looked at the details.)

💬 | SAML Authentication | Disallow login based on attributes

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

> I somehow assumed that the UserSync event would only run for new users

OK, that's cleared up then.

> It does specifically for new users run quite late though, only when already saving the new user. That will also catch and rethrow the exception in the storage handler. This would also break the ability to display a specific message to the user.

Well yeah, interpreting the SAML message and saving the new user is basically the only thing it does. I never considered that "late". (I guess I never cared about the catch + rethrow you mention? What mattered to me only is that it gets caught again in the controller. Also: apologies for the code spaghetti.)

For displaying a message to the user when login (and/or creation / sync) fails, you can throw a Drupal\samlauth\UserVisibleException. Does that help?

💬 | SAML Authentication | Disallow login based on attributes

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

I guess I could throw an exception during the user sync event, but if the attributes change for an existing user, it wouldn't prevent the login.

Not sure I'm interpreting this correctly.

The login is always prevented if you throw an exception.
What you can't do (well I guess you can, but not in a recommended way), is prevent the login but still update the Drupal user data with data from the IdP. I wouldn't expect anyone to want to do that... so... just asking for confirmation: is that really what you want to do? I never considered there would be a use case for that.

📌 | Custom Elements | Field formatter for video