Make WordPress Core

Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#46349 closed enhancement (fixed)

Is your/this admin email still correct

Reported by: andraganescu's profile andraganescu Owned by: andraganescu's profile andraganescu
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: Site Health Keywords: has-patch needs-docs needs-codex has-screenshots has-dev-note
Focuses: ui, administration, ui-copy Cc:

Description

Use a reminder type of notification that checks with the users that some of their details in settings are still up to date.

Rationale:

In the recent discussions on #core-php about the WSOD recovery and the recovery email that should be sent, to announce that the site experienced a fatal error and that they might be locked out of their website's admin, some participants persistently raised the issue of the admin email being either one of:

  • outdated
  • set to a catch all email address which is never checked
  • set automatically by the host in the process of one-click-installs

Since the admin email is by all means the correct value to use when the system decides to send that email we need to make sure we do our best to keep it accurate and not a useless setting nobody cares for.

For now the whole discussion should be about the admin email setting, I was unable to find another candidate so I am unsure if this would require an extensibility API of some kind. However perhaps some plugins like the ones for 2FA could use it.

Solution

We could have a small notification that is triggered by either one of:

  • a certain amount of time since the last login
  • a certain amount of time since the last notification was displayed

This notification explains that some settings are important and need to be revised in order to ensure the security and well functioning of their site. Then it asks about the setting and if it is correct.

Similar approaches

Many current online apps use this style of notification to prompt the user into checking their email, phone number, secondary addresses, even credit card details. This helps prevent many unwanted issues. Of course now I was unable to find the exact screens I am talking about, but I am sure others have seen them :D

How it works

This can be either one or all of:

  • a top bar that leads to a screen where these options can be updated, least invasive
  • a section in the dashboard that does not disappear until it is confirmed or updated, medium invasive
  • a screen right after login that cannot be bypassed until it is confirmed or updated, hardcore! This screen only shows up if the logging in user has the required cap to edit the settings.

We could store the confirmation flag and date using the option API and use WP Cron to check these options once in a while. For the most invasive implementation option then the auth flow needs to be updated to check for the options.

Attachments (13)

5a85cd561df68_Screenshot1.PNG.412c73d5f803d3ed7ead3bb951810ae9.PNG (15.3 KB) - added by boemedia 6 years ago.
Example of email validation screen
Screenshot 2019-03-11 at 10.42.13.jpg (33.5 KB) - added by boemedia 6 years ago.
Example of a software update screen: skip/remind message
Screenshot 2019-03-27 at 21.58.23.png (111.6 KB) - added by andraganescu 6 years ago.
Intermission Form Example
admin_email_reminder.diff (8.2 KB) - added by andraganescu 5 years ago.
Patch that implements @boemedia 's flow, has missing actions on links.
46349.diff (7.2 KB) - added by andraganescu 5 years ago.
46349.1.diff (36.8 KB) - added by azaozz 5 years ago.
admin-email-verufy-wide.png (34.4 KB) - added by azaozz 5 years ago.
admin-email-verufy-narrow.png (25.9 KB) - added by azaozz 5 years ago.
46349.2.diff (41.4 KB) - added by azaozz 5 years ago.
46349.3.diff (42.3 KB) - added by azaozz 5 years ago.
46349.4.diff (3.8 KB) - added by afercia 5 years ago.
46349.5.diff (2.5 KB) - added by azaozz 5 years ago.
admin-email-verification.png (31.3 KB) - added by azaozz 5 years ago.

Download all attachments as: .zip

Change History (93)

This ticket was mentioned in Slack in #design by andraganescu. View the logs.


6 years ago

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


6 years ago

#3 @andraganescu
6 years ago

  • Summary changed from Is your this admin email still correct to Is your/this admin email still correct

#4 @andraganescu
6 years ago

I am currently implementing option 2 "a section in the dashboard that does not disappear until it is confirmed or updated, medium invasive", using the Dashboard Widgets API and provide a simple form in a widget where the user can confirm the email or change it. I will save as a site option the timeout so we don't check with the user again for X amount of time.

#5 @noisysocks
6 years ago

This sounds like a valuable addition to me :)

A dashboard widget aligns nicely with the UX of the Outdated PHP Version Warning that shipped in WordPress 5.1. Pinging @boemedia for her thoughts on design as she mentioned having some in Slack.

#6 @boemedia
6 years ago

Hi @andraganescu ,

First thing that comes to mind is: who do you want to show this notification to? I guess it makes sense to show it to all users with admin rights, not only to the person login in with the email that matches the admin email and not to users with other roles.

How I'd see it as least intrusive: After login as an admin I'd like to to see a subtle reminder:

The admin email for this website is xxx@…. Is this still correct?
Yes > don't show me this notification for the next xxx days/ don't show me this notification again (with warning?) and take me to wp-admin
No > change email (via link to settings). (and trigger something after changing this to send a notification after xx time again)

As for the dashboard widget, I'm not sure about that. The hierarchy in information on the dashboard is not very clear. I wonder how you're going to make this stand out on the page. Do you want to do a full width, like we did with the Gutenberg dashboard widget? Even then, I've found that people sort of ignored the info since it blends in with the rest. Also, as soon as people enter WP-Admin, they tend to look at the black menu on the left and proceed to whatever task they want to perform.

Is it an option to build it in in the login process, before showing WP-admin at all? As an extra screen after hitting 'login'? I see this is your third idea, maybe we could include a 'not now, remind me later' option to that screen (which I hit a lot when I'm asked for software updates...)

Regarding this, I can also imagine having a fallback email address registered, but maybe that should be a different ticket :)

I think we have to think about how dearly we want people to check on their admin email. How important is it? And how often do we think is necessary for them to confirm to keep it up to date?

@boemedia
6 years ago

Example of email validation screen

@boemedia
6 years ago

Example of a software update screen: skip/remind message

#7 @SergeyBiryukov
6 years ago

  • Component changed from Administration to Users
  • Focuses administration added

#8 @TimothyBlynJacobs
6 years ago

  • Keywords servehappy added

#9 @andraganescu
6 years ago

  • Owner set to andraganescu
  • Status changed from new to assigned

First, thank you @boemedia and sorry for the late reply. Excellent work on both planning the UX and the examples!

I am on the same boat with the following things:

  • indeed, the dashboard widgets tend to blend into each other, so it may be that we're not going to serve the purpose of this if the UI is easily ignored
  • even though I called it "hardcore" after reading your comment, the screen after login looks like a better idea to me too.
  • a "remind me later" option would also make it less annoying and easily skippable

I like the UX style of the first example, of course integrated in the WP UI, perhaps using the admin login surrounding elements?

Re. having a fallback email, you are correct, it should be a different ticket.

We need this feature to make sure that any communication we send to an admin address about website health, maybe updates, issues and so on, do reach an admin.

As for rules that govern this extra login screen:

  • the notification should be shown to _one_ admin only in one interval
  • only admins see it, people who have the right to change the Admin Email setting
  • timeline: default to once every six months, remind me later sets a smaller two week interval

I'll continue with switching from widget to extra login screen, we'll see how that "feels" once I have a patch.

@andraganescu
6 years ago

Intermission Form Example

#10 @andraganescu
6 years ago

In order to display the notification right after login I found two ways:

  1. either change the code and before redirecting stick another form into the page like in the Intermission Form Example example above

I don't like this approach mainly because it adds a loat of bloat code in wp-login.php which is already complex: another function to output another form and, plus, we need to pause the redirect flow with more globals.

On the other hand this is easier and faster to implement, I think.

  1. hook into login_redirect and update the redirect process like this:
  • login
  • login_redirect hook called
    • is the admin email recently verified ? (define an ADMIN_EMAIL_REFRESH_TIME in default_constants.php)
      • YES
        • return the normal redirect decided by the system
      • NO
        • return a redirect link to a special place in admin where there is a form to update
  • continue to redirect
  • if special admin place:
    • user updates the email?
      • YES
        • save the new admin email
        • set an admin_email_last_check option via the options API to current time
        • redirect to the original redirect
      • NO
        • set an admin_email_last_check option via the options API to current time - 1/2*ADMIN_EMAIL_REFRESH_TIME
        • redirect to the original redirect

I am working on option (2) but the UX is weird: the best place to update it is on the settings page.

We could direct the users straight to that long form and via some URI param modify the Email Address field to pop out in size and color, and also add a question to it.

Also, we could add a modal overlay with a Form like this in it, which right now is my favourite idea.

LMK if you have other thoughts :)

Last edited 6 years ago by andraganescu (previous) (diff)

#11 @noisysocks
6 years ago

Looking at wp-login.php:498, I see that we're already rendering different form fields depending on the value of the ?action query param. Could we add a ?action=verify query param to support email validation?

#12 @andraganescu
6 years ago

@noisysocks of course we can, it's just that there will me more cruft in there:

  • a new form (e.g. retrievepassword)
  • a new function to update the admin_email option
  • some new "if" statements in the default case of the switch $action block, because we can show this form only after the user is logged in and we know they have their capabilities set up

It's not hard as I said, I just feel that it will make complexity more complex haha and I don't like it. Hooking into login_redirect seems more elegant, albeit more work.

However @boemedia will soon have some time to give us an UX to follow, so that will count a lot in how to implement!

Last edited 6 years ago by andraganescu (previous) (diff)

#13 @andraganescu
6 years ago

  • Keywords has-patch dev-feedback needs-testing needs-design-feedback added

I have a PR here https://github.com/draganescu/wordpress-develop/pull/1 which implements the intermission form UX shown above.

@boemedia this still needs a good design approach, as I've only implemented a functional idea.

I would guess it is a safe approach b/c there is that filter there which is exactly for this kind of job. On the other hand the actual UI might not be the best since (as you can see in the PR's gif) there is a lot of information that needs to be communicated:

  • you should check to see if your admin email is correct
  • you should update your admin email
  • it is important b/c we use it to let you in if your site is broken
  • if you change it you will need to confirm the change via email

The way I did this is pretty basic, but I can implement in that page any number of ideas.

LMK your thoughts!

Last edited 6 years ago by andraganescu (previous) (diff)

This ticket was mentioned in Slack in #core-php by andraganescu. View the logs.


6 years ago

This ticket was mentioned in Slack in #design by boemedia. View the logs.


6 years ago

#16 @boemedia
5 years ago

I never managed to look at the patch (sorry, couldn't get it implemented. But I have been working on an idea for a user flow to change the general admin email.

I abandoned the idea of making it possible to change the emailadress in a separate screen. Instead, I thought it would be better for users to take them to the settings screen for making changes, so they'd recognise it for future reference.
Also, my thinking was, to cause less confusion for users with admin rights who are not registered as the general admin email. We need some additional information pages (on .org?) to help clarify:

a) the purpose of a working general admin email
b) that the admin email given can be different from their own login email

After a user with admin rights hits login, there are 3 possible scenario's:
1) the registered email is verified as correct: the user is taken to the dashboard with a notification 'thank you for verifying'
2) the registered email needs to be changed: the user is taken to the settings page
3) the user hits 'remind me later': the user is taken to the dashboard with 'we'll remind you again in x weeks'

Design wise, I ran into a few things:

  • there's not really a design pattern for these info screens ahead of WP-admin. I didn't think the login screen would be a good basis to work from, so I went with a screen from the install sequence instead.
  • I wasn't sure about the colours for the notification bar to use in this case
  • the dashboard view in Figma is broken, maybe because of importing from Sketch. So please ignore some text and titles being in the wrong place.

I'd love some feedback on the approach/user flow, as well as some guidance on design standards for WordPress, as I wasn't sure if I used the right components at some points.

Technically, I'm not aware of any restrictions regarding my flow, so if a different approach is necessary because of those, please let me know. I'm happy to iterate or work on a different version.

Please take a look at the prototype here: https://www.figma.com/proto/WB24a8gm07lJKqLmVr01ZttB/Change-admin-email-v1?node-id=1%3A2&scaling=min-zoom

All 3 scenario's work. You can move back to the login screen by hitting 'R'.

Looking forward to reading your feedback in this ticket, or if you don't have a .org account, please comment in the Slack design channel: https://wordpress.slack.com/messages/design/

This ticket was mentioned in Slack in #design by boemedia. View the logs.


5 years ago

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


5 years ago

#19 @lessbloat
5 years ago

Hi @boemedia! I recorded a few thoughts for you here: https://cl.ly/8f141b50b65c This is all fairly subjective feedback, so feel free to take/leave any/all of it.

#20 @lessbloat
5 years ago

Okay, take two, since I forgot to hit the audio option before I recorded my first video. :-P

https://cl.ly/31856dbd2324

#21 @karmatosed
5 years ago

  • Keywords needs-design-feedback removed

#22 @karmatosed
5 years ago

  • Keywords ux-feedback removed

#23 @karmatosed
5 years ago

  • Keywords needs-design removed

As this has a design, going to remove the needs design keyword for now. Thanks for moving this on with a design @boemedia.

#24 @andraganescu
5 years ago

Added a new PR https://github.com/draganescu/wordpress-develop/pull/52 which implements the design from @boemedia .
It is still WIP because:

  • I am unsure what happens when I click on "Why is this important" and "Learn more" in the confirmation screen.
  • I am still working on highlighting the Admin Email row on the settings screen
  • I still have to add a message in the dashboard screen if the user confirms or clicks "Remind me later"

#25 @noisysocks
5 years ago

  • Milestone changed from Awaiting Review to 5.3
  • Type changed from feature request to enhancement

#26 @noisysocks
5 years ago

  • Focuses ui added

@andraganescu
5 years ago

Patch that implements @boemedia 's flow, has missing actions on links.

#27 @boemedia
5 years ago

Thanks for creating the patch! For the links to "Why is this important" and "Learn more" in the confirmation screen, I'd like to ping @joostdevalk here and ask for marketing support on this. Ideally, I think these could be two pages written on .org (or one page with two sections) about the general admin email. Which is super useful not only in this case, but also when people set up a new WP Install.

#28 @boemedia
5 years ago

TL;DR

#29 @joostdevalk
5 years ago

@boemedia I think those pages should be on helphub, @clorith and team can help you with that!

#30 @Clorith
5 years ago

Yup, quite happy to help get any user-facing documentation put in the right place, feel free to reach out and I'll facilitate to get any copywriters added where they're needed to make this happen.

#31 @azaozz
5 years ago

Looking at admin_email_reminder.diff, seems to work pretty well here. Got couple of suggestions :)

Generally PHP constants are not great for things that some users may want to change. If we want ADMIN_EMAIL_MAX_AGE to be changeable, how about we set it with a filter? Perhaps something like:

$admin_email_max_age = apply_filters( 'admin_email_max_age', 180 * DAY_IN_SECONDS );

It's used in just one place so this would work well.

Thinking it's not a good idea to do delete_option( 'admin_email_lifespan' ); at the top of wp-login.php. If the option has been deleted, it will go to the DB to try and get it again, and that file may be a subject of brute force login attempts.

Maybe we can keep the option and set it to some value (this is actually the "recommended way" to use options, make sure they exist at all times and don't change the value (write to the DB) for non-authenticated users). That will also remove the need to delete it.

Also wondering if plugins should be able to remove the whole check. This is a good security feature. If we add a filter for ADMIN_EMAIL_MAX_AGE, perhaps this should be hard-coded, not added with a filter on 'login_redirect'.

The rest is minor/nitpicks :) Generally HTML tags should be avoided in translatable strings. Think there was something in the coding standards against nested single if (but may be mixing that with the JS linting/coding standards).

Last edited 5 years ago by azaozz (previous) (diff)

#32 @spacedmonkey
5 years ago

  • Component changed from Users to Site Health

#33 @andraganescu
5 years ago

  • Keywords needs-docs needs-codex added; 2nd-opinion servehappy dev-feedback removed

@azaozz I have updated the diff with your observations except for removing those strong tags in the text, as doing so would make a sentence that has sense broken and translators would need notes and ... too much hassle for a tag, I'd say. Let me know if it looks good/ works for you :) And thanks for reviewing.

So the current status is this:

  • the patch adds the admin email freshness reminder as a feature, it can be disabled by filtering out admin_email_max_age
  • I still don't have links or content for "Why is this important" and "Learn more"

@Clorith are you still able to help with those docs?

@andraganescu
5 years ago

@azaozz
5 years ago

#34 @azaozz
5 years ago

In 46349.1.diff:

  • Based on 46349.diff.
  • Add a nonce check when saving the options for the next admin email verification.
  • Remove the wp_confirm_admin_email() function from user.php. The functionality is moved to wp-login.php, when not outputting to the "interim login" iframe.
  • Add setting of the new option on WP upgrade. It's good to set it for cases where the new screen is not shown for a while after upgrading. For example an admin doesn't log in, only editors and authors do (checking an option that is not set requires one more trip to the DB).
  • Come cleanup and small fixes (white space, printf, "translatable" URL to help page, etc.).

Still to-do: need to make the handbook page with explanation about the admin email.

The patch also contains unrelated WPCS fixes (wp-login.php needed quite a few).

Version 0, edited 5 years ago by azaozz (next)

#35 @azaozz
5 years ago

  • Focuses ui-copy added

Thinking this is working very well. Next step would be a design review/confirmation and trying to make the text/copy as good as possible :)

Last edited 5 years ago by azaozz (previous) (diff)

#36 @andraganescu
5 years ago

Thank you @azaozz this is great, looking forward to wrap this up!

@azaozz
5 years ago

#37 @azaozz
5 years ago

In 46349.2.diff:

  • Fix the nonce check for the "Remind me later" link.
  • Few more WPCS fixes.

Also added sample URL for the "Why is this important?" and "Learn more" links: https://wordpress.org/support/article/administration-email/. It is, of course, still a 404 :)

This ticket was mentioned in Slack in #docs by azaozz. View the logs.


5 years ago

This ticket was mentioned in Slack in #polyglots by azaozz. View the logs.


5 years ago

@azaozz
5 years ago

#40 @azaozz
5 years ago

In 46349.3.diff:

  • Rename Email Address to Administration Email Address in options-general to indicate it is distinctly different than the user's personal email.
  • Update the help/more info URL.
  • A bit more cleanup.

#41 @azaozz
5 years ago

Actually this should be ready. Going to commit for easier testing.

Still pending: design review and copy/wording review, but can adjust when done.

#42 @azaozz
5 years ago

In 45757:

Add admin email verification screen. Shown every six months after an admin has logged in.
Also includes WPCS fixes for wp-login.php.

Props andraganescu, boemedia, lessbloat, azaozz.
See #46349.

#43 @azaozz
5 years ago

In 45758:

Remove trailing white space in upgrade.php after [45757].

See #46349.

This ticket was mentioned in Slack in #design by azaozz. View the logs.


5 years ago

#45 follow-up: @afercia
5 years ago

Looks great! Just a couple things from an a11y and semantics perspective:

  • Why is this important? and Learn more point to the same link. Is this really necessary? Can the wording be reviewed a bit and use just one link?
  • Learn more: the link text is not clear when read out of context, same as the "read more" links: it should provide more context, also depending on previous point

Aside: the double h1 is an issue across most of the install / setup / login screens and I'd tend to think it should be addressed separately.

#46 in reply to: ↑ 45 @azaozz
5 years ago

Replying to afercia:

  • Why is this important? and Learn more point to the same link. Is this really necessary?

Can probably remove the "Why is this important?", but... Thinking it is a good place to link to the help about the importance of keeping the admin email working/current. Guessing that by now many people have seen similar requests for email confirmation in other places/services on the net, but may still be good to explain? :)

  • Learn more: the link text is not clear when read out of context

Yeah, it is part of the paragraph there, cannot be read out of context. Suggestions for changes welcome!

Aside: the double h1 is an issue across most of the install / setup / login screens...

Was wondering about that. Should we make the "Administration email verification" heading h2? Or rather, is it OK when the h1 is just a logo/image and has no text?

@afercia
5 years ago

#47 @afercia
5 years ago

I'm still not sure to understand why there should be 2 links pointing at the same resource :)

  • the two links have different text and same destination: this is also a WCAG violation
  • boht links lack context, as also "Why is this important?" isn't clear when read out of context

If it was up to me, I'd do it as in 46349.4.diff. Screenshot:

http://cldup.com/8CLcRcnEry.png

Not saying that's necessarily the best solution but at least there's just one link. Also, the text learn more about ... is pretty common in core. Found 31 occurrences.

Other things in the patch (some of them should be implemented regardless of the text changes);

  • changed the document title to expand admin to administration
  • links with target="_blank" need the (opens in a new tab) visually hidden text: added
  • the screen-reader-text class requires common.css to be enqueued (unless we'd want to duplicate the rule somewhere but I wouldn't recommend it)
  • common.css also slightly changes the line-height of the H1
  • added some translators comments
  • CS for the whole file

Should we make the "Administration email verification" heading h2? Or rather, is it OK when the h1 is just a logo/image and has no text?

From a practical perspective, the main H1 and the overall headings hierarchy has more to do with the way users find information on a page.

I'd tend to think the logo shouldn't be a H1. Instead, the H1 should identify the main topic of the page. However, I'm not sure all the install/setup/login pages have text to identify the "main topic". Guess this should be reviewed across all these pages and addressed separately.

#48 @boemedia
5 years ago

‘I'm still not sure to understand why there should be 2 links pointing at the same resource :) ‘

@afercia - a quick response from my mobile phone while on a holiday. First of all, thanks for your valuable input. In my original prototype (see a few comment back), I had an idea for two different links. As I recall, one points to a bit of text that explains why having a working general admin email is important.

From my head, the other link explains the difference between the general admin email and the fact that this can be different from the login if someone is an admin but not noted as the general admin ( in case of multiple admins). I experience some of my clients bot knowing the difference.

Currently I cannot check where the links are pointing to. I could test the patch next Tuesday.

As for design review: already asked for this, see my comment before. This is based on the current setup screens. But I feel they are disconnected from tge rest of Wp and I’m lokking for time to redesign all of them.

#49 @afercia
5 years ago

@boemedia thanks for the added context! And for the dedication: commenting while on on a holiday :)

In the current patch both links point to https://wordpress.org/support/article/settings-general-screen/#email-address that's the reason for my concerns. Of course if they will point to two different resources things change and make sense. They will need just a bit more context. Thank you!

#50 @azaozz
5 years ago

I'm still not sure to understand why there should be 2 links pointing at the same resource

...

Of course if they will point to two different resources things change and make sense.

Both links point to the same page as that's the page with help about the admin email. Currently there is only one "section" about it, hence the same link. This can be split in two and the links can point to the two different parts. Then the "URL fragment" (part after the #) will be different.

Thinking that the original design with two links would be best here. There are two distinctly different parts to it: one about what the email is used for and another explaining that it can be different than the user's personal/login email, even for sites with just one user.

Going to ping the Docs team and ask for an update to that section, then change the URLs to match.

Also, the text learn more about ... is pretty common in core. Found 31 occurrences.

...Perhaps that can be replaced with "More info ->" or "Get help" or... something along these lines, but keeping it uniform may be better? :)

#51 follow-up: @garrett-eclipse
5 years ago

Hi @azaozz this prompt is really cool, I just encountered it on a fresh trunk install.

One question: Should I be presented this immediately upon install of WordPress?

Currently after the screen you input your admin email and run setup you get presented with this message which was a little confusing as I just set that admin email during the install. Would it make more sense to only present it after a specific timeframe?

If it is expected to present immediately after install then maybe the verbiage of 'is still correct' can drop the 'still' as that indicates a period of time has passed.

Hope I made sense there, thanks for the new feature.
Cheers

#52 in reply to: ↑ 51 @azaozz
5 years ago

Replying to garrett-eclipse:

Ah, good catch, thanks!

Don't think it should show on the first logging in after installing. Patch coming up :)

@azaozz
5 years ago

#53 @azaozz
5 years ago

In 46349.5.diff:

  • Add the admin_email_lifespan option when installing. The email verification screen will show after six months.
  • Reset the same option when upgrading and the user doing the DB upgrade is not an admin. This will ensure the email verification is shown next time an admin logs in.
  • Use site_url() instead of network_site_url() for the form action. The latter seems needed only for password reset.

#54 @Clorith
5 years ago

Sorry for the late followup (I've also been on vacation :) )

I also don't quite see why two links are needed, I think one location that properly describes the purpose of the admin email is the best approach, and avoids any confusion. We can add in a short line about this not necessarily being your regular login.

I'll see about expanding the text for that particular article (the one already linked to), to make it a bit more readable, and add more details (I noticed it currently just really relates to comment moderation).

#55 @nrqsnchz
5 years ago

Adding some feedback from a user who wasn't able to comment on this ticket (https://make.wordpress.org/design/2019/08/07/design-meeting-notes-for-august-7-2019/#comment-25917)

"I’m just a user, interested in the admin email confirmation because my company has a need for a similar sort of verification, so following the WP decisions about it is providing some helpful perspective. I had an idea and couldn’t comment on the ticket so I figured I’d drop a reply here in case anyone sees it.

I presume (or would hope, at least) that WP has integrated email tracking and you’d be able to tell whether any email to the admin address got opened or not. You could set the 6-month verification timeout based on the last open from that address, rather than arbitrarily every 6 calendar months. As a user, I’d be mildly annoyed to be asked to confirm an address that “you should know” I’m actively using, so this approach could maybe help minimize that."

#56 follow-up: @desrosj
5 years ago

  • Keywords needs-dev-note added

First, I love this! Second, I came to report the same as @garrett-eclipse above. 46349.5.diff looks good to me, but I think the interval should also be passed through the admin_email_check_interval filter so sites and plugins can fully control the frequency from the beginning.

This would also benefit from a dev note, even if short. Marking for one.

#57 @Kelderic
5 years ago

It might be a bit late in the process to point this out, but we are using the term "email" when we are actually referring to "email address". person@website.com is an "email address", which can be sent an "email".

Last edited 5 years ago by Kelderic (previous) (diff)

#58 in reply to: ↑ 56 @azaozz
5 years ago

Replying to desrosj:

Thanks for testing :)

...the interval should also be passed through the admin_email_check_interval filter so sites and plugins can fully control the frequency from the beginning.

Plugins are (usually) not active right after installation when the default options are set. There is another way to override all defaults which I think is sufficient. Another way, of course, would be to change the option directly. That's "not recommended" but.... Pretty sure some will do.

Last edited 5 years ago by azaozz (previous) (diff)

#59 @azaozz
5 years ago

In 45788:

Admin email verification:

  • Add the admin_email_lifespan option when installing. Fixes a bug where the verification screen is shown right after installation.
  • Reset the same option when upgrading and the user doing the DB upgrade is not an admin. This will ensure the email verification is shown next time an admin logs in.
  • Use site_url() instead of network_site_url() for the form action. The latter seems needed only for password reset.

See #46349.

#60 @azaozz
5 years ago

In 45789:

Remove trailing white space in upgrade.php.

See #46349.

#61 @johnbillion
5 years ago

After [45788], does this mean an admin might see this message after every update? That could get annoying and I'm not sure it's necessary.

#62 follow-up: @birgire
5 years ago

1) I wonder if the h1 heading should be capitalized to:

<h1 class="admin-email__heading">
    <?php _e( 'Administration Email Verification' ); ?>
</h1>

2) Then there are existing (blue) primary submit buttons that are capitalized, like the Log In and Get New Password buttons, so should the same apply for the The email is correct button?

3) When I click on the Updatebutton, I'm taken to:

https://example.com/wp-admin/options-general.php?highlight=confirm_admin_email

but I'm not seeing the Administration Email Address part being highlighted.

What is the expected highlighting behavior here?

#63 in reply to: ↑ 62 @azaozz
5 years ago

Replying to johnbillion:

After [45788], does this mean an admin might see this message after every update?

Think no, they will see it only if upgrading from WP 5.2.x or earlier, $wp_current_db_version < 45744 in upgrade.php.

Replying to birgire:

All good questions :) Think @boemedia is currently away and will be able to do a review in a week or so.

What is the expected highlighting behavior here?

Don't think currently there is a "standard" way to highlight specific form field(s) in the settings but looks easy enough to add.

#64 @afercia
5 years ago

About title case: see this ongoing conversation on the Gutenberg GitHub repository https://github.com/WordPress/gutenberg/issues/16764 with some excellent research and background from @tinkerbelly (@sarahmonster on GitHub)

Last edited 5 years ago by afercia (previous) (diff)

#65 @desrosj
5 years ago

Related to the sentence vs title case: #47298.

#66 follow-up: @afercia
5 years ago

Looking a bit into this seems to me some of the things from comment:47 aren't implemented yet. Those are simple, quick, accessibility enhancements that would be great to have in :) They were also implemented in 46349.4.diff

Specifically:

  • changed the document title to expand admin to administration
  • links with target="_blank" need the (opens in a new tab) visually hidden text: added
  • the screen-reader-text class requires common.css to be enqueued (unless we'd want to duplicate the rule somewhere but I wouldn't recommend it)
  • common.css also slightly changes the line-height of the H1
  • added some translators comments (note: not sure if this has already been addressed)

/Cc @azaozz :)

#67 in reply to: ↑ 66 @azaozz
5 years ago

Replying to afercia:

Yep, going to add all of these. Was trying to give some more time to everybody to have a look and try it, both code and UX wise.

Not sure about including common.css on the login screen. Can bring back-compat problems with plugins. Probably better to just copy the screen-reader-text class.

#68 @azaozz
5 years ago

In 46203:

Admin email verification:

  • Fix wording of the HTML title.
  • Add hidden (opens in a new tab) to links that need it.
  • Add the screen-reader-text CSS class to login.css.
  • Add another translator comment.

Props afercia.
See #46349.

#69 follow-up: @johnbillion
5 years ago

  • Keywords needs-screenshots added

Can we get a screenshot of these latest changes?

#70 @johnbillion
5 years ago

  • Version 5.1 deleted

#71 @azaozz
5 years ago

In 46204:

Remove left-over debug code after [46203].

See #46349.

#72 in reply to: ↑ 69 @azaozz
5 years ago

Replying to johnbillion:

Sure. The only visible change is to remove the second (Learn more) link as it points to the same location and that is not accepted for accessibility. Otherwise it follows the original design.

#73 @ocean90
5 years ago

In 46229:

I18N: Remove HTML tags from translatable string in wp-login.php.

See #46349.

#74 @desrosj
5 years ago

Besides a dev note, is there anything else remaining that needs to be addressed in this ticket?

This ticket was mentioned in Slack in #core-site-health by afragen. View the logs.


5 years ago

#76 @desrosj
5 years ago

  • Keywords has-screenshots added; needs-testing needs-screenshots removed
  • Resolution set to fixed
  • Status changed from assigned to closed

With beta 1 happening momentarily, I'm going to close this out. The dev note can happen while this is closed, and bug tickets can be opened to follow up.

#77 @anonymized_11892634
5 years ago

Apologies if I have missed an answer to this in the thread above. What is the recommended way to disable this functionality? Set a very big int via the admin_email_check_interval filter?

#78 @lukefiretoss
5 years ago

Is there still going to be a post on WP repo core related to the filter in core with any filter use examples on it?

#79 @desrosj
5 years ago

@philclothier In the current state, disabling can be done by returning a "falsey" value to the admin_email_check_interval filter.

#48153 was opened to allow the required capability to be filtered, but it has shifted a bit into how to make the feature more flexible.

A dev note will be published as soon as a decision is made on #48153 and details are ironed out.

Note: See TracTickets for help on using tickets.