search

okTurtles / group-income

A decentralized and private (end-to-end encrypted) financial safety net for you and your friends.
https://groupincome.org
GNU Affero General Public License v3.0
330 stars 43 forks source link

Remove the password field in UI when leaving group #1909

Open taoeffect opened 5 months ago

taoeffect commented 5 months ago

Problem

Currently when a user leaves a group, the password field isn't actually verified.

Solution

Verify the password.

Replace the the password field with a username field (see discussion below). I.e. prompt them to enter their own username.

corrideat commented 3 weeks ago

The issue here is that the password doesn't need to be verified since it's not required for the leave action (in theory it could be required, but we need members to be able to remove other members without entering any password, such as when someone is removed from a group).

I'd therefore say that either we use the password (which right now we don't, and I'm not convinced it's entirely warranted) or we remove the password field. I feel that prompts for sensitive information, such as passwords, should be kept to a minimum anyhow to reduce the various risks of compromise.

About how the password would be used:

  1. Figure out a way to do more granular permissions to let the identity CSK be used for everything except the leave action when we're the ones leaving. I think we might be able to use the process or validate functions for this.
  2. Add the IPK to the group contract as a foreign key.
  3. When leaving a group, use the IPK to sign that action (this is the step that'd require using a password).

To reiterate, I'm not sure requiring a password for this is a good idea, but I'd only do it if there's a technical need for a password rather than exclusively as an UI feature with the password being otherwise unused.

taoeffect commented 3 weeks ago

Figure out a way to do more granular permissions to let the identity CSK be used for everything except the leave action when we're the ones leaving. I think we might be able to use the process or validate functions for this.

Sounds painful, let's avoid this.

Add the IPK to the group contract as a foreign key.

Now the group contract is tracking everyone's IPK? Also sounds like unnecessary complexity.

To reiterate, I'm not sure requiring a password for this is a good idea, but I'd only do it if there's a technical need for a password rather than exclusively as an UI feature with the password being otherwise unused.

It sounds like the simple solution here is to simply remove the password field. I'll update the Solution of the OP.

corrideat commented 3 weeks ago

Sounds painful, let's avoid this.

Although I've stated that I don't think we should implement this feature (and we seem to agree), attempting it would be a good exercise to see how granular permissions like these can be implemented.

Now the group contract is tracking everyone's IPK? Also sounds like unnecessary complexity.

It's only tracking CSKs (and PEKs to some extent). Implementing this properly would require also tracking IEKs (on the flip side, it shouldn't change too often)

It sounds like the simple solution here is to simply remove the password field. I'll update the Solution of the OP.

Agreed.