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

Fix (rust crypto): Adjust login procedures to account for rust crypto behaviour. #2603

Merged
merged 2 commits into from
Sep 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 6 additions & 10 deletions src/ClientContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -379,19 +379,15 @@
} catch (err) {
if (err instanceof MatrixError && err.errcode === "M_UNKNOWN_TOKEN") {
// We can't use this session anymore, so let's log it out
try {
const client = await initClient(initClientParams, false); // Don't need the crypto store just to log out)
await client.logout(true);
} catch (err) {
logger.warn(
"The previous session was unable to login, and we couldn't log it out: " +
err,
);
}
logger.log(
"The session from local store is invalid; continuing without a client",
);
clearSession();

Check warning on line 385 in src/ClientContext.tsx

View check run for this annotation

Codecov / codecov/patch

src/ClientContext.tsx#L382-L385

Added lines #L382 - L385 were not covered by tests
// returning null = "no client` pls register" (undefined = "loading" which is the current value when reaching this line)
return null;

Check warning on line 387 in src/ClientContext.tsx

View check run for this annotation

Codecov / codecov/patch

src/ClientContext.tsx#L387

Added line #L387 was not covered by tests
}
throw err;
}
/* eslint-enable camelcase */
} catch (err) {
clearSession();
throw err;
Expand Down
4 changes: 2 additions & 2 deletions src/auth/LoginPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
const { t } = useTranslation();
usePageTitle(t("login_title"));

const { setClient } = useClient();
const login = useInteractiveLogin();
const { client, setClient } = useClient();
const login = useInteractiveLogin(client);

Check warning on line 36 in src/auth/LoginPage.tsx

View check run for this annotation

Codecov / codecov/patch

src/auth/LoginPage.tsx#L35-L36

Added lines #L35 - L36 were not covered by tests
const homeserver = Config.defaultHomeserverUrl(); // TODO: Make this configurable
const usernameRef = useRef<HTMLInputElement>(null);
const passwordRef = useRef<HTMLInputElement>(null);
Expand Down
96 changes: 54 additions & 42 deletions src/auth/useInteractiveLogin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,15 @@

import { initClient } from "../utils/matrix";
import { Session } from "../ClientContext";

export function useInteractiveLogin(): (
/**
* This provides the login method to login using user credentials.
* @param oldClient If there is an already authenticated client it should be passed to this hook
* this allows the interactive login to sign out the client before logging in.
* @returns A async method that can be called/awaited to log in with the provided credentials.
*/
export function useInteractiveLogin(
oldClient?: MatrixClient,

Check warning on line 34 in src/auth/useInteractiveLogin.ts

View check run for this annotation

Codecov / codecov/patch

src/auth/useInteractiveLogin.ts#L33-L34

Added lines #L33 - L34 were not covered by tests
): (
homeserver: string,
username: string,
password: string,
Expand All @@ -36,47 +43,52 @@
username: string,
password: string,
) => Promise<[MatrixClient, Session]>
>(async (homeserver: string, username: string, password: string) => {
const authClient = createClient({ baseUrl: homeserver });
>(
async (homeserver: string, username: string, password: string) => {
const authClient = createClient({ baseUrl: homeserver });

Check warning on line 48 in src/auth/useInteractiveLogin.ts

View check run for this annotation

Codecov / codecov/patch

src/auth/useInteractiveLogin.ts#L47-L48

Added lines #L47 - L48 were not covered by tests

const interactiveAuth = new InteractiveAuth({
matrixClient: authClient,
doRequest: (): Promise<LoginResponse> =>
authClient.login("m.login.password", {
identifier: {
type: "m.id.user",
user: username,
},
password,
}),
stateUpdated: (): void => {},
requestEmailToken: (): Promise<{ sid: string }> => {
return Promise.resolve({ sid: "" });
},
});
const interactiveAuth = new InteractiveAuth({
matrixClient: authClient,
doRequest: (): Promise<LoginResponse> =>
authClient.login("m.login.password", {
identifier: {
type: "m.id.user",
user: username,
},
password,
}),
stateUpdated: (): void => {},
requestEmailToken: (): Promise<{ sid: string }> => {
return Promise.resolve({ sid: "" });
},
});

Check warning on line 64 in src/auth/useInteractiveLogin.ts

View check run for this annotation

Codecov / codecov/patch

src/auth/useInteractiveLogin.ts#L50-L64

Added lines #L50 - L64 were not covered by tests

// XXX: This claims to return an IAuthData which contains none of these
// things - the js-sdk types may be wrong?
/* eslint-disable camelcase,@typescript-eslint/no-explicit-any */
const { user_id, access_token, device_id } =
(await interactiveAuth.attemptAuth()) as any;
const session = {
user_id,
access_token,
device_id,
passwordlessUser: false,
};
// XXX: This claims to return an IAuthData which contains none of these
// things - the js-sdk types may be wrong?
/* eslint-disable camelcase,@typescript-eslint/no-explicit-any */
const { user_id, access_token, device_id } =
(await interactiveAuth.attemptAuth()) as any;
const session = {
user_id,
access_token,
device_id,
passwordlessUser: false,
};

Check warning on line 76 in src/auth/useInteractiveLogin.ts

View check run for this annotation

Codecov / codecov/patch

src/auth/useInteractiveLogin.ts#L69-L76

Added lines #L69 - L76 were not covered by tests

const client = await initClient(
{
baseUrl: homeserver,
accessToken: access_token,
userId: user_id,
deviceId: device_id,
},
false,
);
/* eslint-enable camelcase */
return [client, session];
}, []);
// To not confuse the rust crypto sessions we need to logout the old client before initializing the new one.
await oldClient?.logout(true);
const client = await initClient(
{
baseUrl: homeserver,
accessToken: access_token,
userId: user_id,
deviceId: device_id,
},
false,
);

Check warning on line 88 in src/auth/useInteractiveLogin.ts

View check run for this annotation

Codecov / codecov/patch

src/auth/useInteractiveLogin.ts#L79-L88

Added lines #L79 - L88 were not covered by tests
/* eslint-enable camelcase */
return [client, session];
},
[oldClient],
);

Check warning on line 93 in src/auth/useInteractiveLogin.ts

View check run for this annotation

Codecov / codecov/patch

src/auth/useInteractiveLogin.ts#L90-L93

Added lines #L90 - L93 were not covered by tests
}
26 changes: 16 additions & 10 deletions src/utils/matrix.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,12 @@

/**
* Initialises and returns a new standalone Matrix Client.
* If false is passed for the 'restore' parameter, corresponding crypto
* data is cleared before the client initialization.
* This can only be called safely if no other client is running
* otherwise rust crypto will throw since it is not ready to initialize a new session.
* If another client is running make sure `.logout()` is called before executing this function.
* @param clientOptions Object of options passed through to the client
* @param restore Whether the session is being restored from storage
* @param restore If the rust crypto should be reset before the cient initialization or
* if the initialization should try to restore the crypto state from the indexDB.
* @returns The MatrixClient instance
*/
export async function initClient(
Expand Down Expand Up @@ -130,20 +132,17 @@
fallbackICEServerAllowed: fallbackICEServerAllowed,
});

// In case of registering a new matrix account caused by broken store state. This is particularly needed for:
// In case of logging in a new matrix account but there is still crypto local store. This is needed for:
// - We lost the auth tokens and cannot restore the client resulting in registering a new user.
// - Need to make sure any possible leftover crypto store gets cleared.
// - We start the sign in flow but are registered with a guest user. (It should additionally log out the guest before)
// - A new account is created because of missing LocalStorage: "matrix-auth-store", but the crypto IndexDB is still available.
// This would result in conflicting crypto store userId vs matrixClient userId. Caused by EC 0.6.1
if (!restore) {
client.clearStores();
await client.clearStores();

Check warning on line 140 in src/utils/matrix.ts

View check run for this annotation

Codecov / codecov/patch

src/utils/matrix.ts#L140

Added line #L140 was not covered by tests
}

// Start client store.
// Note: The `client.store` is used to store things like sync results. It's independent of
// the cryptostore, and uses a separate indexeddb database.

// start the client store (totally independent to the crypto store)
try {
await client.store.startup();
} catch (error) {
Expand All @@ -156,7 +155,14 @@
}

// Also creates and starts any crypto related stores.
await client.initRustCrypto();
try {
await client.initRustCrypto();
} catch (err) {
logger.warn(
err,
"Make sure to clear client stores before initializing the rust crypto.",
);
}

Check warning on line 165 in src/utils/matrix.ts

View check run for this annotation

Codecov / codecov/patch

src/utils/matrix.ts#L158-L165

Added lines #L158 - L165 were not covered by tests

client.setGlobalErrorOnUnknownDevices(false);
// Once startClient is called, syncs are run asynchronously.
Expand Down