Skip to content

Commit d0284ac

Browse files
committed
Address degraded-auth review feedback
- Drive.tsx + drive.ts: surface the cloud-unavailable stub on the default category and force `cloud` as the default while degraded, so the failure is visible right after sign-in instead of silently landing on Local. - auth.ts: disable usersMeQuery retries so the degraded UI appears without the ~10 s router-guard stall; key the synthetic-user cache + userId on cognito email instead of the app-wide clientId; guard setUsername. - Settings/data.tsx: keep the Account tab reachable in degraded mode (Cognito-only sections stay visible), hide cloud-dependent sections. - auth.test.ts / degradedAuth.spec.ts: update tests for email-based identifier; drive the 401 path by hand so the form-disappears assertion doesn't fight the immediate unauthorized-recovery logout.
1 parent ca12150 commit d0284ac

6 files changed

Lines changed: 77 additions & 20 deletions

File tree

app/gui/integration-test/dashboard/degradedAuth.spec.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/** @file Verify the degraded-auth mode triggered when `users/me` fails for a non-auth reason. */
22
import { expect, test } from 'integration-test/base'
33

4-
import { TEXT } from '../actions'
4+
import { TEXT, VALID_EMAIL, VALID_PASSWORD } from '../actions'
55

66
const HTTP_INTERNAL_SERVER_ERROR = 500
77
const HTTP_UNAUTHORIZED = 401
@@ -54,11 +54,20 @@ test('401 keeps the existing recovery + logout path, no degraded UI', async ({
5454
loginPage,
5555
cloudApi,
5656
}) => {
57+
// Submit credentials by hand: `loginPage.login()` asserts the login form disappears,
58+
// but the 401-driven unauthorized-recovery flow logs the user back out almost
59+
// immediately, so the form may never stay hidden long enough.
5760
cloudApi.setUsersMeFailureStatus(HTTP_UNAUTHORIZED)
5861

59-
await loginPage.login().do(async (page) => {
60-
// The unauthorized-recovery flow eventually logs the user out, which makes the
61-
// login form visible again. The degraded `cloud-unavailable` stub must never show.
62+
await loginPage.do(async (page) => {
63+
await page.getByPlaceholder(TEXT.emailPlaceholder).fill(VALID_EMAIL)
64+
await page.getByPlaceholder(TEXT.passwordPlaceholder).fill(VALID_PASSWORD)
65+
await page
66+
.getByRole('button', { name: TEXT.login, exact: true })
67+
.getByText(TEXT.login)
68+
.click()
69+
// After unauthorized recovery exhausts, the user is logged out and the login
70+
// screen is shown again. The cloud-unavailable stub must never appear.
6271
await expect(
6372
page.getByRole('button', { name: TEXT.login, exact: true }),
6473
).toBeVisible({ timeout: 30_000 })

app/gui/src/dashboard/layouts/Drive.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { ErrorBoundary } from '#/components/ErrorBoundary'
55
import * as result from '#/components/Result'
66
import SvgMask from '#/components/SvgMask'
77
import { useEventCallback } from '#/hooks/eventCallbackHooks'
8+
import { useLocalStorageState } from '#/hooks/localStoreState'
89
import * as offlineHooks from '#/hooks/offlineHooks'
910
import * as toastAndLogHooks from '#/hooks/toastAndLogHooks'
1011
import AssetsTable from '#/layouts/AssetsTable'
@@ -56,6 +57,13 @@ function DriveInner(props: DriveProperties) {
5657
const { getText } = useText()
5758
const [category, setCategory] = useDriveCurrentCategory()
5859
const { setDefaultCategory } = useDriveLocation()
60+
// The cloud-unavailable status is decoupled from `isCloud`: while the user is on the
61+
// default category (no explicit choice yet) the stub takes over so cloud failure is
62+
// surfaced no matter which category the default logic picked. Once the user explicitly
63+
// picks a category — sidebar click or the "Switch to Local" button — `driveDisplay`
64+
// becomes non-null and we defer to their choice for the local categories.
65+
const [storedDriveDisplay] = useLocalStorageState('driveDisplay')
66+
const hasExplicitCategory = storedDriveDisplay != null
5967

6068
const isCloud = isCloudCategory(category)
6169

@@ -69,7 +77,7 @@ function DriveInner(props: DriveProperties) {
6977

7078
const status =
7179
isCloud && isOffline ? 'offline'
72-
: isCloud && session.isCloudDataUnavailable ? 'cloud-unavailable'
80+
: session.isCloudDataUnavailable && (isCloud || !hasExplicitCategory) ? 'cloud-unavailable'
7381
: isCloud && !user.isEnabled ? 'not-enabled'
7482
: 'ok'
7583

app/gui/src/dashboard/layouts/Settings/data.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,9 @@ export const SETTINGS_TAB_DATA: Readonly<Record<SettingsTabType, SettingsTabData
7070
nameId: 'accountSettingsTab',
7171
settingsTab: SettingsTabType.account,
7272
icon: 'settings',
73-
visible: ({ isCloudDataUnavailable }) => !isCloudDataUnavailable,
73+
// The tab stays visible in degraded-auth mode so the Cognito-only sections
74+
// (password change, 2FA setup) remain reachable; the cloud-dependent sections
75+
// hide themselves via their own `getVisible`.
7476
sections: [
7577
{
7678
nameId: 'userAccountSettingsSection',
@@ -89,6 +91,7 @@ export const SETTINGS_TAB_DATA: Readonly<Record<SettingsTabType, SettingsTabData
8991
IanaTimeZone(getLocalTimeZone()),
9092
),
9193
}),
94+
getVisible: ({ isCloudDataUnavailable }) => !isCloudDataUnavailable,
9295
onSubmit: async (context, { name, timeZone }) => {
9396
const newTimeZone = timeZone != null ? tryGetTimeZoneFromDescription(timeZone) : null
9497
if (newTimeZone != null) {
@@ -229,6 +232,7 @@ export const SETTINGS_TAB_DATA: Readonly<Record<SettingsTabType, SettingsTabData
229232
{
230233
type: 'custom',
231234
aliasesId: 'deleteUserAccountSettingsCustomEntryAliases',
235+
getVisible: ({ isCloudDataUnavailable }) => !isCloudDataUnavailable,
232236
render: () => <DeleteUserAccountSettingsSection />,
233237
},
234238
],
@@ -240,6 +244,7 @@ export const SETTINGS_TAB_DATA: Readonly<Record<SettingsTabType, SettingsTabData
240244
{
241245
type: 'custom',
242246
aliasesId: 'profilePictureSettingsCustomEntryAliases',
247+
getVisible: ({ isCloudDataUnavailable }) => !isCloudDataUnavailable,
243248
render: (context) => <ProfilePictureInput backend={context.backend} />,
244249
},
245250
],

app/gui/src/providers/__tests__/auth.test.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ describe('makeSyntheticUser', () => {
4444
})
4545

4646
it('derives identifiers in the expected newtype shape', () => {
47-
const user = makeSyntheticUser(fakeCognitoSession({ clientId: 'abc' }))
47+
const user = makeSyntheticUser(fakeCognitoSession({ email: 'someone@enso.org' }))
4848
expect(isUserId(user.userId)).toBe(true)
49-
expect(user.userId).toContain('abc')
49+
expect(user.userId).toContain('someone@enso.org')
5050
expect(isOrganizationId(user.organizationId)).toBe(true)
5151
expect(isDirectoryId(user.rootDirectoryId)).toBe(true)
5252
})
@@ -57,8 +57,14 @@ describe('makeSyntheticUser', () => {
5757
expect(user.name).toBe('someone@enso.org')
5858
})
5959

60-
it('handles a missing clientId without producing an empty identifier', () => {
61-
const user = makeSyntheticUser(fakeCognitoSession({ clientId: '' }))
60+
it('keys identifiers on email so two users on the same Cognito app are distinct', () => {
61+
const a = makeSyntheticUser(fakeCognitoSession({ email: 'a@enso.org' }))
62+
const b = makeSyntheticUser(fakeCognitoSession({ email: 'b@enso.org' }))
63+
expect(a.userId).not.toBe(b.userId)
64+
})
65+
66+
it('handles a missing email without producing an empty identifier', () => {
67+
const user = makeSyntheticUser(fakeCognitoSession({ email: '' }))
6268
expect(user.userId).toBe('user-cloud-unavailable-unknown')
6369
})
6470
})

app/gui/src/providers/auth.ts

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ export interface UserSession extends cognitoModule.UserSession {
2323
* `true` when this session is a placeholder synthesized after `users/me` failed with
2424
* a non-auth error. The user is signed in to Cognito and can use local projects, but
2525
* the `user` field is a stub — cloud features must be disabled by callers.
26+
*
27+
* Only set on deployments that have a local backend; cloud-only builds fall back to
28+
* the existing redirect-to-login path when `users/me` fails, so this flag is never
29+
* `true` there.
2630
*/
2731
readonly isCloudDataUnavailable?: boolean
2832
}
@@ -72,6 +76,12 @@ export function createUsersMeQuery(
7276
let refetchCount = 0
7377
return vueQuery.queryOptions({
7478
queryKey: createUsersMeQueryKey(session, remoteBackend),
79+
// Disable the default 3-retry backoff: a failed `users/me` should surface immediately
80+
// so the degraded-auth UI can render instead of stalling navigation for ~10 s while
81+
// the query retries. Unauthorized errors have a dedicated recovery flow in
82+
// {@link useUnauthorizedRecovery}; transient network errors are handled by the
83+
// user clicking the "Retry" button.
84+
retry: false,
7585
queryFn: async (): Promise<UserSession | null> => {
7686
const sessionVal = toValue(session)
7787
if (!sessionVal) {
@@ -102,11 +112,14 @@ export function createUsersMeQuery(
102112
* response is unavailable. All cloud features must be treated as disabled — the
103113
* placeholder mirrors a real user without a licence (`isEnabled: false`,
104114
* `plan: Plan.free`) so the existing "not-enabled" rendering paths apply.
115+
*
116+
* Identifier fields embed the Cognito email so the placeholder is distinct per signed-in
117+
* user (the Cognito app `clientId` is a deployment-wide constant and would alias users).
105118
*/
106119
export function makeSyntheticUser(cognitoSession: cognitoModule.UserSession): backendModule.User {
107-
const clientIdSuffix = cognitoSession.clientId || 'unknown'
120+
const identitySuffix = cognitoSession.email || 'unknown'
108121
return {
109-
userId: backendModule.UserId(`user-cloud-unavailable-${clientIdSuffix}`),
122+
userId: backendModule.UserId(`user-cloud-unavailable-${identitySuffix}`),
110123
organizationId: backendModule.OrganizationId(
111124
'organization-00000000000000000000000000',
112125
),
@@ -162,7 +175,13 @@ function createAuthStore(
162175
})
163176

164177
const setUsername = async (username: string) => {
165-
if (userData.value != null) {
178+
if (isCloudDataUnavailable.value) {
179+
throw new Error('Cannot set username while Enso Cloud is unavailable.')
180+
}
181+
// Branch on the real `users/me` result, not the (possibly synthetic) `userData`:
182+
// a synthetic placeholder would otherwise route us into the update path and hit
183+
// the failing remote.
184+
if (usersMeQuery.data.value != null) {
166185
await updateUserMutation.mutateAsync({ username })
167186
} else {
168187
const orgId = await organizationId()
@@ -190,10 +209,13 @@ function createAuthStore(
190209

191210
const usersMeQuery = vueQuery.useQuery(usersMeQueryOptions)
192211

193-
let syntheticUserCache: { clientId: string; user: backendModule.User } | null = null
212+
// Keyed on `email`, not `clientId`: `clientId` is the Cognito app integration ID and is
213+
// identical across users on the same deployment, so caching on it would surface user A's
214+
// placeholder for user B after a sign-out/sign-in.
215+
let syntheticUserCache: { email: string; user: backendModule.User } | null = null
194216
const getSyntheticUser = (cognitoSession: cognitoModule.UserSession) => {
195-
if (syntheticUserCache?.clientId !== cognitoSession.clientId) {
196-
syntheticUserCache = { clientId: cognitoSession.clientId, user: makeSyntheticUser(cognitoSession) }
217+
if (syntheticUserCache?.email !== cognitoSession.email) {
218+
syntheticUserCache = { email: cognitoSession.email, user: makeSyntheticUser(cognitoSession) }
197219
}
198220
return syntheticUserCache.user
199221
}
@@ -202,7 +224,8 @@ function createAuthStore(
202224
* `true` when Cognito sign-in succeeded but the subsequent `users/me` fetch failed
203225
* with a non-auth error. Auth (401/403) failures are owned by `useUnauthorizedRecovery`
204226
* and excluded here. Requires a local backend so the synthesised session has somewhere
205-
* to land — without one, callers fall back to the redirect-to-login path.
227+
* to land — on cloud-only deployments without `localBackend`, this stays `false` and
228+
* the user falls through to the existing redirect-to-login path.
206229
*/
207230
const isCloudDataUnavailable = computed(() => {
208231
const cognitoSession = session.value

app/gui/src/providers/drive.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { createContextStore } from '@/providers'
44
import { isDirectoryId, type DirectoryId } from 'enso-common/src/services/Backend'
55
import { computed, watch } from 'vue'
66
import * as z from 'zod'
7+
import { useAuth } from './auth'
78
import { useBackends } from './backends'
89
import {
910
CATEGORY_BACKEND,
@@ -39,12 +40,17 @@ export const [provideDriveLocation, useDriveLocation] = createContextStore(
3940
(startReactTransition: (action: () => void) => void) => {
4041
const backends = useBackends()
4142
const categories = useCategories()
43+
const auth = useAuth()
4244
const localStorage = LocalStorage.getInstance()
4345
const storedDriveDisplay = computed(() => localStorage.get('driveDisplay'))
4446

45-
const defaultCategory = computed<Category>(() =>
46-
backends.localBackend != null ? { type: 'local' } : { type: 'cloud' },
47-
)
47+
// In degraded-auth mode the user must land on cloud so the "Enso Cloud is unavailable"
48+
// stub (rendered by the cloud-category view) is visible after sign-in. Otherwise the
49+
// existing local-first default would silently hide the failure.
50+
const defaultCategory = computed<Category>(() => {
51+
if (auth.session?.isCloudDataUnavailable) return { type: 'cloud' }
52+
return backends.localBackend != null ? { type: 'local' } : { type: 'cloud' }
53+
})
4854

4955
const currentCategory = computed({
5056
get: () =>

0 commit comments

Comments
 (0)