Skip to content

Commit ac399eb

Browse files
committed
fix(web): keep session on transient network errors during context init
The web frontend (now maintained in this monorepo under web/) re-establishes the authenticated context on page reload by calling updateContext(), which fetches user data over HTTP. Previously any error thrown there - including transient connectivity failures (ERR_NETWORK, timeouts, fetch TypeErrors) - was treated like an expired or invalid token, forcing a logout and redirect to the access-denied page. This adds an isNetworkError() classifier so connectivity failures preserve the existing session, while genuine auth rejections (carrying an HTTP response such as 401) still trigger the existing logout flow. Applies to the page-reload token-restore path and the addUserLoaded handler. Ports the fix for owncloud/web#12980 (originally owncloud/web#13941; owncloud/web is deprecated and development moved into this repo). Signed-off-by: dj4oC <noreply@github.com>
1 parent fe6a3f5 commit ac399eb

3 files changed

Lines changed: 158 additions & 3 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
Bugfix: Keep the web session on transient network errors during context init
2+
3+
We've fixed a bug in the web frontend where a temporary connectivity failure (e.g. a
4+
fast page reload cancelling in-flight requests, or a short offline period) while
5+
re-establishing the authenticated context on page load was treated like an expired or
6+
invalid token. This forced a full logout and redirected the user to the "trouble
7+
connecting to the login service" page. Network/connectivity errors now preserve the
8+
existing session, while genuine authentication rejections (e.g. a 401) still trigger
9+
the regular logout flow as before.
10+
11+
https://github.com/owncloud/web/issues/12980

web/packages/web-runtime/src/services/auth/authService.ts

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,40 @@ import { PublicLinkType } from '@ownclouders/web-client'
2626
import { WebWorkersStore } from '@ownclouders/web-pkg'
2727
import { isSilentRedirectRoute } from '../../helpers/silentRedirect'
2828

29+
/**
30+
* Distinguishes a transient network/connectivity failure from a genuine
31+
* authentication rejection.
32+
*
33+
* When the context is (re-)established on page reload, user data is fetched over
34+
* HTTP. If that request fails because of a temporary connectivity issue (offline,
35+
* DNS, a page refresh cancelling in-flight requests, ...) we must NOT treat it like
36+
* an expired/invalid token and tear down the session. Only errors that carry an HTTP
37+
* response (e.g. a `401`) are real auth rejections.
38+
*
39+
* Recognised connectivity failures:
40+
* - Axios errors without an HTTP `response`, with code `ERR_NETWORK` (connection
41+
* refused / DNS / CORS / offline) or `ECONNABORTED` (timeout).
42+
* - Browser `fetch` failures, which surface as a `TypeError` (Safari: "Load failed",
43+
* Chromium: "Failed to fetch", Firefox: "NetworkError when attempting to fetch ...").
44+
*/
45+
export const isNetworkError = (e: unknown): boolean => {
46+
if (e instanceof TypeError) {
47+
return /load failed|failed to fetch|networkerror/i.test(e.message)
48+
}
49+
50+
if (e && typeof e === 'object') {
51+
const err = e as { response?: unknown; code?: string }
52+
// An error that carries an HTTP response is an answered request, not a
53+
// connectivity failure - let the regular auth-error handling deal with it.
54+
if (err.response) {
55+
return false
56+
}
57+
return err.code === 'ERR_NETWORK' || err.code === 'ECONNABORTED'
58+
}
59+
60+
return false
61+
}
62+
2963
export class AuthService implements AuthServiceInterface {
3064
private clientService: ClientService
3165
private configStore: ConfigStore
@@ -194,7 +228,11 @@ export class AuthService implements AuthServiceInterface {
194228
this.updateMfaExpiryTimer()
195229
} catch (e) {
196230
console.error(e)
197-
await this.handleAuthError(unref(this.router.currentRoute))
231+
// A transient connectivity failure must not force a logout - keep the
232+
// session and let the regular auth flow recover once back online.
233+
if (!isNetworkError(e)) {
234+
await this.handleAuthError(unref(this.router.currentRoute))
235+
}
198236
}
199237
})
200238

@@ -259,7 +297,11 @@ export class AuthService implements AuthServiceInterface {
259297
}
260298
} catch (e) {
261299
console.error(e)
262-
await this.handleAuthError(unref(this.router.currentRoute))
300+
// A transient connectivity failure must not force a logout - keep the
301+
// session and let the regular auth flow recover once back online.
302+
if (!isNetworkError(e)) {
303+
await this.handleAuthError(unref(this.router.currentRoute))
304+
}
263305
}
264306
}
265307
}

web/packages/web-runtime/tests/unit/services/auth/authService.spec.ts

Lines changed: 103 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { ConfigStore, useAuthStore, useConfigStore } from '@ownclouders/web-pkg'
22
import { mock } from 'vitest-mock-extended'
33
import { Router } from 'vue-router'
4-
import { AuthService } from '../../../../src/services/auth/authService'
4+
import { AuthService, isNetworkError } from '../../../../src/services/auth/authService'
55
import { UserManager } from '../../../../src/services/auth/userManager'
66
import { RouteLocation, createRouter, createTestingPinia } from '@ownclouders/web-test-helpers'
77
import { User } from 'oidc-client-ts'
@@ -224,4 +224,106 @@ describe('AuthService', () => {
224224
expect(mockSignInRedirect).not.toHaveBeenCalled()
225225
})
226226
})
227+
228+
describe('isNetworkError', () => {
229+
it.each([
230+
['axios ERR_NETWORK (offline / connection refused)', { code: 'ERR_NETWORK' }, true],
231+
['axios ECONNABORTED (timeout)', { code: 'ECONNABORTED' }, true],
232+
['fetch TypeError - Safari "Load failed"', new TypeError('Load failed'), true],
233+
['fetch TypeError - Chromium "Failed to fetch"', new TypeError('Failed to fetch'), true],
234+
[
235+
'fetch TypeError - Firefox "NetworkError ..."',
236+
new TypeError('NetworkError when attempting to fetch resource.'),
237+
true
238+
],
239+
['an answered request carrying an HTTP response (401)', { response: { status: 401 } }, false],
240+
[
241+
'an ERR_NETWORK code that nonetheless carries a response',
242+
{ code: 'ERR_NETWORK', response: { status: 500 } },
243+
false
244+
],
245+
['an unrelated TypeError (real bug)', new TypeError('foo is not a function'), false],
246+
['an error with an unknown code', { code: 'ERR_BAD_REQUEST' }, false],
247+
['a null error', null, false],
248+
['a plain string error', 'boom', false]
249+
])('returns %s => %s', (_desc, error, expected) => {
250+
expect(isNetworkError(error)).toBe(expected)
251+
})
252+
})
253+
254+
describe('initializeContext auth-error classification', () => {
255+
const buildAuthService = (rejection: unknown) => {
256+
const authService = new AuthService()
257+
Object.defineProperty(authService, 'userManager', {
258+
value: mock<UserManager>({
259+
getAccessToken: vi.fn().mockResolvedValue('access-token'),
260+
getUser: vi.fn().mockResolvedValue(mock<User>({ expires_in: 3600 })),
261+
updateContext: vi.fn().mockRejectedValue(rejection)
262+
})
263+
})
264+
const router = createRouter()
265+
initAuthService({ authService, router })
266+
const handleAuthErrorSpy = vi
267+
.spyOn(authService, 'handleAuthError')
268+
.mockResolvedValue(undefined)
269+
return { authService, handleAuthErrorSpy }
270+
}
271+
272+
it('preserves the session (no handleAuthError) when updateContext fails with a network error on reload', async () => {
273+
const { authService, handleAuthErrorSpy } = buildAuthService({ code: 'ERR_NETWORK' })
274+
275+
await authService.initializeContext(mock<RouteLocation>({}))
276+
277+
expect(handleAuthErrorSpy).not.toHaveBeenCalled()
278+
})
279+
280+
it('triggers handleAuthError when updateContext fails with a 401 on reload', async () => {
281+
const { authService, handleAuthErrorSpy } = buildAuthService({ response: { status: 401 } })
282+
283+
await authService.initializeContext(mock<RouteLocation>({}))
284+
285+
expect(handleAuthErrorSpy).toHaveBeenCalled()
286+
})
287+
288+
it('addUserLoaded handler preserves the session on a network error but logs out on an auth rejection', async () => {
289+
const authService = new AuthService()
290+
let userLoadedCb: (user: User) => Promise<void>
291+
const updateContext = vi.fn()
292+
Object.defineProperty(authService, 'userManager', {
293+
value: mock<UserManager>({
294+
// skip the reload branch, we drive the addUserLoaded handler directly
295+
getAccessToken: vi.fn().mockResolvedValue(null),
296+
// force event-handler registration (mock<> would otherwise make this truthy)
297+
areEventHandlersRegistered: false,
298+
updateContext,
299+
events: {
300+
addAccessTokenExpired: vi.fn(),
301+
addAccessTokenExpiring: vi.fn(),
302+
addUserLoaded: vi.fn().mockImplementation((cb) => {
303+
userLoadedCb = cb
304+
}),
305+
addUserUnloaded: vi.fn(),
306+
addSilentRenewError: vi.fn()
307+
} as unknown as UserManager['events']
308+
})
309+
})
310+
const router = createRouter()
311+
initAuthService({ authService, router })
312+
const handleAuthErrorSpy = vi
313+
.spyOn(authService, 'handleAuthError')
314+
.mockResolvedValue(undefined)
315+
316+
await authService.initializeContext(mock<RouteLocation>({}))
317+
318+
// network error -> session preserved
319+
updateContext.mockRejectedValueOnce({ code: 'ERR_NETWORK' })
320+
await userLoadedCb(mock<User>({ access_token: 'tok', expires_in: 3600 }))
321+
expect(handleAuthErrorSpy).not.toHaveBeenCalled()
322+
323+
// auth rejection -> existing logout flow
324+
updateContext.mockRejectedValueOnce({ response: { status: 401 } })
325+
await userLoadedCb(mock<User>({ access_token: 'tok', expires_in: 3600 }))
326+
expect(handleAuthErrorSpy).toHaveBeenCalledTimes(1)
327+
})
328+
})
227329
})

0 commit comments

Comments
 (0)