🇺🇸United States @wesleymusgrove

Hey @sandeep_k not sure if you saw what I mentioned above, but it applies to version 3.2.0 of the drupal/recaptcha module, not the latest version.

Also I have some concerns because it's not actually checking the bot score in the response from the Enterprise reCAPTCHA create assessment API.

One thing I noticed about that patch, though, is it's only checking if `$api_response['tokenProperties']['valid'] == true`, but that only indicates that the provided user response token generated by the client-side `grecaptcha.enterprise.execute` is valid. It doesn't really tell you if it's a bot or not. When `valid == false`, the `reason` is specified in `invalidReason`. Google's documentation also says `valid = false` can also indicate that a user has failed to solve a challenge or there is a `siteKey` mismatch.

https://cloud.google.com/recaptcha/docs/interpret-assessment-website

I think two things should be checked in combination with checking if `valid == true`:

1. The bot likelihood `score`. This is going to be different for every site, but they recommend starting out with `0.5`. Anything greater than `0.5` and closer to `1.0` is likely human and anything less than that closer to `0.0` is likely a bot. This value should be made to be configurable in the reCAPTCHA Drupal admin settings form and added to the response handling logic in `Enterprise.php`.

2. The client-side `action` from a user interaction that triggered a reCAPTCHA v2 verification in the first place needs to be compared to the `expectedAction` on the server-side response from the create assessment REST API. Google's documentation says if these don't match, it indicates that an attacker is attempting to falsify actions. You can take actions against the user interaction, such as adding additional verifications or blocking the interaction to prevent any fraudulent activities. Perhaps this `action` could infer some unique `form_id` about the Drupal form it's rendered on and then the server-side `expectedAction` could validate that against a known list of valid actions.

I'm still on `drupal/recaptcha` version 3.2.0 and confirmed that the patch in #5 applies cleanly to that. Looks like there have been some significant updates since 3.2.0 causing a need for the patch to be re-rolled or officially supported as an option by the maintainers.

I have a Google Cloud Project with an Enterprise reCAPTCHA API account and I created a new public site key following these instructions: https://cloud.google.com/recaptcha/docs/create-key-website#create-key

When you create the public site key like that, the GCP admin console doesn't provide you with the private secret key to pair with the public key for the server-side token verification step, like it once did in the classic/legacy reCAPTCHA admin console (before a GCP with an active billing account was required). Now to get the private secret API key, you have to go into the GCP's APIs & Services > Credentials https://console.cloud.google.com/apis/credentials to create the private secret API key.

Of the 5 types of REST authentication methods mentioned here https://cloud.google.com/docs/authentication/rest#types_of_credentials, at the bottom they say you can authenticate with only an API_KEY as the “key” query string parameter. https://cloud.google.com/docs/authentication/rest#api-keys

The patch in #5 performs this server-side request made from Drupal directly to the reCAPTCHA Enterprise REST API create assessment endpoint https://cloud.google.com/recaptcha/docs/create-assessment-website, so it's not be exposed to network traffic visible by the client browser.

Very good and thanks for the link! This can be closed.

wesleymusgrove → created an issue.

wesleymusgrove → created an issue.

wesleymusgrove → created an issue.

I left comment #46 here https://www.drupal.org/project/drupal/issues/3105024#comment-15727018 🐛 Drupal main javascript file can't be loaded with "defer" attribute after upgrade to 8.8.1 Fixed to describe the current state of this problem for me, now that it's recommended to uninstall AdvAgg on D10 and I still want to be able to `defer` all scripts, which was the functionality that AdvAgg Mod was providing.

Unless we have many developers not using advagg that ran into this issue I don't see core are the place to fix this

Since AdvAgg is no longer recommended for D10 (and causes issues like not saving newly generated bundles of JS to disk unless you uncheck "Enable advanced aggregation" 📌 Document which parts of the module are still relevant after aggregation changes in 10.1.0 Needs review ), I've completely uninstalled it, but still needed a way to defer all JS as @nod_ mentioned in #42 🐛 Drupal main javascript file can't be loaded with "defer" attribute after upgrade to 8.8.1 Fixed .

So I added this simple hook to `defer` all scripts:

<?php
/**
 * Implements hook_js_alter().
 */
function custom_js_alter(array &$javascript) {
  foreach ($javascript as &$js) {
    if (is_array($js) && isset($js['attributes'])) {
      $js['attributes']['defer'] = 'defer';
    } 
  }
}
?>

Before uninstalling AdvAgg, I had previously applied the core patch from #45 → , which I re-rolled for D10.

After uninstalling AdvAgg and adding my `custom_js_alter` hook, I started seeing this issue happen again where Drupal behaviors weren't executing if they were attached after drupal.init.js had loaded, as would be the case for all the deferred scripts.

Since @nod_ mentioned that

it is safe to call multiple time[s]

on #38 🐛 Drupal main javascript file can't be loaded with "defer" attribute after upgrade to 8.8.1 Fixed , would a custom module adding some JS like this to call `Drupal.attachBehaviors` one last time after the document is `complete` be an appropriate solution?

(function (Drupal, drupalSettings) {
  var domReady = function domReady(callback) {
    var readyStateCheckInterval = setInterval(function () {
      if (document.readyState === 'complete') {
        callback();
        clearInterval(readyStateCheckInterval);
      }
    }, 100);
  };

  domReady(function () {
    Drupal.attachBehaviors(document, drupalSettings);
  });
})(Drupal, window.drupalSettings);

If someone aggressively defer all scripts without using advagg I would hope they know what they are doing. the drupal.js/drupalsettings scripts are light enough to not have to defer them

I realize this technique _does_ aggressively defer all scripts and that is my goal. Is there a list of core drupal JS files that should be excluded from getting the `defer` attribute for D10 core's `Drupal.attachBehaviors` to continue working as expected even for deferred scripts that define new behaviors?

Thanks! I know this has been closed for a while, but it seemed relevant to this conversation.

@tgoeg all those except for `web/libraries/choices/public/index.html` are non-functional comments essentially referencing that the polyfill code did NOT come from https://cdn.polyfill.io/v2/docs/.

While waiting on an upstream patch, @Luke.Leber mentioned over here 📌 choices/choices 9.0.1 is affected by polyfill.io Active that you can run some post composer install commands to clean it up.

I also mentioned another option in the comment below that 📌 choices/choices 9.0.1 is affected by polyfill.io Active that prevents the entire `choices/choices` library from getting downloaded in the first place. However, I think if that library doesn't exist locally on your server, then it could fallback to using the CDN library, which is the external URL that was fixed in Webform 6.2.3.

Another thing you can do in composer.json is tell Composer not to even try downloading and installing the `choices/choices` library even if it is required by another package, like webform's `composer.libraries.json`. This is informing Composer that I already have a package that replaces the functionality of all versions of `choices/choices` (even though I don't have a replacement). So, Composer will not attempt to download and install `choices/choices`..

"replace": {
    "choices/choices": "*"
}

Hi @ndewhurst

I wanted to provide an update based on what one of my colleagues is experiencing. It seems like the `drupal_image` Twig Tweak function does not support the `bf://uri-to-bf-image` protocol and only allows images to load from `public://uri-to-sites-default-files-image`. This might be related to an existing issue where stream wrappers are needed to load any image outside of the normal `public://` directory?

See:
https://www.drupal.org/forum/support/module-development-and-code-questio... →
https://www.drupal.org/project/drupal/issues/1308152 ✨ Add stream wrappers to access extension files Needs work

However, if we use the file id `fid` of the mirrored Drupal entity Brandfolder image reference, `drupal_image` is able to load that. In tandem with responsive images, this creates a picture tag with a bunch of responsive options for the image size.

Because of this, we are thinking we'll likely need to create some sort of twig function or preprocess hook that takes a Brandfolder asset id and turns it into an entity id.

It's either that or we try to use the Imagecache External → Twig filter function to load the absolute URLs of Brandfolder images from the CDN link.

I'm interested in your thoughts on that approach and what the pros/cons are of not using the `bf://uri-to-bf-image` protocol?

Thanks!

wesleymusgrove → created an issue.

Hi @ndewhurst,

Thanks for the quick response and helpful tips!

Related to @cbrand02's questions, I'm looking at this article https://medium.com/geekculture/responsive-image-optimization-with-media-... and am curious if you could provide any feedback on how the Brandfolder module could be used in conjunction with the Easy Responsive Images → module regarding defining and using a set of responsive image styles defined in Drupal?

Also if we're rendering Brandfolder images in our own custom twig templates, how do you recommend that be integrated with Twig Tweak → and the ability to specify attributes that control whether the image is lazy loaded or eagerly preloaded ✨ Add lazy loading example in Cheat Sheet Fixed , i.e. for important images that need priority for Google Core Web vitals and Largest Contentful Paint reasons?

Thanks!
Wesley