Skip to content

Add logout functionality to helix-front#184

Merged
kabragaurav merged 10 commits into
devfrom
gkabra/add-logout-helix-clean
May 27, 2026
Merged

Add logout functionality to helix-front#184
kabragaurav merged 10 commits into
devfrom
gkabra/add-logout-helix-clean

Conversation

@kabragaurav
Copy link
Copy Markdown
Collaborator

@kabragaurav kabragaurav commented May 25, 2026

Summary

  • Adds POST /user/logout server route that destroys the Express session and clears the connect.sid cookie
  • Adds destroy() to the HelixSession TypeScript interface
  • Adds logout() method to Angular UserService
  • Replaces the user chip with a mat-menu dropdown offering Sign In and Sign Out
  • Adds logout() method to AppComponent with snackbar confirmation

Testing Done

  • Manually verified in browser on localhost:4200:
    • Click username chip → dropdown menu shows Sign In and Sign Out with distinct icons
    • Sign In opens LDAP dialog, authenticates as before
    • Sign Out clears session, chip reverts to "Sign In", snackbar confirms
    • After sign out, write operations correctly require re-authentication
helix-ui_logout_flow.mov

No duplicate HTTP calls:
image

Context

Users had no way to log out or re-authenticate after their identity token expired (~24h), causing 401 errors on write operations while the UI still showed them as logged in. The only workaround was manually clearing cookies in DevTools.

kabragaurav and others added 6 commits May 14, 2026 13:04
Users had no way to log out or re-authenticate after their identity
token expired (~24h), causing 401 errors on write operations while
the UI still showed them as logged in. This adds a POST /user/logout
server route that destroys the session and clears cookies, a logout()
method in UserService, and a dropdown menu on the user chip with
Sign In / Sign Out options.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…or handling

- Show Sign In only when logged out, Sign Out only when logged in
- Clear helixui_identity.token cookie on logout (not just connect.sid)
- Pass explicit { path: '/' } to clearCookie for robustness
- Remove catchError that silently swallowed logout errors

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When not logged in, clicking the user chip opens the login dialog
directly (original behavior). The dropdown menu with Sign Out only
appears when logged in.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread helix-front/src/app/app.component.html Outdated
@@ -28,10 +28,24 @@
<span class="helix-title">Helix</span>
</a>
<span fxFlex="1 1 auto"></span>
<a mat-button (click)="login()">
<a mat-button (click)="login()" *ngIf="(currentUser | async) === 'Sign In'">
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new template references (currentUser | async) 4 times in the toolbar. Each | async opens its own subscription and getCurrentUser() returns this.http.get(...), so a single page load fires 4 GET
/user/current calls.

Expose currentUser$ = this.service.getCurrentUser().pipe(shareReplay(1)) once and bind to that, or resolve the value into a plain field in ngOnInit. The old template was already noisy, this doubles
it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added shareReplay(1) to getCurrentUser() in user.service.ts. All | async pipes now share a single HTTP call per observable instance.

Comment thread helix-front/src/app/app.component.html Outdated
<button
mat-button
[matMenuTriggerFor]="userMenu"
*ngIf="(currentUser | async) !== 'Sign In'"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

=== 'Sign In' / !== 'Sign In' as a logged-in branch is brittle. If /user/current ever returns null, '', or someone happens to be named "Sign In", the UI breaks. Compute an isLoggedIn boolean (or
isLoggedIn$) and gate the two buttons on that.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced string comparison with an isLoggedIn boolean, derived via tap() on the currentUser observable. Template now uses *ngIf="isLoggedIn" / *ngIf="!isLoggedIn". The tap still checks user !== 'Sign In' internally since the server returns that as the default - changing the server return value risks breaking other consumers.

Comment thread helix-front/server/controllers/user.ts Outdated
Comment on lines +60 to +70
protected logout(req: HelixRequest, res: Response) {
req.session.destroy((err) => {
if (err) {
res.status(500).json({ error: 'Logout failed' });
return;
}
res.clearCookie('connect.sid', { path: '/' });
res.clearCookie('helixui_identity.token');
res.json(true);
});
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

req.session is typed session?: HelixSession (optional) in d.ts. If a request lands without a session (store flake, tampered cookie, whatever), req.session.destroy(...) throws TypeError and the client
gets a generic 500.

if (!req.session) { 
  res.json(true); 
  return; 
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added if (!req.session) guard before destroy() call. Returns res.json(true) early if session is missing.

Comment thread helix-front/server/controllers/d.ts Outdated
@@ -11,4 +12,20 @@ interface HelixSession {
identityToken: any;
username: string;
isAdmin: boolean;
destroy(callback: (err?: any) => void): void;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #186 has destroy(callback?: (err?: any) => void): void (optional) which matches express-session's actual signature. This PR has it required. Mirror #186 so the two PRs don't fight over the same
line at merge.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to callback?: (optional) to match express-session's actual signature. Now consistent with #186.

… null check, optional destroy callback

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@kabragaurav kabragaurav force-pushed the gkabra/add-logout-helix-clean branch from 9620b72 to 592baf5 Compare May 27, 2026 06:45
@@ -116,4 +123,18 @@ export class AppComponent implements OnInit {
}
);
}

logout() {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once #186 lands, AppComponent has a 30s setInterval watching the identity cookie. Your manual Sign Out clears the cookie but doesn't stop that interval. ~30s after the user clicks Sign Out, #186's
watcher detects "no token", pops "Session Expired", calls /logout again, and reloads. User experiences: click Sign Out -> snackbar -> 30s of normal UI -> surprise dialog.

Stop the watcher on manual logout. Either expose a stopExpiryWatch() public method from the watcher logic, or move the handle out of AppComponent into a shared service that both #186 and #184's
logout can drive.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added clearInterval(this.expiryCheckHandle) at the top of logout() before the API call. Stops the 30s watcher immediately so it won't detect the cleared cookie and fire a surprise dialog.

Gaurav Kabra and others added 3 commits May 27, 2026 18:13
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…store session null check

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@LZD-PratyushBhatt LZD-PratyushBhatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kabragaurav kabragaurav merged commit b81fbdf into dev May 27, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants