Core & Multiple modules: activity controls

[dgirardi]

Type of change

• Feature

Description of change

This is an implementation of the activity controls infrastructure, and a refactoring of many of the current control mechanisms:

• storage access control (bidderSettings.*.storageAllowed, deviceAccess) are now routed through the accessDevice activity
• user sync controls (filterSettings) go through syncUser
• all the checks done by the GDPR enforcement module now use activities (in additions to the above, they also affect fetchBids and enrichUids)
• there are completely new checks for the transmit* and enrich* activites, for both bid adapters and RTD modules

Other information

Closes #9546
Documentations coming soon!

[robertrmartinez]

Just couple minor comments.

Beast of a PR but very very cool!

I tested it very thoroughly using mock CMP's, all the different Activities, FPD transmit and updates and it all seems to work as expected.

One minor comment which we spoke in slack about is that for storage for bidders the default is NOT ALLOW.

And there is no easy way with Activity Set Config to flip this

This does not work for example:

                allowActivities: {
                    accessDevice: {
                        default: true,
                    }
                }

Because there is special bidder handling, bidder settings still denies bidders from storage.

So a workaround is like so:

                    accessDevice: {
                        rules: [
                            {
                                condition: ({componentType, componentName}) => {
                                    return true
                                },
                                allow: true
                            }
                        ]
                    }

To allow all access to storage (or of course using standard bidderSettings.)

Which I think is fine.

Another thing we spoke about offline is moving the cfg.js logic (handling of activity setConfig stuff) to an optional module.

Lastly, When using Activity controls, many console logs come up and clutter the console. Especially when adapters use storage. We spoke a bit on some ways to suppress them but not sure there is a nice clean way besides a debounce which may not be worth doing. Users can just filter out Activity in their consoles if needed really. -Activity will do this in chrome.

Anyways, this is really really well done. Thanks @dgirardi . Great stuff

[robertrmartinez]

cfg is used twice here, inside the getConfig callback as the entire config, and inside the activities for each callback as the specific activity. Is there a reason to keep them same name or could we rename the inner one to be more clear?

[robertrmartinez]

So if exception, we default to not allow?

Which is different than the default of allow when no rule is set right?

[dgirardi]

correct, my reasoning is when it throws the pub clearly meant to do something else, so failing loudly seems the best strategy.

[robertrmartinez]

What is the reasoning behind ruleRegistry returning array vs object for these exports. I have not come across it much and curious to learn more!

[dgirardi]

it's a desperate attempt to save half a byte :) object property names are not mangled by the minifier so returning {someName, someOtherName} will keep the full names in the final bundle, but as an array it will become something like [a, b].

It's not a great technique because if the objects get just a little more complex they are unreadable as arrays, but I haven't found yet a way to tell uglify "please mangle this class" for example. I'm also open to learning more!

[robertrmartinez]

Ahhh yes makes sense. Awesome thanks and makes sense. Every byte counts when people are always complaining haha!

[dgirardi]

condition can be omitted if the rule is always active (referring to the workaround above), e.g.

setConfig({
   allowActivities: {
     accessDevice: {
        rules: [
            {
                allow: true
            }
        ]
      }
   }
})

I do plan to update this very soon with configuration logic extracted into a module and cleaner logging (yes it can be filtered out, but it'd bother me if I had to).

[dgirardi]

noting this for myself - this should be localStorageIsEnabled

[pm-harshad-mane]

Thanks @robertrmartinez for a very detailed review.
I did a high level review of this GRAND PR and it looks great to me 👍
Thanks a lot @dgirardi for working this huge and highly important feature.

[pm-harshad-mane]

@dgirardi there seems an issue with E2E tests, can you please take a look?