fix(oidc): only perform RP-Initiated Logout when OIDC_POST_LOGOUT_REDIRECT_URI is set

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.
This commit is contained in:
Dries Peeters
2025-10-17 12:51:43 +02:00
parent 5bb0959596
commit 04ed5ef8ae
5 changed files with 394 additions and 19 deletions

125
OIDC_LOGOUT_FIX_SUMMARY.md Normal file
View File

@@ -0,0 +1,125 @@
# OIDC Logout Fix Summary
## Issue Description
When `OIDC_POST_LOGOUT_REDIRECT_URI` was not set (unset/None), the application was still attempting RP-Initiated Logout at the OIDC provider. This caused issues with Identity Providers like Authelia that don't support RP-Initiated Logout yet.
For example, users would be incorrectly redirected to:
```
https://auth.example.de/api/oidc/revocation?id_token_hint=...
```
This would fail because the provider doesn't support the endpoint, instead of simply logging out locally and returning to the TimeTracker login page.
## Root Cause
In `app/routes/auth.py`, the logout function had this logic:
```python
post_logout = getattr(Config, 'OIDC_POST_LOGOUT_REDIRECT_URI', None) or url_for('auth.login', _external=True)
```
The problem was the `or url_for('auth.login', _external=True)` fallback. Even when `OIDC_POST_LOGOUT_REDIRECT_URI` was `None` (not configured), it would fall back to generating a logout redirect URL, causing the application to always attempt RP-Initiated Logout.
## Solution
Modified the logout logic to only perform RP-Initiated Logout if `OIDC_POST_LOGOUT_REDIRECT_URI` is **explicitly configured**:
```python
if auth_method in ('oidc', 'both'):
# Only perform RP-Initiated Logout if OIDC_POST_LOGOUT_REDIRECT_URI is explicitly configured
post_logout = getattr(Config, 'OIDC_POST_LOGOUT_REDIRECT_URI', None)
if post_logout:
# ... proceed with RP-Initiated Logout
```
Now:
- **If `OIDC_POST_LOGOUT_REDIRECT_URI` is NOT set**: Users are logged out locally and redirected to TimeTracker's login page
- **If `OIDC_POST_LOGOUT_REDIRECT_URI` IS set**: Users are redirected to the provider's logout endpoint (RP-Initiated Logout)
## Files Changed
### 1. `app/routes/auth.py`
- Modified logout function to check if `OIDC_POST_LOGOUT_REDIRECT_URI` is set before attempting provider logout
- Added comment explaining the behavior
### 2. `docs/OIDC_SETUP.md`
- Updated documentation for `OIDC_POST_LOGOUT_REDIRECT_URI` to clarify it's optional
- Added guidance: only set if your provider supports end_session_endpoint
- Updated troubleshooting section with specific guidance for providers like Authelia
### 3. `env.example`
- Added clear comments explaining when to set `OIDC_POST_LOGOUT_REDIRECT_URI`
- Noted that if unset, logout will be local only (recommended for providers without RP-Initiated Logout support)
### 4. `tests/test_oidc_logout.py` (NEW)
- Created comprehensive test suite with 9 tests covering:
- Unit tests for logout without `OIDC_POST_LOGOUT_REDIRECT_URI` configured
- Unit tests for logout with `OIDC_POST_LOGOUT_REDIRECT_URI` configured
- Tests for different auth methods (local, oidc, both)
- Tests for provider metadata loading failures
- Tests for session token cleanup
- Smoke tests for configuration validation
## Behavior Matrix
| AUTH_METHOD | OIDC_POST_LOGOUT_REDIRECT_URI | Logout Behavior |
|-------------|-------------------------------|-----------------|
| `local` | (any) | Local logout → `/login` |
| `oidc` or `both` | Not set (None) | Local logout → `/login` |
| `oidc` or `both` | Set | RP-Initiated Logout → Provider logout endpoint |
## Testing
All tests pass:
- ✅ 9 new OIDC logout tests (all passing)
- ✅ Existing logout tests remain passing (backward compatibility confirmed)
- ✅ No linter errors
Run tests with:
```bash
python -m pytest tests/test_oidc_logout.py -v
```
## Migration Guide
### For Users with Authelia or Similar Providers
If your OIDC provider doesn't support RP-Initiated Logout:
1. **Remove or comment out** `OIDC_POST_LOGOUT_REDIRECT_URI` from your environment:
```bash
# OIDC_POST_LOGOUT_REDIRECT_URI=https://yourapp.example.com/
```
2. Restart the application
3. Test logout - you should now be redirected to TimeTracker's login page instead of the provider's revocation endpoint
### For Users with Providers Supporting RP-Initiated Logout
No changes needed. If you have `OIDC_POST_LOGOUT_REDIRECT_URI` configured, the behavior remains the same.
## Security Considerations
This fix does not reduce security:
- Users are still logged out of TimeTracker (session invalidated)
- The ID token is removed from the session
- For providers that support RP-Initiated Logout, full logout still occurs when configured
The only difference is that providers without RP-Initiated Logout support no longer receive logout requests they cannot handle.
## Compatibility
- ✅ Backward compatible - existing configurations continue to work
- ✅ Forward compatible - new optional behavior
- ✅ Works with all OIDC providers (Azure AD, Okta, Keycloak, Authelia, Google, etc.)
- ✅ No database migration required
- ✅ No breaking changes
## References
- Issue: Authelia doesn't support RP-Initiated Logout
- OIDC Spec: [RP-Initiated Logout](https://openid.net/specs/openid-connect-rpinitiated-1_0.html)
- Related: end_session_endpoint is optional in OIDC providers

View File

@@ -118,22 +118,24 @@ def logout():
flash(_('Goodbye, %(username)s!', username=username), 'info')
if auth_method in ('oidc', 'both'):
client = oauth.create_client('oidc')
if client:
try:
# Build end-session URL if provider supports it
metadata = client.load_server_metadata()
end_session_endpoint = metadata.get('end_session_endpoint') or metadata.get('revocation_endpoint')
if end_session_endpoint:
params = {}
if id_token:
params['id_token_hint'] = id_token
post_logout = getattr(Config, 'OIDC_POST_LOGOUT_REDIRECT_URI', None) or url_for('auth.login', _external=True)
params['post_logout_redirect_uri'] = post_logout
from urllib.parse import urlencode
return redirect(f"{end_session_endpoint}?{urlencode(params)}")
except Exception:
pass
# Only perform RP-Initiated Logout if OIDC_POST_LOGOUT_REDIRECT_URI is explicitly configured
post_logout = getattr(Config, 'OIDC_POST_LOGOUT_REDIRECT_URI', None)
if post_logout:
client = oauth.create_client('oidc')
if client:
try:
# Build end-session URL if provider supports it
metadata = client.load_server_metadata()
end_session_endpoint = metadata.get('end_session_endpoint') or metadata.get('revocation_endpoint')
if end_session_endpoint:
params = {}
if id_token:
params['id_token_hint'] = id_token
params['post_logout_redirect_uri'] = post_logout
from urllib.parse import urlencode
return redirect(f"{end_session_endpoint}?{urlencode(params)}")
except Exception:
pass
return redirect(url_for('auth.login'))

View File

@@ -50,7 +50,9 @@ OIDC_GROUPS_CLAIM=groups
OIDC_ADMIN_GROUP=timetracker-admins # If your IdP issues a groups claim
OIDC_ADMIN_EMAILS=alice@company.com,bob@company.com
# Optional logout behavior
# Optional: RP-Initiated Logout (set only if your provider supports end_session_endpoint)
# If unset, users will be logged out locally and redirected to TimeTracker's login page.
# If set, TimeTracker will redirect to the provider's logout endpoint after local logout.
OIDC_POST_LOGOUT_REDIRECT_URI=https://your-app.example.com/
```
@@ -134,8 +136,9 @@ services:
- “User is not admin”
- Verify `OIDC_ADMIN_GROUP` matches the group claim value, or add the users email to `OIDC_ADMIN_EMAILS`.
- Logout keeps me signed in
- Not all IdPs support end-session. If supported, we redirect to the providers end-session endpoint with `post_logout_redirect_uri`.
- "Logout keeps me signed in" or "Logout redirects to provider error page"
- Not all IdPs support RP-Initiated Logout (end-session). If your provider doesn't support it (e.g., Authelia), **do not set** `OIDC_POST_LOGOUT_REDIRECT_URI`. TimeTracker will then perform local logout only and redirect to the login page.
- If your provider supports end-session and you want to log out from the IdP too, set `OIDC_POST_LOGOUT_REDIRECT_URI` to your desired post-logout landing page.
### 9) Routes Reference

View File

@@ -45,6 +45,9 @@ AUTH_METHOD=local
# OIDC_GROUPS_CLAIM=groups
# OIDC_ADMIN_GROUP=
# OIDC_ADMIN_EMAILS=
# Optional: RP-Initiated Logout. Only set if your provider supports end_session_endpoint.
# If unset, logout will be local only (recommended for providers like Authelia).
# If set, TimeTracker will redirect to the provider's logout endpoint.
# OIDC_POST_LOGOUT_REDIRECT_URI=https://yourapp.example.com/
# Backup settings

242
tests/test_oidc_logout.py Normal file
View File

@@ -0,0 +1,242 @@
"""
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)