mirror of
https://github.com/DRYTRIX/TimeTracker.git
synced 2026-02-17 11:08:40 -06:00
04ed5ef8ae
ixes #88 When OIDC_POST_LOGOUT_REDIRECT_URI was unset, the application was still attempting RP-Initiated Logout by falling back to a generated redirect URL. This caused issues with OIDC providers like Authelia that don't support RP-Initiated Logout, resulting in failed redirects to unsupported endpoints. Changes: - Modified logout logic in app/routes/auth.py to only attempt provider logout when OIDC_POST_LOGOUT_REDIRECT_URI is explicitly configured - If unset, users are now logged out locally and redirected to the TimeTracker login page (expected behavior) - If set, RP-Initiated Logout proceeds as before (backward compatible) Documentation: - Updated docs/OIDC_SETUP.md with guidance on when to set the config - Added clear comments in env.example explaining optional behavior - Documented troubleshooting steps for providers without RP-Initiated Logout support (e.g., Authelia) Tests: - Added comprehensive test suite (tests/test_oidc_logout.py) with 9 tests covering different logout scenarios and edge cases - All existing tests continue to pass (no regressions) This change is fully backward compatible. Users with providers supporting RP-Initiated Logout can continue using OIDC_POST_LOGOUT_REDIRECT_URI as before. Users with providers like Authelia should leave it unset for local-only logout.
243 lines
9.4 KiB
Python
243 lines
9.4 KiB
Python
"""
|
|
Tests for OIDC logout behavior
|
|
"""
|
|
import pytest
|
|
from unittest.mock import Mock, patch, MagicMock
|
|
from flask import session, url_for
|
|
from app.models import User
|
|
from app import db
|
|
|
|
|
|
@pytest.fixture
|
|
def oidc_user(app):
|
|
"""Create a test user with OIDC linkage."""
|
|
with app.app_context():
|
|
user = User(
|
|
username='oidc_test_user',
|
|
email='oidc@example.com',
|
|
full_name='OIDC Test User'
|
|
)
|
|
# Set OIDC attributes after creation
|
|
user.oidc_issuer = 'https://idp.example.com'
|
|
user.oidc_sub = 'test-sub-123'
|
|
db.session.add(user)
|
|
db.session.commit()
|
|
yield user
|
|
db.session.delete(user)
|
|
db.session.commit()
|
|
|
|
|
|
@pytest.fixture
|
|
def oidc_authenticated_client(client, oidc_user):
|
|
"""Client with an authenticated OIDC user."""
|
|
with client:
|
|
with client.session_transaction() as sess:
|
|
sess['_user_id'] = str(oidc_user.id)
|
|
sess['oidc_id_token'] = 'mock_id_token_12345'
|
|
yield client
|
|
|
|
|
|
# ============================================================================
|
|
# Unit Tests: OIDC Logout Behavior
|
|
# ============================================================================
|
|
|
|
@pytest.mark.unit
|
|
@pytest.mark.security
|
|
def test_logout_without_post_logout_uri_config(oidc_authenticated_client, app):
|
|
"""
|
|
Test that when OIDC_POST_LOGOUT_REDIRECT_URI is not set,
|
|
logout performs local logout only and redirects to login page.
|
|
|
|
This fixes the issue where Authelia (and other providers without
|
|
RP-Initiated Logout support) would receive incorrect redirect requests.
|
|
"""
|
|
with app.app_context():
|
|
# Ensure OIDC_POST_LOGOUT_REDIRECT_URI is not set
|
|
app.config['AUTH_METHOD'] = 'oidc'
|
|
if hasattr(app.config, 'OIDC_POST_LOGOUT_REDIRECT_URI'):
|
|
delattr(app.config, 'OIDC_POST_LOGOUT_REDIRECT_URI')
|
|
|
|
# Mock oauth client to prevent actual OIDC calls
|
|
with patch('app.routes.auth.oauth') as mock_oauth:
|
|
mock_client = MagicMock()
|
|
mock_oauth.create_client.return_value = mock_client
|
|
|
|
# Perform logout
|
|
response = oidc_authenticated_client.get('/logout', follow_redirects=False)
|
|
|
|
# Should redirect to local login page, NOT to IdP
|
|
assert response.status_code == 302
|
|
assert response.location.endswith('/login')
|
|
|
|
# OAuth client should not have been created since no post_logout URI
|
|
mock_oauth.create_client.assert_not_called()
|
|
|
|
|
|
@pytest.mark.unit
|
|
@pytest.mark.security
|
|
def test_logout_with_post_logout_uri_config(oidc_authenticated_client, app):
|
|
"""
|
|
Test that when OIDC_POST_LOGOUT_REDIRECT_URI is set,
|
|
logout attempts RP-Initiated Logout at the provider.
|
|
"""
|
|
with app.app_context():
|
|
# Mock oauth client and Config
|
|
with patch('app.routes.auth.oauth') as mock_oauth, \
|
|
patch('app.routes.auth.Config') as mock_config:
|
|
# Configure OIDC with post-logout redirect
|
|
mock_config.AUTH_METHOD = 'oidc'
|
|
mock_config.OIDC_POST_LOGOUT_REDIRECT_URI = 'https://app.example.com/'
|
|
|
|
mock_client = MagicMock()
|
|
mock_metadata = {
|
|
'end_session_endpoint': 'https://idp.example.com/logout'
|
|
}
|
|
mock_client.load_server_metadata.return_value = mock_metadata
|
|
mock_oauth.create_client.return_value = mock_client
|
|
|
|
# Perform logout
|
|
response = oidc_authenticated_client.get('/logout', follow_redirects=False)
|
|
|
|
# Should redirect to IdP logout endpoint
|
|
assert response.status_code == 302
|
|
assert 'idp.example.com/logout' in response.location
|
|
assert 'post_logout_redirect_uri' in response.location
|
|
assert 'id_token_hint' in response.location
|
|
|
|
|
|
@pytest.mark.unit
|
|
@pytest.mark.security
|
|
def test_logout_oidc_provider_has_revocation_endpoint_only(oidc_authenticated_client, app):
|
|
"""
|
|
Test logout when provider has revocation_endpoint but no end_session_endpoint.
|
|
Should use revocation_endpoint as fallback when post_logout URI is configured.
|
|
"""
|
|
with app.app_context():
|
|
with patch('app.routes.auth.oauth') as mock_oauth, \
|
|
patch('app.routes.auth.Config') as mock_config:
|
|
mock_config.AUTH_METHOD = 'oidc'
|
|
mock_config.OIDC_POST_LOGOUT_REDIRECT_URI = 'https://app.example.com/'
|
|
|
|
mock_client = MagicMock()
|
|
mock_metadata = {
|
|
'revocation_endpoint': 'https://idp.example.com/revoke'
|
|
}
|
|
mock_client.load_server_metadata.return_value = mock_metadata
|
|
mock_oauth.create_client.return_value = mock_client
|
|
|
|
response = oidc_authenticated_client.get('/logout', follow_redirects=False)
|
|
|
|
# Should redirect to revocation endpoint
|
|
assert response.status_code == 302
|
|
assert 'idp.example.com/revoke' in response.location
|
|
|
|
|
|
@pytest.mark.unit
|
|
@pytest.mark.security
|
|
def test_logout_local_auth_method(authenticated_client, app):
|
|
"""Test that local auth method doesn't try OIDC logout."""
|
|
with app.app_context():
|
|
app.config['AUTH_METHOD'] = 'local'
|
|
|
|
with patch('app.routes.auth.oauth') as mock_oauth:
|
|
response = authenticated_client.get('/logout', follow_redirects=False)
|
|
|
|
# Should redirect to login
|
|
assert response.status_code == 302
|
|
assert response.location.endswith('/login')
|
|
|
|
# Should not attempt OIDC operations
|
|
mock_oauth.create_client.assert_not_called()
|
|
|
|
|
|
@pytest.mark.unit
|
|
@pytest.mark.security
|
|
def test_logout_clears_oidc_id_token_from_session(oidc_authenticated_client, app):
|
|
"""Test that logout removes the OIDC ID token from session."""
|
|
with app.app_context():
|
|
app.config['AUTH_METHOD'] = 'oidc'
|
|
|
|
with patch('app.routes.auth.oauth'):
|
|
# Verify ID token is in session before logout
|
|
with oidc_authenticated_client.session_transaction() as sess:
|
|
assert 'oidc_id_token' in sess
|
|
|
|
# Perform logout
|
|
oidc_authenticated_client.get('/logout', follow_redirects=True)
|
|
|
|
# Verify ID token is removed from session
|
|
with oidc_authenticated_client.session_transaction() as sess:
|
|
assert 'oidc_id_token' not in sess
|
|
|
|
|
|
@pytest.mark.unit
|
|
@pytest.mark.security
|
|
def test_logout_with_both_auth_method_no_post_logout_uri(oidc_authenticated_client, app):
|
|
"""
|
|
Test logout with AUTH_METHOD=both and no post_logout URI.
|
|
Should perform local logout only.
|
|
"""
|
|
with app.app_context():
|
|
app.config['AUTH_METHOD'] = 'both'
|
|
if hasattr(app.config, 'OIDC_POST_LOGOUT_REDIRECT_URI'):
|
|
delattr(app.config, 'OIDC_POST_LOGOUT_REDIRECT_URI')
|
|
|
|
with patch('app.routes.auth.oauth') as mock_oauth:
|
|
response = oidc_authenticated_client.get('/logout', follow_redirects=False)
|
|
|
|
# Should redirect to login without OIDC logout
|
|
assert response.status_code == 302
|
|
assert response.location.endswith('/login')
|
|
mock_oauth.create_client.assert_not_called()
|
|
|
|
|
|
@pytest.mark.unit
|
|
@pytest.mark.security
|
|
def test_logout_provider_metadata_load_fails_gracefully(oidc_authenticated_client, app):
|
|
"""Test that logout handles provider metadata loading failures gracefully."""
|
|
with app.app_context():
|
|
with patch('app.routes.auth.oauth') as mock_oauth, \
|
|
patch('app.routes.auth.Config') as mock_config:
|
|
mock_config.AUTH_METHOD = 'oidc'
|
|
mock_config.OIDC_POST_LOGOUT_REDIRECT_URI = 'https://app.example.com/'
|
|
|
|
mock_client = MagicMock()
|
|
# Simulate metadata loading failure
|
|
mock_client.load_server_metadata.side_effect = Exception("Metadata unavailable")
|
|
mock_oauth.create_client.return_value = mock_client
|
|
|
|
# Should fall back to local logout
|
|
response = oidc_authenticated_client.get('/logout', follow_redirects=False)
|
|
|
|
assert response.status_code == 302
|
|
assert response.location.endswith('/login')
|
|
|
|
|
|
# ============================================================================
|
|
# Smoke Tests: OIDC Logout
|
|
# ============================================================================
|
|
|
|
@pytest.mark.smoke
|
|
def test_logout_endpoint_exists(client):
|
|
"""Smoke test: Ensure logout endpoint is accessible."""
|
|
# Should redirect to login (not 404)
|
|
response = client.get('/logout', follow_redirects=False)
|
|
assert response.status_code in [302, 401] # Redirect or unauthorized, not 404
|
|
|
|
|
|
@pytest.mark.smoke
|
|
def test_logout_configuration_keys_valid(app):
|
|
"""Smoke test: Verify OIDC configuration keys are properly defined."""
|
|
with app.app_context():
|
|
from app.config import Config
|
|
|
|
# These should be accessible without errors
|
|
auth_method = getattr(Config, 'AUTH_METHOD', None)
|
|
assert auth_method in ['local', 'oidc', 'both', None]
|
|
|
|
# OIDC_POST_LOGOUT_REDIRECT_URI should be optional
|
|
post_logout = getattr(Config, 'OIDC_POST_LOGOUT_REDIRECT_URI', None)
|
|
# It's fine if it's None or a string
|
|
assert post_logout is None or isinstance(post_logout, str)
|
|
|