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.

Sign up for GitHub

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

Jump to bottom

feat!: rename "Capability" to "Role", attach ID #462

Merged
merged 5 commits into from
Feb 7, 2024
Merged

feat!: rename "Capability" to "Role", attach ID #462

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
20541ab
feat!: rename "Capability" to "Role", attach ID
EvanHahn Jan 31, 2024
ce304b3
Merge branch 'main' into role-slash-capability-improvements
EvanHahn Feb 6, 2024
b8742e4
Improve types for `Role`
EvanHahn Feb 6, 2024
885071e
Tweak types so ROLES object works
EvanHahn Feb 6, 2024
f24c85e
Add runtime type checks
EvanHahn Feb 6, 2024
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
14 changes: 7 additions & 7 deletions src/mapeo-manager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import { getFastifyServerAddress } from './fastify-plugins/utils.js'
import { LocalPeers } from './local-peers.js'
import { InviteApi } from './invite-api.js'
import { LocalDiscovery } from './discovery/local-discovery.js'
import { Capabilities } from './capabilities.js'
import { Roles } from './roles.js'
import NoiseSecretStream from '@hyperswarm/secret-stream'
import { Logger } from './logger.js'
import { kSyncState } from './sync/sync-api.js'
Expand Down Expand Up @@ -590,7 +590,7 @@ export class MapeoManager extends TypedEmitter {
}

/**
* Sync initial data: the `auth` cores which contain the capability messages,
* Sync initial data: the `auth` cores which contain the role messages,
* and the `config` cores which contain the project name & custom config (if
* it exists). The API consumer should await this after `client.addProject()`
* to ensure that the device is fully added to the project.
Expand All @@ -605,26 +605,26 @@ export class MapeoManager extends TypedEmitter {
* @returns {Promise<boolean>}
*/
async #waitForInitialSync(project, { timeoutMs = 5000 } = {}) {
const [capability, projectSettings] = await Promise.all([
project.$getOwnCapabilities(),
const [ownRole, projectSettings] = await Promise.all([
project.$getOwnRole(),
project.$getProjectSettings(),
])
const {
auth: { localState: authState },
config: { localState: configState },
} = project.$sync[kSyncState].getState()
const isCapabilitySynced = capability !== Capabilities.NO_ROLE_CAPABILITIES
const isRoleSynced = ownRole !== Roles.NO_ROLE
const isProjectSettingsSynced =
projectSettings !== MapeoProject.EMPTY_PROJECT_SETTINGS
// Assumes every project that someone is invited to has at least one record
// in the auth store - the capability record for the invited device
// in the auth store - the row record for the invited device
const isAuthSynced = authState.want === 0 && authState.have > 0
// Assumes every project that someone is invited to has at least one record
// in the config store - defining the name of the project.
// TODO: Enforce adding a project name in the invite method
const isConfigSynced = configState.want === 0 && configState.have > 0
if (
isCapabilitySynced &&
isRoleSynced &&
isProjectSettingsSynced &&
isAuthSynced &&
isConfigSynced
Expand Down
22 changes: 11 additions & 11 deletions src/mapeo-project.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ import {
import {
BLOCKED_ROLE_ID,
COORDINATOR_ROLE_ID,
Capabilities,
Roles,
LEFT_ROLE_ID,
} from './capabilities.js'
} from './roles.js'
import {
getDeviceId,
projectKeyToId,
Expand Down Expand Up @@ -71,7 +71,7 @@ export class MapeoProject extends TypedEmitter {
#dataTypes
#blobStore
#coreOwnership
#capabilities
#roles
/** @ts-ignore */
#ownershipWriteDone
#sqlite
Expand Down Expand Up @@ -246,7 +246,7 @@ export class MapeoProject extends TypedEmitter {
coreKeypairs,
identityKeypair,
})
this.#capabilities = new Capabilities({
this.#roles = new Roles({
dataType: this.#dataTypes.role,
coreOwnership: this.#coreOwnership,
coreManager: this.#coreManager,
Expand All @@ -256,7 +256,7 @@ export class MapeoProject extends TypedEmitter {

this.#memberApi = new MemberApi({
deviceId: this.#deviceId,
capabilities: this.#capabilities,
roles: this.#roles,
coreOwnership: this.#coreOwnership,
encryptionKeys,
projectKey,
Expand Down Expand Up @@ -298,7 +298,7 @@ export class MapeoProject extends TypedEmitter {

this.#syncApi = new SyncApi({
coreManager: this.#coreManager,
capabilities: this.#capabilities,
roles: this.#roles,
logger: this.#l,
})

Expand All @@ -325,7 +325,7 @@ export class MapeoProject extends TypedEmitter {
}

// When a new peer is found, try to replicate (if it is not a member of the
// project it will fail the capability check and be ignored)
// project it will fail the role check and be ignored)
localPeers.on('peer-add', onPeerAdd)

// This happens whenever a peer replicates a core to the stream. SyncApi
Expand Down Expand Up @@ -494,8 +494,8 @@ export class MapeoProject extends TypedEmitter {
}
}

async $getOwnCapabilities() {
return this.#capabilities.getCapabilities(this.#deviceId)
async $getOwnRole() {
return this.#roles.getRole(this.#deviceId)
}

/**
Expand Down Expand Up @@ -567,7 +567,7 @@ export class MapeoProject extends TypedEmitter {
throw new Error('Cannot leave a project as a blocked device')
}

const knownDevices = Object.keys(await this.#capabilities.getAll())
const knownDevices = Object.keys(await this.#roles.getAll())
const projectCreatorDeviceId = await this.#coreOwnership.getOwner(
this.#projectId
)
Expand Down Expand Up @@ -629,7 +629,7 @@ export class MapeoProject extends TypedEmitter {
// 3.2 Clear indexed data

// 4. Assign LEFT role for device
await this.#capabilities.assignRole(this.#deviceId, LEFT_ROLE_ID)
await this.#roles.assignRole(this.#deviceId, LEFT_ROLE_ID)
}
}

Expand Down
36 changes: 18 additions & 18 deletions src/member-api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import { TypedEmitter } from 'tiny-typed-emitter'
import { InviteResponse_Decision } from './generated/rpc.js'
import { projectKeyToId } from './utils.js'
import { DEFAULT_CAPABILITIES } from './capabilities.js'
import { ROLES } from './roles.js'

/** @typedef {import('./datatype/index.js').DataType<import('./datastore/index.js').DataStore<'config'>, typeof import('./schema/project.js').deviceInfoTable, "deviceInfo", import('@mapeo/schema').DeviceInfo, import('@mapeo/schema').DeviceInfoValue>} DeviceInfoDataType */
/** @typedef {import('./datatype/index.js').DataType<import('./datastore/index.js').DataStore<'config'>, typeof import('./schema/client.js').projectSettingsTable, "projectSettings", import('@mapeo/schema').ProjectSettings, import('@mapeo/schema').ProjectSettingsValue>} ProjectDataType */
/** @typedef {{ deviceId: string, name?: import('@mapeo/schema').DeviceInfo['name'], capabilities: import('./capabilities.js').Capability }} MemberInfo */
/** @typedef {{ deviceId: string, name?: import('@mapeo/schema').DeviceInfo['name'], role: import('./roles.js').Role }} MemberInfo */

export class MemberApi extends TypedEmitter {
#ownDeviceId
#capabilities
#roles
#coreOwnership
#encryptionKeys
#projectKey
Expand All @@ -19,7 +19,7 @@ export class MemberApi extends TypedEmitter {
/**
* @param {Object} opts
* @param {string} opts.deviceId public key of this device as hex string
* @param {import('./capabilities.js').Capabilities} opts.capabilities
* @param {import('./roles.js').Roles} opts.roles
* @param {import('./core-ownership.js').CoreOwnership} opts.coreOwnership
* @param {import('./generated/keys.js').EncryptionKeys} opts.encryptionKeys
* @param {Buffer} opts.projectKey
Expand All @@ -30,7 +30,7 @@ export class MemberApi extends TypedEmitter {
*/
constructor({
deviceId,
capabilities,
roles,
coreOwnership,
encryptionKeys,
projectKey,
Expand All @@ -39,7 +39,7 @@ export class MemberApi extends TypedEmitter {
}) {
super()
this.#ownDeviceId = deviceId
this.#capabilities = capabilities
this.#roles = roles
this.#coreOwnership = coreOwnership
this.#encryptionKeys = encryptionKeys
this.#projectKey = projectKey
Expand All @@ -51,15 +51,15 @@ export class MemberApi extends TypedEmitter {
* @param {string} deviceId
*
* @param {Object} opts
* @param {import('./capabilities.js').RoleId} opts.roleId
* @param {import('./roles.js').RoleIdAssignableToOthers} opts.roleId
* @param {string} [opts.roleName]
* @param {string} [opts.roleDescription]
* @param {number} [opts.timeout]
*
* @returns {Promise<import('./generated/rpc.js').InviteResponse_Decision>}
*/
async invite(deviceId, { roleId, roleName, roleDescription, timeout }) {
if (!DEFAULT_CAPABILITIES[roleId]) {
if (!ROLES[roleId]) {
Copy link
Member

Choose a reason for hiding this comment

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

I think this needs to be stricter now, since we do not want to allow inviting someone as a project creator. This should be the "assignable to others" role list, although we should probably exclude the blocked role from that, but maybe as a to-do because I think some tests invite as blocked, just because I was lazy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in f24c85e.

throw new Error('Invalid role id')
}

Expand All @@ -82,14 +82,14 @@ export class MemberApi extends TypedEmitter {
projectKey: this.#projectKey,
encryptionKeys: this.#encryptionKeys,
projectInfo: { name: project.name },
roleName: roleName || DEFAULT_CAPABILITIES[roleId].name,
roleName: roleName || ROLES[roleId].name,
roleDescription,
invitorName: deviceName,
timeout,
})

if (response === InviteResponse_Decision.ACCEPT) {
await this.#capabilities.assignRole(deviceId, roleId)
await this.#roles.assignRole(deviceId, roleId)
}

return response
Expand All @@ -100,10 +100,10 @@ export class MemberApi extends TypedEmitter {
* @returns {Promise<MemberInfo>}
*/
async getById(deviceId) {
const capabilities = await this.#capabilities.getCapabilities(deviceId)
const role = await this.#roles.getRole(deviceId)

/** @type {MemberInfo} */
const result = { deviceId, capabilities }
const result = { deviceId, role }

try {
const configCoreId = await this.#coreOwnership.getCoreId(
Expand All @@ -129,15 +129,15 @@ export class MemberApi extends TypedEmitter {
* @returns {Promise<Array<MemberInfo>>}
*/
async getMany() {
const [allCapabilities, allDeviceInfo] = await Promise.all([
this.#capabilities.getAll(),
const [allRoles, allDeviceInfo] = await Promise.all([
this.#roles.getAll(),
this.#dataTypes.deviceInfo.getMany(),
])

return Promise.all(
Object.entries(allCapabilities).map(async ([deviceId, capabilities]) => {
Object.entries(allRoles).map(async ([deviceId, role]) => {
/** @type {MemberInfo} */
const memberInfo = { deviceId, capabilities }
const memberInfo = { deviceId, role }

try {
const configCoreId = await this.#coreOwnership.getCoreId(
Expand All @@ -163,10 +163,10 @@ export class MemberApi extends TypedEmitter {

/**
* @param {string} deviceId
* @param {import('./capabilities.js').RoleId} roleId
* @param {import('./roles.js').RoleIdAssignableToOthers} roleId
* @returns {Promise<void>}
*/
async assignRole(deviceId, roleId) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a TODO note here or open an issue to have stricter runtime checks here.
I also wonder if "assigning a blocked role" is confusing for the API consumer, and we should expose that separately as $member.block(deviceId) and limit assignRole() to just assignable roles within the project (currently participant and coordinator).

Copy link
Member

Choose a reason for hiding this comment

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

Also in the future we might want to add options to how we block users (which internally we might implement as different roles, or additional fields on membership records - maybe the latter better for many scenarios), for example:

  1. Removed with "self-destruct", e.g. when a device discovers it is removed it deletes all data including its own. Would be useful for compromised phones.
  2. Removed with removed encryption keys and keeping own data (but deleting others) - useful for when removing a device that we know has important data on it, that we want to sync off it in the future, without allowing that device to access it. Device would need to self-delete its encryption keys when it finds out it is blocked this way.
  3. Removed with ignore all data from device - would re-index and ignore all edits or created data from the removed device.

Having a separate API to remove devices would be helpful for implementing these.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a stricter runtime check in f24c85e. Let me know if you still want me to add a TODO/issue.

return this.#capabilities.assignRole(deviceId, roleId)
return this.#roles.assignRole(deviceId, roleId)
}
}
Loading
Loading