mirror of https://github.com/langgenius/dify.git
fix: handle null email/name from GitHub API for private-email users (#33882)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: crazywoola <100913391+crazywoola@users.noreply.github.com> Co-authored-by: QuantumGhost <obelisk.reg+git@gmail.com>
This commit is contained in:
parent
abd68d2ea6
commit
244f9e0c11
|
|
@ -1,4 +1,5 @@
|
||||||
import logging
|
import logging
|
||||||
|
import urllib.parse
|
||||||
|
|
||||||
import httpx
|
import httpx
|
||||||
from flask import current_app, redirect, request
|
from flask import current_app, redirect, request
|
||||||
|
|
@ -112,6 +113,9 @@ class OAuthCallback(Resource):
|
||||||
error_text = e.response.text
|
error_text = e.response.text
|
||||||
logger.exception("An error occurred during the OAuth process with %s: %s", provider, error_text)
|
logger.exception("An error occurred during the OAuth process with %s: %s", provider, error_text)
|
||||||
return {"error": "OAuth process failed"}, 400
|
return {"error": "OAuth process failed"}, 400
|
||||||
|
except ValueError as e:
|
||||||
|
logger.warning("OAuth error with %s", provider, exc_info=True)
|
||||||
|
return redirect(f"{dify_config.CONSOLE_WEB_URL}/signin?message={urllib.parse.quote(str(e))}")
|
||||||
|
|
||||||
if invite_token and RegisterService.is_valid_invite_token(invite_token):
|
if invite_token and RegisterService.is_valid_invite_token(invite_token):
|
||||||
invitation = RegisterService.get_invitation_by_token(token=invite_token)
|
invitation = RegisterService.get_invitation_by_token(token=invite_token)
|
||||||
|
|
|
||||||
|
|
@ -1,16 +1,19 @@
|
||||||
|
import logging
|
||||||
import sys
|
import sys
|
||||||
import urllib.parse
|
import urllib.parse
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from typing import NotRequired
|
from typing import NotRequired
|
||||||
|
|
||||||
import httpx
|
import httpx
|
||||||
from pydantic import TypeAdapter
|
from pydantic import TypeAdapter, ValidationError
|
||||||
|
|
||||||
if sys.version_info >= (3, 12):
|
if sys.version_info >= (3, 12):
|
||||||
from typing import TypedDict
|
from typing import TypedDict
|
||||||
else:
|
else:
|
||||||
from typing_extensions import TypedDict
|
from typing_extensions import TypedDict
|
||||||
|
|
||||||
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
JsonObject = dict[str, object]
|
JsonObject = dict[str, object]
|
||||||
JsonObjectList = list[JsonObject]
|
JsonObjectList = list[JsonObject]
|
||||||
|
|
||||||
|
|
@ -30,8 +33,8 @@ class GitHubEmailRecord(TypedDict, total=False):
|
||||||
class GitHubRawUserInfo(TypedDict):
|
class GitHubRawUserInfo(TypedDict):
|
||||||
id: int | str
|
id: int | str
|
||||||
login: str
|
login: str
|
||||||
name: NotRequired[str]
|
name: NotRequired[str | None]
|
||||||
email: NotRequired[str]
|
email: NotRequired[str | None]
|
||||||
|
|
||||||
|
|
||||||
class GoogleRawUserInfo(TypedDict):
|
class GoogleRawUserInfo(TypedDict):
|
||||||
|
|
@ -127,9 +130,14 @@ class GitHubOAuth(OAuth):
|
||||||
response.raise_for_status()
|
response.raise_for_status()
|
||||||
user_info = GITHUB_RAW_USER_INFO_ADAPTER.validate_python(_json_object(response))
|
user_info = GITHUB_RAW_USER_INFO_ADAPTER.validate_python(_json_object(response))
|
||||||
|
|
||||||
email_response = httpx.get(self._EMAIL_INFO_URL, headers=headers)
|
try:
|
||||||
email_info = GITHUB_EMAIL_RECORDS_ADAPTER.validate_python(_json_list(email_response))
|
email_response = httpx.get(self._EMAIL_INFO_URL, headers=headers)
|
||||||
primary_email = next((email for email in email_info if email.get("primary") is True), None)
|
email_response.raise_for_status()
|
||||||
|
email_info = GITHUB_EMAIL_RECORDS_ADAPTER.validate_python(_json_list(email_response))
|
||||||
|
primary_email = next((email for email in email_info if email.get("primary") is True), None)
|
||||||
|
except (httpx.HTTPStatusError, ValidationError):
|
||||||
|
logger.warning("Failed to retrieve email from GitHub /user/emails endpoint", exc_info=True)
|
||||||
|
primary_email = None
|
||||||
|
|
||||||
return {**user_info, "email": primary_email.get("email", "") if primary_email else ""}
|
return {**user_info, "email": primary_email.get("email", "") if primary_email else ""}
|
||||||
|
|
||||||
|
|
@ -137,8 +145,11 @@ class GitHubOAuth(OAuth):
|
||||||
payload = GITHUB_RAW_USER_INFO_ADAPTER.validate_python(raw_info)
|
payload = GITHUB_RAW_USER_INFO_ADAPTER.validate_python(raw_info)
|
||||||
email = payload.get("email")
|
email = payload.get("email")
|
||||||
if not email:
|
if not email:
|
||||||
email = f"{payload['id']}+{payload['login']}@users.noreply.github.com"
|
raise ValueError(
|
||||||
return OAuthUserInfo(id=str(payload["id"]), name=str(payload.get("name", "")), email=email)
|
'Dify currently not supports the "Keep my email addresses private" feature,'
|
||||||
|
" please disable it and login again"
|
||||||
|
)
|
||||||
|
return OAuthUserInfo(id=str(payload["id"]), name=str(payload.get("name") or ""), email=email)
|
||||||
|
|
||||||
|
|
||||||
class GoogleOAuth(OAuth):
|
class GoogleOAuth(OAuth):
|
||||||
|
|
|
||||||
|
|
@ -95,13 +95,11 @@ class TestGitHubOAuth(BaseOAuthTest):
|
||||||
],
|
],
|
||||||
"primary@example.com",
|
"primary@example.com",
|
||||||
),
|
),
|
||||||
# User with no emails - fallback to noreply
|
# User with private email (null email and name from API)
|
||||||
({"id": 12345, "login": "testuser", "name": "Test User"}, [], "12345+testuser@users.noreply.github.com"),
|
|
||||||
# User with only secondary email - fallback to noreply
|
|
||||||
(
|
(
|
||||||
{"id": 12345, "login": "testuser", "name": "Test User"},
|
{"id": 12345, "login": "testuser", "name": None, "email": None},
|
||||||
[{"email": "secondary@example.com", "primary": False}],
|
[{"email": "primary@example.com", "primary": True}],
|
||||||
"12345+testuser@users.noreply.github.com",
|
"primary@example.com",
|
||||||
),
|
),
|
||||||
],
|
],
|
||||||
)
|
)
|
||||||
|
|
@ -118,9 +116,54 @@ class TestGitHubOAuth(BaseOAuthTest):
|
||||||
user_info = oauth.get_user_info("test_token")
|
user_info = oauth.get_user_info("test_token")
|
||||||
|
|
||||||
assert user_info.id == str(user_data["id"])
|
assert user_info.id == str(user_data["id"])
|
||||||
assert user_info.name == user_data["name"]
|
assert user_info.name == (user_data["name"] or "")
|
||||||
assert user_info.email == expected_email
|
assert user_info.email == expected_email
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
("user_data", "email_data"),
|
||||||
|
[
|
||||||
|
# User with no emails
|
||||||
|
({"id": 12345, "login": "testuser", "name": "Test User"}, []),
|
||||||
|
# User with only secondary email
|
||||||
|
(
|
||||||
|
{"id": 12345, "login": "testuser", "name": "Test User"},
|
||||||
|
[{"email": "secondary@example.com", "primary": False}],
|
||||||
|
),
|
||||||
|
# User with private email and no primary in emails endpoint
|
||||||
|
(
|
||||||
|
{"id": 12345, "login": "testuser", "name": None, "email": None},
|
||||||
|
[],
|
||||||
|
),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
@patch("httpx.get", autospec=True)
|
||||||
|
def test_should_raise_error_when_no_primary_email(self, mock_get, oauth, user_data, email_data):
|
||||||
|
user_response = MagicMock()
|
||||||
|
user_response.json.return_value = user_data
|
||||||
|
|
||||||
|
email_response = MagicMock()
|
||||||
|
email_response.json.return_value = email_data
|
||||||
|
|
||||||
|
mock_get.side_effect = [user_response, email_response]
|
||||||
|
|
||||||
|
with pytest.raises(ValueError, match="Keep my email addresses private"):
|
||||||
|
oauth.get_user_info("test_token")
|
||||||
|
|
||||||
|
@patch("httpx.get", autospec=True)
|
||||||
|
def test_should_raise_error_when_email_endpoint_fails(self, mock_get, oauth):
|
||||||
|
user_response = MagicMock()
|
||||||
|
user_response.json.return_value = {"id": 12345, "login": "testuser", "name": "Test User"}
|
||||||
|
|
||||||
|
email_response = MagicMock()
|
||||||
|
email_response.raise_for_status.side_effect = httpx.HTTPStatusError(
|
||||||
|
"Forbidden", request=MagicMock(), response=MagicMock()
|
||||||
|
)
|
||||||
|
|
||||||
|
mock_get.side_effect = [user_response, email_response]
|
||||||
|
|
||||||
|
with pytest.raises(ValueError, match="Keep my email addresses private"):
|
||||||
|
oauth.get_user_info("test_token")
|
||||||
|
|
||||||
@patch("httpx.get", autospec=True)
|
@patch("httpx.get", autospec=True)
|
||||||
def test_should_handle_network_errors(self, mock_get, oauth):
|
def test_should_handle_network_errors(self, mock_get, oauth):
|
||||||
mock_get.side_effect = httpx.RequestError("Network error")
|
mock_get.side_effect = httpx.RequestError("Network error")
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue