Merge pull request #532 from isair/fix/pkce-code-verifier
Fix PKCE code verifier not generated for initial OAuth flow
This commit is contained in:
@@ -295,6 +295,7 @@ def create_oauth_flow(
|
||||
redirect_uri: str,
|
||||
state: Optional[str] = None,
|
||||
code_verifier: Optional[str] = None,
|
||||
autogenerate_code_verifier: bool = True,
|
||||
) -> Flow:
|
||||
"""Creates an OAuth flow using environment variables or client secrets file."""
|
||||
flow_kwargs = {
|
||||
@@ -306,6 +307,12 @@ def create_oauth_flow(
|
||||
flow_kwargs["code_verifier"] = code_verifier
|
||||
# Preserve the original verifier when re-creating the flow in callback.
|
||||
flow_kwargs["autogenerate_code_verifier"] = False
|
||||
else:
|
||||
# Generate PKCE code verifier for the initial auth flow.
|
||||
# google-auth-oauthlib's from_client_* helpers pass
|
||||
# autogenerate_code_verifier=None unless explicitly provided, which
|
||||
# prevents Flow from generating and storing a code_verifier.
|
||||
flow_kwargs["autogenerate_code_verifier"] = autogenerate_code_verifier
|
||||
|
||||
# Try environment variables first
|
||||
env_config = load_client_secrets_from_env()
|
||||
@@ -520,6 +527,7 @@ def handle_auth_callback(
|
||||
redirect_uri=redirect_uri,
|
||||
state=state,
|
||||
code_verifier=state_info.get("code_verifier"),
|
||||
autogenerate_code_verifier=False,
|
||||
)
|
||||
|
||||
# Exchange the authorization code for credentials
|
||||
|
||||
118
tests/auth/test_google_auth_pkce.py
Normal file
118
tests/auth/test_google_auth_pkce.py
Normal file
@@ -0,0 +1,118 @@
|
||||
"""Regression tests for OAuth PKCE flow wiring."""
|
||||
|
||||
import os
|
||||
import sys
|
||||
from unittest.mock import patch
|
||||
|
||||
|
||||
sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), "../..")))
|
||||
|
||||
from auth.google_auth import create_oauth_flow # noqa: E402
|
||||
|
||||
|
||||
DUMMY_CLIENT_CONFIG = {
|
||||
"web": {
|
||||
"client_id": "dummy-client-id.apps.googleusercontent.com",
|
||||
"client_secret": "dummy-secret",
|
||||
"auth_uri": "https://accounts.google.com/o/oauth2/auth",
|
||||
"token_uri": "https://oauth2.googleapis.com/token",
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
def test_create_oauth_flow_autogenerates_verifier_when_missing():
|
||||
expected_flow = object()
|
||||
with (
|
||||
patch(
|
||||
"auth.google_auth.load_client_secrets_from_env",
|
||||
return_value=DUMMY_CLIENT_CONFIG,
|
||||
),
|
||||
patch(
|
||||
"auth.google_auth.Flow.from_client_config",
|
||||
return_value=expected_flow,
|
||||
) as mock_from_client_config,
|
||||
):
|
||||
flow = create_oauth_flow(
|
||||
scopes=["openid"],
|
||||
redirect_uri="http://localhost/callback",
|
||||
state="oauth-state-1",
|
||||
)
|
||||
|
||||
assert flow is expected_flow
|
||||
args, kwargs = mock_from_client_config.call_args
|
||||
assert args[0] == DUMMY_CLIENT_CONFIG
|
||||
assert kwargs["autogenerate_code_verifier"] is True
|
||||
assert "code_verifier" not in kwargs
|
||||
|
||||
|
||||
def test_create_oauth_flow_preserves_callback_verifier():
|
||||
expected_flow = object()
|
||||
with (
|
||||
patch(
|
||||
"auth.google_auth.load_client_secrets_from_env",
|
||||
return_value=DUMMY_CLIENT_CONFIG,
|
||||
),
|
||||
patch(
|
||||
"auth.google_auth.Flow.from_client_config",
|
||||
return_value=expected_flow,
|
||||
) as mock_from_client_config,
|
||||
):
|
||||
flow = create_oauth_flow(
|
||||
scopes=["openid"],
|
||||
redirect_uri="http://localhost/callback",
|
||||
state="oauth-state-2",
|
||||
code_verifier="saved-verifier",
|
||||
)
|
||||
|
||||
assert flow is expected_flow
|
||||
args, kwargs = mock_from_client_config.call_args
|
||||
assert args[0] == DUMMY_CLIENT_CONFIG
|
||||
assert kwargs["code_verifier"] == "saved-verifier"
|
||||
assert kwargs["autogenerate_code_verifier"] is False
|
||||
|
||||
|
||||
def test_create_oauth_flow_file_config_still_enables_pkce():
|
||||
expected_flow = object()
|
||||
with (
|
||||
patch("auth.google_auth.load_client_secrets_from_env", return_value=None),
|
||||
patch("auth.google_auth.os.path.exists", return_value=True),
|
||||
patch(
|
||||
"auth.google_auth.Flow.from_client_secrets_file",
|
||||
return_value=expected_flow,
|
||||
) as mock_from_file,
|
||||
):
|
||||
flow = create_oauth_flow(
|
||||
scopes=["openid"],
|
||||
redirect_uri="http://localhost/callback",
|
||||
state="oauth-state-3",
|
||||
)
|
||||
|
||||
assert flow is expected_flow
|
||||
_args, kwargs = mock_from_file.call_args
|
||||
assert kwargs["autogenerate_code_verifier"] is True
|
||||
assert "code_verifier" not in kwargs
|
||||
|
||||
|
||||
def test_create_oauth_flow_allows_disabling_autogenerate_without_verifier():
|
||||
expected_flow = object()
|
||||
with (
|
||||
patch(
|
||||
"auth.google_auth.load_client_secrets_from_env",
|
||||
return_value=DUMMY_CLIENT_CONFIG,
|
||||
),
|
||||
patch(
|
||||
"auth.google_auth.Flow.from_client_config",
|
||||
return_value=expected_flow,
|
||||
) as mock_from_client_config,
|
||||
):
|
||||
flow = create_oauth_flow(
|
||||
scopes=["openid"],
|
||||
redirect_uri="http://localhost/callback",
|
||||
state="oauth-state-4",
|
||||
autogenerate_code_verifier=False,
|
||||
)
|
||||
|
||||
assert flow is expected_flow
|
||||
_args, kwargs = mock_from_client_config.call_args
|
||||
assert kwargs["autogenerate_code_verifier"] is False
|
||||
assert "code_verifier" not in kwargs
|
||||
Reference in New Issue
Block a user