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.

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

rxdb-server replication endpoint user data is not refreshed for query modifier #6593

Open
HRM opened this issue Nov 16, 2024 · 6 comments
Open
Labels

Comments

@HRM
Copy link
Contributor

HRM commented Nov 16, 2024

Im using rxdb-server with the according replication plugin. An issue I noticed is when I change the auth data of a user, the data sent by the pull stream doesn't reflect the change. I found the source of the behaviour below.

rxdb-server\src\plugins\server\endpoint-replication.ts 179-235

this.server.adapter.get(this.server.serverApp, '/' + this.urlPath + '/pullStream', async (req, res) => {

            const authData = await getAuthDataByRequest<AuthType, any, any>(this.server, req, res);
            if (!authData) { return; }

            adapter.setSSEHeaders(res);
            const docDataMatcherStream = getDocAllowedMatcher(this, ensureNotFalsy(authData));
            const subscription = replicationHandler.masterChangeStream$.pipe(
                mergeMap(async (changes) => {
                    /**
                     * The auth-data might be expired
                     * so we re-run the auth parsing each time
                     * before emitting an event.
                     */
                    let authData: RxServerAuthData<AuthType>;
                    try {
                        authData = await server.authHandler(adapter.getRequestHeaders(req));
                    } catch (err) {
                        adapter.closeConnection(res, 401, 'Unauthorized');
                        return null;
                    }

                    if (changes === 'RESYNC') {
                        return changes;
                    } else {
                        const useDocs = changes.documents.filter(d => docDataMatcherStream(d as any));
                        return {
                            documents: useDocs,
                            checkpoint: changes.checkpoint
                        };
                    }
                }),
                filter(f => f !== null && (f === 'RESYNC' || f.documents.length > 0))
            ).subscribe(filteredAndModified => {
                if (filteredAndModified === 'RESYNC') {
                    adapter.responseWrite(res, 'data: ' + JSON.stringify(filteredAndModified) + '\n\n');
                } else {
                    const responseDocuments = ensureNotFalsy(filteredAndModified).documents.map(d => removeServerOnlyFields(d as any));
                    adapter.responseWrite(
                        res,
                        'data: ' + JSON.stringify({
                            documents: responseDocuments,
                            checkpoint: ensureNotFalsy(filteredAndModified).checkpoint
                        }) + '\n\n'
                    );
                }

            });

So, we create the docDataMatcherStream at the beginning of the event stream and we don't change it till the request closes. My question would be: Are there any reasons why the docDataMatcherStream is not updated? We retrieve the authData for every change anyway.

If updateing the docDataMatcherStream is a fine approach and the behaviour wouldn't contradict anything, I would create a fix for this issue.

@pubkey
Copy link
Owner

pubkey commented Nov 17, 2024

We retrieve the authData for every change anyway.

I do not understand. The authData is send to the SSE endpoint once, it will not be send when changed or does it?

@HRM
Copy link
Contributor Author

HRM commented Nov 18, 2024

under the comment "The auth-data might be expired..." the auth data is reloaded (with the same request data). If I understand it correctly it is reloaded every time, we receive a change on masterChangeStream$. We reload it to see if the request is still valid. My problem is that docDataMatcherStream is not updated, so if we have a more granular access change, like the user gets ccess to new objects, the pull stream won't reflect this new state.

@pubkey
Copy link
Owner

pubkey commented Nov 18, 2024

No, this will only account for ttls of the auth data. It will not magically get new updated auth data from the client.
If your auth changes, just restart the replication.

@HRM
Copy link
Contributor Author

HRM commented Nov 18, 2024

Yeah sorry by the way I didn't open this as an issue, it was sad that before contributing feature changes, the contributor should open an issue for it. It just seems to me that recomputing docDataMatcherStream is a low hanging fruit as we already recompute the auth data required for it. This wouldn't allow the automatic handling of auth data change of course, it would only allow to extend the auth data in some scenarios (I'm in a scenario where it would help). Maybe It would also match a bit better how checkpoint iteration reacts to auth data change, as the next cycle there is done with the new auth data if it changes. But as another point of view, it seems to me that changing auth data is a bit of an anti-pattern, only using an id as auth data and denormalizing any other access to the stored objects is the correct way to go, and that way it doesn't necessarily makes sense to change one undefined behaviour to another.

@pubkey
Copy link
Owner

pubkey commented Nov 20, 2024

Computing the auth data is easy, so we do that. SEE are one-directional things, we do not have client-side-updated authData without manually sending it again or reopening the stream.

Copy link

stale bot commented Nov 27, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed soon. Please update it or it may be closed to keep our repository organized. The best way is to add some more information or make a pull request with a test case. Also you might get help in fixing it at the RxDB Community Chat If you know you will continue working on this, just write any message to the issue (like "ping") to remove the stale tag.

@stale stale bot added the stale label Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants