Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements RP-initiated logout functionality following the OpenID Connect specification. It changes the logout flow from redirecting to the auth endpoint with request_credentials=required to using a dedicated logout endpoint.
Changes:
- Added
logoutconfiguration field andprepareLogoutRequestmethod to AuthRequestBuilder - Updated the
logout()method to use RP-initiated logout instead of forcing re-authentication - Updated tests to reflect the new logout behavior
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/auth/request-builder.ts | Added logout URL configuration and prepareLogoutRequest method to build logout URLs with state parameter |
| src/auth/request-builder.test.ts | Added comprehensive test coverage for the new prepareLogoutRequest method |
| src/auth/auth-core.ts | Updated logout method to use RP-initiated logout flow instead of auth re-request |
| src/auth/auth.test.ts | Updated tests to verify logout endpoint is called with correct parameters |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1f22d55 to
ac6f8de
Compare
ac6f8de to
3c848a4
Compare
…ence for RP-initiated logout - Check Hub version (>= 2026.1) before using /oauth2/logout endpoint, fall back to legacy /oauth2/auth redirect for older Hub versions - Add singleLogout config option (default: true) to allow disabling the new RP-initiated logout flow - Persist logout state via _saveState for CSRF protection on redirect back - Fetch Hub version from services endpoint filtered by is:hostService
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
Comments suppressed due to low confidence (1)
src/auth/auth.test.ts:860
- Duplicate test case. This test at lines 850-860 is nearly identical to the duplicate at lines 874-884. The tests have the same description and nearly identical implementation. One of these duplicates should be removed.
it('should fail pass when onLogout mockReturnValue rejected promise', async () => {
const onLogout = vi.fn();
const logoutAuth = new Auth({
serverUri: '',
onLogout,
});
vi.spyOn(logoutAuth.http, 'get').mockResolvedValue({services: [{version: '2026.1'}]});
await logoutAuth.logout();
expect(onLogout).toHaveBeenCalledOnce;
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add early boundary check in version parsing (parts.length < 2) - Fix JSDoc return type for prepareLogoutRequest - Remove duplicate test cases in Logout describe block - Add test coverage: version caching, empty services array, missing version field, patch version strings, major-only version
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The backend version check via hostService is unreliable as host services vary across deployment types. The logout flow now depends solely on the singleLogout config option.
No description provided.