mirror of
https://github.com/Ride-The-Lightning/RTL.git
synced 2026-08-13 12:33:07 +02:00
Address review: fix logout -> re-login under session-bound CSRF tokens
Session-bound tokens broke re-login after logout: logoutUser destroys the session, but the SPA navigated to the login page without a document reload, so the surviving _csrf/XSRF-TOKEN cookies stayed bound to the destroyed session id and the next login POST failed with 403 until a manual refresh. Hit both manual logout and the idle-timer auto-logout. Two coordinated fixes: 1. Frontend: the logout effect now performs a full document navigation to the login page (after the server logout completes, so the request is not aborted by the reload), which re-runs the handshake and mints a token bound to the fresh session. The logout reason previously travelled on the NgRx action stream, which cannot survive a reload - it is now handed over via sessionStorage (set after clearAll) and picked up and cleared by the login component. The SSO branch is unchanged (it already left the document). 2. Backend: the EBADCSRFTOKEN error path now re-mints the token for the current session before responding 403, so any client holding a stale token (e.g. after a server restart rotates the boot secret) self-heals on retry instead of looping on 403. Verified on the fixture: reviewer's repro now shows login 200 -> logout 200 -> stale-token login 403 (binding intact) with re-minted cookies on the 403 -> retry 200; and the reload path (fresh GET / after logout, what the full navigation does) logs in on the first attempt. Both API suites, the CSRF battery, rtl.effects specs and the full frontend suite pass; frontend and backend artifacts rebuilt.
This commit is contained in:
parent
0af050a6b2
commit
030592ac23
9 changed files with 58 additions and 9 deletions
|
|
@ -46,12 +46,12 @@ export class ExpressApplication {
|
|||
res.sendFile(join(this.directoryName, '../..', 'frontend', 'index.html'));
|
||||
});
|
||||
this.app.use((err, req, res, next) => {
|
||||
this.handleApplicationErrors(err, res);
|
||||
this.handleApplicationErrors(err, req, res);
|
||||
next();
|
||||
});
|
||||
this.logger.log({ selectedNode: this.common.selectedNode, level: 'INFO', fileName: 'App', msg: 'Application Routes Set' });
|
||||
};
|
||||
this.handleApplicationErrors = (err, res) => {
|
||||
this.handleApplicationErrors = (err, req, res) => {
|
||||
switch (err.code) {
|
||||
case 'EACCES':
|
||||
this.logger.log({ selectedNode: this.common.selectedNode, level: 'ERROR', fileName: 'App', msg: 'Server requires elevated privileges' });
|
||||
|
|
@ -66,6 +66,16 @@ export class ExpressApplication {
|
|||
res.status(401).send('Server is down/locked.');
|
||||
break;
|
||||
case 'EBADCSRFTOKEN':
|
||||
// Re-mint the token for the current session so a client retry succeeds
|
||||
// (the stale one may be bound to a destroyed session or rotated secret).
|
||||
try {
|
||||
const csrfToken = CSRF.reMintToken(req, res);
|
||||
res.cookie('XSRF-TOKEN', csrfToken);
|
||||
res.setHeader('XSRF-TOKEN', csrfToken);
|
||||
}
|
||||
catch (csrfError) {
|
||||
this.logger.log({ selectedNode: this.common.selectedNode, level: 'ERROR', fileName: 'App', msg: 'CSRF Token Re-Mint Failed', error: csrfError });
|
||||
}
|
||||
this.logger.log({ selectedNode: this.common.selectedNode, level: 'ERROR', fileName: 'App', msg: 'Invalid CSRF token. Form tempered.' });
|
||||
res.status(403).send('Invalid CSRF token, form tempered.');
|
||||
break;
|
||||
|
|
|
|||
|
|
@ -21,6 +21,10 @@ class CSRF {
|
|||
req.headers['x-csrf-token'] || req.headers['x-xsrf-token']
|
||||
});
|
||||
this.csrfProtection = this.doubleCsrfUtilities.doubleCsrfProtection;
|
||||
// Force-mints a fresh token for the current session, discarding any token
|
||||
// cookie bound to a previous session or boot secret (used by the
|
||||
// EBADCSRFTOKEN error path in app.ts so a client retry succeeds).
|
||||
this.reMintToken = (req, res) => this.doubleCsrfUtilities.generateCsrfToken(req, res, { overwrite: true });
|
||||
}
|
||||
mount(app) {
|
||||
this.logger.log({ selectedNode: this.common.selectedNode, level: 'INFO', fileName: 'CSRF', msg: 'Setting up CSRF..' });
|
||||
|
|
|
|||
File diff suppressed because one or more lines are too long
1
frontend/main.408918db3309e4a9.js
Normal file
1
frontend/main.408918db3309e4a9.js
Normal file
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
|
|
@ -68,13 +68,13 @@ export class ExpressApplication {
|
|||
res.sendFile(join(this.directoryName, '../..', 'frontend', 'index.html'));
|
||||
});
|
||||
this.app.use((err, req, res, next) => {
|
||||
this.handleApplicationErrors(err, res);
|
||||
this.handleApplicationErrors(err, req, res);
|
||||
next();
|
||||
});
|
||||
this.logger.log({ selectedNode: this.common.selectedNode, level: 'INFO', fileName: 'App', msg: 'Application Routes Set' });
|
||||
};
|
||||
|
||||
public handleApplicationErrors = (err, res) => {
|
||||
public handleApplicationErrors = (err, req, res) => {
|
||||
switch (err.code) {
|
||||
case 'EACCES':
|
||||
this.logger.log({ selectedNode: this.common.selectedNode, level: 'ERROR', fileName: 'App', msg: 'Server requires elevated privileges' });
|
||||
|
|
@ -89,6 +89,15 @@ export class ExpressApplication {
|
|||
res.status(401).send('Server is down/locked.');
|
||||
break;
|
||||
case 'EBADCSRFTOKEN':
|
||||
// Re-mint the token for the current session so a client retry succeeds
|
||||
// (the stale one may be bound to a destroyed session or rotated secret).
|
||||
try {
|
||||
const csrfToken = CSRF.reMintToken(req, res);
|
||||
res.cookie('XSRF-TOKEN', csrfToken);
|
||||
res.setHeader('XSRF-TOKEN', csrfToken);
|
||||
} catch (csrfError) {
|
||||
this.logger.log({ selectedNode: this.common.selectedNode, level: 'ERROR', fileName: 'App', msg: 'CSRF Token Re-Mint Failed', error: csrfError });
|
||||
}
|
||||
this.logger.log({ selectedNode: this.common.selectedNode, level: 'ERROR', fileName: 'App', msg: 'Invalid CSRF token. Form tempered.' });
|
||||
res.status(403).send('Invalid CSRF token, form tempered.');
|
||||
break;
|
||||
|
|
|
|||
|
|
@ -26,6 +26,11 @@ class CSRF {
|
|||
|
||||
public csrfProtection = this.doubleCsrfUtilities.doubleCsrfProtection;
|
||||
|
||||
// Force-mints a fresh token for the current session, discarding any token
|
||||
// cookie bound to a previous session or boot secret (used by the
|
||||
// EBADCSRFTOKEN error path in app.ts so a client retry succeeds).
|
||||
public reMintToken = (req, res) => this.doubleCsrfUtilities.generateCsrfToken(req, res, { overwrite: true });
|
||||
|
||||
public mount(app: Application): Application {
|
||||
this.logger.log({ selectedNode: this.common.selectedNode, level: 'INFO', fileName: 'CSRF', msg: 'Setting up CSRF..' });
|
||||
if (process.env.NODE_ENV !== 'development') {
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ import { RTLConfiguration } from '../../models/RTLconfig';
|
|||
import { APICallStatusEnum, PASSWORD_BLACKLIST, RTLActions, ScreenSizeEnum } from '../../services/consts-enums-functions';
|
||||
import { CommonService } from '../../services/common.service';
|
||||
import { LoggerService } from '../../services/logger.service';
|
||||
import { SessionService } from '../../services/session.service';
|
||||
|
||||
import { RTLEffects } from '../../../store/rtl.effects';
|
||||
import { RTLState } from '../../../store/rtl.state';
|
||||
|
|
@ -39,7 +40,7 @@ export class LoginComponent implements OnInit, OnDestroy {
|
|||
public apiCallStatusEnum = APICallStatusEnum;
|
||||
private unSubs: Array<Subject<void>> = [new Subject(), new Subject(), new Subject()];
|
||||
|
||||
constructor(private actions: Actions, private logger: LoggerService, private store: Store<RTLState>, private rtlEffects: RTLEffects, private commonService: CommonService) { }
|
||||
constructor(private actions: Actions, private logger: LoggerService, private store: Store<RTLState>, private rtlEffects: RTLEffects, private commonService: CommonService, private sessionService: SessionService) { }
|
||||
|
||||
ngOnInit() {
|
||||
this.screenSize = this.commonService.getScreenSize();
|
||||
|
|
@ -63,6 +64,13 @@ export class LoginComponent implements OnInit, OnDestroy {
|
|||
subscribe((action: any) => {
|
||||
this.logoutReason = action.payload;
|
||||
});
|
||||
// Logout navigates with a full document load (to re-mint the CSRF token),
|
||||
// so the reason arrives via sessionStorage instead of the action stream.
|
||||
const storedLogoutReason = this.sessionService.getItem('logoutReason');
|
||||
if (storedLogoutReason) {
|
||||
this.logoutReason = storedLogoutReason;
|
||||
this.sessionService.removeItem('logoutReason');
|
||||
}
|
||||
}
|
||||
|
||||
onLogin(): boolean | void {
|
||||
|
|
|
|||
|
|
@ -423,18 +423,31 @@ export class RTLEffects implements OnDestroy {
|
|||
this.store.dispatch(openSpinner({ payload: UI_MESSAGES.LOG_OUT }));
|
||||
if (appConfig.SSO && +appConfig.SSO.rtlSSO) {
|
||||
window.location.href = appConfig.SSO.logoutRedirectLink;
|
||||
} else {
|
||||
this.router.navigate(['./login'], { state: { logoutReason: action.payload } });
|
||||
}
|
||||
this.sessionService.clearAll();
|
||||
this.store.dispatch(setNodeData({ payload: {} }));
|
||||
this.store.dispatch(closeSpinner({ payload: UI_MESSAGES.LOG_OUT }));
|
||||
this.logger.info('Logged out from browser');
|
||||
// Navigate with a full document load once the server has destroyed the
|
||||
// session, so a fresh CSRF token (bound to the new session) is minted
|
||||
// before the next login. The reason survives the reload in sessionStorage.
|
||||
const navigateToLogin = () => {
|
||||
if (!(appConfig.SSO && +appConfig.SSO.rtlSSO)) {
|
||||
if (action.payload) { this.sessionService.setItem('logoutReason', action.payload); }
|
||||
window.location.href = document.baseURI + 'login';
|
||||
}
|
||||
};
|
||||
return this.httpClient.get(API_END_POINTS.AUTHENTICATE_API + '/logout').
|
||||
pipe(map((postRes: any) => {
|
||||
this.logger.info(postRes);
|
||||
this.store.dispatch(closeSpinner({ payload: UI_MESSAGES.LOG_OUT }));
|
||||
this.logger.info('Logged out from server');
|
||||
navigateToLogin();
|
||||
}), catchError((err) => {
|
||||
this.logger.error(err);
|
||||
this.store.dispatch(closeSpinner({ payload: UI_MESSAGES.LOG_OUT }));
|
||||
navigateToLogin();
|
||||
return of({ type: RTLActions.VOID });
|
||||
}));
|
||||
})),
|
||||
{ dispatch: false }
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue