🇸🇰Slovakia @poker10

MR tests are green: https://git.drupalcode.org/project/drupal/-/pipelines/345271

poker10 → created an issue.

heine → credited poker10 → .

xjm → credited poker10 → .

xjm → credited poker10 → .

xjm → credited poker10 → .

poker10 → credited poker10 → .

poker10 → credited poker10 → .

@ivnish I would probably not do that in 8.x-1.x, as it is going to drop support for anything below 10.3, which can be disruptive for existing sites?

Thanks for creating the other issue.

Is this related / duplicate of 📌 Review the messages that appear on the initial page after install Postponed ?

diff --git a/recipes/drupal_cms_starter/recipe.yml b/recipes/drupal_cms_starter/recipe.yml
--- a/recipes/drupal_cms_starter/recipe.yml
+++ b/recipes/drupal_cms_starter/recipe.yml
@@ -104,50 +86,34 @@ config:
-        register: admin_only

Do we want to allow registrations in Drupal CMS installations for everyone by default? I think the default is register: visitors, if the code above will be removed. This change is not mentioned in previous comments, if I have not missed anything.

I think this is related to 🐛 Test files are publicly accessible Needs review (or maybe even a duplicate?).

Yes, I can commit this and create a release. I looked at the MR and all changes looks good to me (tests are passing with these changes). I can't tell without a deeper check, if the MR is not missing something else, but if tests are green, probably not.

One thing I am not sure about is the status of the 2.x branch. There seems to be some additional commits in comparision with the 8.x-1.x branch. Yes, I can roll a 2.x-beta, but have anyone tested the 2.x-dev if it is really working with all changes currently committed? Sorry, I do not have time to check it commit by commit now.

Thanks!

Should this target 11.x, instead of 7.x, because of this?

This is present in Drupal 8 too in \Drupal\user\Form\UserPasswordForm::buildForm().

I know that the google_analytics module still does not have D11 compatibility fixed, but I am curious, what is the more common way to add tracking to the sites for ambitious marketers? Is it to use the GA module directly (where the Google Analytics configuration is easier and more straightforward) or using the Google Tag manager, where you need a bit more experience to configure it right?

If we do not allow "access user profiles" for authenticated/anonymous users, then will the user pictures have any benefits? Probably the most common thing to display these avatars is in comments or in articles. But I am not sure if we plan to use them there.

longwave → credited poker10 → .

@matthieuscarset Thanks for the answer. The mentioned change record is a draft and the issue it is referencing is still not committed - 🐛 Untranslated menu items are displayed in menus Needs work . Until then, the module is still very usable to hide menu links which are not supposed to be displayed in a specific language. Thanks!

greggles → credited poker10 → .

poker10 → credited poker10 → .

It would be great if we can get this committed and if a new D11 compatible release can be created. Thanks!

Re: maintainership. I am one of the maintainers of this module, but was maintaining only the 7.x branch. I do not have permission to add another maintainers. So I think we need at least one issue - either from me to get full permissions, or from someone else to get on board. Thanks!

@fgm Would it be possible to tag a D11 compatible release (see also: 📌 Tag new release? Active )? Thanks!

@voleger Would it be possible to create a D11 compatible release? Thanks!

This is an important commerce module. It would be great if you could create a D11 compatible release with Commerce 3.0.x support (see the second issue). Thanks!

@matthieuscarset It would be great if you can create the release you mentioned in #4. Thanks!

Re#25:

but only publish the first one and keep the other 2 unpublished. That way, the site owner can enable them when required in their context.

Is there any reason why we need to publish anything by default? This content will be incomplete / "ugly". Every published content is visible to everyone, indexed by bots and affects SEO. I suggest that we do not publish any such content without explicitely doing so by the site admin.

Other points looks good to me.

I am not sure what we need to add to the issue summary, but agree that the MR should be updated - at least we need another elseif in the existing condition, so that we do not end up with Password reset form was submitted with an unknown or inactive account: %name. when actually the account exists, it just does not have an email set. So I think we should move the check to a new elseif with a new separate watchdog message, so that is clear, that a form was submitted for an account without email address. The printed message seems to be correct then, and it is as proposed in IS.

Probably we should also add a simple test for this behavior as well? Something like - create user without email, log out and try to send the password for that username. I do not think we are hiding a larger issue here, simply the PhpMail does not expect to get a NULL email (which is a case, if a user account does not have an email set).

Thanks!

poker10 → created an issue.

Thank you for the explanation @a.dmitriiev. I haven't noticed the new beta release of the 3.x branch from the last week). When we tried the 3.x branch few months ago, in that time it was not possible to use it as a replacement for 2.x branch, as it still had a lot of issues. We will take a look at the new changes :)

Facets have a lot of open issues and personally I am not sure if we should ever include it unless these issues are fixed:

🌱 [META] Overview of Facets + Ajax issues Active
🐛 Facets with AJAX not working in most of situations Needs review

It is problematic to run Facets even on Drupal 10.3 if you want to enable ajax. Yes, we can probably ship it without ajax enabled, but if someone start to explore these options, it will be very easy to break everything.

So +1 from me to removing Facets for now.

kim.pepper → credited poker10 → .

poker10 → created an issue.

greggles → credited poker10 → .

Created an MR with proposed fix (using another function if the db driver is PostgreSQL). Probably needs a manual testing.

@japerry Is there a follow-up for an issue mentioned in #9 and #13? We run into the same situation. Updated the module on D10 (because there was no reason/incompatibility to prevent this and then after updating to D11, the update hook was not run again. So the module is still installed.

I have not tested it on Drupal CMS, but on Simplytest.me with Drupal 10.3.6 - if you put <script>alert("TEST");</script> to the body with full HTML format, it is not filtered and the alert appears once the node is displayed.

It does not seems like that FullHTML format set by full_html_format_editor recipe limits the allowed tags by default (https://api.drupal.org/api/drupal/core%21recipes%21full_html_format_edit...). So even if the iframe is not in the ckeditor toolbar, it still seems to be allowed by default (if I have not missed anything).

greggles → credited poker10 → .

greggles → credited poker10 → .

Can we explore if the option "Run only on form change." is currently usable and if we could use it to reduce the load of the autosave feature? I personally do not have any experience with it, but maybe someone has.

The registration form is disabled by default. I have never heard of needing bot protection for the forgot password form, is this something others have had issues with?

We usually add it to forgot password forms as well, but mostly in cases when the user registration is turned on, or there is a commerce store or other ways to register accounts. The reason is that a bot can create an account with a fake email address (even if there is an antispam protection) and then the bot can use the forgot password form to spam that email address (which belongs to the account) asking for activation/password reset links. It is not a 100% full protection, only a hardening, but when "fighting" with bots, even small things count. On the other hand, we typically do not use these protections on login forms.

The registration form is disabled by default.
...
In order to apply it as part of the base recipe, I think it needs to be protecting at least one form.

I didn't know that the registration will be disabled. So yes, this seems reasonable to enable it only in case there will be something to protect.

@jurgenhaas I see the Klaro! consent manager JS-Library is open-source for client-side usage, but paid for another server-side features. Are we certain that the licensing will not change if we make it more popular? These 3rd party services tend to tighten the usage policies like cookiebot and some others have done in the past.

I do not think that currently the failing MRs or MRs which needs to be rebased, are moved to NW automatically (except by NR queue bot). Probably this is more relevant issue: 📌 Mark issues needs work when MRs can't be merged or pipelines fail Active , but since this is a policy issue, I am not sure we should close this without a decision.

@atul_ghate Thank you for working on this.

Users can access that password reset form just by entering the URL to the browser. I think it will be better idea to move the message to the UserPasswordForm itself and just disallow to submit the form (see the "Proposed resolution" in the issue summary). Yes, then, we can think, if we also need to remove the link from the AccountForm, but this change itself will not fix the root cause of the problem, so this is maybe an issue for a followup. Thanks!

The last release of D7 is currently planned for 2024-12-04 , so I think we can create that issue and at least check, what issues there will be with automated testing and try to fix them. I think that it could still be possible to fix them until the release (or EOL) if there will be enough interest and if they won't require some major changes to D7 code.

@liam morland This is a meta issue for tracking all child issues needed to fix PHP 8.3 compatibility. So far we have identified only two (#3397882 and #3446569). I have created an MR here with the two fixes to see, if both will be committed, if our tests will be green. So there will be no commit from this meta issue, we will commit both fixes from the separate issues I mentioned.

mr.baileys → credited poker10 → .

poker10 → credited poker10 → .

mr.baileys → credited poker10 → .

poker10 → credited poker10 → .

Is there a deadline until when this needs to be decided? I mean the date when the blocking modules will be removed, in case they will have no D11 supported release. Thanks!

Can we maybe use a help of https://www.drupal.org/project/puwg → ? I think that except some big modules (like webform), D11 should be a priority over some small upgrade-blocking modules.

Discussed this briefly on Slack with @catch and he suggested to solve this by changing the markup of the form to inform a user that is unable to use/submit this form, because of a missing email address.

I have updated the issue summary.

@gaurav gupta I was thinking about keeping the UI message the same, even if the username does not have an email associated, exactly because of this information disclosure issue: #1521996: Password reset form reveals whether an email or username is in use → . If we change the error message for this case, it will be possible to use this for username enumeration (especially on sites where email addresses are not required/used). I do not think that is a good idea.

I suggest to keep the behavior the same as now, so everytime the form is submitted, the same message is printed and only add another condition/or change existing conditions to handle the case a username does not have an email address.

@zartab farooquee We cannot block the form submit, since this issue is about anonymous users doing password resets. So we will know if the entered username has email address only after the form is submitted.

The similar issue is here, which is for logged in users: 🐛 Accounts without an email address will display a warning message when resetting their password on the edit page Active

Thanks!

@akalata By my comment about the "third thing" I meant that the MR is hashing the password to $form_state['values']['pass'], but the password is still saved to $account->password in a plaintext. So I was not sure about the benefit of the solution, if we still keep the plaintext password on another place. I will try to take a look at this, what other possible solutions we have here. Thanks!

Thank you for reporting and working on this!

I have tested the MR also manually on a test site - checked the install.php, the update.php and a regular page served via index.php. I can confirm that with the patch from the MR, the DRUPAL_ROOT path is stripped from the error messages on all these three places correctly. I have also tested it on windows, where it seems to work as well.

Tests-only job seems to fail as expected: https://git.drupalcode.org/project/drupal/-/jobs/2975882

All other tests seems to pass: https://git.drupalcode.org/project/drupal/-/pipelines/302957

There is still a "Needs tests" tag, but I am not sure if we need more tests - do you think so? I would say the added tests covers the check.

I have also updated the issue summary with the current proposed resolution. Do we need a change record for D7? D11 fix did not have it, but since we are stripping this globally, should we consider it?

In overall, I think this looks good, thanks!

Thanks for creating an MR for D7. It looks like a clean backport of the D8+ fixes.

I am a bit concerned about changing the markup on the form for logged in users from "textfield" to "item" - for BC reasons (sites can have custom styling which can potentially break, ...). Yes, it is the cleanest way how to fix this in D7, but maybe we should consider something with lesser impact - for example to rewrite the email and name in the contact_personal_form_submit? According to the backport policy for D7 ( https://www.drupal.org/about/core/policies/core-change-policies/what-pat... → ), changes like these are allowed only if the issue is Critical.

(not creating a separe D7 issue just yet, in case we decide to commit it in this form - otherwise we should create a separate issue as per current backport policy)

Adding a missing tag.

Thank you for working on this!

I see there are some differences with the original D10 code:

1. The issue title is modules and themes are not filtered on output - I think the D7 backport is sanitizing the data a way sooner, than on output. I did not have time to review it if that have a potential to cause any troubles yet, but another option would be to move the sanitization to output - for example to system_modules and system_themes_page (or theme_system_themes_page)

2. Currently the code is adding check_plain on 'name', 'version' and 'package' attributes. According to the documentation, it seems correct, that these three should be in plaintext ( https://www.drupal.org/docs/7/creating-custom-modules/writing-module-inf... → ). In D10 code, there does not seems to be a direct sanitization, but I suppose the automatic twig sanitization is used (anything between {{ }} gets automatically sanitized ( https://www.drupal.org/docs/administering-a-drupal-site/security-in-drup... → )).

3. package in D10 is sanitized using D10's version of filter_xss_admin, see: '#title' => Markup::create(Xss::filterAdmin($this->t($package))),. In D7 is sanitized using check_plain. We should check, what is a correct approach (the output of the package name seems very similar to me - in D10 it is a title in "details" form element, in D7 it is a title in "fieldset" form element).

4. D7 backport is missing tests. Can we add at least a simple test to check the output of name and description for script tags (like it is done in the D10 test)?

Thanks!

Thanks for reporting and working on this. There are two older issues:

#2909652: [D7] Parsing a URL with another URL in the query arguments throws undefined offset notice → - URL with another URL in the query arguments

#1875540: drupal_parse_url() fails to parse fragment from external url when query is not present → - fragment in external url when query is not present

Normally we should close this issue as a duplicate. But in this case, we can consider fixing both problems here, since the first issue does not have any progress and the second only have one incomplete Drupal 7 patch. We can transfer credits when doing commit, so that is not an issue.

I will leave this open for further opinions.

I have also left some comments in the MR (mostly for PHP 5.6 compatibility), so moving to Needs Work. Thanks!

Thank you for working on this! The tests are failing because of array to string conversion: https://git.drupalcode.org/issue/drupal-3482184/-/jobs/3111321

I also left a few comments/suggestions in the MR (including the reason for the failure mentioned above). Moving to Needs Work.

I think that the new documentation page: https://www.drupal.org/about/core/policies/maintainers/subsystem-maintainer → (see #43 and #62).

Unfortunatelly the 8.x version of the EU Cookie Compliance modules has a pretty annoying bug/feature: #3209352: 8.x: disabled_javascripts runs scripts which wouldn't otherwise be on the page → . In the 7.x version, only scripts which were removed by the module were added later. But in the 8.x version, this is not checked (see the issue I linked), so the module is adding all JS scripts even if some of these should not be on a specific page.

It seems like this is still an issue in Drupal 7.x-dev and also in Drupal 11.x-dev, so we should probably move it for 11.x-dev (as per backport policy).

I personally think this is by design and that it is only a documentation issue.

Relevant documentation page is probably this: https://www.drupal.org/docs/8/api/database-api/dynamic-queries/expressions →

Thanks!

I think that the examples from "Actual result" and "Expected result:" seems to be swapped, because based on my testing, currently if you use the same placeholder multiple times, it has the same value substituted.

I tried it on Drupal 7.x-dev and this is the result on PostgreSQL:

SELECT u.uid AS uid, u.name AS name, n.nid AS nid, n.title AS title, c.cid AS cid, c.subject AS subject
FROM 
{users} u
INNER JOIN {node} n ON n.uid = u.uid AND n.created > '1000000'
INNER JOIN {comment} c ON c.uid = u.uid AND c.nid = n.nid AND c.created < '1000000'
WHERE  (u.created > '500000') 

So I have corrected the example from the IS.

I think this is a duplicate of: #1587370: Repetitive use of the same placeholder name in join causes wrong query generation → , so let's continue in the older issue. Thanks!

There is an older related issue which proposes to make the password field optional (when creating an account for another user) - #397846: When creating an account for another user, make password field optional → .

It seems to me that the change from https://git.drupalcode.org/project/tour/-/merge_requests/83.diff was done in the commit in #3473594 (https://git.drupalcode.org/project/tour/-/commit/6618943547927c7b8ff7e86...), though a bit differently.

poker10 → created an issue.

poker10 → created an issue.

@pameeela Sorry I was not clear enough, but my second part of the comment was a reaction to the quoted text, but that is related mostly to a separate issue @lauriii mentioned earlier, not this change itself. It was not really clear to me, what other benefits (except that admin would not have to enter a random password) can be achieved by generating a password and sending the email vs generating and not sending it (with regards to the generation itself), if the notification email will be still the same in both cases (when password will be generated, or manually entered) - it will just contain a link to reset the password. But yes, this question belongs to a separate issue (or probably also here 📌 Automatically generate a password on user creation Active ). This issue is just about the checkbox/configuration setting.

Is there any progress with this?

We need to develop a more detailed project plan, coordinated with the Drupal Association, for what we'd like to have in place by DrupalCon Barcelona and what we need to have by the initial launch.

I am mostly curious about what is expected to change on January 15. Is the https://www.drupal.org/download → or other places on d.o., where is a reference to Drupal core, going to change with the first stable release of Drupal CMS? Or is this a plan for some next phases? Thanks!

From my experience , when creating an account manually, it is probably 50:50 between situations, where notification email is desired and when it is not desired. I think the default off is reasonable there. The primary way to register on the site is by users themselves and there the emails are sent.

This was the reason I revisited this, I felt that we should sort this out first since it doesn't make sense to generate the password and NOT send an email.

There is a reason. Sending plaintext passwords in emails is not a good idea from the security point of view and that was a reason why core is not doing this anymore from 2010 ( #660302-64: Migration path for changed user email tokens; don't complicate translation of default messages → ). I am strongly against the idea of sending plaintext password in emails again.

Sending plaintext passwords in email is not considered a best practice and that was the reason, why password tokens were removed from core in #660302-64: Migration path for changed user email tokens; don't complicate translation of default messages → . Do we really want to send passwords in email in Drupal CMS by default?

We have the password reset links just for this specific use-case, so that passwords are not leaked to emails and 3rd party services parsing emails.

If we plan to use the https://www.drupal.org/project/genpass → module just for the password generation, I think that will be OK. Thanks!

poker10 → created an issue.

Actually the deprecation message was in 10.3.x, in 11.x-dev I get TypeError when trying to access the password reset page with user without email address:

TypeError: Drupal\Component\Utility\Html::escape(): Argument #1 ($text) must be of type string, null given, called in /core/lib/Drupal/Component/Render/FormattableMarkup.php on line 238 in Drupal\Component\Utility\Html::escape() (line 431 of /core/lib/Drupal/Component/Utility/Html.php).

#0 /core/lib/Drupal/Component/Render/FormattableMarkup.php(238): Drupal\Component\Utility\Html::escape(NULL)
#1 /core/lib/Drupal/Component/Render/FormattableMarkup.php(211): Drupal\Component\Render\FormattableMarkup::placeholderEscape(NULL)
#2 /core/lib/Drupal/Core/StringTranslation/TranslatableMarkup.php(195): Drupal\Component\Render\FormattableMarkup::placeholderFormat('Password reset ...', Array)
#3 /core/lib/Drupal/Component/Utility/ToStringTrait.php(15): Drupal\Core\StringTranslation\TranslatableMarkup->render()
#4 /core/lib/Drupal/Core/Render/Renderer.php(472): Drupal\Core\StringTranslation\TranslatableMarkup->__toString()
#5 /core/lib/Drupal/Core/Render/Renderer.php(459): Drupal\Core\Render\Renderer->doRender(Array)
#6 /core/lib/Drupal/Core/Render/Renderer.php(203): Drupal\Core\Render\Renderer->doRender(Array, false)
#7 /core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(238): Drupal\Core\Render\Renderer->render(Array, false)
#8 /core/lib/Drupal/Core/Render/Renderer.php(593): Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}()
#9 /core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(231): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure))
#10 /core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(128): Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\CurrentRouteMatch))
#11 /core/lib/Drupal/Core/EventSubscriber/MainContentViewSubscriber.php(90): Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\CurrentRouteMatch))
#12 /vendor/symfony/event-dispatcher/EventDispatcher.php(246): Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object(Symfony\Component\HttpKernel\Event\ViewEvent), 'kernel.view', Object(Symfony\Component\EventDispatcher\EventDispatcher))
#13 /vendor/symfony/event-dispatcher/EventDispatcher.php(206): Symfony\Component\EventDispatcher\EventDispatcher::Symfony\Component\EventDispatcher\{closure}(Object(Symfony\Component\HttpKernel\Event\ViewEvent), 'kernel.view', Object(Symfony\Component\EventDispatcher\EventDispatcher))
#14 /vendor/symfony/event-dispatcher/EventDispatcher.php(56): Symfony\Component\EventDispatcher\EventDispatcher->callListeners(Array, 'kernel.view', Object(Symfony\Component\HttpKernel\Event\ViewEvent))
#15 /vendor/symfony/http-kernel/HttpKernel.php(188): Symfony\Component\EventDispatcher\EventDispatcher->dispatch(Object(Symfony\Component\HttpKernel\Event\ViewEvent), 'kernel.view')
#16 /vendor/symfony/http-kernel/HttpKernel.php(76): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1)
#17 /core/lib/Drupal/Core/StackMiddleware/Session.php(53): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#18 /core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(48): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#19 /core/lib/Drupal/Core/StackMiddleware/ContentLength.php(28): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#20 /core/modules/big_pipe/src/StackMiddleware/ContentLength.php(32): Drupal\Core\StackMiddleware\ContentLength->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#21 /core/modules/page_cache/src/StackMiddleware/PageCache.php(116): Drupal\big_pipe\StackMiddleware\ContentLength->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#22 /core/modules/page_cache/src/StackMiddleware/PageCache.php(90): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#23 /core/modules/ban/src/BanMiddleware.php(50): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#24 /core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(48): Drupal\ban\BanMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#25 /core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(51): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#26 /core/lib/Drupal/Core/StackMiddleware/AjaxPageState.php(36): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#27 /core/lib/Drupal/Core/StackMiddleware/StackedHttpKernel.php(51): Drupal\Core\StackMiddleware\AjaxPageState->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#28 /core/lib/Drupal/Core/DrupalKernel.php(709): Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#29 /index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
#30 {main}

Updated the issue summary again.

Update the issue summary.

This still needs work, because I do not think the attempt to make the email required is correct in this issue. If you want to make email required, please use 🐛 Email Address should be a required field in the "Add User" form Needs work or 🐛 User email is missing and no longer required Needs work

This issue should just fix the deprecation message and maybe remove the link if a user does not have email, so it is not confusing.

Thanks!

We do have the following quote from ACF implying unlike D.O. where this is common, is rare in the WP community.

I do not think that this is common on d.o. Can you provide an example? We are talking about "A plugin under active development", which all projects going thru ( https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or... → ) are not, because you need at least 14+14 days without an answer from all maintainers (contacted via issue queue and via contact form / email).

xjm → credited poker10 → .

This seems to be still an issue. I think the steps to reproduce can be the following:

1. install the module
2. try to use the SessionBasedTempStore::get() method for example in event subscriber subscribing to KernelEvents::REQUEST

Calling the get() method:

      $temp_store_factory = \Drupal::service('session_based_temp_store');
      $temp_store = $temp_store_factory->get('xxx');

in that event subscriber will call SessionBasedTempStore::getOwner(), which will then try to evaluate this condition:

      if (!$session->has('core.tempstore.private.owner')) {
        // Remember that we did not have started a session before.
        $session_already_started = FALSE;
        // This generates a unique identifier for the user.
        $session->set('core.tempstore.private.owner', Crypt::randomBytesBase64());
      }

The issue is, that this condition can pass even for authenticated users, causing $session_already_started = FALSE; to be set for logged-in users. Then, there is another code, which is run later:

      // Clear session if there was no session before.
      if (!$session_already_started) {
        $session->clear();
      }

Which will logout the logged-in user.

Maybe we need to generate $session_store_id for logged-in users differently, or fix the problematic condition in SessionBasedTempStore::getOwner()?

Thanks!

We probably need to add something like this $this->config('captcha.captcha_point.user_login_form')->set('status', 0)->save(); to InteractiveInstallTest::testPostInstallState(), just before the login attempt, as the recipe is turning on the protection of the login form.

I agree with @jurgenhaas that this is most likely a base recipe feature, not a feature of the contact form. It is important to have at least forgotten password and registration forms protected as well, which is not what I would expect from the contact form track.

Other than that, I still think that we do not need to remove the honeypot and antibot together with this new addition. Why do not we just add friendlycaptcha?

Thanks!

100% agree that we should not rely on 3rd party service like CookieBot, because, among other things, they continuosly tend to restrict the terms of usage or free usage limits, so we could get trapped easily.

I have not used it personally, but have also heard some good words about https://github.com/orestbida/cookieconsent . Though not sure if it does have similar features like the eu_cookie_compliance module (like removing disallowed cookies by default, and similar).

I don't see how...? Every patch we've been needing for core is either in progress, or has been committed to 11.x, and I believe that includes 11.0.x.

I was just asking. Not saying it must happen, but was curious if there are any risks for doing this.

I'd like to move to D11 as soon as possible too, but our dependencies make that impossible. The biggest bugaboo is Webform, which is currently stuck on D10 (although progress is being made to port it to D11).

This sounds good. Thanks!

If we want to switch to 11.0.x ( 📌 Switch our base to Drupal 11 Active ), would not this conflict that goal? Since 10.4.x -> 11.0.x will be a downgrade.

greggles → credited poker10 → .

poker10 → credited poker10 → .

greggles → credited poker10 → .

greggles → credited poker10 → .

Seems like the possibility to run it completely locally is a big plus. We have also used https://www.drupal.org/project/hcaptcha → , but I do not think it can be run just locally (do we need to evaluate more captcha modules?).

Other question is, do we need to remove Honeypot and Antibot? We use combination of reCaptcha, Honeypot and Antibot on a lot of sites and efficiency has alway been better when we used more than one module for a bot protection (though not sure about the FriendlyCaptcha efficiency, because we have not used it).

poker10 → created an issue.

benjifisher → credited poker10 → .

poker10 → credited poker10 → .

poker10 → credited poker10 → .

greggles → credited poker10 → .

Thanks for drafting this, looks good to me!

Would not it be better to fix this in Pathauto (if possible - but maintainers announced it will be removed in next major release) and not introduce additional workarounds, especially in this phase? That will also help other sites using Pathauto module. Agree that if this will be still an issue later, before Drupal CMS release, then we should probably use this, so that we have a cleaner codebase.

Adding related issue.