Page MenuHomePhabricator
Differential D14463

[lib] Add a migration that adds new permissions
AcceptedPublic
Actions

Authored by tomek on Wed, Mar 19, 7:52 AM.
Tags
None
Referenced Files
F5082988: D14463.diff
Sat, Mar 29, 10:29 AM
Unknown Object (File)
Fri, Mar 28, 8:45 PM
Unknown Object (File)
Thu, Mar 27, 8:53 PM
Unknown Object (File)
Wed, Mar 26, 7:46 PM
Unknown Object (File)
Wed, Mar 26, 2:44 PM
Unknown Object (File)
Wed, Mar 26, 10:21 AM
Unknown Object (File)
Wed, Mar 26, 7:00 AM
Unknown Object (File)
Wed, Mar 26, 4:47 AM
View All 13 Files
Subscribers
None

Details

Reviewers
kamil
ashoat
Summary

All the admins should have both new permissions. All the members should have only DELETE_OWN_MESSAGES permission.

https://linear.app/comm/issue/ENG-10323/add-keyserver-migrations

Depends on D14462

Test Plan

Run the migrations and checked if the permissions were updated in the DB. Also modified roles in the UI and made sure the DB was updated.

Diff Detail

Repository
rCOMM Comm
Branch
delete-messages
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

tomek created this revision.Wed, Mar 19, 7:52 AM
Harbormaster completed remote builds in B33769: Diff 47461.Wed, Mar 19, 8:10 AM
tomek requested review of this revision.Wed, Mar 19, 8:10 AM
ashoat accepted this revision.Wed, Mar 19, 9:52 AM
ashoat added inline comments.
keyserver/src/database/migration-config.js
994–995

It looks like this migration was initially added for a special situation, where we had accidentally removed a permission. I guess that adding a new permission from scratch requires the same stuff as fixing an accidentally removed permission?

keyserver/src/fetchers/thread-fetchers.js
337–338

If we land this with NEXT_CODE_VERSION, we need to make sure that we land a client migration at the same time. Otherwise you can consider switching to FUTURE_CODE_VERSION

lib/shared/thread-utils.js
836–841

Does the threadPermissionMembershipPrefixes.MEMBER prefix get stripped before we get there?

This revision is now accepted and ready to land.Wed, Mar 19, 9:52 AM
tomek updated this revision to Diff 47575.Wed, Mar 26, 11:56 AM

Updated permissions filtering logic

keyserver/src/database/migration-config.js
994–995

Yes, exactly

keyserver/src/fetchers/thread-fetchers.js
337–338

The client migrations are introduced in the next diff, and I'm going to make sure both diffs land simultaneously.

lib/shared/thread-utils.js
836–841

After my testing in the next diff I realized that it wasn't stripped. Updated the approach to filter out all the permissions based on the suffix which should work regardless of the prefixes.

tomek added a child revision: D14505: [lib] Migrate permissions on clients.Wed, Mar 26, 12:06 PM
Harbormaster completed remote builds in B33847: Diff 47575.Wed, Mar 26, 12:31 PM

Revision Contents
Changeset List

PathSize
keyserver/
src/
database/
121 lines
fetchers/
13 lines
lib/
shared/
13 lines

Diff 47461

View Options

keyserver/src/database/migration-config.js

Show First 20 Lines • Show All 65 Lines • ▼ Show 20 Lines case State::DATA_HASH: {
} }
response->set_backupid(this->backupID); response->set_backupid(this->backupID);
this->holder = tools::generateHolder(this->dataHash, this->backupID); this->holder = tools::generateHolder(this->dataHash, this->backupID);
put_client_initialize_cxx(this->holder.c_str()); put_client_initialize_cxx(this->holder.c_str());
put_client_write_cxx( put_client_write_cxx(
this->holder.c_str(), this->holder.c_str(),
tools::getBlobPutField(blob::PutRequest::DataCase::kHolder), tools::getBlobPutField(blob::PutRequest::DataCase::kHolder),
this->holder.c_str()); this->holder.c_str());
put_client_blocking_read_cxx( put_client_blocking_read_cxx(this->holder.c_str());
this->holder.c_str()); // todo this should be avoided
// (blocking); we should be able to
// ignore responses; we probably want to
// delegate performing ops to separate
// threads in the base reactors
put_client_write_cxx( put_client_write_cxx(
this->holder.c_str(), this->holder.c_str(),
tools::getBlobPutField(blob::PutRequest::DataCase::kBlobHash), tools::getBlobPutField(blob::PutRequest::DataCase::kBlobHash),
this->dataHash.c_str()); this->dataHash.c_str());
rust::String responseStr = put_client_blocking_read_cxx( rust::String responseStr =
this->holder.c_str()); // todo this should be avoided put_client_blocking_read_cxx(this->holder.c_str());
// (blocking); we should be able to
// ignore responses; we probably
// want to delegate performing ops
// to separate threads in the base
// reactors
// data exists? // data exists?
if ((bool)tools::charPtrToInt(responseStr.c_str())) { if ((bool)tools::charPtrToInt(responseStr.c_str())) {
return std::make_unique<ServerBidiReactorStatus>( return std::make_unique<ServerBidiReactorStatus>(
grpc::Status::OK, true); grpc::Status::OK, true);
} }
return nullptr; return nullptr;
} }
case State::DATA_CHUNKS: { case State::DATA_CHUNKS: {
if (request.mutable_newcompactionchunk()->empty()) { if (request.mutable_newcompactionchunk()->empty()) {
return std::make_unique<ServerBidiReactorStatus>(grpc::Status::OK); return std::make_unique<ServerBidiReactorStatus>(grpc::Status::OK);
} }
put_client_write_cxx( put_client_write_cxx(
this->holder.c_str(), this->holder.c_str(),
tools::getBlobPutField(blob::PutRequest::DataCase::kDataChunk), tools::getBlobPutField(blob::PutRequest::DataCase::kDataChunk),
std::string(std::move(*request.mutable_newcompactionchunk())) std::string(std::move(*request.mutable_newcompactionchunk()))
.c_str()); .c_str());
put_client_blocking_read_cxx( put_client_blocking_read_cxx(this->holder.c_str());
this->holder.c_str()); // todo this should be avoided
// (blocking); we should be able to
// ignore responses; we probably want to
// delegate performing ops to separate
// threads in the base reactors
return nullptr; return nullptr;
} }
} }
throw std::runtime_error("new backup - invalid state"); throw std::runtime_error("new backup - invalid state");
} }
void CreateNewBackupReactor::terminateCallback() { void CreateNewBackupReactor::terminateCallback() {
Show All 18 Lines
View Options

keyserver/src/fetchers/thread-fetchers.js

Show First 20 Lines • Show All 65 Lines • ▼ Show 20 Lines case State::DATA_HASH: {
} }
response->set_backupid(this->backupID); response->set_backupid(this->backupID);
this->holder = tools::generateHolder(this->dataHash, this->backupID); this->holder = tools::generateHolder(this->dataHash, this->backupID);
put_client_initialize_cxx(this->holder.c_str()); put_client_initialize_cxx(this->holder.c_str());
put_client_write_cxx( put_client_write_cxx(
this->holder.c_str(), this->holder.c_str(),
tools::getBlobPutField(blob::PutRequest::DataCase::kHolder), tools::getBlobPutField(blob::PutRequest::DataCase::kHolder),
this->holder.c_str()); this->holder.c_str());
put_client_blocking_read_cxx( put_client_blocking_read_cxx(this->holder.c_str());
this->holder.c_str()); // todo this should be avoided
// (blocking); we should be able to
// ignore responses; we probably want to
// delegate performing ops to separate
// threads in the base reactors
put_client_write_cxx( put_client_write_cxx(
this->holder.c_str(), this->holder.c_str(),
tools::getBlobPutField(blob::PutRequest::DataCase::kBlobHash), tools::getBlobPutField(blob::PutRequest::DataCase::kBlobHash),
this->dataHash.c_str()); this->dataHash.c_str());
rust::String responseStr = put_client_blocking_read_cxx( rust::String responseStr =
this->holder.c_str()); // todo this should be avoided put_client_blocking_read_cxx(this->holder.c_str());
// (blocking); we should be able to
// ignore responses; we probably
// want to delegate performing ops
// to separate threads in the base
// reactors
// data exists? // data exists?
if ((bool)tools::charPtrToInt(responseStr.c_str())) { if ((bool)tools::charPtrToInt(responseStr.c_str())) {
return std::make_unique<ServerBidiReactorStatus>( return std::make_unique<ServerBidiReactorStatus>(
grpc::Status::OK, true); grpc::Status::OK, true);
} }
return nullptr; return nullptr;
} }
case State::DATA_CHUNKS: { case State::DATA_CHUNKS: {
if (request.mutable_newcompactionchunk()->empty()) { if (request.mutable_newcompactionchunk()->empty()) {
return std::make_unique<ServerBidiReactorStatus>(grpc::Status::OK); return std::make_unique<ServerBidiReactorStatus>(grpc::Status::OK);
} }
put_client_write_cxx( put_client_write_cxx(
this->holder.c_str(), this->holder.c_str(),
tools::getBlobPutField(blob::PutRequest::DataCase::kDataChunk), tools::getBlobPutField(blob::PutRequest::DataCase::kDataChunk),
std::string(std::move(*request.mutable_newcompactionchunk())) std::string(std::move(*request.mutable_newcompactionchunk()))
.c_str()); .c_str());
put_client_blocking_read_cxx( put_client_blocking_read_cxx(this->holder.c_str());
this->holder.c_str()); // todo this should be avoided
// (blocking); we should be able to
// ignore responses; we probably want to
// delegate performing ops to separate
// threads in the base reactors
return nullptr; return nullptr;
} }
} }
throw std::runtime_error("new backup - invalid state"); throw std::runtime_error("new backup - invalid state");
} }
void CreateNewBackupReactor::terminateCallback() { void CreateNewBackupReactor::terminateCallback() {
Show All 18 Lines
View Options

lib/shared/thread-utils.js

Show First 20 Lines • Show All 65 Lines • ▼ Show 20 Lines case State::DATA_HASH: {
} }
response->set_backupid(this->backupID); response->set_backupid(this->backupID);
this->holder = tools::generateHolder(this->dataHash, this->backupID); this->holder = tools::generateHolder(this->dataHash, this->backupID);
put_client_initialize_cxx(this->holder.c_str()); put_client_initialize_cxx(this->holder.c_str());
put_client_write_cxx( put_client_write_cxx(
this->holder.c_str(), this->holder.c_str(),
tools::getBlobPutField(blob::PutRequest::DataCase::kHolder), tools::getBlobPutField(blob::PutRequest::DataCase::kHolder),
this->holder.c_str()); this->holder.c_str());
put_client_blocking_read_cxx( put_client_blocking_read_cxx(this->holder.c_str());
this->holder.c_str()); // todo this should be avoided
// (blocking); we should be able to
// ignore responses; we probably want to
// delegate performing ops to separate
// threads in the base reactors
put_client_write_cxx( put_client_write_cxx(
this->holder.c_str(), this->holder.c_str(),
tools::getBlobPutField(blob::PutRequest::DataCase::kBlobHash), tools::getBlobPutField(blob::PutRequest::DataCase::kBlobHash),
this->dataHash.c_str()); this->dataHash.c_str());
rust::String responseStr = put_client_blocking_read_cxx( rust::String responseStr =
this->holder.c_str()); // todo this should be avoided put_client_blocking_read_cxx(this->holder.c_str());
// (blocking); we should be able to
// ignore responses; we probably
// want to delegate performing ops
// to separate threads in the base
// reactors
// data exists? // data exists?
if ((bool)tools::charPtrToInt(responseStr.c_str())) { if ((bool)tools::charPtrToInt(responseStr.c_str())) {
return std::make_unique<ServerBidiReactorStatus>( return std::make_unique<ServerBidiReactorStatus>(
grpc::Status::OK, true); grpc::Status::OK, true);
} }
return nullptr; return nullptr;
} }
case State::DATA_CHUNKS: { case State::DATA_CHUNKS: {
if (request.mutable_newcompactionchunk()->empty()) { if (request.mutable_newcompactionchunk()->empty()) {
return std::make_unique<ServerBidiReactorStatus>(grpc::Status::OK); return std::make_unique<ServerBidiReactorStatus>(grpc::Status::OK);
} }
put_client_write_cxx( put_client_write_cxx(
this->holder.c_str(), this->holder.c_str(),
tools::getBlobPutField(blob::PutRequest::DataCase::kDataChunk), tools::getBlobPutField(blob::PutRequest::DataCase::kDataChunk),
std::string(std::move(*request.mutable_newcompactionchunk())) std::string(std::move(*request.mutable_newcompactionchunk()))
.c_str()); .c_str());
put_client_blocking_read_cxx( put_client_blocking_read_cxx(this->holder.c_str());
this->holder.c_str()); // todo this should be avoided
// (blocking); we should be able to
// ignore responses; we probably want to
// delegate performing ops to separate
// threads in the base reactors
return nullptr; return nullptr;
} }
} }
throw std::runtime_error("new backup - invalid state"); throw std::runtime_error("new backup - invalid state");
} }
void CreateNewBackupReactor::terminateCallback() { void CreateNewBackupReactor::terminateCallback() {
Show All 18 Lines