Page MenuHomePhabricator
Differential D14465

[identity] Support service-to-service token in auth RPCs
ClosedPublic
Actions

Authored by bartek on Mar 20 2025, 3:58 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 18, 2:31 AM
Unknown Object (File)
Fri, Apr 18, 1:04 AM
Unknown Object (File)
Thu, Apr 10, 12:30 AM
Unknown Object (File)
Thu, Apr 3, 7:16 PM
Unknown Object (File)
Wed, Apr 2, 6:54 AM
Unknown Object (File)
Wed, Apr 2, 4:32 AM
Unknown Object (File)
Mon, Mar 31, 7:40 PM
Unknown Object (File)
Sun, Mar 30, 7:32 AM
View All 23 Files
Subscribers
ashoat

Details

Reviewers
kamil
tomek
Commits
rCOMMa241d420bbb6: [identity] Support service-to-service token in auth RPCs
Summary

To call authenticated RPCs from other services, we have to support providing s2s token.
Modified the interceptor to look for the token, in addition to traditional Auth Metadata (uid + did + csat).

Test Plan

Tested locally, using GUI gRPC client and default localstack-stored service-to-service token.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

bartek created this revision.Mar 20 2025, 3:58 AM
bartek held this revision as a draft.
Herald added a subscriber: ashoat. · View Herald TranscriptMar 20 2025, 3:58 AM
bartek added a child revision: D14466: [grpc-clients] Add helpers for services grpc client.Mar 20 2025, 3:59 AM
Harbormaster completed remote builds in B33771: Diff 47475.Mar 20 2025, 4:16 AM
bartek published this revision for review.Mar 23 2025, 11:48 PM
kamil accepted this revision.Mar 24 2025, 3:13 AM
kamil added inline comments.
services/identity/src/grpc_services/authenticated.rs
101–107

Why are we not using verify_auth_credential here? It seems to be doing the same thing as for AuthorizationCredential::UserToken. Maybe this could make the code simpler.

This revision is now accepted and ready to land.Mar 24 2025, 3:13 AM
bartek added inline comments.Mar 24 2025, 6:04 AM
services/identity/src/grpc_services/authenticated.rs
101–107

verify_auth_credential is for non-Identity services. It calls the VerifyUserAccessToken Identity RPC which does this DDB call internally. It would create an unnecessary network call to self.

Closed by commit rCOMMa241d420bbb6: [identity] Support service-to-service token in auth RPCs (authored by bartek). · Explain WhyMar 24 2025, 6:55 AM
This revision was automatically updated to reflect the committed changes.
bartek added a commit: rCOMMa241d420bbb6: [identity] Support service-to-service token in auth RPCs.

Revision Contents
Changeset List

PathSize
services/
identity/
src/
1 line
grpc_services/
44 lines
10 lines

Diff 47475

View Options

services/identity/src/constants.rs

Show First 20 Lines • Show All 861 Lines • ▼ Show 20 Lines
// persisted. However, we aren't able to specify nested keys in `blacklist`. // persisted. However, we aren't able to specify nested keys in `blacklist`.
// As a result, if we want to prevent nested keys from being persisted we'll // As a result, if we want to prevent nested keys from being persisted we'll
// need to use `createTransform(...)` to specify an `inbound` function that // need to use `createTransform(...)` to specify an `inbound` function that
// allows us to modify the `state` object before it's passed through // allows us to modify the `state` object before it's passed through
// `JSON.stringify(...)` and written to disk. We specify the keys for which // `JSON.stringify(...)` and written to disk. We specify the keys for which
// this transformation should be executed in the `whitelist` property of the // this transformation should be executed in the `whitelist` property of the
// `config` object that's passed to `createTransform(...)`. // `config` object that's passed to `createTransform(...)`.
// eslint-disable-next-line no-unused-vars // eslint-disable-next-line no-unused-vars
type PersistedThreadMessageInfo = {
+startReached: boolean,
};
type PersistedMessageStore = { type PersistedMessageStore = {
+local: { +[id: string]: LocalMessageInfo }, +local: { +[id: string]: LocalMessageInfo },
+currentAsOf: { +[keyserverID: string]: number }, +currentAsOf: { +[keyserverID: string]: number },
+threads: { +[threadID: string]: PersistedThreadMessageInfo },
}; };
const messageStoreMessagesBlocklistTransform: Transform = createTransform( const messageStoreMessagesBlocklistTransform: Transform = createTransform(
(state: MessageStore): PersistedMessageStore => { (state: MessageStore): PersistedMessageStore => {
const { messages, threads, ...messageStoreSansMessages } = state; const { messages, threads, ...messageStoreSansMessages } = state;
// We also do not want to persist `messageStore.threads[ID].messageIDs` return { ...messageStoreSansMessages };
// because they can be deterministically computed based on messages we have
// from SQLite
const threadsToPersist = {};
for (const threadID in threads) {
const { messageIDs, ...threadsData } = threads[threadID];
threadsToPersist[threadID] = threadsData;
}
return { ...messageStoreSansMessages, threads: threadsToPersist };
}, },
(state: MessageStore): MessageStore => { (state: MessageStore): MessageStore => {
const { threads: persistedThreads, ...messageStore } = state;
const threads = {};
for (const threadID in persistedThreads) {
threads[threadID] = { ...persistedThreads[threadID], messageIDs: [] };
}
// We typically expect `messageStore.messages` to be `undefined` because // We typically expect `messageStore.messages` to be `undefined` because
// messages are persisted in the SQLite `messages` table rather than via // messages are persisted in the SQLite `messages` table rather than via
// `redux-persist`. In this case we want to set `messageStore.messages` // `redux-persist`. In this case we want to set `messageStore.messages`
// to {} so we don't run into issues with `messageStore.messages` being // to {} so we don't run into issues with `messageStore.messages` being
// `undefined` (https://phab.comm.dev/D5545). // `undefined` (https://phab.comm.dev/D5545).
// //
// However, in the case that a user is upgrading from a client where // However, in the case that a user is upgrading from a client where
// `persistConfig.version` < 31, we expect `messageStore.messages` to // `persistConfig.version` < 31, we expect `messageStore.messages` to
// contain messages stored via `redux-persist` that we need in order // contain messages stored via `redux-persist` that we need in order
// to correctly populate the SQLite `messages` table in migration 31 // to correctly populate the SQLite `messages` table in migration 31
// (https://phab.comm.dev/D2600). // (https://phab.comm.dev/D2600).
// //
// However, because `messageStoreMessagesBlocklistTransform` modifies // However, because `messageStoreMessagesBlocklistTransform` modifies
// `messageStore` before migrations are run, we need to make sure we aren't // `messageStore` before migrations are run, we need to make sure we aren't
// inadvertently clearing `messageStore.messages` (by setting to {}) before // inadvertently clearing `messageStore.messages` (by setting to {}) before
// messages are stored in SQLite (https://linear.app/comm/issue/ENG-2377). // messages are stored in SQLite (https://linear.app/comm/issue/ENG-2377).
return { ...messageStore, threads, messages: messageStore.messages ?? {} }; return {
...state,
threads: state.threads ?? {},
messages: state.messages ?? {},
};
}, },
{ whitelist: ['messageStore'] }, { whitelist: ['messageStore'] },
); );
type PersistedReportStore = $Diff< type PersistedReportStore = $Diff<
ReportStore, ReportStore,
{ +queuedReports: $ReadOnlyArray<ClientReportCreationRequest> }, { +queuedReports: $ReadOnlyArray<ClientReportCreationRequest> },
>; >;
▲ Show 20 Lines • Show All 70 Lines • ▼ Show 20 Lines
}; };
const codeVersion: number = commCoreModule.getCodeVersion(); const codeVersion: number = commCoreModule.getCodeVersion();
// This local exists to avoid a circular dependency where redux-setup needs to // This local exists to avoid a circular dependency where redux-setup needs to
// import all the navigation and screen stuff, but some of those screens want to // import all the navigation and screen stuff, but some of those screens want to
// access the persistor to purge its state. // access the persistor to purge its state.
let storedPersistor = null; let storedPersistor = null;
atulUnsubmitted
Not Done
Inline Actions

Is your editor auto formatter maybe adding these newlines? Feel free to disregard if intentional, but noticed these D9682 as well.

atul: Is your editor auto formatter maybe adding these newlines? Feel free to disregard if…
kamilAuthorUnsubmitted
Done
Inline Actions

looks like it's my IDE fault, it is adding this while committing, I'll take a look

kamil: looks like it's my IDE fault, it is adding this while committing, I'll take a look
atulUnsubmitted
Not Done
Inline Actions

Sweet, thanks for taking a look!

Not a huge deal regardless, guess it clutters up git blame history a bit.. but not end of the world

atul: Sweet, thanks for taking a look! Not a huge deal regardless, guess it clutters up `git blame`…
function setPersistor(persistor: *) { function setPersistor(persistor: *) {
storedPersistor = persistor; storedPersistor = persistor;
} }
function getPersistor(): empty { function getPersistor(): empty {
invariant(storedPersistor, 'should be set'); invariant(storedPersistor, 'should be set');
return storedPersistor; return storedPersistor;
} }
export { persistConfig, codeVersion, setPersistor, getPersistor }; export { persistConfig, codeVersion, setPersistor, getPersistor };
View Options

services/identity/src/grpc_services/authenticated.rs

Show First 20 Lines • Show All 861 Lines • ▼ Show 20 Lines
// persisted. However, we aren't able to specify nested keys in `blacklist`. // persisted. However, we aren't able to specify nested keys in `blacklist`.
// As a result, if we want to prevent nested keys from being persisted we'll // As a result, if we want to prevent nested keys from being persisted we'll
// need to use `createTransform(...)` to specify an `inbound` function that // need to use `createTransform(...)` to specify an `inbound` function that
// allows us to modify the `state` object before it's passed through // allows us to modify the `state` object before it's passed through
// `JSON.stringify(...)` and written to disk. We specify the keys for which // `JSON.stringify(...)` and written to disk. We specify the keys for which
// this transformation should be executed in the `whitelist` property of the // this transformation should be executed in the `whitelist` property of the
// `config` object that's passed to `createTransform(...)`. // `config` object that's passed to `createTransform(...)`.
// eslint-disable-next-line no-unused-vars // eslint-disable-next-line no-unused-vars
type PersistedThreadMessageInfo = {
+startReached: boolean,
};
type PersistedMessageStore = { type PersistedMessageStore = {
+local: { +[id: string]: LocalMessageInfo }, +local: { +[id: string]: LocalMessageInfo },
+currentAsOf: { +[keyserverID: string]: number }, +currentAsOf: { +[keyserverID: string]: number },
+threads: { +[threadID: string]: PersistedThreadMessageInfo },
}; };
const messageStoreMessagesBlocklistTransform: Transform = createTransform( const messageStoreMessagesBlocklistTransform: Transform = createTransform(
(state: MessageStore): PersistedMessageStore => { (state: MessageStore): PersistedMessageStore => {
const { messages, threads, ...messageStoreSansMessages } = state; const { messages, threads, ...messageStoreSansMessages } = state;
// We also do not want to persist `messageStore.threads[ID].messageIDs` return { ...messageStoreSansMessages };
// because they can be deterministically computed based on messages we have
// from SQLite
const threadsToPersist = {};
for (const threadID in threads) {
const { messageIDs, ...threadsData } = threads[threadID];
threadsToPersist[threadID] = threadsData;
}
return { ...messageStoreSansMessages, threads: threadsToPersist };
}, },
(state: MessageStore): MessageStore => { (state: MessageStore): MessageStore => {
const { threads: persistedThreads, ...messageStore } = state;
const threads = {};
for (const threadID in persistedThreads) {
threads[threadID] = { ...persistedThreads[threadID], messageIDs: [] };
}
// We typically expect `messageStore.messages` to be `undefined` because // We typically expect `messageStore.messages` to be `undefined` because
// messages are persisted in the SQLite `messages` table rather than via // messages are persisted in the SQLite `messages` table rather than via
// `redux-persist`. In this case we want to set `messageStore.messages` // `redux-persist`. In this case we want to set `messageStore.messages`
// to {} so we don't run into issues with `messageStore.messages` being // to {} so we don't run into issues with `messageStore.messages` being
// `undefined` (https://phab.comm.dev/D5545). // `undefined` (https://phab.comm.dev/D5545).
// //
// However, in the case that a user is upgrading from a client where // However, in the case that a user is upgrading from a client where
// `persistConfig.version` < 31, we expect `messageStore.messages` to // `persistConfig.version` < 31, we expect `messageStore.messages` to
// contain messages stored via `redux-persist` that we need in order // contain messages stored via `redux-persist` that we need in order
// to correctly populate the SQLite `messages` table in migration 31 // to correctly populate the SQLite `messages` table in migration 31
// (https://phab.comm.dev/D2600). // (https://phab.comm.dev/D2600).
// //
// However, because `messageStoreMessagesBlocklistTransform` modifies // However, because `messageStoreMessagesBlocklistTransform` modifies
// `messageStore` before migrations are run, we need to make sure we aren't // `messageStore` before migrations are run, we need to make sure we aren't
// inadvertently clearing `messageStore.messages` (by setting to {}) before // inadvertently clearing `messageStore.messages` (by setting to {}) before
// messages are stored in SQLite (https://linear.app/comm/issue/ENG-2377). // messages are stored in SQLite (https://linear.app/comm/issue/ENG-2377).
return { ...messageStore, threads, messages: messageStore.messages ?? {} }; return {
...state,
threads: state.threads ?? {},
messages: state.messages ?? {},
};
}, },
{ whitelist: ['messageStore'] }, { whitelist: ['messageStore'] },
); );
type PersistedReportStore = $Diff< type PersistedReportStore = $Diff<
ReportStore, ReportStore,
{ +queuedReports: $ReadOnlyArray<ClientReportCreationRequest> }, { +queuedReports: $ReadOnlyArray<ClientReportCreationRequest> },
>; >;
▲ Show 20 Lines • Show All 70 Lines • ▼ Show 20 Lines
}; };
const codeVersion: number = commCoreModule.getCodeVersion(); const codeVersion: number = commCoreModule.getCodeVersion();
// This local exists to avoid a circular dependency where redux-setup needs to // This local exists to avoid a circular dependency where redux-setup needs to
// import all the navigation and screen stuff, but some of those screens want to // import all the navigation and screen stuff, but some of those screens want to
// access the persistor to purge its state. // access the persistor to purge its state.
let storedPersistor = null; let storedPersistor = null;
atulUnsubmitted
Not Done
Inline Actions

Is your editor auto formatter maybe adding these newlines? Feel free to disregard if intentional, but noticed these D9682 as well.

atul: Is your editor auto formatter maybe adding these newlines? Feel free to disregard if…
kamilAuthorUnsubmitted
Done
Inline Actions

looks like it's my IDE fault, it is adding this while committing, I'll take a look

kamil: looks like it's my IDE fault, it is adding this while committing, I'll take a look
atulUnsubmitted
Not Done
Inline Actions

Sweet, thanks for taking a look!

Not a huge deal regardless, guess it clutters up git blame history a bit.. but not end of the world

atul: Sweet, thanks for taking a look! Not a huge deal regardless, guess it clutters up `git blame`…
function setPersistor(persistor: *) { function setPersistor(persistor: *) {
storedPersistor = persistor; storedPersistor = persistor;
} }
function getPersistor(): empty { function getPersistor(): empty {
invariant(storedPersistor, 'should be set'); invariant(storedPersistor, 'should be set');
return storedPersistor; return storedPersistor;
} }
export { persistConfig, codeVersion, setPersistor, getPersistor }; export { persistConfig, codeVersion, setPersistor, getPersistor };
View Options

services/identity/src/main.rs

Show First 20 Lines • Show All 861 Lines • ▼ Show 20 Lines
// persisted. However, we aren't able to specify nested keys in `blacklist`. // persisted. However, we aren't able to specify nested keys in `blacklist`.
// As a result, if we want to prevent nested keys from being persisted we'll // As a result, if we want to prevent nested keys from being persisted we'll
// need to use `createTransform(...)` to specify an `inbound` function that // need to use `createTransform(...)` to specify an `inbound` function that
// allows us to modify the `state` object before it's passed through // allows us to modify the `state` object before it's passed through
// `JSON.stringify(...)` and written to disk. We specify the keys for which // `JSON.stringify(...)` and written to disk. We specify the keys for which
// this transformation should be executed in the `whitelist` property of the // this transformation should be executed in the `whitelist` property of the
// `config` object that's passed to `createTransform(...)`. // `config` object that's passed to `createTransform(...)`.
// eslint-disable-next-line no-unused-vars // eslint-disable-next-line no-unused-vars
type PersistedThreadMessageInfo = {
+startReached: boolean,
};
type PersistedMessageStore = { type PersistedMessageStore = {
+local: { +[id: string]: LocalMessageInfo }, +local: { +[id: string]: LocalMessageInfo },
+currentAsOf: { +[keyserverID: string]: number }, +currentAsOf: { +[keyserverID: string]: number },
+threads: { +[threadID: string]: PersistedThreadMessageInfo },
}; };
const messageStoreMessagesBlocklistTransform: Transform = createTransform( const messageStoreMessagesBlocklistTransform: Transform = createTransform(
(state: MessageStore): PersistedMessageStore => { (state: MessageStore): PersistedMessageStore => {
const { messages, threads, ...messageStoreSansMessages } = state; const { messages, threads, ...messageStoreSansMessages } = state;
// We also do not want to persist `messageStore.threads[ID].messageIDs` return { ...messageStoreSansMessages };
// because they can be deterministically computed based on messages we have
// from SQLite
const threadsToPersist = {};
for (const threadID in threads) {
const { messageIDs, ...threadsData } = threads[threadID];
threadsToPersist[threadID] = threadsData;
}
return { ...messageStoreSansMessages, threads: threadsToPersist };
}, },
(state: MessageStore): MessageStore => { (state: MessageStore): MessageStore => {
const { threads: persistedThreads, ...messageStore } = state;
const threads = {};
for (const threadID in persistedThreads) {
threads[threadID] = { ...persistedThreads[threadID], messageIDs: [] };
}
// We typically expect `messageStore.messages` to be `undefined` because // We typically expect `messageStore.messages` to be `undefined` because
// messages are persisted in the SQLite `messages` table rather than via // messages are persisted in the SQLite `messages` table rather than via
// `redux-persist`. In this case we want to set `messageStore.messages` // `redux-persist`. In this case we want to set `messageStore.messages`
// to {} so we don't run into issues with `messageStore.messages` being // to {} so we don't run into issues with `messageStore.messages` being
// `undefined` (https://phab.comm.dev/D5545). // `undefined` (https://phab.comm.dev/D5545).
// //
// However, in the case that a user is upgrading from a client where // However, in the case that a user is upgrading from a client where
// `persistConfig.version` < 31, we expect `messageStore.messages` to // `persistConfig.version` < 31, we expect `messageStore.messages` to
// contain messages stored via `redux-persist` that we need in order // contain messages stored via `redux-persist` that we need in order
// to correctly populate the SQLite `messages` table in migration 31 // to correctly populate the SQLite `messages` table in migration 31
// (https://phab.comm.dev/D2600). // (https://phab.comm.dev/D2600).
// //
// However, because `messageStoreMessagesBlocklistTransform` modifies // However, because `messageStoreMessagesBlocklistTransform` modifies
// `messageStore` before migrations are run, we need to make sure we aren't // `messageStore` before migrations are run, we need to make sure we aren't
// inadvertently clearing `messageStore.messages` (by setting to {}) before // inadvertently clearing `messageStore.messages` (by setting to {}) before
// messages are stored in SQLite (https://linear.app/comm/issue/ENG-2377). // messages are stored in SQLite (https://linear.app/comm/issue/ENG-2377).
return { ...messageStore, threads, messages: messageStore.messages ?? {} }; return {
...state,
threads: state.threads ?? {},
messages: state.messages ?? {},
};
}, },
{ whitelist: ['messageStore'] }, { whitelist: ['messageStore'] },
); );
type PersistedReportStore = $Diff< type PersistedReportStore = $Diff<
ReportStore, ReportStore,
{ +queuedReports: $ReadOnlyArray<ClientReportCreationRequest> }, { +queuedReports: $ReadOnlyArray<ClientReportCreationRequest> },
>; >;
▲ Show 20 Lines • Show All 70 Lines • ▼ Show 20 Lines
}; };
const codeVersion: number = commCoreModule.getCodeVersion(); const codeVersion: number = commCoreModule.getCodeVersion();
// This local exists to avoid a circular dependency where redux-setup needs to // This local exists to avoid a circular dependency where redux-setup needs to
// import all the navigation and screen stuff, but some of those screens want to // import all the navigation and screen stuff, but some of those screens want to
// access the persistor to purge its state. // access the persistor to purge its state.
let storedPersistor = null; let storedPersistor = null;
atulUnsubmitted
Not Done
Inline Actions

Is your editor auto formatter maybe adding these newlines? Feel free to disregard if intentional, but noticed these D9682 as well.

atul: Is your editor auto formatter maybe adding these newlines? Feel free to disregard if…
kamilAuthorUnsubmitted
Done
Inline Actions

looks like it's my IDE fault, it is adding this while committing, I'll take a look

kamil: looks like it's my IDE fault, it is adding this while committing, I'll take a look
atulUnsubmitted
Not Done
Inline Actions

Sweet, thanks for taking a look!

Not a huge deal regardless, guess it clutters up git blame history a bit.. but not end of the world

atul: Sweet, thanks for taking a look! Not a huge deal regardless, guess it clutters up `git blame`…
function setPersistor(persistor: *) { function setPersistor(persistor: *) {
storedPersistor = persistor; storedPersistor = persistor;
} }
function getPersistor(): empty { function getPersistor(): empty {
invariant(storedPersistor, 'should be set'); invariant(storedPersistor, 'should be set');
return storedPersistor; return storedPersistor;
} }
export { persistConfig, codeVersion, setPersistor, getPersistor }; export { persistConfig, codeVersion, setPersistor, getPersistor };