gtm-support / vue-gtm

Simple implementation of Google Tag Manager for Vue
https://www.npmjs.com/package/@gtm-support/vue-gtm
MIT License
198 stars 23 forks source link

Bug: nonce config does not output nonce to script tag #354

Closed Orrison closed 2 months ago

Orrison commented 1 year ago

Info

Tool Version
Plugin v2.0.0
Vue v3.2.47
Node v16.19.0
OS mac

Input

createGtm({
    id: 'GTM-XXXXXXX',
    enabled: true,
    loadScript: true,
    vueRouter: router,
    nonce: '2726c7f26c',
})

Output or Error

Nonce is not output into Script tag

Expected Output

nonce should be added to script tag created by the package

Additional Context

It looks like the nonce setting does not work, setting this to any value does not result in the nonce being output in the script tag generated by the package.

This also seems to be the same for the defer config value. Making defer true does not add defer to the script

Shinigami92 commented 1 year ago

There are tests https://github.com/gtm-support/vue-gtm/blob/18fd3374ac4a970c12aaa1cae97ae43b82a0fd6f/tests/vue-use.test.ts#L129 Could you please check if the test does not fulfill some expectations you have? Maybe we need to add another test with some specific configurations?

Orrison commented 1 year ago

@Shinigami92 hmm, that test seems to make sense and validate the expectation I would have. But this does not seem to be happening in practice outside of tests.

My configuration is simple, only slightly different from the documentation.

import { createApp } from 'vue';
import App from './App.vue';
import { IonicVue } from '@ionic/vue';
import { createPinia } from 'pinia';
import router from './router';

const pinia = createPinia();

const app = createApp(App)
    .use(IonicVue)
    .use(pinia)
    .use(router)
    .use(
        createGtm({
            id: 'GTM-DEMO',
            enabled: true,
            loadScript: true,
            vueRouter: router,
            nonce: '2726c7f26c',
        })
    );

With this configuration, the output script tag does NOT include the nonce in practice.

<script async="" src="https://www.googletagmanager.com/gtm.js?id=GTM-DEMO"></script>

Even if I remove all my other configuration to simplify this:

import { createApp } from 'vue';
import App from './App.vue';

const app = createApp(App)
    .use(
        createGtm({
            id: 'GTM-DEMO',
            enabled: true,
            loadScript: true,
            vueRouter: router,
            nonce: '2726c7f26c',
        })
    );

Sidebar: I was wrong about defer, that is working as expected.

Orrison commented 1 year ago

@Shinigami92 see a live example of this not working here: https://codesandbox.io/s/hopeful-tharp-tvv97x?file=/src/main.js

Shinigami92 commented 1 year ago

Here the nonce will be set: https://github.com/gtm-support/core/blob/9391b2d691dcff0dd75446c4ddbce3b916e60652/src/utils.ts#L95-L97 I tried to tracked the path, but just in GitHub without checking out the project To me I could not identify any issue :thinking:

Would you like to checkout the repo (core) and try to debug it? It's really not much code, you just need to track down the options with nonce.

Shinigami92 commented 11 months ago

@Shinigami92 As a a side note, if I understand the vue-gtm code base correctly, the script is only added in a browser context anyways, so the purpose of using a CSP nonce is kinda defeated or not?

I just quickly read through https://content-security-policy.com/nonce/ to check what CSP nonce are. Based on that, I'm not sure what you mean by that it is defeated 🤔

Orrison commented 11 months ago

@Shinigami92 As a a side note, if I understand the vue-gtm code base correctly, the script is only added in a browser context anyways, so the purpose of using a CSP nonce is kinda defeated or not?

I just quickly read through https://content-security-policy.com/nonce/ to check what CSP nonce are. Based on that, I'm not sure what you mean by that it is defeated 🤔

Hey @Shinigami92, unsure of which comment you are replying to here.

The intention of a nonce is to generate it every time on the page load server side and supply it to the GTAG script so that it can set the nonce for the scripts it contains. This provides security and is a great way to have a robust CSP policy while still allowing GTAG managers to add scripts without consulting development.

This package offers a option to set nonce and says "// Will add nonce to the script tag". Though the test is passing it seems that in a real usage, the nonce is not being set on the Script tag created by the package . As you can see in my example above.

By setting the nonce attribute on the script tag calling the GTAG, GTAG is able to supply that to the scripts it will load onto the page. This is a GTAG functionality that works in any other context where a GTAG script is loaded onto the page and executed.

Shinigami92 commented 11 months ago

@Orrison you can ignore the comment. Someone posted a comment (you can read the comment in the quote of my previous post) and then deleted the comment again. I assume because they realized that the question was incorrect at all. That deletion just confused you, sorry for that 🙂

Orrison commented 11 months ago

@Orrison you can ignore the comment. Someone posted a comment (you can read the comment in the quote of my previous post) and then deleted the comment again. I assume because they realized that the question was incorrect at all. That deletion just confused you, sorry for that 🙂

Got it, no problem at all!

dariasamo commented 3 months ago

@Shinigami92 it seems like the problem is here where nonce is set as a property not as attribute, so it doesn't get synced to the script tag. If this line is changed to the following it seems to work as expected: script.setAttribute('nonce', config.nonce);

Shinigami92 commented 3 months ago

@Shinigami92 it seems like the problem is here where nonce is set as a property not as attribute, so it doesn't get synced to the script tag. If this line is changed to the following it seems to work as expected: script.setAttribute('nonce', config.nonce);

Would you like to open a PR? Is it possible to write a test for this?