Should not access group after being leaved #1728
Conversation
…-group-unless-group-member-tries
…-group-unless-group-member-tries
There was a problem hiding this comment.
Could you test this PR and try it out?
The PR seems to fix the issue, but after the last step in #1716 I get this error:
Should I approve and merge anyway as-is?
| if (rootState.contracts[params.contractID]?.type !== 'gi.contracts/group') { | ||
| if (!rootState.contracts[params.contractID]) { | ||
| // NOTE: already kicked from the group by someone else | ||
| return |
There was a problem hiding this comment.
This return looks like it'd fix the issue
frontend/controller/actions/group.js
Outdated
| @@ -316,7 +316,9 @@ export default (sbp('sbp/selectors/register', { | |||
| // action if we haven't done so yet (because we were previously waiting for | |||
| // the keys), or (c) already a member and ready to interact with the group. | |||
| 'gi.actions/group/join': async function (params: $Exact<ChelKeyRequestParams> & { options?: { skipUsableKeysCheck?: boolean; skipInviteAccept?: boolean } }) { | |||
| sbp('okTurtles.data/set', 'JOINING_GROUP-' + params.contractID, true) | |||
| if (!params.options?.skipInviteAccept) { | |||
| sbp('okTurtles.data/set', 'JOINING_GROUP-' + params.contractID, true) | |||
There was a problem hiding this comment.
I need to test this, but I'd think it breaks waiting for an invite upon logging in (i.e., logging in before the keys have been received).
There was a problem hiding this comment.
'JOINING_GROUP-${contractID}' is being used inside the sideEffect functions of group contracts.
It should be true only when joining group, not when try to log in or to see if the user has key.
That's why I added if conditional statement.
frontend/controller/actions/group.js
Outdated
| @@ -524,6 +527,7 @@ export default (sbp('sbp/selectors/register', { | |||
| console.error('gi.actions/group/join failed!', e) | |||
| throw new GIErrorUIRuntimeError(L('Failed to join the group: {codeError}', { codeError: e.message })) | |||
| } finally { | |||
| sbp('okTurtles.data/set', 'JOINING_GROUP-' + params.contractID, false) | |||
There was a problem hiding this comment.
This function returning does not mean that the joining process is complete. It's complete when keys are received.
|
I was able to reproduce, and I now need to text this PR. However, I expect it'd break in the following case:
Personally, I'd go with something like this: diff --git a/frontend/controller/actions/group.js b/frontend/controller/actions/group.js
index bde4403e..65ecd0a6 100644
--- a/frontend/controller/actions/group.js
+++ b/frontend/controller/actions/group.js
@@ -513,6 +513,15 @@ export default (sbp('sbp/selectors/register', {
}
sbp('okTurtles.data/set', 'JOINING_GROUP-' + params.contractID, false)
+ // We don't have the group secret keys, we're not waiting for a key
+ // share and we're marked as having left. This means we synced the contract
+ // and we've been kicked out
+ } else if (!hasSecretKeys && !sbp('chelonia/contract/isWaitingForKeyShare', state) && state.profiles[username].departedDate) {
+ sbp('okTurtles.data/set', 'JOINING_GROUP-' + params.contractID, false)
+ // DO SOMETHING HERE
+ // SOMETHING MEANS: Separate the logic for leaving, so that we can do it
+ // even if it's not an action we're processing in contracts/group.js
+
// We have already sent a key request that hasn't been answered. We cannot
// do much at this point, so we do nothing.
// This could happen, for example, after logging in if we still haven't |
|
I've tested this PR, and it fails under simpler scenarios than I described. For example,
|
taoeffect
added
the
Note:Reassigned
|
Closing in favor of recent changes in |
Silver-IT
deleted the
1716-should-not-be-accessible-to-the-group-unless-group-member-tries
branch
Summary of Changes
JOINING_GROUP_CHAT#1766