Add BidStreamClient and SharingClient by asloobq · Pull Request #31 · IABTechLab/uid2-client-python

asloobq · 2024-03-19T19:26:59Z

Introduce two new clients BidStreamClient and SharingClient and deprecate the existing Uid2PublisherClient.

Accept parameter domain_name for decrypting ad tokens, but allow None for now. The actual domain_name checks will follow in a future PR
Change encrypt / decrypt / refresh interface to return a *response object always that contains the value or exception reason. Keeps it consistent with C# SDK
Checks if bidstream tokens and sharing tokens have a valid lifetime as returned from operator
Return expiry_time in response if decryption fails with TOKEN_EXPIRED error
Returns if decrypted token was client side generated and whether its email/phone
Fixed a bug where created_at was being set to 1 hr earlier
Included a fix from https://github.com/IABTechLab/uid2-client-python/pull/10/files to generate consistent prefix

asloobq · 2024-03-19T19:30:22Z

uid2_client/client.py

        """
-        return encryption.decrypt(token, self._keys)
-
-    def _parse_keys_json(self, resp_body):


moved to refresh_keys_util.py

asloobq · 2024-03-19T19:30:30Z

uid2_client/client.py

        Returns:
            EncryptionKeysCollection containing the keys
        """
-        req, nonce = make_v2_request(self._secret_key, dt.datetime.now(tz=timezone.utc))


moved to refresh_keys_util.py

examples/sample_bidstream_client.py

uid2_client/bid_stream_client.py

mcollins-ttd · 2024-03-27T03:53:01Z

uid2_client/client.py

I think this should remain unchanged

thanks, I missed this

mcollins-ttd · 2024-03-27T04:02:39Z

uid2_client/sharing_client.py

Ideally we don't want to have a now parameter on these public methods. In the .NET SDK the DateTime param is on the internal method.

good catch. I missed that the C# method is internal. Changed the method signature

mcollins-ttd · 2024-03-27T04:05:08Z

uid2_client/sharing_client.py

Default argument values are evaluated at the point of function definition, so now doesn't behave the way we want it to. This is a per-existing issue but we should fix it, at least in new code.

very interesting. I didn't know this and assumed they are evaluated when the function is invoked. Thanks!

mcollins-ttd · 2024-03-27T04:06:35Z

uid2_client/publisher_client.py

@@ -1,7 +1,4 @@
-"""Internal module for holding the Uid2Client class.
-
-Do not use this module directly, import through uid2_client module instead, e.g.


Why was this comment removed?

The UID2Client class is being deprecated and participants can use this directly. The publisher guide mentions this

mcollins-ttd · 2024-03-27T04:08:51Z

uid2_client/refresh_keys_util.py

Why is this inside the keys loop?

that's right, it doesn't need to be in the for loop. My indentation is incorrect

mcollins-ttd · 2024-03-27T04:11:36Z

uid2_client/refresh_keys_util.py

Can we do

keyset_id = key.get("keyset_id")

yes, that's more elegant

caroline-ttd · 2024-03-28T01:44:02Z

pyproject.toml

Just curious, when do we need to change the version?

@cYKatherine @thomasm-ttd is this handled by the github action now, so we shouldn't change version manually like this?

caroline-ttd · 2024-03-28T01:59:05Z

uid2_client/refresh_keys_util.py

+    if resp_body["identity_scope"] == "EUID":
+        identity_scope = IdentityScope.EUID
+    for key in resp_body["keys"]:
+        keyset_id = key.get("keyset_id")


"keyset_id" is removed from key objects. Should we also remove it here?

https://atlassian.thetradedesk.com/confluence/display/UID2/UID2-1858+DSP+Check+-+Validate+30-day+token+is+not+in+bid+stream#UID21858DSPCheckValidate30daytokenisnotinbidstream-SDKs

NVM, this function is also used in the deprecated client class.

its being removed from /key/bidstream response but it will continue to exist in /key/sharing response

caroline-ttd · 2024-03-28T02:11:09Z

uid2_client/refresh_keys_util.py

+        keyset_id = key.get("keyset_id")
+        key = EncryptionKey(key['id'],
+                            key.get('site_id', -1),
+                            _make_dt(key['created']),


Would you like to replace all method to get(), like key.get("created")?

for response fields which are necessary for the key to work, it makes sense to alert the user in case of a missing value. So key['created'] will throw an exception if the field is absent.
Using key.get("created") it will silently fail. I am assuming that alerting the user by throwing an exception is the right way to go

uid2_client/bid_stream_client.py

examples/sample_bidstream_client.py

examples/sample_sharing_client.py

tests/test_sharing_client.py

jon8787 · 2024-03-28T06:39:56Z

tests/test_sharing_client.py

Is this the PhoneTest equivalent? Let's use the same order between SDKs to make it easier

I actually did reorganize the test cases between the SDKs :)

Could you elaborate? We need to keep them in the same order to make it easier for future maintainers. If you want to propose a better order, let's discuss but you'll need to change them in all SDKs.

I just meant that I sync'd the tests in python SDK with the C# one. They were not in the same order in Python SDK.

tests/test_sharing_client.py

jon8787 · 2024-04-03T06:23:02Z

examples/sample_bidstream_client.py

identity_scope belongs at the BidstreamClient level rather than the token level. It's probably not needed there either since it's the consumer who chooses whether to connect to EUID vs UID2.

Ok. will remove this

Seems you've still left some identity_scopes at the token level, could you please check?

tests/test_sharing_client.py

mcollins-ttd · 2024-04-05T03:23:31Z

uid2_client/bid_stream_client.py

Should this be named bidstream_client.py?

mcollins-ttd · 2024-04-05T03:24:14Z

uid2_client/bid_stream_client.py

s/BidStream/Bidstream/

mcollins-ttd · 2024-04-05T03:25:08Z

uid2_client/bid_stream_client.py

decrypt_token_into_raw_uid

mcollins-ttd · 2024-04-05T03:26:00Z

uid2_client/bid_stream_client.py

BidstreamClient

mcollins-ttd · 2024-04-05T03:30:48Z

uid2_client/encryption.py

Bidstream / decrypt_token_into_raw_uid

mcollins-ttd · 2024-04-05T03:32:10Z

uid2_client/bid_stream_client.py

decrypt_token_into_raw_uid

mcollins-ttd · 2024-04-05T03:33:48Z

uid2_client/client_type.py

Should we make these UPPER_CASE to match DecryptionStatus enum values?

Yes. I copied it from C# SDk and forgot to change

mcollins-ttd · 2024-04-05T03:38:15Z

uid2_client/encryption_data_response.py

Can we delete this line

mcollins-ttd · 2024-04-05T04:03:47Z

examples/sample_sharing.py

Isn't this still called refresh_keys?

mcollins-ttd · 2024-04-05T04:11:00Z

uid2_client/encryption.py

    """

-    def __init__(self, uid2, established, site_id, site_key_site_id):
-        self.uid2 = uid2


This makes this release a breaking change, so we need to bump the major version.

Thanks for pointing this out. I was trying not to break any existing functionality, for now I added a property alias uid2 for existing clients to work. At the same time the overall changes in the PR look big enough to me to warrant a major version upgrade.

asloobq commented Mar 19, 2024

View reviewed changes

asloobq marked this pull request as ready for review March 26, 2024 00:34

mcollins-ttd reviewed Mar 27, 2024

View reviewed changes

caroline-ttd reviewed Mar 28, 2024

View reviewed changes

uid2_client/bid_stream_client.py Outdated Show resolved Hide resolved

jon8787 reviewed Mar 28, 2024

View reviewed changes

jon8787 reviewed Apr 3, 2024

View reviewed changes

asloobq mentioned this pull request Apr 4, 2024

[UID2-3105] Update python SDK guide with new interface for DSP check IABTechLab/uid2docs#511

Merged

mcollins-ttd reviewed Apr 5, 2024

View reviewed changes

asloobq added 18 commits April 8, 2024 15:05

Add BidStreamClient and SharingClient

0d19378

Check token lifetime in bidstream client

08629c2

Fix Bidstream client to call the correct key fetch API

b763e62

Revert unintended change in encryption.py

7c83750

Fix existing tests

66a4bb8

Fix creation of key collection and added unit tests

a251134

Add unit tests for bidstream and sharing client

ea8bd0a

Set default value for allow clock skew seconds

9070b91

Raise an exception when invalid token lifetime is met

21d9537

Add test cases for invalid lifetime check

c30dbe4

Add token version and scope to decryption response

9ca5aea

Simplify logic for checking clock skew

d1d7030

Add more test cases to sharing client

5bfd9fe

Tests refactor

24136f3

More test cases for bidstream client

e7334a3

Deprecate encryptData and decryptData methods

a871524

Add optional parameter for encrypt/decrypt to specify timestamp

0899c54

Add new test case and fix checking exception in the unit tests

c35abb1

asloobq and others added 26 commits April 8, 2024 15:05

throw error if token size less than 4

b449b20

Fix unittest exception checking

e921197

bump version

cb9ddd6

changed signatures to match C# SDK and fixed time bug

8812e6e

Moved encrypt decrypt methods with timestamp to internal

d4eca84

Fix bug which set token creation time 1hr earlier

304319f

Fix bug which set token creation time 1hr earlier

f17901c

Address some review comments

4c7a471

Don't throw exception. Reply with encryption-decryption response

c49be71

don't throw exception for encryption errors

cfbda5c

Expose expiry time in decryption response

5918b26

Store identity type in decrypted token

e4434af

Store is CSTG token in response

12de071

Return refresh response instead of throwing exception

3d6403b

More review comments

54e8896

Some cleanup

e3ac39a

Complete bidstream tests

d4bcab5

Add internal refreshJson method to Bidstream client

828a8e8

expose refresh_json in sharing_client and use for testing

667eed7

Make fetching identity scope optional

b416319

refactor tests

846d65a

Respond with encryption error status enums

e6e9a28

Cleaned up code

1e33190

renamed bid_stream_client.py

8896006

Address review comments

3c3cf97

Rename encryption status

588e980

asloobq force-pushed the aaq-UID2-2877-dsp-check-client-refactor branch from 89738e5 to 588e980 Compare April 8, 2024 22:06

jon8787 approved these changes Apr 8, 2024

View reviewed changes

asloobq merged commit f1329a4 into main Apr 10, 2024

asloobq deleted the aaq-UID2-2877-dsp-check-client-refactor branch April 10, 2024 00:05

		@@ -1,7 +1,4 @@
		"""Internal module for holding the Uid2Client class.

		Do not use this module directly, import through uid2_client module instead, e.g.

Conversation

asloobq commented Mar 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

asloobq commented Mar 19, 2024 •

edited

Loading