🇬🇧United Kingdom @alexharries

Thank you very much, @apaderno :)

/A

Thank you for your help, Vishal :) Very much appreciated.

Best wishes

> The feedback comment related to README.md and the .module file has not been addressed.

I have updated the Readme per https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or... →

Hooks are contained in includes/really_simple_google_tag.hooks.inc so that description is more appropriate there.

Accordingly, no change has been made in this respect.

Thank you, Vishal. I believe all of those fixes have now been committed to the master branch.

/Alex

Hi MarcellinoStroosnijder,

> Please reroll the patch of #38, but with the systems routes added in :)

I'm away from that project now so won't be able to do this, but feel free to do the re-roll yourself :)

/Alex

Hmm, I'm not having any luck fixing this issue on the command line or in PHPStorm despite following the steps in #44 and Selwynpolit's guide (thank you for the guide! Very helpful).

Following in hope of a resolution :)

Alex

Hi Vishal, done :)

alexharries → created an issue.

Hello folks, just a very minor suggestion to improve the experience for first-time visitors to this page who want to apply.

This page contains a bit of a wall of text with no clear call to action; perhaps we could:

1. Change the heading "Application process description" to "How to apply"

2. Bold step 4 - "Once ready, create a new issue in the Drupal.org security advisory coverage applications queue."

That's all :)

Alex

Solotandem, in the kindest-possible way, I think you need to take a step back and stop taking this as a personal attack.

You’ve proven yourself very good at writing posts containing wildly-excessive accusations.

If this was the first time in this issue queue that you’d been rude, I’d be prepared to believe that I’m the problem here.

But it’s not the first time, and - respectfully - I’m not the problem here.

:)

That’s ok; we’re just going to stop using your module on all our sites (-:

All the best,

Alex

alexharries → created an issue.

alexharries → created an issue.

Yes, it certainly looks like the same issue.

Perhaps removal of the skip/cancel button is the simplest solution?

I'm sure there's a use case where it could prove useful but from looking at the code, creating a universal fix which for example would resolve #2932071 might be more squeeze than the juice is worth... ¯\_(ツ)_/¯

/A

alexharries → created an issue.

Commit added to also handle empty keys when trying to _encrypt_, too.

Optimistically moving to "needs review"... ¯\_(ツ)_/¯

alexharries → created an issue.

Hi @cmlara, apologies - I provided the patch in #38 in good faith to fix the broken patch in #35. I'm not working with the 2.x branch due to SA-CONTRIB-2024-003, so wasn't able to provide a patch against that branch.

Hopefully the patch at #38 will help those still on 8.x-1.x ¯\_(ツ)_/¯

/A

Playing whack-a-mole with indentation... [facepalm.gif]

Found another incorrect indent - updated patch attached.

Hello all,

I've spotted four issues with the patch in #35:

- Hunk in tfa.schema.yml doesn't apply because indentation of two subsequent lines is missing a space

- Indentation is 4 spaces instead of two

- Hiding the Allowed Validation plugins field when "force redirection" is enabled seems like a bad idea to me; when hidden, the administrator can't change which validation plugins are selected, and because this field is below the "force" checkbox, users working from top to bottom down the form might never see the allowed validation plugins field.

- A typo; "FTA"

Attaching a patch which:

- Fixes broken indentation - bad:

@@ -11,6 +11,9 @@ tfa.settings:
       sequence:
         type: string
         label: 'Role'
+    forced:
+      type: integer
+      label: 'Force required roles to setup when on last validation skip'
     send_plugins:
       type: sequence
       label: 'Enabled send plugins'

Good (i.e. the last two lines):

@@ -11,6 +11,9 @@ tfa.settings:
       sequence:
         type: string
         label: 'Role'
+    forced:
+      type: integer
+      label: 'Force required roles to setup when on last validation skip'
     send_plugins:
        type: sequence
        label: 'Enabled send plugins'

- Fixes indents - bad:

+    public function __construct(RouteMatchInterface $route_match, MessengerInterface $messenger, AccountProxyInterface $current_user, ConfigFactoryInterface $config_factory,
+                                EntityTypeManagerInterface $entity_type_manager, UserDataInterface $user_data, TfaValidationPluginManager $tfa_validation_manager, TfaLoginPluginManager $tfa_plugin_manager) {
+        $this->messenger = $messenger;
+        $this->routeMatch = $route_match;
+        $this->currentUser = $current_user;
+        $this->userStorage = $entity_type_manager->getStorage('user');
+        $this->tfaSettings = $config_factory->get('tfa.settings');
+        $this->userData = $user_data;
+        $this->tfaValidationManager = $tfa_validation_manager;
+        $this->tfaLoginManager = $tfa_plugin_manager;
+    }
+
+    /**
+     * Redirect users to TFA overview when no remaining skips.
+     *
+     * @param \Symfony\Component\HttpKernel\Event\RequestEvent $event
+     *   The request event.
+     */
+    public function redirect(RequestEvent $event): void {
+        /** @var \Drupal\user\UserInterface $user */
+        $user = $this->userStorage->load($this->currentUser->id());
+        $this->setUser($user);
+
+        if ($this->isReady() || !$this->forceTFASetup()) {
+            return;
+        }
+
+        $this->messenger->addWarning(t('You need to enable Two Factor Authentication.'));
+
+        // Don't redirect the user if on password/profile edit page,
+        // as it is possible the user used one-time login URL
+        // and need to change the password.
+        $ignored_route_names = [
+            'user.login',
+            'user.logout',
+            'user.pass',
+            'user.edit',
+            'entity.user.edit_form',
+            'user.reset.login',
+            'user.reset',
+            'user.reset.form',
+            'user.well-known.change_password',
+            'tfa.entry',
+            'tfa.login',
+            'tfa.overview',
+            'tfa.validation.setup',
+            'tfa.disable',
+            'tfa.plugin.reset',
+        ];
+        if (in_array($this->routeMatch->getRouteName(), $ignored_route_names)) {
+            return;
+        }
+
+        $tfa_overview_url = Url::fromRoute('tfa.overview', ['user' => $this->user->id()]);
+        $event->setResponse(new RedirectResponse($tfa_overview_url->toString()));
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public static function getSubscribedEvents() {
+        $events[KernelEvents::REQUEST][] = ['redirect', 32];
+        return $events;
+    }
+
+}

Good:

+    public function __construct(RouteMatchInterface $route_match, MessengerInterface $messenger, AccountProxyInterface $current_user, ConfigFactoryInterface $config_factory,
+                                EntityTypeManagerInterface $entity_type_manager, UserDataInterface $user_data, TfaValidationPluginManager $tfa_validation_manager, TfaLoginPluginManager $tfa_plugin_manager) {
+      $this->messenger = $messenger;
+      $this->routeMatch = $route_match;
+      $this->currentUser = $current_user;
+      $this->userStorage = $entity_type_manager->getStorage('user');
+      $this->tfaSettings = $config_factory->get('tfa.settings');
+      $this->userData = $user_data;
+      $this->tfaValidationManager = $tfa_validation_manager;
+      $this->tfaLoginManager = $tfa_plugin_manager;
+    }
+
+    /**
+     * Redirect users to TFA overview when no remaining skips.
+     *
+     * @param \Symfony\Component\HttpKernel\Event\RequestEvent $event
+     *   The request event.
+     */
+    public function redirect(RequestEvent $event): void {
+      /** @var \Drupal\user\UserInterface $user */
+      $user = $this->userStorage->load($this->currentUser->id());
+      $this->setUser($user);
+
+      if ($this->isReady() || !$this->forceTFASetup()) {
+          return;
+      }
+
+      $this->messenger->addWarning(t('You need to enable Two Factor Authentication.'));
+
+      // Don't redirect the user if on password/profile edit page,
+      // as it is possible the user used one-time login URL
+      // and need to change the password.
+      $ignored_route_names = [
+        'user.login',
+        'user.logout',
+        'user.pass',
+        'user.edit',
+        'entity.user.edit_form',
+        'user.reset.login',
+        'user.reset',
+        'user.reset.form',
+        'user.well-known.change_password',
+        'tfa.entry',
+        'tfa.login',
+        'tfa.overview',
+        'tfa.validation.setup',
+        'tfa.disable',
+        'tfa.plugin.reset',
+      ];
+      if (in_array($this->routeMatch->getRouteName(), $ignored_route_names)) {
+          return;
+      }
+
+      $tfa_overview_url = Url::fromRoute('tfa.overview', ['user' => $this->user->id()]);
+      $event->setResponse(new RedirectResponse($tfa_overview_url->toString()));
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public static function getSubscribedEvents() {
+      $events[KernelEvents::REQUEST][] = ['redirect', 32];
+      return $events;
+    }
+
+}

- Removes the behaviour to hide allowed validation plugins - i.e. this chunk:

@@ -186,7 +194,14 @@ class SettingsForm extends ConfigFormBase {
       '#default_value' => $config->get('allowed_validation_plugins') ?: ['tfa_totp'],
       '#description' => $this->t('Plugins that can be setup by users for various TFA processes.'),
       // Show only when TFA is enabled.
-      '#states' => $enabled_state,
+      '#states' => [
+        'visible' => [
+          [
+            ':input[name="tfa_enabled"]' => ['checked' => TRUE],
+            ':input[name="tfa_forced"]' => ['checked' => FALSE],
+          ],
+        ],
+      ],
       '#required' => TRUE,
     ];
     $form['tfa_validate'] = [

- Fixes the typo "FTA":

+      '#description' => $this->t('Force TFA setup on login, redirect user to FTA overview page.'),

... to:

+      '#description' => $this->t('Force TFA setup on login, redirect user to TFA overview page.'),

alexharries → created an issue.

Just coming back to say that I've run into this problem again and Googling the issue brought me back to my own post which I'd forgotten I'd made some time ago

/Alex

alexharries → created an issue.

+1 to the patch at #13 - this issue is preventing me from adjusting the order of elements on the taxonomy term edit form.

Thank you @mrweiner :)

/A

Thank you. It’s possibly-related to #5 but I don’t suspect it’s a duplicate. :-)

/A

May be linked to #2528214 🐛 "Restrict images to this site" blocks image style derivatives Needs work .

First patch had a coding error :) Attaching new patch.

alexharries → created an issue.

Thank you @Grevil and @Anybody for fixing this so quickly :)

/A

Patch attached for 8.x-1.x-dev.

alexharries → created an issue.

alexharries → created an issue.

Patch attached.

alexharries → created an issue.

Thank you, @anybody - I filed #3278738 and was considering having a go at creating a patch to convert to the correct template approach, but simply haven't managed to find the time yet (and I expect many other folks yourself included could probably write a better patch and better-written test coverage, in fairness...).

Unless a fail-safe automated way to migrate to Layout Paragraphs is possible, and the EX stays more-or-less the same, we won't be migrating at all.

Following with interest. :)

A

Thank you @Occupant - that module is perfect for our needs.

A

Just posting that this has happened on our D9 sites too.

Surely there's a more-elegant way for Migrate Tools to have made this breaking change?

alexharries → created an issue.

alexharries → created an issue.

alexharries → created an issue.

alexharries → created an issue.

So it looks like there is a sandbox module which should provide this functionality:

COOKiES Google Tag Manager Consent: https://www.drupal.org/sandbox/gresko8/3267405 →

I am posting an issue in the project's queue to ask if we can help to get the module out of the sandbox.

I wonder whether it might be useful to post a link to this sandbox project on the COOKiES homepage, as it will be a crucial link between the COOKiES module and the Google Tag module for users in e.g. the European Union who need to implement Google's Consent Mode in order to get accurate analytics tracking data.

/Al

Thank you @tijsdeboeck.

So it looks like there is a sandbox module which should provide this functionality:
https://www.drupal.org/sandbox/gresko8/3267405 →

Downloading and testing now - many thanks.

/Al

Just to clarify the link in #4 should be:

https://www.drupal.org/sandbox/gresko8/3267405 →

Downloading and testing now - many thanks.

/Al

Thank you @Anybody.

The issue here I think (/assume) is that there might need to be some change in how the GTag module loads the Google Tag script in order to let Google know that the user has declined consent.

When the COOKiES module is installed, the GTag module can determine if the user has declined consent from the COOKiES module - how I don't yet know - but it would be this check and the subsequent change to how the Google Tag is loaded which would presumably either need to happen in the GTag module, or potentially through a couple of strategically-placed hook alters in the GTag module which an external module could make use of to change how the tag script is loaded.

All of the above is vapourware/presumption until we can get a clear understanding of how this might work and what capability we already have in the GTag and COOKiES modules.

¯\_(ツ)_/¯

/Al

I'm sad to see there hasn't been a response to your suggestion @JFeltkamp.

I'm unfortunately not knowledgeable enough about the Google Tag module to know whether I can help you code up a solution, but the organisation I work for use Google Tag and COOKiES and we have received a request to evaluate whether using Google's Consent Mode can be achieved.

If we can create a patch for the GTag module in-house which we think might have some benefit to other users, we will be sure to post it here.

Following this topic in the hope that someone much more capable comes up with a decent solution in the meantime!

/Al

I can confirm this is a problem with "Restore client IP address".

Will update the issue title and description accordingly.

alexharries → created an issue.