Azure E2E: Add read client signal and audience tests #20042
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
dependencies
scottn12
previously approved these changes
Mar 11, 2024
azure/packages/test/end-to-end-tests/src/test/AzureTokenFactory.ts
Outdated
Show resolved
Hide resolved
|
Need to see if I can "force read" via server changes based on token scopes rather than the client config change. |
…/signal-audience-reader-tests
znewton
added a commit
that referenced
this pull request
Mar 27, 2024
## Description
When a client joins with AzureClient, it will always try to join in
`write` even if it only has `read` permissions according to its token
claims. On the server (Nexus Lambda), we handle that correctly by
creating a read-scoped connection. However, the server would incorrectly
handle client details and send connection info like this:
```js
socket.emit("connect_document_success", { /* ... */, mode: "read");
// ...
socket.emitToRoom(
"tenantId/documentId",
"signal",
{
/* ... join signal message ...*/,
client: { /* ... */, mode: "write"
});
```
I'm not sure about the specifics of what's happening on the client, but
when testing read client Audience and Signals usage in #20042, I
realized that the read client audience is not working appropriately
without this change. My understanding is it's because Audience is trying
to use Quorum (Join Ops) instead of Join Signals because the Join Signal
client details said `{ mode: "write" }` even though Connection details
said `{ mode: "read" }`
github-actions
bot
removed
the
area: server
znewton
added a commit
to znewton/FluidFramework
that referenced
this pull request
May 16, 2024
## Description
When a client joins with AzureClient, it will always try to join in
`write` even if it only has `read` permissions according to its token
claims. On the server (Nexus Lambda), we handle that correctly by
creating a read-scoped connection. However, the server would incorrectly
handle client details and send connection info like this:
```js
socket.emit("connect_document_success", { /* ... */, mode: "read");
// ...
socket.emitToRoom(
"tenantId/documentId",
"signal",
{
/* ... join signal message ...*/,
client: { /* ... */, mode: "write"
});
```
I'm not sure about the specifics of what's happening on the client, but
when testing read client Audience and Signals usage in microsoft#20042, I
realized that the read client audience is not working appropriately
without this change. My understanding is it's because Audience is trying
to use Quorum (Join Ops) instead of Join Signals because the Join Signal
client details said `{ mode: "write" }` even though Connection details
said `{ mode: "read" }`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Azure Fluid Relay is planning work to begin officially supporting Signals. Part of that work is also making sure that Audience management works correctly for Read-only clients.
This PR adds 4 new Azure E2E tests.
Also, it adds a fix in Nexus Lambda to update the Connection messages Clientmodetoread/writedepending on Claims scopes.Reviewer Guidance
A couple of the new tests involve semi-complicated (in a testing sense) scenarios. I know tests are supposed to test 1 thing and then have another test to test another thing, however, each of these tests creates a new Container, so adding too many tests can become costly when running frequently against production environments. Open to feedback on alternative ways to handle this self-imposed limitation.