Skip to content
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

Show UI on front end #15

Closed
iandunn opened this issue Nov 14, 2022 · 16 comments
Closed

Show UI on front end #15

iandunn opened this issue Nov 14, 2022 · 16 comments
Assignees
Milestone

Comments

@iandunn
Copy link
Member

iandunn commented Nov 14, 2022

Part of #5

We know that we want to show the UI on the front end, but need to pick a location and render the settings there.

  1. https://wordpress.org/support/users/profile/edit/ this is where passwords are set, so it could make sense to group it with this. There's already a lot on that page, though.
  2. https://profiles.wordpress.org/profile/edit/group/1 - This makes less sense to me than the Support profile, but is another place where account settings live.
  3. login.w.org/settings (or something like that) - This doesn't make as much sense to me, since we already have 2 other profile screens, and this would add a 3rd.

Regardless of which domain it goes on, I think it'd be best to create a dedicated page, rather than appending it to an existing page. 2FA is a relatively complex thing to setup, so having it's own page would be a better UX IMO.

This depends on WordPress/two-factor#261. If we just built own custom UI that would be fragile, and upstream changes could silently introduce conflicts/vulnerabilities.

@iandunn iandunn added this to the MVP milestone Nov 14, 2022
@dd32
Copy link
Member

dd32 commented Nov 16, 2022

https://wordpress.org/support/users/profile/edit/

I think this is the current best place for it.

There's already a lot on that page, though.

I think it would make sense to split the "Account" subsection that has Email/Password onto a new tab, and list that along with 2FA on that page. Alternatively, I guess just starting off with 2FA on it's own dedicated tab would make sense for now.

https://profiles.wordpress.org/profile/edit/group/1

This is where I think it should live, although I admit it's not currently where it makes the most sense. The reason this isn't the de-facto login-management page is because previously it didn't have Memcache, so you couldn't change your password here. There's nothing stopping us removing Password/Email management from Support and moving it to Profiles now, Realistically, The Forum profile tab should not really need be used by regular users.

@pkevan
Copy link
Contributor

pkevan commented Nov 16, 2022

Agree that a dedicated page/tab would make sense for the password/2FA settings - the current password flow feels a little awkward but maybe something to improve at a later stage?

There is some good examples of either checkbox security (WP.com), and grouping of similar items (Github.com), but again perhaps something to push later.

@iandunn
Copy link
Member Author

iandunn commented Nov 16, 2022

@dd32 I assigned this to you because it looks like you've already got some code written, but feel free to un-assign if that's wrong. I just wanted to avoid someone else picking it up and duplicating efforts.

@dd32 dd32 removed their assignment Nov 17, 2022
@dd32
Copy link
Member

dd32 commented Nov 17, 2022

I've cleared the assignment; Only because I haven't written any code.

The integration into the bbPress user-edit screen is done by bbPress itself, as it displays user-profile page custom sections on the front-end. The two-factor plugin mostly works there, although not 100% as the plugin is tied to the notion it's on the wp-admin/profile.php screen.

@dd32
Copy link
Member

dd32 commented Nov 17, 2022

As noted, this depends upon WordPress/two-factor#261 but also, as noted in #5 I anticipate we'll need to customise whatever approach the two-factor plugin takes significantly for our own UI if we were to have reasonably good UX.

@iandunn
Copy link
Member Author

iandunn commented Nov 17, 2022

Ah, I didn't realize it was already doing that 👍🏻

IMO it'd be ideal if we can make most of the changes from #5 upstream, to benefit everyone and avoid having to maintain a forked experience. We'll probably want to do some customization no matter what though, even if it's just CSS to fit the Support theme.

@iandunn
Copy link
Member Author

iandunn commented Nov 17, 2022

Even if we don't have time to contribute the visual stuff upstream, at least having a stable API upstream for saving options etc would make it safe to customize the UI.

@iandunn
Copy link
Member Author

iandunn commented Nov 18, 2022

These should be updated as part of this issue.

// todo Change this to match the front-end URL once that's implemented.
return 'https://' . $primary_site->domain . trailingslashit( $primary_site->path ) . 'wp-admin/profile.php';

function render_enable_2fa_notice() : void {
// @todo change this to use front-end URL/styles when 2FA settings are moved there.
?>
<div class="notice notice-error">
<p>
<?php echo wp_kses_data( sprintf(
__(
'Your account requires two-factor authentication, which adds an extra layer of protection against hackers. You cannot make any changes to the site until you <a href="%s">enable it</a>.',

@iandunn
Copy link
Member Author

iandunn commented Nov 18, 2022

There's nothing stopping us removing Password/Email management from Support and moving it to Profiles now

I like that 👍🏻 Ideally we'd go farther and unify the two profiles, but only if we have time.

@dd32
Copy link
Member

dd32 commented Nov 24, 2022

There's nothing stopping us removing Password/Email management from Support and moving it to Profiles now

I like that 👍🏻 Ideally we'd go farther and unify the two profiles, but only if we have time.

As a follow up to this, I would like to say I was wrong.

BuddyPress has a UI for changing email/password but it's not viable for our usage at this point in time without a lot of work.
Adding 2FA options (sans Password) to a BuddyPress page is possible, but is better off kept on the support profile for the time being.

When we redesign the profiles, rebuilding it as a block theme, we'll likely remove a lot of the BuddyPress templating and build our own minimal UI that does what we need. At that point in time we can Unify profiles.

@tellyworth tellyworth mentioned this issue Nov 24, 2022
@pkevan
Copy link
Contributor

pkevan commented Nov 24, 2022

Started looking at this - it's not without it's challenges, and firstly I'm going to recreate a flow that might be appropriate within a sub-page in wp-admin, with the intent of making the methods of save etc callable. Taking inspiration from some work done a while ago in https://github.com/WordPress/two-factor/blob/add/totp-ajax/.

The more I look, the more complications I seem to find, which i'll note later.

@pkevan pkevan self-assigned this Nov 25, 2022
@iandunn
Copy link
Member Author

iandunn commented Nov 28, 2022

Does it look like something that could be broken into multiple issues that could be worked on in parallel? This is probably the biggest chunk of work left, so working in parallel would help us meet the deadline.

If you stub out the basics and merge that, then all 3 of us could be working on filling in the gaps and polishing different parts of it.

@pkevan
Copy link
Contributor

pkevan commented Nov 29, 2022

Does it look like something that could be broken into multiple issues that could be worked on in parallel?

Yes - let me check back in with where I got, then I'll start doing some committing and create some other issues.

Primarily I've been looking at the backup codes and adding some additional logic/functionality.

@iandunn
Copy link
Member Author

iandunn commented Dec 7, 2022

I created #26 for the task mentioned above about moving the Account settings to a separate screen.

@iandunn
Copy link
Member Author

iandunn commented Dec 14, 2022

Based on our other discussions, I committed stubs for the UI in d457c55 (related: a697745 and 77ddf2b). That will hopefully give us something tangible to start building on, but it's still rough so feel free to propose big changes.

I also created issues #27, #28, #29, #30, and #31, so that we can work on individual parts in parallel with a minimum of merge conflicts.

Is there anything left to do in this issue, or is it superseded by the other ones now?

@iandunn
Copy link
Member Author

iandunn commented Dec 20, 2022

Closing since it seems like this is done/superseded now, but feel free to reopen if I missed something.

@iandunn iandunn closed this as completed Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants