User tests: Successful: Unsuccessful:
This information should not be displayed in the user profile. It is only relevant to display it in the edit form.
This PR adds a check for that and also removes code which will no longer be needed.
To test create a front end menu item to user profile and set to registered.
Log in to the site and select user profile
Now click on the edit profile button and make sure that the webauthn fields are still displayed there
@nikosdion as discussed
Status | New | ⇒ | Pending |
Category | ⇒ | Front End Plugins |
Labels |
Added:
?
|
if ($layout = $this->app->input->get('layout') !== 'edit')
I did try that originally but it gave an error
0 Call to a member function get() on null
I'm not sure ether but that means we no longer have status information on the overview page?
I dont understand your question?
You remove the information from the profile overview page, isn't this information useful?
Beside that I don't know if any form validation is effected if you only load the information in the edit
layout. Because that means after saving the information the form doesn't have the addition fields in the validation/save function (not sure if it's needed)
the information is only useful if it is complete
What you mean with "if it is complete" what's missing?
Accessibility Settings
Multi-Factor Authentication
and probably a few more things
ok make sense to remove information on a overview page instead of adding the missing parts.
Anywas does it still works? does it needs the form for validation or saving?
of course it still works - but you could of course verify it for yourself
of course it still works - but you could of course verify it for yourself
I not going to test this because I think it's wrong.
/me confused
you said it makes sense
you said it makes sense
sorry lost in translation, that was ironic... I think we shouldn't loose such important information like which authentications are configured.
@HLeithner This PR is not wrong, the way we're currently doing it is wrong.
Remember that per the discussion we had with @SniperSister we will move to a solution where Joomla will have multiple authentication mechanisms implemented as System plugins. It makes no sense for each and every plugin to render the number of authenticators / its status in a separate area in the profile overview page. With just two of these authenticators the overview become crowded, busy, and unreadable — I know that because I am using both WebAuthn and SocialLogin on my own blog.
It makes more sense NOT having this area rendered by each plugin.
In the future, when we implement the common API for all system plugin authenticators, we can allow the site integrator to choose if a summary of this information for all authenticators (authentication or system plugins) should be displayed in the profile summary.
Also, in case we eventually make the summary public (makes sense, that's a common requirement for sites which can currently only be implemented with very heavy third party extensions) it would also let us hide this privileged information from the public display more easily.
Think about all that. I am seeing the bigger picture and realise that showing the WebAuthn status in the profile summary has been a mistake. We were too narrow-minded when testing that PR two years ago.
Sorry I have not the same opinion @nikosdion especially if you have multiple authentication method active it's interesting which of them you have without having to click into the edit page. Maybe it's not useful to list all non used authenticators, but showing what I have is the use case for the overview page and not having all data available looks like an error.
About the public user profile, if we build such thing we have to remove everything except the "name" and the dates, everything else maybe irrelevant or problematic.
not having all data available looks like an error.
Which is why I created the issue on the other PR and was told to do this instead
@HLeithner The Joomla profile list page is an unmitigated clusterf..k from a UX point of view. An endless data vomit of useless details giving everyone a headache. There is no hierarchy, there's too much information and it's all a bloody goddamn mess!
Here's a much better layout and even this is too much information IMHO:
You might also notice using anything other than Joomla that the profile overview page is an overview of important information, not a data dump of all possible options.
Anyway, it was made clear in October 2015 that Joomla doesn't care about UX. You want a data vomit on a page, you'll get a data vomit on a page. I don't care. I am now doing template overrides because I got tired of my sites' users telling me the profile page is unusable before I started doing that.
That it looks like garbage is a different sorry and I think the same. This should be addressed but sadly not much frontend developer/designer are available to help Joomla! on this front.
But that doesn't mean someone who creates it's own template (or buys one) can't style it like they want. For myself (which is maybe irrelevant) I create only custom templates per customer and I override everything I use and need. When I don't have the information I can't make them pretty^^
Following your logic, it's a bug that we do not by default display the password hash in the profile overview page. Do you want me to make a PR for it?
Do you think it makes sense to be polemical?
love it when non-native english speakers use a word that I have to look up in a dictionary and it's absolutely perfect
@HLeithner Not more than it makes sense to have authentication information for WebAuthn in the profile overview. There is zero consistency in what goes there and what doesn't. As I said, it's currently designed as a data vomit. This is not me being belligerent, it's me telling you the obvious truth.
love it when non-native english speakers use a word that I have to look up in a dictionary and it's absolutely perfect
I love it when English people make snide remarks without understanding how accidentally ironic against them they truly are.
The word "polemical" is Greek. Root: πόλεμος (war). Original Greek word: πολεμικός (that related to war).
The more "English" word would be "belligerent" insofar that you consider wholesale adoption of the Latin root belli (war) as English, despite the fact that English proper is teutonic language. I guess the occupation of modern day England by the Roman Empire might be considered reason enough for the adoption of a foreign root in the same way that some modern Greek words have obvious Turkish (e.g. κότα vs the Greek όρνιθα) or Italian (e.g. πόρτα vs the Greek θύρα) origins.
Yeah, I am fun at parties.
In any case, you want the MFA info dumped on the page, you will get the MFA info dumped on the page the same way as the WebAuthn info: a long, impossible to understand string. You ask for consistency, I'll give you consistency.
it was not a snide remark. it was a genuine comment that I love it when I learn a new word etc (and yes I did see that it was a word of greek origin when I looked it up in a dictionary)
It sounded very much like a snide remark. It happens when communicating over text, I guess.
My favourite source for vocabulary enrichment is reading English authors' books. Unlike American authors they tend to use more obscure words and complex syntax. I like linguistic complexity when used artfully. I reckon I wouldn't have chosen to become a software developer if I didn't
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-05-30 13:51:59 |
Closed_By | ⇒ | brianteeman |
closed. no point in doing what others ask to be done when its not appreciated
Yeah, that definitely works. I cannot remember what was the reason we had been reluctant to check the application input on the system plugin but it was two years ago and it was based on in-person discussion we had with @HLeithner over breakfast in the Forum For The Future meeting in January 2020. I honestly can't recall the details but I also don't see why not to do that either. Therefore I will definitely accept it as a good idea in this context.
Just one thing I'd do differently. I would not call
Factory::getApplication()
from a plugin anymore. We already define the magic$app
properly in the main plugin extension classJoomla\Plugin\Multifactorauth\Webauthn\Extension\Webauthn
. This is magically assigned the application object by the plugin's constructor.So I'd rewrite this line:
if ($layout = Factory::getApplication()->input->get('layout') !== 'edit')
to
if ($layout = $this->app->input->get('layout') !== 'edit')
The reason is that at some point (if not Joomla 5.0 then very likely throughout the 5.x lifetime, per tangential discussions we had in other PRs with @laoneo and @HLeithner) we will most likely move from defining a magic
$app
property in the plugin to injecting the application object in the plugin's service provider. Not having a call to a static method of a god object like Factory in random places will help with that transition.