M2M Manual methods #152

vincent-stytch · 2023-08-09T16:58:15Z

Add M2M Manual methods

logan-stytch · 2023-08-09T17:08:02Z

stytch/b2b/api/sessions.py

    def authenticate_jwt_local(
        self,
        session_jwt: str,
        max_token_age_seconds: Optional[int] = None,
        leeway: int = 0,
-    ) -> Optional[MemberSession]:
+    ) -> Optional[Tuple[Dict[str, Any], Dict[str, Any]]]:


Generally I don't really care for Tuple because it's such a weak type (you have to just know what the ordering of returned items means).

Furthermore, you're changing the return type of the public interface, so this will break someone's flow. I think it's okay if you make some other helper function (you could consider a shared/ folder like we do for stytch-node), but this interface should not change.

logan-stytch · 2023-08-09T17:08:23Z

stytch/b2b/api/sessions.py

+    # ENDMANUAL(authenticate_jwt_local)
+
+    # MANUAL(authenticate_m2m_jwt_local)(SERVICE_METHOD)
+    # ADDIMPORT: import time


[sand] import time isn't used in this function

logan-stytch · 2023-08-09T18:51:32Z

stytch/b2b/models/sessions.py

+    def __init__(self, sub, scope, custom_claims):
+        self.sub = sub
+        self.scope = scope
+        self.custom_claims = custom_claims


Add typing for everything

But this method is unnecessary -- anything inheriting from pydantic.BaseModel will automatically be parseable and get an __init__ method for free

But also this isn't a ResponseBase -- you're not going to have a status_code or request_id. Make this inherit from pydantic.BaseModel instead

logan-stytch · 2023-08-09T18:52:06Z

stytch/consumer/api/m2m.py

+        client_secret: str,
+        scopes: Optional[List[str]] = None,
+    ) -> Optional[GetTokenResponse]:
+        """Rtrieves an access token for the given M2M Client.


[dust] spelling

Suggested change

"""Rtrieves an access token for the given M2M Client.

"""Retrieves an access token for the given M2M Client.

logan-stytch · 2023-08-09T18:52:39Z

stytch/consumer/api/m2m.py

+            data["scope"] = " ".join(scopes)
+
+        url = self.api_base.url_for(
+            "/v1/public/" + self.project_id + "/oauth2/token", data


Use an f-string

logan-stytch · 2023-08-09T18:57:29Z

stytch/consumer/api/m2m.py

+        if required_scopes:
+            missing_scopes = filter(lambda scope: scope not in scopes, required_scopes)
+            if missing_scopes:
+                return None


Use comprehensions instead of map/filter/reduce. Also, this can all be simplified.

One more nit-picky thing: I prefer to be really explicit with conditionals instead of relying on "truthy" values

required_scopes = required_scopes or [] missing_scopes = [scope for scope in scopes if scope not in required_scopes] if len(missing_scopes) != 0: return None

logan-stytch · 2023-08-09T18:58:05Z

stytch/consumer/api/sessions.py

    def authenticate_jwt_local(
        self,
        session_jwt: str,
        max_token_age_seconds: Optional[int] = None,
        leeway: int = 0,
-    ) -> Optional[Session]:
+    ) -> Optional[Tuple[Dict[str, Any], Dict[str, Any]]]:


Not a fan of tuples

This breaks a public interface; let's create a shared helper

stytch/consumer/models/m2m.py

logan-stytch · 2023-08-09T18:58:46Z

stytch/consumer/models/m2m.py

+    def __init__(self, client_id, scopes, custom_claims):
+        self.client_id = client_id
+        self.scopes = scopes
+        self.custom_claims = custom_claims


logan-stytch · 2023-08-09T18:58:56Z

stytch/consumer/models/sessions.py

+    def __init__(self, sub, scope, custom_claims):
+        self.sub = sub
+        self.scope = scope
+        self.custom_claims = custom_claims


logan-stytch

Approving to unblock. Remaining comments are really minor. Two things, though:

You need to bump version.py
Please add a screenshot of you manually testing M2M auth + sessions auth since we've refactored local authentication logic
a. And related, add a Linear ticket (you can assign to me) for adding new automated tests

logan-stytch · 2023-08-10T16:19:26Z

stytch/b2b/api/sessions.py

-
-        # Unpack the session claim to match the detached session format.
-        claim = payload[_session_claim]
+        )  # Unpack the session claim to match the detached session format.


This comment is outdated -- just remove it

logan-stytch · 2023-08-10T16:20:16Z

stytch/consumer/api/m2m.py

        self.clients = Clients(api_base, sync_client, async_client)
+
+    # MANUAL(m2m.token)(SERVICE_METHOD)
+    # ADDIMPORT: from typing import Any, Dict, List


You also use Optional here

logan-stytch · 2023-08-10T16:21:11Z

stytch/consumer/api/m2m.py

+        Fields:
+          - client_id: The ID of the client.
+          - client_secret: The secret of the client.
+          - scopes: An array scopes requested. If omitted, all scopes assigned to the client will be returned.


[sand] missing of in description

Suggested change

- scopes: An array scopes requested. If omitted, all scopes assigned to the client will be returned.

- scopes: An array of scopes requested. If omitted, all scopes assigned to the client will be returned.

logan-stytch · 2023-08-10T16:22:48Z

stytch/consumer/api/sessions.py

-
-        # Unpack the session claim to match the detached session format.
-        claim = payload[_session_claim]
+        )  # Unpack the session claim to match the detached session format.


This comment is outdated -- just remove it

logan-stytch · 2023-08-10T16:23:12Z

stytch/consumer/api/sessions.py

@@ -279,76 +280,33 @@ async def authenticate_jwt_async(
    # MANUAL(authenticate_jwt_local)(SERVICE_METHOD)
    # ADDIMPORT: import time


This import is no longer used

stytch/consumer/models/m2m.py

logan-stytch · 2023-08-10T16:24:55Z

stytch/consumer/models/m2m.py

+
+# ENDMANUAL(GetTokenResponse)
+
+# MANUAL(M2MJWTClaims)(TYPES)


add imports for any, dict, list, and optional

logan-stytch · 2023-08-10T16:26:17Z

stytch/shared/jwt_helpers.py

+def authenticate_jwt_local(
+    *,
+    jwks_client: pyjwt.PyJWKClient,
+    project_id: str,
+    jwt: str,
+    max_token_age_seconds: Optional[int] = None,
+    leeway: int = 0,
+) -> Optional[GenericClaims]:


You know, it would be really cool if we had a stytch/shared/test/test_jwt_helpers.py file :)
I think I'm empathetic if you don't want to do that right now, but could you at least go back and add a test in this PR summary to show that m2m auth + sessions auth are working as expected?

vincent-stytch requested a review from a team as a code owner August 9, 2023 16:58

vincent-stytch requested a review from danny-stytch August 9, 2023 16:58

vincent-stytch changed the base branch from main to v/autogenbeforem2m August 9, 2023 16:58

Base automatically changed from v/autogenbeforem2m to main August 9, 2023 17:48

vincent-stytch changed the title ~~[work in progress, do not review] M2M Manual methods~~ M2M Manual methods Aug 9, 2023

logan-stytch requested changes Aug 9, 2023

View reviewed changes

Add M2M Manual Methods

6ba5fbf

vincent-stytch force-pushed the m2m2 branch from dc30cec to 6ba5fbf Compare August 9, 2023 22:32

vincent-stytch requested a review from logan-stytch August 10, 2023 15:26

logan-stytch approved these changes Aug 10, 2023

View reviewed changes

vincent-stytch added 3 commits August 10, 2023 10:39

comments

6831afd

Merge branch 'main' into m2m2

e0c664d

bump version

c9877ae

vincent-stytch merged commit 57912cb into main Aug 10, 2023

vincent-stytch deleted the m2m2 branch August 10, 2023 18:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

M2M Manual methods #152

M2M Manual methods #152

vincent-stytch commented Aug 9, 2023 •

edited

Loading

logan-stytch Aug 9, 2023

logan-stytch Aug 9, 2023

logan-stytch Aug 9, 2023

logan-stytch Aug 9, 2023

logan-stytch Aug 9, 2023

logan-stytch Aug 9, 2023

logan-stytch Aug 9, 2023

logan-stytch Aug 9, 2023

logan-stytch Aug 9, 2023

logan-stytch left a comment

logan-stytch Aug 10, 2023

logan-stytch Aug 10, 2023

logan-stytch Aug 10, 2023

logan-stytch Aug 10, 2023

logan-stytch Aug 10, 2023

logan-stytch Aug 10, 2023

logan-stytch Aug 10, 2023

	"""Rtrieves an access token for the given M2M Client.
	"""Retrieves an access token for the given M2M Client.

	- scopes: An array scopes requested. If omitted, all scopes assigned to the client will be returned.
	- scopes: An array of scopes requested. If omitted, all scopes assigned to the client will be returned.

		@@ -279,76 +280,33 @@ async def authenticate_jwt_async(
		# MANUAL(authenticate_jwt_local)(SERVICE_METHOD)
		# ADDIMPORT: import time

M2M Manual methods #152

M2M Manual methods #152

Conversation

vincent-stytch commented Aug 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

logan-stytch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vincent-stytch commented Aug 9, 2023 •

edited

Loading