IDP token introspection #229
Conversation
vincent-stytch
commented
Jan 10, 2025
•
vincent-stytch
commented
stytch/consumer/api/idp.py
Outdated
| self.jwks_client = jwks_client | ||
| self.project_id = project_id | ||
|
|
||
| def introspect_idp_access_token( |
There was a problem hiding this comment.
We don't need introspect_idp_access_token since unlike Session JWTs there is no case where an access token can fail local validation and pass remote validation. If the access token is expired locally, it is also guaranteed to be expired serverside.
stytch/consumer/api/idp.py
Outdated
| return AccessTokenJWTClaims( | ||
| subject=jwtResponse.sub, | ||
| scope=jwtResponse.scope, | ||
| custom_claims={}, |
There was a problem hiding this comment.
We should return custom claims that we retrieve from the introspection response here
stytch/consumer/api/idp.py
Outdated
| token_type_hint: str = "access_token", | ||
| ) -> Optional[AccessTokenJWTClaims]: | ||
| """Introspects a token JWT from an authorization code response. | ||
| Access tokens and refresh tokens are JWTs signed with the project's JWKs. |
There was a problem hiding this comment.
Refresh Tokens are not JWTs - this endpoint supports both Access Token JWTs and Refresh Token opaque tokens
stytch/consumer/api/idp.py
Outdated
| access_token, client_id, client_secret, token_type_hint | ||
| ) | ||
|
|
||
| def introspect_idp_access_token_network( |
There was a problem hiding this comment.
Naming nits:
- Since the classname is already
idp- do we need the secondidpin the name?idp.introspect_idp_could beidp.introspect_ - Since this works for both access and refresh tokens, suggest removing
access_from the name
I think introspect_token_network works better.
stytch/consumer/api/idp.py
Outdated
| Access tokens contain a standard set of claims as well as any custom claims generated from templates. | ||
|
|
||
| Fields: | ||
| - access_token: The access token (or refresh token) to introspect. |
There was a problem hiding this comment.
If this field takes in both access tokens and refresh tokens, we should name it token
stytch/consumer/api/idp.py
Outdated
| not_before=jwtResponse.nbf, | ||
| ) | ||
|
|
||
| def introspect_idp_access_token_local( |
There was a problem hiding this comment.
in contrast this method is only valid for access tokens
stytch/core/http/client.py
Outdated
| @@ -66,6 +68,17 @@ def post( | |||
| resp = requests.post(url, json=json, headers=final_headers, auth=self.auth) | |||
| return self._response_from_request(resp) | |||
|
|
|||
| def postForm( | |||
There was a problem hiding this comment.
can we name this post_form instead? We use snake case everywhere else in this package
| final_headers = self.headers.copy() | ||
| final_headers.update(headers or {}) | ||
| resp = requests.post(url, data=form, headers=final_headers, auth=self.auth) | ||
| return self._response_from_request(resp) |
There was a problem hiding this comment.
Why does one flavor of post_form use _response_from_post_form_request and the other not?
There was a problem hiding this comment.
the sync flavor uses the requests library which plays well with _response_from_request, but async's aiohttp doesn't play well here
