-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
Refactor settings controllers #14767
Conversation
Gargron
commented
Sep 9, 2020
•
edited
edited
- Disallow suspended accounts from revoking sessions and apps
- Allow suspended accounts to access exports
52ffe13
to
7d4b7ad
Compare
7d4b7ad
to
603d633
Compare
Why tough? If you are suspended you should still be allowed to revoke access from everything. |
If you are suspended, we retain some data to prevent ban evasions, e.g. you can’t sign up with the identical email address anymore. IPs and used apps may provide further insight into finding alternative accounts, so it doesn’t make sense that the suspended user can simply clean up all the tracks. |
It's the part after the + filtered? IPs are most of the time pretty useless. Mine for example changes every day and I can easily switch it.
If my account got suspended by action taken on another device which maybe got stolen or the session cookie was phished etc. I can't revoke that device. I should be able to revoke those sessions without cleaning up all my tracks. Also revoking or disabling 2FA does not clean up any track or provides any useful information to anyone. It is just a measure which maybe locks people out of their account without any real advantage. |
Why should you be able to change 2fa settings if you’re not allowed to change your password or email? It makes more sense to me to keep it consistent: suspended means the account’s done, no further changes. |
Before I actually review: I see no issue with being able to change 2FA settings or password when suspended. It would also be good to be able to revoke active sessions and change e-mail address, but I also understand how this is tied with the anti-abuse features so I think that's an acceptable compromise. |
app/controllers/settings/two_factor_authentication/confirmations_controller.rb
Outdated
Show resolved
Hide resolved
app/controllers/settings/two_factor_authentication/otp_authentication_controller.rb
Outdated
Show resolved
Hide resolved
app/controllers/settings/two_factor_authentication/recovery_codes_controller.rb
Outdated
Show resolved
Hide resolved
app/controllers/settings/two_factor_authentication/webauthn_credentials_controller.rb
Outdated
Show resolved
Hide resolved
app/controllers/settings/two_factor_authentication_methods_controller.rb
Outdated
Show resolved
Hide resolved
- Disallow suspended accounts from revoking sessions and apps - Allow suspended accounts to access exports
603d633
to
a055990
Compare
- Disallow suspended accounts from revoking sessions and apps - Allow suspended accounts to access exports