Skip to content

Conversation

@lukasmasuch
Copy link
Collaborator

@lukasmasuch lukasmasuch commented Jan 12, 2026

Describe your changes

Implementing auth-related features from #12693 and #12693 for the new Starlette implementation. This also applies some refactoring for better reuse and adds an e2e test.

Testing Plan

  • Added unit and e2e tests.

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

Copilot AI review requested due to automatic review settings January 12, 2026 20:40
@snyk-io
Copy link
Contributor

snyk-io bot commented Jan 12, 2026

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 12, 2026

✅ PR preview is ready!

Name Link
📦 Wheel file https://core-previews.s3-us-west-2.amazonaws.com/pr-13571/streamlit-1.53.0-py3-none-any.whl
📦 @streamlit/component-v2-lib Download from artifacts
🕹️ Preview app pr-13571.streamlit.app (☁️ Deploy here if not accessible)

@lukasmasuch
Copy link
Collaborator Author

@cursor review

@lukasmasuch lukasmasuch added change:bugfix PR contains bug fix implementation impact:users PR changes affect end users security-assessment-completed Security assessment has been completed for PR starlette-tests If applied to PR, will run all e2e tests with the Starlette implementation. labels Jan 12, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds support for OIDC end_session_endpoint logout flow to Streamlit's authentication system, enabling proper logout from OAuth providers. The implementation refactors common auth logic into shared utility functions and adds the provider field to user cookies to support provider-specific logout URLs.

Changes:

  • Introduced three new shared functions in auth_util.py: get_validated_redirect_uri(), get_origin_from_redirect_uri(), and build_logout_url() to centralize redirect URI handling and logout URL construction
  • Updated both Tornado and Starlette auth routes to use the shared utilities and implement provider logout URL retrieval via _get_provider_logout_url()
  • Added provider field to user cookies in both auth callback handlers to enable logout URL construction
  • Enhanced OIDC mock server with end_session_endpoint support and added E2E test for the logout flow

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
lib/streamlit/auth_util.py Added three shared utility functions for redirect URI validation, origin extraction, and logout URL building with proper docstrings and type hints
lib/streamlit/web/server/oauth_authlib_routes.py Refactored to use shared auth utilities, removed duplicate redirect URI and origin extraction logic, and integrated build_logout_url()
lib/streamlit/web/server/starlette/starlette_auth_routes.py Implemented provider logout URL retrieval, added cookie value extraction helper, and integrated shared auth utilities for consistency
lib/tests/streamlit/auth_util_test.py Added comprehensive unit tests for the three new utility functions covering various edge cases and port placeholder substitution
lib/tests/streamlit/web/server/starlette/starlette_auth_routes_test.py Updated tests for refactored origin extraction logic and added extensive tests for provider logout URL retrieval and redirect behavior
e2e_playwright/shared/oidc_mock_server.py Enhanced mock OIDC server to include end_session_endpoint in metadata and handle logout redirects
e2e_playwright/auth.py Added logout button and st.logout() call to test script for E2E testing
e2e_playwright/auth_test.py Added E2E test verifying complete logout flow with end_session_endpoint redirect

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no bugs!

@lukasmasuch lukasmasuch added the ai-review If applied to PR or issue will run AI review workflow label Jan 12, 2026
@github-actions github-actions bot removed the ai-review If applied to PR or issue will run AI review workflow label Jan 12, 2026
@github-actions
Copy link
Contributor

Summary

This PR implements auth-related features from PR #12693 for the new Starlette implementation. The key changes include:

  1. Refactoring shared utility functions - Extracts get_validated_redirect_uri(), get_origin_from_redirect_uri(), and build_logout_url() into auth_util.py for code reuse between Tornado and Starlette backends
  2. Adding OIDC logout support for Starlette - Implements _get_provider_logout_url() to redirect users to the OAuth provider's end_session_endpoint during logout
  3. Updating the OIDC mock server - Adds an end_session_endpoint (/logout) for e2e testing
  4. Comprehensive test coverage - Adds unit tests for all new functions and an e2e test for the logout flow

Code Quality

The code quality is excellent and follows Streamlit's best practices:

Type annotations: All new functions are fully typed with proper return type hints
Docstrings: NumPy-style docstrings with clear descriptions and parameter documentation
Logging: Uses the standard get_logger(__name__) pattern
Error handling: Proper exception handling with logging for edge cases
Code reuse: Good refactoring to share code between Tornado (oauth_authlib_routes.py) and Starlette (starlette_auth_routes.py) implementations

Specific highlights:

  • lib/streamlit/auth_util.py:197-229 - The build_logout_url() function is well-designed with clear parameter documentation and proper URL encoding using urlencode()
  • lib/streamlit/web/server/starlette/starlette_auth_routes.py:405-463 - The _get_provider_logout_url() function handles all edge cases gracefully (no cookie, no provider, no end_session_endpoint, invalid redirect_uri)

Test Coverage

The test coverage is comprehensive and well-structured:

Unit Tests (auth_util_test.py)

TestGetValidatedRedirectUri - 5 tests covering: no auth section, no redirect_uri, invalid suffix, valid URI, and port placeholder substitution
TestGetOriginFromRedirectUri - 4 tests covering: no auth section, no redirect_uri, correct extraction, localhost with port
TestBuildLogoutUrl - 3 tests covering: basic URL building, id_token_hint inclusion, and URL encoding

Unit Tests (starlette_auth_routes_test.py)

TestGetOriginFromSecrets - 3 tests verifying the wrapper function behavior
TestGetProviderLogoutUrl - 5 tests covering: no user cookie, no provider in cookie, no end_session_endpoint, valid logout URL, and invalid redirect_uri
TestLogoutWithProviderRedirect - 2 tests for redirect behavior

E2E Test (auth_test.py)

test_logout_with_end_session_endpoint - Tests the full login → logout flow with provider redirect

Negative assertions are properly included:

  • auth_util_test.py:648 - Verifies id_token_hint is NOT present when not provided
  • auth_test.py:161-165 - Verifies logged-in content is no longer visible after logout

Backwards Compatibility

No breaking changes - The changes are additive and maintain full backwards compatibility
Both backends updated - Tornado (oauth_authlib_routes.py) and Starlette implementations are updated consistently to use the shared functions
Cookie handling preserved - The cookie format and handling remain unchanged

The refactoring from private method _get_redirect_uri() in oauth_authlib_routes.py to the shared get_validated_redirect_uri() function is clean and maintains the same behavior.

Security & Risk

Standard OIDC practices - The logout implementation follows standard OIDC logout flow (RP-Initiated Logout)
Proper URL encoding - Parameters are properly URL-encoded using urlencode()
Redirect URI validation - The redirect URI is validated to end with /oauth2callback before use
Optional id_token_hint - The id_token_hint parameter is properly handled as optional per OIDC spec
Error handling - All error paths return None rather than raising exceptions, preventing information leakage

No security concerns identified.

Recommendations

  1. Minor - E2E test timing (non-blocking): The new e2e test uses app.wait_for_timeout(2_000) which the e2e best practices guide recommends avoiding in favor of wait_until. However, this is consistent with the existing tests in auth_test.py, so this is acceptable for consistency.

  2. Documentation consideration (optional): Consider adding a brief inline comment in _get_provider_logout_url explaining why the function returns None in various cases rather than raising exceptions (to fail gracefully and fall back to base redirect).

Verdict

APPROVED: This is a well-implemented PR that properly adds OIDC logout support for the Starlette backend with excellent code quality, comprehensive test coverage, and no breaking changes or security concerns.


This is an automated AI review. Please verify the feedback and use your judgment.

if id_token:
logout_params["id_token_hint"] = id_token

return f"{end_session_endpoint}?{urlencode(logout_params)}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: This looks to always include a ?, will this break if the endpoint already has existing query params in it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it's usually a clean URL, but I've updated the code to handle existing query params defensively for non-standard providers (on tornado and starlette side)

"""
get_button(app, button_label).click()
# Wait for OAuth redirect chain to complete and return to app root
app.wait_for_url(re.compile(r"http://localhost:\d+/$"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: Is using localhost resilient enough for our testing harness?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think localhost is somewhat fine, its also used in the AUTH_SECRETS_TEMPLATE. But I made it more explicit by using the actual part the app is running on.

@lukasmasuch lukasmasuch merged commit 5e3792d into develop Jan 17, 2026
44 checks passed
@lukasmasuch lukasmasuch deleted the fix/support-new-auth-logic-in-starlette branch January 17, 2026 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:bugfix PR contains bug fix implementation impact:users PR changes affect end users security-assessment-completed Security assessment has been completed for PR starlette-tests If applied to PR, will run all e2e tests with the Starlette implementation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants