[wp-meta] [Making WordPress.org] #6509: Forum profile: "Also viewing" checkbox sometimes gets unchecked

[Making WordPress.org]

#6509: Forum profile: "Also viewing" checkbox sometimes gets unchecked
----------------------------+----------------------
 Reporter:  zoonini         |       Owner:  Clorith
     Type:  defect (bug)    |      Status:  closed
 Priority:  normal          |   Milestone:
Component:  Support Forums  |  Resolution:  fixed
 Keywords:                  |
----------------------------+----------------------

Comment (by Clorith):

 Initially posted this on Slack to the same remark, but responding here for
 the sake of history :)

 So the reason it's happening is because these profile field updates are
 hooking into `bbp_profile_update`, which is just an extension that runs
 through cores own `profile_update` action.

 The latter of these actions is triggered any time a user is inserted, or
 updated, and by extension so is the bbPress one then. By having user
 settings tied to a named checkbox's existence in a request means that the
 code basically says "if this isn't present, the user decided not to tick
 the box", which isn't true when the user can be updated from "other"
 places.

 In the most recent case, the Two-Factor plugin, as used under the wporg-
 two-factor one, will update a user and trigger this cascade of actions,
 and because we (previously) had no safeguards on these specific form
 fields, they would be treated as "unticked by the user", and be wiped.

 Some alternative approaches though, if you'd rather, could for example be;
 - add a nonce for each field, but don't throw a nonce-error if it doesn't
 pass, just return early.
 - Convert checkboxes to select boxes with yes/no type options and a switch
 statement in the form handler
 - Specifically check the referrer URL of a post request

 I chose not to go the nonce route, since that would add 2 DB entries per
 user for nonce-references, which seemed overkill.
 Likewise, the idea of select boxes for on/off statements is a bit
 backwards, and referrals are unreliable.

 So with that in mind, the implementation of a `can_*` field entry, that
 gets posted alongside the value we check for, was the simplest one to both
 ensure these non-bbPress-core user settings we've implemented do not get
 wiped from unintentional "external" actions, in a period where users
 suddenly got a whiff of a new security feature that's been long requested
 and are likely to jump on :slightly_smiling_face: (and it's not just the
 two-factor bit that would potentially cause this, any action that triggers
 a user update within the w.org/forums subsite would have the potential to
 clear them previously)

-- 
Ticket URL: <https://meta.trac.wordpress.org/ticket/6509#comment:7>
Making WordPress.org <https://meta.trac.wordpress.org/>
Making WordPress.org