Page MenuHomePhabricator
Differential D7657

Establish olm notifs session on both sides during registration
ClosedPublic
Actions

Authored by marcin on Apr 27 2023, 6:40 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 20, 4:26 AM
Unknown Object (File)
Sat, Apr 19, 10:37 AM
Unknown Object (File)
Sat, Apr 19, 10:06 AM
Unknown Object (File)
Fri, Apr 18, 5:01 PM
Unknown Object (File)
Fri, Apr 18, 2:18 PM
Unknown Object (File)
Fri, Apr 18, 12:59 PM
Unknown Object (File)
Fri, Apr 18, 11:28 AM
Unknown Object (File)
Fri, Apr 18, 7:03 AM
View All Files
Subscribers
ashoat

Details

Reviewers
tomek
atul
bartek
ashoat
Commits
rCOMM7cbc1822445f: Establish olm notifs session on both sides during registration
rCOMM29f99ee4dfad: Establish olm notifs session on both sides during registration
Summary

This differential implements olm notifs session establishment during account creation.

Test Plan

Create new account. Ensure new entry in olm_sessions table appears

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

marcin created this revision.Apr 27 2023, 6:40 AM
Herald added a subscriber: ashoat. · View Herald TranscriptApr 27 2023, 6:40 AM
Harbormaster completed remote builds in B18859: Diff 25828.Apr 27 2023, 6:58 AM
marcin requested review of this revision.Apr 27 2023, 6:58 AM
marcin added a parent revision: D7654: Establish olm sessions for notifs on both sides during log-in.Apr 27 2023, 7:02 AM
marcin added a child revision: D7669: Establish olm notifs sessions during siwe account creation.Apr 27 2023, 8:15 AM
ashoat requested changes to this revision.Apr 27 2023, 1:10 PM
ashoat added inline comments.
keyserver/src/creators/account-creator.js
132–138 ↗(On Diff #25828)

See feedback in D7654. Please be more thoughtful when introducing new await keywords

native/account/register-panel.react.js
295–297 ↗(On Diff #25828)

See feedback in D7654. Please be more thoughtful when introducing new await keywords

This revision now requires changes to proceed.Apr 27 2023, 1:10 PM
marcin updated this revision to Diff 26006.May 2 2023, 8:35 AM

Better usage of awaits. Ensure cookie iscreated prior to olm session.

Harbormaster completed remote builds in B18989: Diff 26006.May 2 2023, 8:57 AM
ashoat accepted this revision.May 2 2023, 10:13 AM
This revision is now accepted and ready to land.May 2 2023, 10:13 AM
marcin updated this revision to Diff 26399.May 11 2023, 5:53 AM

Rebase

Harbormaster completed remote builds in B19286: Diff 26399.May 11 2023, 6:11 AM
marcin updated this revision to Diff 26742.May 22 2023, 1:48 AM

Rebase before landing

Harbormaster completed remote builds in B19561: Diff 26742.May 22 2023, 2:15 AM
marcin updated this revision to Diff 26751.May 22 2023, 3:29 AM

Rename call to get and rebase before landing

Harbormaster completed remote builds in B19570: Diff 26751.May 22 2023, 3:47 AM
marcin updated this revision to Diff 27173.May 29 2023, 11:35 AM

Rebase before landing

Harbormaster completed remote builds in B19842: Diff 27173.May 29 2023, 12:02 PM
Closed by commit rCOMM29f99ee4dfad: Establish olm notifs session on both sides during registration (authored by marcin). · Explain WhyMay 29 2023, 1:41 PM
This revision was automatically updated to reflect the committed changes.
marcin added a commit: rCOMM29f99ee4dfad: Establish olm notifs session on both sides during registration.
ashoat added a reverting change: rCOMM080355ee6040: Revert "Establish olm notifs session on both sides during registration".May 31 2023, 4:51 AM
marcin reopened this revision.May 31 2023, 8:36 AM
This revision is now accepted and ready to land.May 31 2023, 8:36 AM
marcin updated this revision to Diff 27341.Jun 1 2023, 6:08 AM

Rebase before landing

Harbormaster completed remote builds in B19951: Diff 27341.Jun 1 2023, 6:32 AM
Closed by commit rCOMM7cbc1822445f: Establish olm notifs session on both sides during registration (authored by marcin). · Explain WhyJun 1 2023, 6:55 AM
This revision was automatically updated to reflect the committed changes.
marcin added a commit: rCOMM7cbc1822445f: Establish olm notifs session on both sides during registration.

Revision Contents
Changeset List

PathSize
keyserver/
src/
creators/
18 lines
responders/
1 line
lib/
types/
1 line
native/
account/
14 lines

Diff 26006

View Options

keyserver/src/creators/account-creator.js

Show All 21 Lines
import { dbQuery, SQL } from '../database/database.js'; import { dbQuery, SQL } from '../database/database.js';
import { import {
fetchThreadInfos, fetchThreadInfos,
fetchServerThreadInfos, fetchServerThreadInfos,
rawThreadInfosFromServerThreadInfos, rawThreadInfosFromServerThreadInfos,
} from '../fetchers/thread-fetchers.js'; } from '../fetchers/thread-fetchers.js';
import { checkThreadPermission } from '../fetchers/thread-permission-fetchers.js'; import { checkThreadPermission } from '../fetchers/thread-permission-fetchers.js';
import type { Viewer } from '../session/viewer.js'; import type { Viewer } from '../session/viewer.js';
import { updateRole } from '../updaters/thread-updaters.js';
type InitialRoles = { type InitialRoles = {
+default: RoleInfo, +default: RoleInfo,
+creator: RoleInfo, +creator: RoleInfo,
}; };
async function createInitialRolesForNewThread( async function createInitialRolesForNewThread(
threadID: string, threadID: string,
threadType: ThreadType, threadType: ThreadType,
▲ Show 20 Lines • Show All 50 Lines • ▼ Show 20 Lines const hasPermission = await checkThreadPermission(
viewer, viewer,
request.community, request.community,
threadPermissions.CHANGE_ROLE, threadPermissions.CHANGE_ROLE,
); );
if (!hasPermission) { if (!hasPermission) {
throw new ServerError('invalid_credentials'); throw new ServerError('invalid_credentials');
} }
const { community, name, permissions, action } = request; const { community, existingRoleID, name, permissions, action } = request;
for (const permission of permissions) { for (const permission of permissions) {
if (!userSurfacedPermissionsSet.has(permission)) { if (!userSurfacedPermissionsSet.has(permission)) {
throw new ServerError('invalid_parameters'); throw new ServerError('invalid_parameters');
} }
} }
const [id] = await createIDs('roles', 1); const [id] = await createIDs('roles', 1);
Show All 31 Lines): Promise<RoleModificationResult> {
let query = SQL``; let query = SQL``;
if (action === 'create_role') { if (action === 'create_role') {
query = SQL` query = SQL`
INSERT INTO roles (id, thread, name, permissions, creation_time) INSERT INTO roles (id, thread, name, permissions, creation_time)
VALUES (${row}) VALUES (${row})
`; `;
} else if (action === 'edit_role') { } else if (action === 'edit_role') {
throw new ServerError("unimplemented: can't edit roles yet"); query = SQL`
UPDATE roles
SET name = ${name}, permissions = ${permissionsBlob}
WHERE id = ${existingRoleID}
`;
} }
await dbQuery(query); await dbQuery(query);
// The `updateRole` needs to occur after the role has been updated
// in the database because it will want the most up to date role info
// (permissions / name)
if (action === 'edit_role') {
atulUnsubmitted
Not Done
Inline Actions

Instead of two separate (action === 'edit_role') blocks, could we merge the two?

We could move await dbQuery(query); within both the (action === 'create_role') (line 139) and else if (action === 'edit_role') (line 144) blocks?

Not a huge deal, whatever you prefer

atul: Instead of two separate `(action === 'edit_role')` blocks, could we merge the two? We could…
rohanAuthorUnsubmitted
Done
Inline Actions

Yeah we can, that should be cleaner too

rohan: Yeah we can, that should be cleaner too
if (!existingRoleID) {
throw new ServerError('invalid_parameters');
}
const membersWithRole = threadInfo.members.filter(
memberInfo => memberInfo.role === existingRoleID,
);
await updateRole(
viewer,
{
threadID: community,
role: existingRoleID,
memberIDs: membersWithRole.map(memberInfo => memberInfo.id),
atulUnsubmitted
Not Done
Inline Actions

Should we add this .map to the end of the .filter on lines 162-164?

atul: Should we add this `.map` to the end of the `.filter` on lines 162-164?
rohanAuthorUnsubmitted
Done
Inline Actions

Sure that makes sense

rohan: Sure that makes sense
},
{ silenceNewMessages: true },
);
}
const fetchServerThreadInfosResult = await fetchServerThreadInfos({ const fetchServerThreadInfosResult = await fetchServerThreadInfos({
threadID: community, threadID: community,
}); });
const { threadInfos: serverThreadInfos } = fetchServerThreadInfosResult; const { threadInfos: serverThreadInfos } = fetchServerThreadInfosResult;
const serverThreadInfo = serverThreadInfos[community]; const serverThreadInfo = serverThreadInfos[community];
if (!serverThreadInfo) { if (!serverThreadInfo) {
throw new ServerError('internal_error'); throw new ServerError('internal_error');
Show All 32 Lines
View Options

keyserver/src/responders/user-responders.js

Show All 21 Lines
import { dbQuery, SQL } from '../database/database.js'; import { dbQuery, SQL } from '../database/database.js';
import { import {
fetchThreadInfos, fetchThreadInfos,
fetchServerThreadInfos, fetchServerThreadInfos,
rawThreadInfosFromServerThreadInfos, rawThreadInfosFromServerThreadInfos,
} from '../fetchers/thread-fetchers.js'; } from '../fetchers/thread-fetchers.js';
import { checkThreadPermission } from '../fetchers/thread-permission-fetchers.js'; import { checkThreadPermission } from '../fetchers/thread-permission-fetchers.js';
import type { Viewer } from '../session/viewer.js'; import type { Viewer } from '../session/viewer.js';
import { updateRole } from '../updaters/thread-updaters.js';
type InitialRoles = { type InitialRoles = {
+default: RoleInfo, +default: RoleInfo,
+creator: RoleInfo, +creator: RoleInfo,
}; };
async function createInitialRolesForNewThread( async function createInitialRolesForNewThread(
threadID: string, threadID: string,
threadType: ThreadType, threadType: ThreadType,
▲ Show 20 Lines • Show All 50 Lines • ▼ Show 20 Lines const hasPermission = await checkThreadPermission(
viewer, viewer,
request.community, request.community,
threadPermissions.CHANGE_ROLE, threadPermissions.CHANGE_ROLE,
); );
if (!hasPermission) { if (!hasPermission) {
throw new ServerError('invalid_credentials'); throw new ServerError('invalid_credentials');
} }
const { community, name, permissions, action } = request; const { community, existingRoleID, name, permissions, action } = request;
for (const permission of permissions) { for (const permission of permissions) {
if (!userSurfacedPermissionsSet.has(permission)) { if (!userSurfacedPermissionsSet.has(permission)) {
throw new ServerError('invalid_parameters'); throw new ServerError('invalid_parameters');
} }
} }
const [id] = await createIDs('roles', 1); const [id] = await createIDs('roles', 1);
Show All 31 Lines): Promise<RoleModificationResult> {
let query = SQL``; let query = SQL``;
if (action === 'create_role') { if (action === 'create_role') {
query = SQL` query = SQL`
INSERT INTO roles (id, thread, name, permissions, creation_time) INSERT INTO roles (id, thread, name, permissions, creation_time)
VALUES (${row}) VALUES (${row})
`; `;
} else if (action === 'edit_role') { } else if (action === 'edit_role') {
throw new ServerError("unimplemented: can't edit roles yet"); query = SQL`
UPDATE roles
SET name = ${name}, permissions = ${permissionsBlob}
WHERE id = ${existingRoleID}
`;
} }
await dbQuery(query); await dbQuery(query);
// The `updateRole` needs to occur after the role has been updated
// in the database because it will want the most up to date role info
// (permissions / name)
if (action === 'edit_role') {
atulUnsubmitted
Not Done
Inline Actions

Instead of two separate (action === 'edit_role') blocks, could we merge the two?

We could move await dbQuery(query); within both the (action === 'create_role') (line 139) and else if (action === 'edit_role') (line 144) blocks?

Not a huge deal, whatever you prefer

atul: Instead of two separate `(action === 'edit_role')` blocks, could we merge the two? We could…
rohanAuthorUnsubmitted
Done
Inline Actions

Yeah we can, that should be cleaner too

rohan: Yeah we can, that should be cleaner too
if (!existingRoleID) {
throw new ServerError('invalid_parameters');
}
const membersWithRole = threadInfo.members.filter(
memberInfo => memberInfo.role === existingRoleID,
);
await updateRole(
viewer,
{
threadID: community,
role: existingRoleID,
memberIDs: membersWithRole.map(memberInfo => memberInfo.id),
atulUnsubmitted
Not Done
Inline Actions

Should we add this .map to the end of the .filter on lines 162-164?

atul: Should we add this `.map` to the end of the `.filter` on lines 162-164?
rohanAuthorUnsubmitted
Done
Inline Actions

Sure that makes sense

rohan: Sure that makes sense
},
{ silenceNewMessages: true },
);
}
const fetchServerThreadInfosResult = await fetchServerThreadInfos({ const fetchServerThreadInfosResult = await fetchServerThreadInfos({
threadID: community, threadID: community,
}); });
const { threadInfos: serverThreadInfos } = fetchServerThreadInfosResult; const { threadInfos: serverThreadInfos } = fetchServerThreadInfosResult;
const serverThreadInfo = serverThreadInfos[community]; const serverThreadInfo = serverThreadInfos[community];
if (!serverThreadInfo) { if (!serverThreadInfo) {
throw new ServerError('internal_error'); throw new ServerError('internal_error');
Show All 32 Lines
View Options

lib/types/account-types.js

Show All 21 Lines
import { dbQuery, SQL } from '../database/database.js'; import { dbQuery, SQL } from '../database/database.js';
import { import {
fetchThreadInfos, fetchThreadInfos,
fetchServerThreadInfos, fetchServerThreadInfos,
rawThreadInfosFromServerThreadInfos, rawThreadInfosFromServerThreadInfos,
} from '../fetchers/thread-fetchers.js'; } from '../fetchers/thread-fetchers.js';
import { checkThreadPermission } from '../fetchers/thread-permission-fetchers.js'; import { checkThreadPermission } from '../fetchers/thread-permission-fetchers.js';
import type { Viewer } from '../session/viewer.js'; import type { Viewer } from '../session/viewer.js';
import { updateRole } from '../updaters/thread-updaters.js';
type InitialRoles = { type InitialRoles = {
+default: RoleInfo, +default: RoleInfo,
+creator: RoleInfo, +creator: RoleInfo,
}; };
async function createInitialRolesForNewThread( async function createInitialRolesForNewThread(
threadID: string, threadID: string,
threadType: ThreadType, threadType: ThreadType,
▲ Show 20 Lines • Show All 50 Lines • ▼ Show 20 Lines const hasPermission = await checkThreadPermission(
viewer, viewer,
request.community, request.community,
threadPermissions.CHANGE_ROLE, threadPermissions.CHANGE_ROLE,
); );
if (!hasPermission) { if (!hasPermission) {
throw new ServerError('invalid_credentials'); throw new ServerError('invalid_credentials');
} }
const { community, name, permissions, action } = request; const { community, existingRoleID, name, permissions, action } = request;
for (const permission of permissions) { for (const permission of permissions) {
if (!userSurfacedPermissionsSet.has(permission)) { if (!userSurfacedPermissionsSet.has(permission)) {
throw new ServerError('invalid_parameters'); throw new ServerError('invalid_parameters');
} }
} }
const [id] = await createIDs('roles', 1); const [id] = await createIDs('roles', 1);
Show All 31 Lines): Promise<RoleModificationResult> {
let query = SQL``; let query = SQL``;
if (action === 'create_role') { if (action === 'create_role') {
query = SQL` query = SQL`
INSERT INTO roles (id, thread, name, permissions, creation_time) INSERT INTO roles (id, thread, name, permissions, creation_time)
VALUES (${row}) VALUES (${row})
`; `;
} else if (action === 'edit_role') { } else if (action === 'edit_role') {
throw new ServerError("unimplemented: can't edit roles yet"); query = SQL`
UPDATE roles
SET name = ${name}, permissions = ${permissionsBlob}
WHERE id = ${existingRoleID}
`;
} }
await dbQuery(query); await dbQuery(query);
// The `updateRole` needs to occur after the role has been updated
// in the database because it will want the most up to date role info
// (permissions / name)
if (action === 'edit_role') {
atulUnsubmitted
Not Done
Inline Actions

Instead of two separate (action === 'edit_role') blocks, could we merge the two?

We could move await dbQuery(query); within both the (action === 'create_role') (line 139) and else if (action === 'edit_role') (line 144) blocks?

Not a huge deal, whatever you prefer

atul: Instead of two separate `(action === 'edit_role')` blocks, could we merge the two? We could…
rohanAuthorUnsubmitted
Done
Inline Actions

Yeah we can, that should be cleaner too

rohan: Yeah we can, that should be cleaner too
if (!existingRoleID) {
throw new ServerError('invalid_parameters');
}
const membersWithRole = threadInfo.members.filter(
memberInfo => memberInfo.role === existingRoleID,
);
await updateRole(
viewer,
{
threadID: community,
role: existingRoleID,
memberIDs: membersWithRole.map(memberInfo => memberInfo.id),
atulUnsubmitted
Not Done
Inline Actions

Should we add this .map to the end of the .filter on lines 162-164?

atul: Should we add this `.map` to the end of the `.filter` on lines 162-164?
rohanAuthorUnsubmitted
Done
Inline Actions

Sure that makes sense

rohan: Sure that makes sense
},
{ silenceNewMessages: true },
);
}
const fetchServerThreadInfosResult = await fetchServerThreadInfos({ const fetchServerThreadInfosResult = await fetchServerThreadInfos({
threadID: community, threadID: community,
}); });
const { threadInfos: serverThreadInfos } = fetchServerThreadInfosResult; const { threadInfos: serverThreadInfos } = fetchServerThreadInfosResult;
const serverThreadInfo = serverThreadInfos[community]; const serverThreadInfo = serverThreadInfos[community];
if (!serverThreadInfo) { if (!serverThreadInfo) {
throw new ServerError('internal_error'); throw new ServerError('internal_error');
Show All 32 Lines
View Options

native/account/register-panel.react.js

Show All 21 Lines
import { dbQuery, SQL } from '../database/database.js'; import { dbQuery, SQL } from '../database/database.js';
import { import {
fetchThreadInfos, fetchThreadInfos,
fetchServerThreadInfos, fetchServerThreadInfos,
rawThreadInfosFromServerThreadInfos, rawThreadInfosFromServerThreadInfos,
} from '../fetchers/thread-fetchers.js'; } from '../fetchers/thread-fetchers.js';
import { checkThreadPermission } from '../fetchers/thread-permission-fetchers.js'; import { checkThreadPermission } from '../fetchers/thread-permission-fetchers.js';
import type { Viewer } from '../session/viewer.js'; import type { Viewer } from '../session/viewer.js';
import { updateRole } from '../updaters/thread-updaters.js';
type InitialRoles = { type InitialRoles = {
+default: RoleInfo, +default: RoleInfo,
+creator: RoleInfo, +creator: RoleInfo,
}; };
async function createInitialRolesForNewThread( async function createInitialRolesForNewThread(
threadID: string, threadID: string,
threadType: ThreadType, threadType: ThreadType,
▲ Show 20 Lines • Show All 50 Lines • ▼ Show 20 Lines const hasPermission = await checkThreadPermission(
viewer, viewer,
request.community, request.community,
threadPermissions.CHANGE_ROLE, threadPermissions.CHANGE_ROLE,
); );
if (!hasPermission) { if (!hasPermission) {
throw new ServerError('invalid_credentials'); throw new ServerError('invalid_credentials');
} }
const { community, name, permissions, action } = request; const { community, existingRoleID, name, permissions, action } = request;
for (const permission of permissions) { for (const permission of permissions) {
if (!userSurfacedPermissionsSet.has(permission)) { if (!userSurfacedPermissionsSet.has(permission)) {
throw new ServerError('invalid_parameters'); throw new ServerError('invalid_parameters');
} }
} }
const [id] = await createIDs('roles', 1); const [id] = await createIDs('roles', 1);
Show All 31 Lines): Promise<RoleModificationResult> {
let query = SQL``; let query = SQL``;
if (action === 'create_role') { if (action === 'create_role') {
query = SQL` query = SQL`
INSERT INTO roles (id, thread, name, permissions, creation_time) INSERT INTO roles (id, thread, name, permissions, creation_time)
VALUES (${row}) VALUES (${row})
`; `;
} else if (action === 'edit_role') { } else if (action === 'edit_role') {
throw new ServerError("unimplemented: can't edit roles yet"); query = SQL`
UPDATE roles
SET name = ${name}, permissions = ${permissionsBlob}
WHERE id = ${existingRoleID}
`;
} }
await dbQuery(query); await dbQuery(query);
// The `updateRole` needs to occur after the role has been updated
// in the database because it will want the most up to date role info
// (permissions / name)
if (action === 'edit_role') {
atulUnsubmitted
Not Done
Inline Actions

Instead of two separate (action === 'edit_role') blocks, could we merge the two?

We could move await dbQuery(query); within both the (action === 'create_role') (line 139) and else if (action === 'edit_role') (line 144) blocks?

Not a huge deal, whatever you prefer

atul: Instead of two separate `(action === 'edit_role')` blocks, could we merge the two? We could…
rohanAuthorUnsubmitted
Done
Inline Actions

Yeah we can, that should be cleaner too

rohan: Yeah we can, that should be cleaner too
if (!existingRoleID) {
throw new ServerError('invalid_parameters');
}
const membersWithRole = threadInfo.members.filter(
memberInfo => memberInfo.role === existingRoleID,
);
await updateRole(
viewer,
{
threadID: community,
role: existingRoleID,
memberIDs: membersWithRole.map(memberInfo => memberInfo.id),
atulUnsubmitted
Not Done
Inline Actions

Should we add this .map to the end of the .filter on lines 162-164?

atul: Should we add this `.map` to the end of the `.filter` on lines 162-164?
rohanAuthorUnsubmitted
Done
Inline Actions

Sure that makes sense

rohan: Sure that makes sense
},
{ silenceNewMessages: true },
);
}
const fetchServerThreadInfosResult = await fetchServerThreadInfos({ const fetchServerThreadInfosResult = await fetchServerThreadInfos({
threadID: community, threadID: community,
}); });
const { threadInfos: serverThreadInfos } = fetchServerThreadInfosResult; const { threadInfos: serverThreadInfos } = fetchServerThreadInfosResult;
const serverThreadInfo = serverThreadInfos[community]; const serverThreadInfo = serverThreadInfos[community];
if (!serverThreadInfo) { if (!serverThreadInfo) {
throw new ServerError('internal_error'); throw new ServerError('internal_error');
Show All 32 Lines