-
Notifications
You must be signed in to change notification settings - Fork 53
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
[hotfix] add naver email, nickname fields(optinal). #153
Conversation
if require email and nickname, must check its.
fastapi_sso/sso/naver.py
Outdated
@@ -25,7 +25,8 @@ async def get_discovery_document(self) -> DiscoveryDocument: | |||
async def openid_from_response(self, response: dict, session: Optional["httpx.AsyncClient"] = None) -> OpenID: | |||
return OpenID( | |||
id=response["response"]["id"], | |||
display_name=response["response"]["nickname"], | |||
email=response["response"]["email"] or None, |
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.
Will the field still be there? Maybe response["response"].get("email")
would be safer (same for the other fields), what do you say?
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.
Actually, I didn't know much about this part. If response["response"].get() is used, will None be automatically returned in fields that do not exist?
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.
That's 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.
The signature of get
is like get(name, default = None)
, if "name" is not present in the dict, default
is returned (None
if not specified)
No problem, thanks! I'd just maybe use |
I used the .get method as suggested. If there is no field after searching, None is returned if there is no separate setting. thanks for letting me know. And thank you for telling me to do well on the exam! |
fastapi_sso/sso/naver.py
Outdated
@@ -25,7 +25,8 @@ async def get_discovery_document(self) -> DiscoveryDocument: | |||
async def openid_from_response(self, response: dict, session: Optional["httpx.AsyncClient"] = None) -> OpenID: | |||
return OpenID( | |||
id=response["response"]["id"], | |||
display_name=response["response"]["nickname"], | |||
email=response["response"]["email"] or None, |
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.
That's right!
fastapi_sso/sso/naver.py
Outdated
@@ -25,7 +25,8 @@ async def get_discovery_document(self) -> DiscoveryDocument: | |||
async def openid_from_response(self, response: dict, session: Optional["httpx.AsyncClient"] = None) -> OpenID: | |||
return OpenID( | |||
id=response["response"]["id"], | |||
display_name=response["response"]["nickname"], | |||
email=response["response"]["email"] or None, |
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.
The signature of get
is like get(name, default = None)
, if "name" is not present in the dict, default
is returned (None
if not specified)
Sorry about its.
Naver sets the DisplayName, email, and Picture fields as optional fields. Photo options include: I didn't check this part properly when doing PR. I'm sorry for uploading a hotfix again with duplicate content.