fix: resolve OIDC session loss and migration failures (#1023)
* fix: resolve OIDC session loss and migration failures This commit fixes three critical issues affecting OIDC/SSO authentication: 1. Session Not Saved Before Redirect - Added explicit req.session.save() callback in OIDC callback handler - Ensures session is persisted before redirecting to /today - Prevents 401 errors after successful SSO authentication 2. Migration Resilience - Added DROP TABLE IF EXISTS users_new in migration - Prevents "table already exists" errors from failed migrations - Created cleanup script for orphaned migration tables 3. Trust Proxy Documentation - Documented TUDUDI_TRUST_PROXY requirement for reverse proxy deployments - Added troubleshooting guide for session loss issues - Updated .env.example with OIDC configuration examples Fixes session loss when deployed behind reverse proxies (nginx, Traefik, etc.) Changes: - backend/modules/oidc/controller.js: Add session.save() before redirect - backend/migrations/20260420000004-make-password-optional.js: Add DROP TABLE IF EXISTS - backend/scripts/cleanup-failed-migration.js: New cleanup utility - backend/.env.example: Add OIDC and trust proxy examples - docs/10-oidc-sso.md: Add trust proxy configuration and troubleshooting - docs/feature-plans/00-oidc-sso.md: Document required environment variables * fix: prettier formatting in cleanup script
This commit is contained in:
Chris
GitHub
parent
3e839e3bee
commit
ca77222eae
6 changed files with 151 additions and 4 deletions
|
|
@ -30,4 +30,29 @@ DISABLE_TELEGRAM=false
|
|||
# Feature Flags
|
||||
FF_ENABLE_MCP=false
|
||||
|
||||
# Trust proxy (REQUIRED when behind reverse proxy - nginx, Traefik, etc.)
|
||||
# Uncomment this if you're getting session issues or rate limiter errors
|
||||
# TUDUDI_TRUST_PROXY=true
|
||||
|
||||
# OIDC/SSO Configuration
|
||||
# See docs/10-oidc-sso.md for detailed setup instructions
|
||||
OIDC_ENABLED=false
|
||||
# BASE_URL=https://your-domain.com # Required for OIDC callbacks
|
||||
|
||||
# Single provider configuration
|
||||
# OIDC_PROVIDER_NAME=PocketID
|
||||
# OIDC_PROVIDER_SLUG=pocketid
|
||||
# OIDC_ISSUER_URL=https://pocketid.app
|
||||
# OIDC_CLIENT_ID=your-client-id
|
||||
# OIDC_CLIENT_SECRET=your-client-secret
|
||||
# OIDC_SCOPE=openid profile email
|
||||
# OIDC_AUTO_PROVISION=true
|
||||
# OIDC_ADMIN_EMAIL_DOMAINS=example.com,company.com
|
||||
|
||||
# For multiple providers, use numbered variables:
|
||||
# OIDC_PROVIDER_1_NAME=Google
|
||||
# OIDC_PROVIDER_1_SLUG=google
|
||||
# OIDC_PROVIDER_1_ISSUER=https://accounts.google.com
|
||||
# OIDC_PROVIDER_1_CLIENT_ID=xxx.apps.googleusercontent.com
|
||||
# OIDC_PROVIDER_1_CLIENT_SECRET=xxx
|
||||
# OIDC_PROVIDER_1_AUTO_PROVISION=true
|
||||
|
|
|
|||
|
|
@ -4,6 +4,8 @@ module.exports = {
|
|||
async up(queryInterface, Sequelize) {
|
||||
await queryInterface.sequelize.query('PRAGMA foreign_keys = OFF;');
|
||||
|
||||
await queryInterface.sequelize.query('DROP TABLE IF EXISTS users_new;');
|
||||
|
||||
await queryInterface.sequelize.query(`
|
||||
CREATE TABLE users_new (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
|
|
@ -63,6 +65,8 @@ module.exports = {
|
|||
async down(queryInterface, Sequelize) {
|
||||
await queryInterface.sequelize.query('PRAGMA foreign_keys = OFF;');
|
||||
|
||||
await queryInterface.sequelize.query('DROP TABLE IF EXISTS users_new;');
|
||||
|
||||
await queryInterface.sequelize.query(`
|
||||
CREATE TABLE users_new (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
|
|
|
|||
|
|
@ -79,7 +79,16 @@ async function handleCallback(req, res) {
|
|||
slug
|
||||
);
|
||||
|
||||
res.redirect('/today');
|
||||
req.session.save((err) => {
|
||||
if (err) {
|
||||
console.error('Error saving session after OIDC login:', err);
|
||||
return res.redirect(
|
||||
'/login?error=' +
|
||||
encodeURIComponent('Failed to establish session')
|
||||
);
|
||||
}
|
||||
res.redirect('/today');
|
||||
});
|
||||
} catch (error) {
|
||||
console.error('Error handling OIDC callback:', error);
|
||||
|
||||
|
|
|
|||
49
backend/scripts/cleanup-failed-migration.js
Normal file
49
backend/scripts/cleanup-failed-migration.js
Normal file
|
|
@ -0,0 +1,49 @@
|
|||
#!/usr/bin/env node
|
||||
|
||||
/**
|
||||
* Cleanup script for failed migration 20260420000004-make-password-optional
|
||||
*
|
||||
* This script removes the leftover users_new table if it exists,
|
||||
* allowing the migration to run again cleanly.
|
||||
*
|
||||
* Usage:
|
||||
* node backend/scripts/cleanup-failed-migration.js
|
||||
*/
|
||||
|
||||
const { sequelize } = require('../models');
|
||||
|
||||
async function cleanup() {
|
||||
console.log('🔍 Checking for leftover migration tables...');
|
||||
|
||||
try {
|
||||
const [results] = await sequelize.query(`
|
||||
SELECT name FROM sqlite_master
|
||||
WHERE type='table' AND name='users_new'
|
||||
`);
|
||||
|
||||
if (results.length > 0) {
|
||||
console.log(
|
||||
'⚠️ Found leftover users_new table from failed migration'
|
||||
);
|
||||
console.log('🧹 Dropping users_new table...');
|
||||
|
||||
await sequelize.query('DROP TABLE users_new');
|
||||
|
||||
console.log('✅ Cleanup completed successfully');
|
||||
console.log(
|
||||
'💡 You can now run migrations again: npm run db:migrate'
|
||||
);
|
||||
} else {
|
||||
console.log('✅ No cleanup needed - no leftover tables found');
|
||||
}
|
||||
|
||||
await sequelize.close();
|
||||
process.exit(0);
|
||||
} catch (error) {
|
||||
console.error('❌ Cleanup failed:', error.message);
|
||||
await sequelize.close();
|
||||
process.exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
cleanup();
|
||||
|
|
@ -180,6 +180,23 @@ BASE_URL=http://localhost:3002 # Development
|
|||
BASE_URL=https://tududi.example.com # Production
|
||||
```
|
||||
|
||||
**Trust Proxy Configuration (Required for Production):**
|
||||
|
||||
If Tududi is deployed behind a reverse proxy (nginx, Traefik, Apache, etc.), you **must** configure Express to trust the proxy:
|
||||
|
||||
```bash
|
||||
TUDUDI_TRUST_PROXY=true
|
||||
```
|
||||
|
||||
This is required for:
|
||||
- Proper session handling after OIDC login
|
||||
- Rate limiting based on actual client IP addresses
|
||||
- Correct IP logging in audit trails
|
||||
|
||||
Without this setting, you may experience:
|
||||
- Session loss after SSO login (401 errors)
|
||||
- Rate limiter errors: `ValidationError: The 'X-Forwarded-For' header is set but the Express 'trust proxy' setting is false`
|
||||
|
||||
---
|
||||
|
||||
## Provider Setup Guides
|
||||
|
|
@ -530,6 +547,37 @@ If a user was created via SSO and has no password:
|
|||
|
||||
## Troubleshooting
|
||||
|
||||
### Session Lost After SSO Login (401 Errors)
|
||||
|
||||
**Symptoms:**
|
||||
- Successfully authenticate with SSO provider
|
||||
- Redirected to `/today` page
|
||||
- Immediately see login screen again
|
||||
- API calls return 401 "Authentication required"
|
||||
- Browser logs show repeated 401 errors on `/api/profile`
|
||||
|
||||
**Cause:** Express is not configured to trust the reverse proxy, causing session cookies to not be set properly.
|
||||
|
||||
**Solution:**
|
||||
|
||||
Add to your `.env` file:
|
||||
```bash
|
||||
TUDUDI_TRUST_PROXY=true
|
||||
```
|
||||
|
||||
Then restart the Tududi server:
|
||||
```bash
|
||||
docker compose restart # For Docker
|
||||
npm start # For standalone
|
||||
```
|
||||
|
||||
**Verification:**
|
||||
|
||||
After restarting, the error log should no longer show:
|
||||
```
|
||||
ValidationError: The 'X-Forwarded-For' header is set but the Express 'trust proxy' setting is false
|
||||
```
|
||||
|
||||
### "Provider not found" Error
|
||||
|
||||
**Cause:** The provider slug in the URL doesn't match any configured provider.
|
||||
|
|
|
|||
|
|
@ -771,15 +771,27 @@ OIDC_PROVIDER_3_AUTO_PROVISION=true
|
|||
- **Azure AD:** `https://login.microsoftonline.com/{tenant-id}/v2.0`
|
||||
- **Generic:** Any OIDC-compliant provider with `.well-known/openid-configuration`
|
||||
|
||||
### Required Environment Variable
|
||||
### Required Environment Variables
|
||||
|
||||
The following environment variables must be set for OAuth redirects:
|
||||
|
||||
The `BASE_URL` environment variable must be set for OAuth redirects:
|
||||
```bash
|
||||
# Base URL for callback redirects
|
||||
BASE_URL=http://localhost:3002 # Development
|
||||
BASE_URL=https://tududi.example.com # Production
|
||||
|
||||
# Trust proxy (REQUIRED for production behind reverse proxy)
|
||||
TUDUDI_TRUST_PROXY=true
|
||||
```
|
||||
|
||||
This is used to construct the callback URL: `${BASE_URL}/api/oidc/callback/{slug}`
|
||||
**Why TUDUDI_TRUST_PROXY is Required:**
|
||||
|
||||
When deployed behind a reverse proxy (nginx, Traefar, Apache), Express must be configured to trust the proxy headers. Without this:
|
||||
- Sessions may not be saved properly after OIDC callback
|
||||
- Rate limiting will fail with `X-Forwarded-For` errors
|
||||
- Users will experience 401 errors after successful SSO login
|
||||
|
||||
The `BASE_URL` is used to construct the callback URL: `${BASE_URL}/api/oidc/callback/{slug}`
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue