Skip to content
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.

Sign up for GitHub

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

Handle facebook limited login #113

Merged
merged 8 commits into from
Jan 8, 2025
Merged

Handle facebook limited login #113

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6157751
Handle facebook limited login
carlosvstokkur Aug 28, 2024
46375ea
PR Fixes
carlosvstokkur Aug 30, 2024
3a6573a
Merge remote-tracking branch 'origin/master' into facebook-limited-login
vthorsteinsson Jan 3, 2025
e493b12
Modified caching; other small changes
vthorsteinsson Jan 3, 2025
43a3107
Add user ID validation for Facebook OAuth token
carlosvstokkur Jan 7, 2025
f4870bc
Refactor Facebook OAuth token validation and return public key correctly
carlosvstokkur Jan 8, 2025
159c157
Formatting; token verification for Facebook limited login
vthorsteinsson Jan 8, 2025
8013a78
Merge remote-tracking branch 'origin/master' into facebook-limited-login
vthorsteinsson Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
167 changes: 124 additions & 43 deletions src/auth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
DEFAULT_LOCALE,
FACEBOOK_APP_SECRET,
FACEBOOK_APP_ID,
FACEBOOK_NONCE,
APPLE_CLIENT_ID,
DEV_SERVER,
FlaskConfig,
Expand All @@ -61,6 +62,9 @@
"https://graph.facebook.com/debug_token?input_token={0}&access_token={1}|{2}"
)

# Facebook public key endpoint
FACEBOOK_JWT_ENDPOINT = "https://www.facebook.com/.well-known/oauth/openid/jwks/"

# Apple public key and JWT stuff
APPLE_TOKEN_VALIDATION_URL = "https://appleid.apple.com/auth/token"
APPLE_JWKS_URL = "https://appleid.apple.com/auth/keys"
Expand All @@ -80,6 +84,8 @@
FIREBASE_FORBIDDEN_CHARS, "_" * len(FIREBASE_FORBIDDEN_CHARS)
)

FACEBOOK_PUBLIC_KEY: Optional[str] = None


# Utility function for making account IDs compatible with Firebase
# key restrictions
Expand Down Expand Up @@ -108,6 +114,30 @@ def rq_get(request: Request, key: str) -> str:
return ""


def get_facebook_public_key() -> Optional[str]:
"""Get the public key from Facebook's JWKS endpoint"""
global FACEBOOK_PUBLIC_KEY
if FACEBOOK_PUBLIC_KEY:
# Already fetched successfully: re-use cached value
return FACEBOOK_PUBLIC_KEY
try:
response = requests.get(FACEBOOK_JWT_ENDPOINT)
response.raise_for_status() # Raise an error for bad status codes
jwks = response.json()
# Extract the public key in PEM format
key_data = jwks['keys'][0]
return FACEBOOK_PUBLIC_KEY := cast( # type: ignore
Optional[str],
jwt.algorithms.RSAAlgorithm.from_jwk(key_data), # type: ignore
)
except requests.exceptions.RequestException as e:
logging.error(f"An error occurred while fetching the public key: {e}")
return None # Return None and failure status
except KeyError as e:
logging.error(f"Key error: {e}")
return None # Return None and failure status


def oauth2callback(request: Request) -> ResponseType:
# Note that HTTP GETs to the /oauth2callback URL are handled in web.py,
# this route is only for HTTP POSTs
Expand Down Expand Up @@ -321,57 +351,108 @@ def oauth_fb(request: Request) -> ResponseType:
)

token = user.get("token", "")
# Validate the Facebook token
# Perform basic validation
if not token or len(token) > 1024 or not token.isalnum():
return jsonify({"status": "invalid", "msg": "Invalid Facebook token"}), 401
r = requests.get(
FACEBOOK_TOKEN_VALIDATION_URL.format(
token, facebook_app_id, facebook_app_secret
)
)
if r.status_code != 200:
# Error from the Facebook API: communicate it back to the client
msg = ""

# Check whether this is a limited login (used by iOS clients only)
is_limited_login = rq.get_bool("isLimitedLogin", False)
if is_limited_login:
# Validate Limited Login token
try:
msg = cast(Any, r).json()["error"]["message"]
msg = f": {msg}"
except (KeyError, ValueError):
pass
return (
jsonify(
{
"status": "invalid",
"msg": f"Unable to verify Facebook token{msg}", # Lack of space intentional
}
),
401,
# Get the (cached) public key from Facebook's JWKS endpoint
public_key = get_facebook_public_key()
if not public_key:
return (
jsonify(
{"status": "invalid", "msg": "Missing Facebook public key",}
),
401,
)
decoded_token = jwt.decode(
token,
public_key,
algorithms=["RS256"],
audience=facebook_app_id,
options={"verify_exp": True}
)
if decoded_token.get("nonce") != FACEBOOK_NONCE:
return (
jsonify(
{"status": "invalid", "msg": "Invalid Facebook nonce token",}
),
401,
)
account = decoded_token.get("sub", "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the other case (non-limited login), the account id is compared with the original string from the login request, and if different, the login is aborted. Does the same logic apply here, in the limited login case? @kadyvillicana

except jwt.ExpiredSignatureError:
return (
jsonify(
{"status": "invalid", "msg": "Token has expired",}
),
401,
)
except jwt.InvalidTokenError:
return (
jsonify(
{"status": "invalid", "msg": "Invalid Facebook token",}
),
401,
)
if account != user.get("id"):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me like we need instead, in line 386, something like this:

account_in_token = decoded_token.get("sub", "")
if account_in_token != account:
    return (jsonify(...), 401)  # User account mismatch

We already assigned the account variable in line 326 and the user id in the token ("sub") must match that account id.

return (
jsonify({"status": "invalid", "msg": "Wrong user id in Facebook token"}),
401,
)
else:
# Validate regular Facebook token
r = requests.get(
FACEBOOK_TOKEN_VALIDATION_URL.format(
token, facebook_app_id, facebook_app_secret
)
)
if r.status_code != 200:
# Error from the Facebook API: communicate it back to the client
msg = ""
try:
msg = cast(Any, r).json()["error"]["message"]
msg = f": {msg}"
except (KeyError, ValueError):
pass
return (
jsonify(
{
"status": "invalid",
"msg": f"Unable to verify Facebook token{msg}", # Lack of space intentional
}
),
401,
)
response: Dict[str, Any] = cast(Any, r).json()
if not response or not (rd := response.get("data")):
return (
jsonify(
{"status": "invalid", "msg": "Invalid format of Facebook token data"}
),
401,
)
if (
facebook_app_id != rd.get("app_id")
or "USER" != rd.get("type")
or not rd.get("is_valid")
):
return (
jsonify({"status": "invalid", "msg": "Facebook token data mismatch"}),
401,
)
if account != rd.get("user_id"):
return (
jsonify({"status": "invalid", "msg": "Wrong user id in Facebook token"}),
401,
)
# So far, so good: double check that token data are as expected
name = user.get("full_name", "")
image = user.get("image", "")
email = user.get("email", "").lower()
response: Dict[str, Any] = cast(Any, r).json()
if not response or not (rd := response.get("data")):
return (
jsonify(
{"status": "invalid", "msg": "Invalid format of Facebook token data"}
),
401,
)
if (
facebook_app_id != rd.get("app_id")
or "USER" != rd.get("type")
or not rd.get("is_valid")
):
return (
jsonify({"status": "invalid", "msg": "Facebook token data mismatch"}),
401,
)
if account != rd.get("user_id"):
return (
jsonify({"status": "invalid", "msg": "Wrong user id in Facebook token"}),
401,
)
# Make sure that Facebook account ids are different from Google/OAuth ones
# by prefixing them with 'fb:'
account = "fb:" + account
Expand Down
4 changes: 4 additions & 0 deletions src/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ class FlaskConfig(TypedDict):
), f"FACEBOOK_APP_SECRET not set correctly in {CLIENT_SECRET_ID}"
assert FACEBOOK_APP_ID, f"FACEBOOK_APP_ID not set correctly in {CLIENT_SECRET_ID}"

# Facebook nonce for limited login verification
FACEBOOK_NONCE: str = j.get("FACEBOOK_NONCE", "")
assert FACEBOOK_NONCE, f"FACEBOOK_NONCE not set correctly in {CLIENT_SECRET_ID}"

# Firebase configuration
FIREBASE_API_KEY: str = j.get("FIREBASE_API_KEY", "")
FIREBASE_SENDER_ID: str = j.get("FIREBASE_SENDER_ID", "")
Expand Down
8 changes: 2 additions & 6 deletions src/skrafldb.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -737,9 +737,7 @@ def fetch_multi(cls, user_ids: Iterable[str]) -> List[Optional[UserModel]]:
len_keys = len(keys)
recs = cast(
List[Optional[UserModel]],
# The following cast is due to strange typing
# in ndb (the use of 'Type' is almost certainly a bug there)
ndb.get_multi(cast(Sequence[Type[Key[UserModel]]], keys)),
ndb.get_multi(cast(Sequence[Key[UserModel]], keys)),
)
if ix == 0 and len_keys == end:
# Most common case: just a single, complete read
Expand Down Expand Up @@ -1132,9 +1130,7 @@ def fetch_keys() -> None:
nonlocal result, keys
recs = cast(
List[Optional[EloModel]],
# The following cast is due to strange typing
# in ndb (the use of 'Type' is almost certainly a bug there)
ndb.get_multi(cast(Sequence[Type[Key[EloModel]]], keys)),
ndb.get_multi(cast(Sequence[Key[EloModel]], keys)),
)
for em in recs:
if em is not None:
Expand Down