Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalidate queries when the user goes online + Fix offline mode #11944

Merged
merged 10 commits into from
Jan 14, 2025
2 changes: 2 additions & 0 deletions app/common/src/queryClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,15 @@ export function createQueryClient<TStorageValue = string>(
const shouldAwaitInvalidates = mutation.meta?.awaitInvalidates ?? false
const refetchType = mutation.meta?.refetchType ?? 'active'
const invalidates = mutation.meta?.invalidates ?? []

const invalidatesToAwait = (() => {
if (Array.isArray(shouldAwaitInvalidates)) {
return shouldAwaitInvalidates
} else {
return shouldAwaitInvalidates ? invalidates : []
}
})()

const invalidatesToIgnore = invalidates.filter(
(queryKey) => !invalidatesToAwait.includes(queryKey),
)
Expand Down
19 changes: 7 additions & 12 deletions app/gui/src/dashboard/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ export default function App(props: AppProps) {
const { mutate: executeBackgroundUpdate } = useMutation({
mutationKey: ['refetch-queries', { isOffline }],
scope: { id: 'refetch-queries' },
mutationFn: () => queryClient.refetchQueries({ type: 'all' }),
mutationFn: () => queryClient.refetchQueries({ type: 'all', queryKey: [RemoteBackend.type] }),
networkMode: 'online',
onError: () => {
toastify.toast.error(getText('refetchQueriesError'), {
Expand Down Expand Up @@ -297,7 +297,7 @@ export interface AppRouterProps extends AppProps {
* component as the component that defines the provider.
*/
function AppRouter(props: AppRouterProps) {
const { isAuthenticationDisabled, onAuthenticated, projectManagerInstance } = props
const { onAuthenticated, projectManagerInstance } = props
const httpClient = useHttpClientStrict()
const logger = useLogger()
const navigate = router.useNavigate()
Expand Down Expand Up @@ -400,8 +400,6 @@ function AppRouter(props: AppRouterProps) {

const authService = useInitAuthService(props)

const userSession = authService.cognito.userSession.bind(authService.cognito)
const refreshUserSession = authService.cognito.refreshUserSession.bind(authService.cognito)
const registerAuthEventListener = authService.registerAuthEventListener

React.useEffect(() => {
Expand Down Expand Up @@ -531,18 +529,15 @@ function AppRouter(props: AppRouterProps) {
return (
<RouterProvider navigate={navigate}>
<SessionProvider
saveAccessToken={authService.cognito.saveAccessToken.bind(authService.cognito)}
onLogout={() => {
localStorage.clearUserSpecificEntries()
}}
authService={authService.cognito}
mainPageUrl={mainPageUrl}
userSession={userSession}
registerAuthEventListener={registerAuthEventListener}
refreshUserSession={refreshUserSession}
>
<BackendProvider remoteBackend={remoteBackend} localBackend={localBackend}>
<AuthProvider
shouldStartInOfflineMode={isAuthenticationDisabled}
authService={authService}
onAuthenticated={onAuthenticated}
>
<AuthProvider onAuthenticated={onAuthenticated}>
<InputBindingsProvider inputBindings={inputBindings}>
<LocalBackendPathSynchronizer />
<VersionChecker />
Expand Down
8 changes: 2 additions & 6 deletions app/gui/src/dashboard/authentication/cognito.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@ import type * as amplify from '@aws-amplify/auth'
import type * as cognito from 'amazon-cognito-identity-js'
import * as results from 'ts-results'

import type * as loggerProvider from '#/providers/LoggerProvider'

import type * as service from '#/authentication/service'

import * as original from './cognito.js'
import * as listen from './listen.mock.js'

Expand Down Expand Up @@ -75,9 +71,9 @@ export class Cognito {

/** Create a new Cognito wrapper. */
constructor(
private readonly logger: loggerProvider.Logger,
private readonly logger: null,
private readonly supportsDeepLinks: boolean,
private readonly amplifyConfig: service.AmplifyConfig,
private readonly amplifyConfig: null,
) {
const username = localStorage.getItem(MOCK_EMAIL_KEY)
if (username != null) {
Expand Down
100 changes: 91 additions & 9 deletions app/gui/src/dashboard/authentication/cognito.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,12 @@ interface UserAttributes {
}
/* eslint-enable @typescript-eslint/naming-convention */

/** The type of multi-factor authentication (MFA) that the user has set up. */
export type MfaType = 'NOMFA' | 'SMS_MFA' | 'SOFTWARE_TOKEN_MFA' | 'TOTP'
/** The type of multi-factor authentication (MFA) including non-specified MFA */
export type MfaType = MfaProtectionTypes | 'NOMFA' | 'TOTP'
/**
* MFA protection types that the user can set up.
*/
export type MfaProtectionTypes = 'SMS_MFA' | 'SOFTWARE_TOKEN_MFA'

/**
* The type of challenge that the user is currently facing after signing in.
Expand Down Expand Up @@ -184,6 +188,82 @@ interface CognitoError {
readonly message: string
}

/**
* Return type for Confirm sign up endpoint
*/
export type ConfirmSignInReturn = Promise<
results.Err<AmplifyError> | results.Ok<cognito.CognitoUser>
>

/**
* Return type for Setup TOTP endpoint
*/
export interface SetupTOTPReturn {
/** The URL to scan the QR code */
readonly url: string
/** The secret to use for the TOTP */
readonly secret: string
}

/**
* Interface that represents Auth Provider API
* Currently, it's tightly coupled with Cognito, but in the future, it should be decoupled from
* Cognito and be able to be used with other Auth Providers.
*
* Currently used in unit tests to mock the Auth Provider API
*/
export interface ISessionProvider {
Copy link
Contributor

Choose a reason for hiding this comment

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

we typically don't do IInterfaceName in TS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, worth noting that we use interface as much as possible. And in most of the cases we don't need the I prefix, but for these interfaces, that supposed to be as implementation for classes, I think this might make sense, WDYT?

readonly userSession: () => Promise<UserSession | null>
readonly organizationId: () => Promise<string | null>
readonly email: () => Promise<string>
readonly signUp: (
username: string,
password: string,
organizationId: string | null,
) => Promise<results.Err<SignUpError> | results.Ok<unknown>>
readonly confirmSignUp: (
email: string,
code: string,
) => Promise<results.Err<ConfirmSignUpError> | results.Ok<unknown>>
readonly signInWithGoogle: () => Promise<void>
readonly signInWithGitHub: () => Promise<void>
readonly signInWithPassword: (
username: string,
password: string,
) => Promise<results.Err<SignInWithPasswordError> | results.Ok<amplify.CognitoUser>>
readonly refreshUserSession: () => Promise<UserSession | null>
readonly signOut: () => Promise<void>
readonly forgotPassword: (
email: string,
) => Promise<results.Err<ForgotPasswordError> | results.Ok<unknown>>
readonly forgotPasswordSubmit: (
email: string,
code: string,
password: string,
) => Promise<results.Err<ForgotPasswordSubmitError> | results.Ok<unknown>>
readonly changePassword: (
oldPassword: string,
newPassword: string,
) => Promise<results.Err<AmplifyError> | results.Ok<unknown>>
readonly setupTOTP: () => Promise<results.Err<AmplifyError> | results.Ok<SetupTOTPReturn>>
readonly verifyTotpSetup: (
totpToken: string,
) => Promise<results.Err<AmplifyError> | results.Ok<unknown>>
readonly updateMFAPreference: (
mfaMethod: MfaType,
) => Promise<results.Err<AmplifyError> | results.Ok<void>>
readonly getMFAPreference: () => Promise<results.Err<AmplifyError> | results.Ok<MfaType>>
readonly verifyTotpToken: (
totpToken: string,
) => Promise<results.Err<AmplifyError> | results.Ok<boolean>>
readonly saveAccessToken: (accessTokenPayload: saveAccessToken.AccessToken | null) => void
readonly confirmSignIn: (
user: cognito.CognitoUser,
otp: string,
mfaType: MfaProtectionTypes,
) => ConfirmSignInReturn
}

// ===============
// === Cognito ===
// ===============
Expand All @@ -193,7 +273,7 @@ interface CognitoError {
* This way, the methods don't throw all errors, but define exactly which errors they return.
* The caller can then handle them via pattern matching on the {@link results.Result} type.
*/
export class Cognito {
export class Cognito implements ISessionProvider {
/** Create a new Cognito wrapper. */
constructor(
private readonly logger: loggerProvider.Logger,
Expand Down Expand Up @@ -468,9 +548,9 @@ export class Cognito {
const cognitoUserResult = await currentAuthenticatedUser()
if (cognitoUserResult.ok) {
const cognitoUser = cognitoUserResult.unwrap()
const result = await results.Result.wrapAsync(
async () => await amplify.Auth.setPreferredMFA(cognitoUser, mfaMethod),
)
const result = await results.Result.wrapAsync(async () => {
await amplify.Auth.setPreferredMFA(cognitoUser, mfaMethod)
})
return result.mapErr(intoAmplifyErrorOrThrow)
} else {
return results.Err(cognitoUserResult.val)
Expand Down Expand Up @@ -503,7 +583,9 @@ export class Cognito {
const cognitoUser = cognitoUserResult.unwrap()

return (
await results.Result.wrapAsync(() => amplify.Auth.verifyTotpToken(cognitoUser, totpToken))
await results.Result.wrapAsync(() =>
amplify.Auth.verifyTotpToken(cognitoUser, totpToken).then(() => true),
)
).mapErr(intoAmplifyErrorOrThrow)
} else {
return results.Err(cognitoUserResult.val)
Expand All @@ -514,8 +596,8 @@ export class Cognito {
async confirmSignIn(
user: amplify.CognitoUser,
confirmationCode: string,
mfaType: 'SMS_MFA' | 'SOFTWARE_TOKEN_MFA',
) {
mfaType: MfaProtectionTypes,
): ConfirmSignInReturn {
const result = await results.Result.wrapAsync(() =>
amplify.Auth.confirmSignIn(user, confirmationCode, mfaType),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ export default function ProjectIcon(props: ProjectIconProps) {
case backendModule.ProjectState.created:
return (
<ariaComponents.Button
size="custom"
size="large"
variant="icon"
icon={PlayIcon}
aria-label={getTooltip(getText('openInEditor'))}
Expand Down
5 changes: 4 additions & 1 deletion app/gui/src/dashboard/layouts/InfoMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import SvgMask from '#/components/SvgMask'
import AboutModal from '#/modals/AboutModal'
import { useAuth } from '#/providers/AuthProvider'
import { useSetModal } from '#/providers/ModalProvider'
import { useSessionAPI } from '#/providers/SessionProvider.tsx'
import { useText } from '#/providers/TextProvider'

// ================
Expand All @@ -23,7 +24,9 @@ export interface InfoMenuProps {
/** A menu containing info about the app. */
export default function InfoMenu(props: InfoMenuProps) {
const { hidden = false } = props
const { signOut, session } = useAuth()

const { signOut } = useSessionAPI()
const { session } = useAuth()
const { setModal } = useSetModal()
const { getText } = useText()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import ConfirmDeleteUserModal from '#/modals/ConfirmDeleteUserModal'

/** Settings tab for deleting the current user. */
export default function DeleteUserAccountSettingsSection() {
const { signOut, deleteUser } = authProvider.useAuth()
const { deleteUser } = authProvider.useAuth()
const { getText } = textProvider.useText()

return (
Expand All @@ -37,7 +37,6 @@ export default function DeleteUserAccountSettingsSection() {
<ConfirmDeleteUserModal
doDelete={async () => {
await deleteUser()
await signOut()
}}
/>
</ariaComponents.DialogTrigger>
Expand Down
45 changes: 12 additions & 33 deletions app/gui/src/dashboard/layouts/Settings/SetupTwoFaForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
} from '#/components/AriaComponents'
import { ErrorBoundary } from '#/components/ErrorBoundary'
import { Suspense } from '#/components/Suspense'
import { useAuth } from '#/providers/AuthProvider'
import { useSessionAPI } from '#/providers/SessionProvider'
import { useText } from '#/providers/TextProvider'
import { useMutation, useSuspenseQuery } from '@tanstack/react-query'
import { lazy } from 'react'
Expand All @@ -39,31 +39,17 @@ const LazyQRCode = lazy(() =>
*/
export function SetupTwoFaForm() {
const { getText } = useText()
const { cognito } = useAuth()
const { getMFAPreference, updateMFAPreference, verifyTotpToken } = useSessionAPI()

const { data } = useSuspenseQuery({
queryKey: ['twoFaPreference'],
queryFn: () =>
cognito.getMFAPreference().then((res) => {
if (res.err) {
throw res.val
} else {
return res.unwrap()
}
}),
queryFn: () => getMFAPreference(),
})

const MFAEnabled = data !== 'NOMFA'

const updateMFAPreferenceMutation = useMutation({
mutationFn: (preference: MfaType) =>
cognito.updateMFAPreference(preference).then((res) => {
if (res.err) {
throw res.val
} else {
return res.unwrap()
}
}),
mutationFn: (preference: MfaType) => updateMFAPreference(preference),
meta: { invalidates: [['twoFaPreference']] },
})

Expand Down Expand Up @@ -100,11 +86,11 @@ export function SetupTwoFaForm() {
formOptions={{ mode: 'onSubmit' }}
method="dialog"
onSubmit={({ otp }) =>
cognito.verifyTotpToken(otp).then((res) => {
if (res.ok) {
verifyTotpToken(otp).then((passed) => {
if (passed) {
return updateMFAPreferenceMutation.mutateAsync('NOMFA')
} else {
throw res.val
throw new Error('Invalid OTP')
}
})
}
Expand Down Expand Up @@ -139,11 +125,11 @@ export function SetupTwoFaForm() {
defaultValues={{ enabled: false, display: 'QR' }}
onSubmit={async ({ enabled, otp }) => {
if (enabled) {
return cognito.verifyTotpToken(otp).then((res) => {
if (res.ok) {
return verifyTotpToken(otp).then((passed) => {
if (passed) {
return updateMFAPreferenceMutation.mutateAsync('TOTP')
} else {
throw res.val
throw new Error('Invalid OTP')
}
})
}
Expand All @@ -170,19 +156,12 @@ export function SetupTwoFaForm() {

/** Two Factor Authentication Setup Form. */
function TwoFa() {
const { cognito } = useAuth()
const { setupTOTP } = useSessionAPI()
const { getText } = useText()

const { data } = useSuspenseQuery({
queryKey: ['setupTOTP'],
queryFn: () =>
cognito.setupTOTP().then((res) => {
if (res.err) {
throw res.val
} else {
return res.unwrap()
}
}),
queryFn: () => setupTOTP(),
})

return (
Expand Down
5 changes: 3 additions & 2 deletions app/gui/src/dashboard/layouts/Settings/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ import { useEventCallback } from '#/hooks/eventCallbackHooks'
import { useSearchParamsState } from '#/hooks/searchParamsStateHooks'
import { useToastAndLog } from '#/hooks/toastAndLogHooks'
import SearchBar from '#/layouts/SearchBar'
import { useAuth, useFullUserSession } from '#/providers/AuthProvider'
import { useFullUserSession } from '#/providers/AuthProvider'
import { useLocalBackend, useRemoteBackend } from '#/providers/BackendProvider'
import { useLocalStorageState } from '#/providers/LocalStorageProvider'
import { useSessionAPI } from '#/providers/SessionProvider'
import { useText } from '#/providers/TextProvider'
import type Backend from '#/services/Backend'
import { Path } from '#/services/ProjectManager'
Expand Down Expand Up @@ -53,7 +54,7 @@ export default function Settings() {
includesPredicate(Object.values(SettingsTabType)),
)
const { user, accessToken } = useFullUserSession()
const { changePassword } = useAuth()
const { changePassword } = useSessionAPI()
const { getText } = useText()
const toastAndLog = useToastAndLog()
const [query, setQuery] = React.useState('')
Expand Down
Loading
Loading