-
Notifications
You must be signed in to change notification settings - Fork 25
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
Subscription Authentication Security #9
Comments
Is there a workaround available to stop the publication from server side? Imagine a user can stream changes for |
@smeijer not sure what you mean, I think that instead of killing the stream, a solution would be not to send them at all based on your criteria ? |
If I understand correct; you're suggesting to fix it in a resolver? Let's take this subscription: Subscription: {
messages: {
resolve: payload => payload,
subscribe(_, { groupId }, { userId, db }, ast) {
if (!userId) {
throw new Error('unauthorized');
}
// check if user has access to group
const group = db.groups.findOne({
_id: groupId,
members: userId,
});
if (!group) {
throw new Error('unauthorized');
}
const observable = db.messages.find({
groupId,
});
return asyncIterator(observable);
},
},
}, The users receives new messages for eternity. But how do I stop the pub/sub when he's being kicked out of the group? It's of course possible to do a db check in the resolver before returning the document. But is that what we want here? And also; if I do add a check in the resolver, how do we then end the thing? Once the The Now I think about it, we could use the Subscription: {
messages: {
resolve: (payload, args, { userId, db }, info) => {
const group = db.groups.findOne({
_id: groupId,
members: userId,
});
if (!group) {
// end publication
throw new Error('unauthorized');
}
return payload;
},
subscribe(_, { groupId }, { userId, db }, ast) {
}
}
} I think my examples are exactly what you described in the opening post; so my question still stands. Is there currently a solution / work around to end the publication from the server side? |
@smeijer there must be a solution that would allow us to reset a certain connection based on the user's id. I don't know it yet I have to do digging. But this is very important for security. Meteor had it by default and we didn't care about it. If you find a way or some specs that would allow us to do that I'll do it, otherwise this will be in my queue and will be done in the next few weeks |
@theodorDiaconu, did you ever find a solution to this problem? I'm again working with graphql subscriptions after a while of absence and directly experienced this issue again. |
@smeijer it wasn't a priority for us currently. The solution is to somehow reset the websocket connection from server. |
We check the token only once when the websocket connects. But what if, in the meantime, the user gets deleted or something about him changes.
It's not enough only do run this check on every subscription request, but also have a sort of interval for this check, as a user may already have been subscribed and authorised. And if, for example, the token is no longer available we close down the connection.
The text was updated successfully, but these errors were encountered: