-
Notifications
You must be signed in to change notification settings - Fork 39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: oauth provider #549
feat: oauth provider #549
Conversation
def redirect(self, url: str) -> BaseResponse: | ||
if not self.response_sent: | ||
self.set_header("Location", url) | ||
self.set_status_code(302) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't this need to set response_sent
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
def redirect(self, url: str) -> BaseResponse: | ||
if not self.response_sent: | ||
self.set_header("Location", url) | ||
self.set_status_code(302) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't this need to set response_sent
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
try: | ||
session = await get_session( | ||
api_options.request, | ||
False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer if this was a named param, but it doesn't matter too much.
authorization_header = api_options.request.get_header("authorization") | ||
|
||
if authorization_header is None or not authorization_header.startswith("Bearer "): | ||
api_options.response.set_header( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this needs to be ported to node
|
||
# Verify token signature using session recipe's JWKS | ||
session_recipe = SessionRecipe.get_instance() | ||
matching_keys = get_latest_keys(session_recipe.config, access_token_obj.kid) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this goes through caching right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes it does
* fix: cdi and fdi versions * fix: deps * fix: sdk version * fix: tests * fix: website reset * fix: website tests * fix: test scripts for oauth2 * fix: website tests * fix: unit test * fix: unit test * fix: django headers * fix: better types * fix: review comments * fix: test server
Summary of change
(A few sentences about this PR)
Related issues
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)
Documentation changes
(If relevant, please create a PR in our docs repo, or create a checklist here highlighting the necessary changes)
Checklist for important updates
coreDriverInterfaceSupported.json
file has been updated (if needed)supertokens_python/constants.py
frontendDriverInterfaceSupported.json
file has been updated (if needed)setup.py
supertokens_python/constants.py
git tag
) in the formatvX.Y.Z
, and then find the latest branch (git branch --all
) whoseX.Y
is greater than the latest released tag.supertokens_python/utils.py
file to include that in theFRAMEWORKS
variablesyncio
/asyncio
functions are consistent.tests/sessions/test_access_token_version.py
to account for any new claims that are optional or omitted by the coreRemaining TODOs for this PR