Paths

Table of Contentst

-
keyserver/src/updaters/
-
src/
-
updaters/
-
thread-updaters.js

[keyserver] Update usage of threads.default_role in thread-updaters.js
ClosedPublic
Actions

Authored by rohan on Nov 24 2023, 9:53 AM.

Details

Reviewers

atul
ginsu
tomek

Commits

rCOMMe83074e9bf06: [keyserver] Update usage of threads.default_role in thread-updaters.js

Summary

We need to get rid of usages of threads.default_role throughout the codebase so we can eventually drop the column.

This diff handles the usage in thread-updaters.js

Depends on D9976

Addresses part of ENG-5833

Test Plan

Tested both use cases of threads.default_role separately in this file:

Attempts to fetch the role permissions for the default role in the thread (expected return result size is one row)
- Ran the query before and after in my MariaDB on my local database and saw the same results printed out
Fetches the user, role and default_role from memberships to determine whether the user is a 'default role user'
- Ran the query before and after in my MariaDB on my local database and saw the same results printed out

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

rohan created this revision.Nov 24 2023, 9:53 AM

Herald added a subscriber: ashoat. · View Herald TranscriptNov 24 2023, 9:53 AM

rohan added a child revision: D9978: [keyserver] Update usage of threads.default_role in validate-role-permissions.js.Nov 24 2023, 9:55 AM

Harbormaster completed remote builds in B24404: Diff 33603.Nov 24 2023, 10:26 AM

rohan requested review of this revision.Nov 24 2023, 10:26 AM

ashoat added inline comments.Nov 26 2023, 4:53 PM

keyserver/src/updaters/thread-updaters.js
211 ↗	(On Diff #33603)	Makes me wonder if we should update the `thread` index on `roles` to be a `thread`, `special_role` index

tomek accepted this revision.Nov 27 2023, 1:53 AM

tomek added inline comments.

keyserver/src/updaters/thread-updaters.js
211 ↗	(On Diff #33603)	Makes sense, but shouldn't matter too much, as we probably don't expect more than a couple of roles per thread

This revision is now accepted and ready to land.Nov 27 2023, 1:53 AM

rohan added inline comments.Nov 27 2023, 9:47 AM

keyserver/src/updaters/thread-updaters.js
211 ↗	(On Diff #33603)	Addressed in D9961!

Closed by commit rCOMMe83074e9bf06: [keyserver] Update usage of threads.default_role in thread-updaters.js (authored by rohan). · Explain WhyNov 27 2023, 10:36 AM

This revision was automatically updated to reflect the committed changes.

rohan added a commit: rCOMMe83074e9bf06: [keyserver] Update usage of threads.default_role in thread-updaters.js.

Revision Contents
Changeset List

Path

Size

keyserver/

src/

updaters/

thread-updaters.js

9 lines

Diff 33755

View Options

keyserver/src/updaters/thread-updaters.js

	import ExpoModulesCore			import ExpoModulesCore
				import CryptoKit

	public class AESCryptoModule: Module {			public class AESCryptoModule: Module {
	public func definition() -> ModuleDefinition {			public func definition() -> ModuleDefinition {
	Name("AESCrypto")			Name("AESCrypto")

	Function("hello") { () -> String in			Function("generateKey", generateKey)
	return "Hello world! 👋"
	}			}
	}			}

				// MARK: - Function implementations

				private func generateKey(destination: Uint8Array) throws {
				let key = SymmetricKey(size: .bits256)
				guard destination.byteLength * 8 == key.bitCount else {
				throw InvalidKeyLengthException()
				}
				atulUnsubmitted Not Done Inline Actions Before we even generate `key` via `SymmetricKey`, should we check that `destination.byteLength` is `32` and throw `InvalidDestinationSizeException()` if not? It seems like `destination` is the input that we want to validate, so we should just do that directly? It's not really so much that `SymmetricKey` is giving us incorrect size `key` since it's (I'm assuming) always going to give us exactly the size we're asking for? Feel free to re-request review if there's something I'm missing here. atul: Before we even generate `key` via `SymmetricKey`, should we check that `destination.byteLength`…
				bartekAuthorUnsubmitted Done Inline Actions The only reason the guard is below `SymmetricKey` is that I wanted to use its `bitCount` property to get the desired destination length, but you're right that it can be done this way: guard destination.byteLength == KEY_SIZE else { throw InvalidKeyLengthException() } let key = SymmetricKey(size: .bits256) I'll refactor this as it is more clear solution bartek: The only reason the guard is below `SymmetricKey` is that I wanted to use its `bitCount`…

				key.withUnsafeBytes { bytes in
				let _ = bytes.copyBytes(to: destination.rawBufferPtr())
				}
				}

				// MARK: - Utilities

				extension TypedArray {
				func rawBufferPtr() -> UnsafeMutableRawBufferPointer {
				UnsafeMutableRawBufferPointer(start: self.rawPointer,
				count: self.byteLength)
				}
				}

				// MARK: - Exception definitions

				private class InvalidKeyLengthException: Exception {
				override var reason: String {
				"The AES key has invalid length"
				}
	}			}

[keyserver] Update usage of threads.default_role in thread-updaters.jsClosedPublicActions