Skip to content

Commit

Permalink
Improved OIDC error reporting to make it easier to identify issues
Browse files Browse the repository at this point in the history
  • Loading branch information
jordan-dalby committed Nov 24, 2024
1 parent 8f7d19f commit 83d6ed4
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 19 deletions.
12 changes: 7 additions & 5 deletions client/src/components/auth/LoginPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { useToast } from '../../hooks/useToast';
import { ROUTES } from '../../constants/routes';
import { OIDCConfig } from '../../types/auth';
import { apiClient } from '../../utils/api/apiClient';
import { handleOIDCError } from '../../utils/oidcErrorHandler';

export const LoginPage: React.FC = () => {
const [username, setUsername] = useState('');
Expand All @@ -18,12 +19,13 @@ export const LoginPage: React.FC = () => {

useEffect(() => {
const params = new URLSearchParams(window.location.search);
if (params.get('error') === 'auth_failed') {
addToast('Authentication failed. Please try again.', 'error');
} else if (params.get('error') === 'registration_disabled') {
addToast('Registration is disabled on this ByteStash instance.', 'error');
const error = params.get('error');
const message = params.get('message');

if (error) {
handleOIDCError(error, addToast, oidcConfig?.displayName, message || undefined);
}
}, []);
}, [addToast, oidcConfig]);

useEffect(() => {
const fetchOIDCConfig = async () => {
Expand Down
15 changes: 11 additions & 4 deletions client/src/components/auth/oidc/OIDCCallback.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,31 @@ import { useNavigate } from 'react-router-dom';
import { useAuth } from '../../../hooks/useAuth';
import { Loader2 } from 'lucide-react';
import { PageContainer } from '../../common/layout/PageContainer';
import { useToast } from '../../../hooks/useToast';
import { handleOIDCError } from '../../../utils/oidcErrorHandler';

export const OIDCCallback: React.FC = () => {
const navigate = useNavigate();
const { login } = useAuth();
const { addToast } = useToast();

useEffect(() => {
const params = new URLSearchParams(window.location.search);
const token = params.get('token');
const error = params.get('error');
const message = params.get('message');

if (token) {
// Store the token and redirect
login(token, null);
navigate('/', { replace: true });
} else if (error) {
handleOIDCError(error, addToast, undefined, message || undefined);
navigate('/login', { replace: true });
} else {
// Handle error case
navigate('/login?error=auth_failed', { replace: true });
handleOIDCError('auth_failed', addToast);
navigate('/login', { replace: true });
}
}, [login, navigate]);
}, [login, navigate, addToast]);

return (
<PageContainer>
Expand Down
14 changes: 8 additions & 6 deletions client/src/contexts/ToastContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ export interface Toast {
id: number;
message: string;
type: ToastType;
duration: number;
duration: number | null;
}

interface ToastContextType {
addToast: (message: string, type?: ToastType, duration?: number) => void;
export interface ToastContextType {
addToast: (message: string, type?: ToastType, duration?: number | null) => void;
removeToast: (id: number) => void;
}

Expand Down Expand Up @@ -65,7 +65,7 @@ const ToastComponent: React.FC<ToastProps> = ({
clearInterval(interval);
return 0;
}
return prev - (100 / (duration / 100));
return prev - (100 / ((duration || 0) / 100));
});
}, 100);

Expand Down Expand Up @@ -106,11 +106,13 @@ export const ToastProvider: React.FC<{ children: React.ReactNode }> = ({ childre
const addToast = useCallback((
message: string,
type: ToastType = 'info',
duration = 3000
duration: number | null = 3000
) => {
const id = Date.now();
setToasts(prev => [...prev, { id, message, type, duration }]);
setTimeout(() => removeToast(id), duration);
if (duration !== null) {
setTimeout(() => removeToast(id), duration);
}
}, [removeToast]);

return (
Expand Down
55 changes: 55 additions & 0 deletions client/src/utils/oidcErrorHandler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { ToastType } from '../contexts/ToastContext';

interface OIDCErrorConfig {
message: string;
type: ToastType;
duration?: number | null;
}

const OIDC_ERROR_CONFIGS: Record<string, OIDCErrorConfig> = {
auth_failed: {
message: "Authentication failed. This could be due to a cancelled login attempt or an expired session. Please try again.",
type: 'error',
duration: 8000
},
registration_disabled: {
message: "New account registration is currently disabled on this ByteStash instance. Please contact your administrator.",
type: 'error',
duration: null
},
provider_error: {
message: "The identity provider encountered an error or is unavailable. Please try again later or contact your administrator.",
type: 'error',
duration: 8000
},
config_error: {
message: "There was an error with the SSO configuration. Please contact your administrator.",
type: 'error',
duration: null
},
default: {
message: "An unexpected error occurred during authentication. Please try again.",
type: 'error',
duration: 8000
}
};

export const handleOIDCError = (
error: string,
addToast: (message: string, type: ToastType, duration?: number | null) => void,
providerName?: string,
additionalMessage?: string
) => {
const config = OIDC_ERROR_CONFIGS[error] || OIDC_ERROR_CONFIGS.default;
let message = config.message;

if (providerName) {
message = message.replace('identity provider', providerName);
}

if (additionalMessage) {
message = `${message}\n\nError details: ${additionalMessage}`;
}

addToast(message, config.type, config.duration);
};
2 changes: 1 addition & 1 deletion server/src/app.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import express from 'express';
import { initializeDatabase } from './config/database.js';
import { initializeDatabase, shutdownDatabase } from './config/database.js';
import snippetRoutes from './routes/snippetRoutes.js';
import authRoutes from './routes/authRoutes.js';
import shareRoutes from './routes/shareRoutes.js';
Expand Down
23 changes: 20 additions & 3 deletions server/src/routes/oidcRoutes.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ router.get('/auth', async (req, res) => {
try {
const oidc = await OIDCConfig.getInstance();
if (!oidc.isEnabled()) {
return res.status(404).json({ error: 'OIDC not enabled' });
return res.redirect('/login?error=config_error');
}

const baseUrl = getBaseUrl(req);
Expand All @@ -58,7 +58,8 @@ router.get('/auth', async (req, res) => {
res.redirect(authUrl);
} catch (error) {
Logger.error('OIDC auth error:', error);
res.redirect('/login?error=auth_failed');
const errorMessage = encodeURIComponent(error.message || 'Unknown error');
res.redirect(`/login?error=provider_error&message=${errorMessage}`);
}
});

Expand Down Expand Up @@ -118,7 +119,23 @@ router.get('/callback', async (req, res) => {
res.redirect(`/auth/callback?token=${token}`);
} catch (error) {
Logger.error('OIDC callback error:', error);
res.redirect('/login?error=auth_failed');
let errorType = 'auth_failed';
let errorDetails = '';

if (error.message?.includes('state parameter')) {
errorType = 'auth_failed';
errorDetails = 'Your authentication session has expired';
} else if (error.message?.includes('accounts disabled')) {
errorType = 'registration_disabled';
} else if (error.message?.includes('OIDC configuration')) {
errorType = 'config_error';
} else if (error.response?.status === 401 || error.response?.status === 403) {
errorType = 'provider_error';
errorDetails = 'Authorization denied by identity provider';
}

const messageParam = errorDetails ? `&message=${encodeURIComponent(errorDetails)}` : '';
res.redirect(`/login?error=${errorType}${messageParam}`);
}
});

Expand Down

0 comments on commit 83d6ed4

Please sign in to comment.