? Language Change Composer Dependency Changed Release Blocker ? Pending

User tests: Successful: Unsuccessful:

avatar nikosdion
nikosdion
1 Sep 2022

Pull Request for Issue #38233 and #38654 .

Summary of Changes

  • Use a copy of fido.jwt supplied with Joomla in the plugins/system/webauthn folder.
  • Script to update this file in the build directory.
  • Update of build/build.php to automatically run this script when building a new version of Joomla.
  • Integrate the lazy loading of the MDS information from #38661

Testing Instructions

You need to have a host where you see a performance loss when the webauth system plugin is activated e.g. blackhole the domain mds.fidoalliance.org.

Actual result BEFORE applying this Pull Request

Slow page load (+5 seconds than usual).

Expected result AFTER applying this Pull Request

Fast page load.

Documentation Changes Required

None.

Votes

# of Users Experiencing Issue
1/1
Average Importance Score
5.00

avatar nikosdion nikosdion - open - 1 Sep 2022
avatar nikosdion nikosdion - change - 1 Sep 2022
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 1 Sep 2022
Category Administration Language & Strings Repository Front End Plugins
avatar richard67
richard67 - comment - 1 Sep 2022

@nikosdion Does this PR fix #38662 , too?

avatar nikosdion
nikosdion - comment - 1 Sep 2022

Yes, it does! Since we no longer go through the network there is no performance penalty when the FIDO server is down.

avatar Fedik
Fedik - comment - 1 Sep 2022

One performance issue remains, but now it more "local one" so probably not much critical.

Use of json_decode(json_encode($entry->metadataStatement), true).
Because $entry->metadataStatement contain base64 encoded icon, it slows down json_decode, and so whole thing.

avatar nikosdion
nikosdion - comment - 1 Sep 2022

@Fedik But it no longer happens on every request. It only happens when you are registering an authenticator or viewing a list of authenticators. In these instances a total page load performance impact between 7 and 30 milliseconds is perfectly acceptable.

avatar wilsonge
wilsonge - comment - 1 Sep 2022

Can I suggest we distribute the ability to update the file over CLI too within the CMS as a command. So people can choose to update the local file too? Understand it will always get overridden by update - but feels relatively acceptable.

avatar Fedik
Fedik - comment - 1 Sep 2022

But it no longer happens on every request.

yes right, that what I meant with: "it now local" :)

avatar nikosdion
nikosdion - comment - 1 Sep 2022

@wilsonge If you're on CLI does running something like php /path/to/cli/joomla.php core:webauthn:update make more sense than wget "https://mds.fidoalliance.org" -O /path/to/plugins/system/webauthn/fido.jwt? We really just retrieve and store a file, we don't process it any further.

avatar nikosdion
nikosdion - comment - 1 Sep 2022

@Fedik Removing that double encode-decode step will cause fatal errors on PHP 8 with some authenticators. It would also only remove 1-5 milliseconds. Too big a risk for too little gains?

avatar Fedik
Fedik - comment - 1 Sep 2022

It would also only remove 1-5 milliseconds

Sometimes it hard to stop to make it faster, when see that it can be faster ?

cause fatal errors on PHP 8 with some authenticators

That interesting.
I would probably then just unset icon (and set it back if it used somwhere in code)

$array = $entry->metadataStatement;
unset($array['icon']);
$array = json_decode(json_encode($array), true);

But it is not related to current PR.

avatar nikosdion nikosdion - change - 1 Sep 2022
Labels Added: Language Change Release Blocker ?
avatar joomla-cms-bot joomla-cms-bot - change - 1 Sep 2022
Category Administration Language & Strings Repository Front End Plugins Repository Administration Language & Strings External Library Composer Change Front End Plugins
avatar nikosdion nikosdion - change - 1 Sep 2022
Labels Added: Composer Dependency Changed
avatar wilsonge
wilsonge - comment - 1 Sep 2022

@wilsonge If you're on CLI does running something like php /path/to/cli/joomla.php core:webauthn:update make more sense than wget "https://mds.fidoalliance.org" -O /path/to/plugins/system/webauthn/fido.jwt? We really just retrieve and store a file, we don't process it any further.

On a VM it's fine - in containers it's more complicated because alpine doesn't ship with wget for instance - so having it "php native" can make things easier in some environments.

avatar brianteeman
brianteeman - comment - 1 Sep 2022

do we need to add administrator/cache/fido.jwt to the list of files removed on upgrade?

avatar richard67
richard67 - comment - 1 Sep 2022

do we need to add administrator/cache/fido.jwt to the list of files removed on upgrade?

@brianteeman Seems so. But as said many times before, we don't bother the authors of PRs with that and leave that to the release leads (or their helpers like I am one).

avatar richard67
richard67 - comment - 1 Sep 2022

P.S.: Question is if it's safe to remove it on update or if it still might be used during that process.

avatar nikosdion
nikosdion - comment - 1 Sep 2022

It is safe to remove, yes.

avatar laoneo
laoneo - comment - 1 Sep 2022

@nikosdion are the testing instructions still valid? Guess now is only a composer install needed or the drone build should work out of the box? I would like to get here some tests as it looks like all the issues are ironed out.

avatar nikosdion
nikosdion - comment - 2 Sep 2022

@laoneo If the FIDO service becomes available then, yes, the testing instructions will work using the Drone build.

Since FIDO servers are down you need to manually copy fido.jwt into plugins/system/webauthn before testing. That's what I was telling in the chat yesterday. The FIDO server seems down so we have to do that step manually when building 4.2.2.

avatar roland-d
roland-d - comment - 2 Sep 2022

@nikosdion Not sure what is going on but when I run the php build/build.php the update_fido_cache.php is not run. Do I run composer install manually, it is triggered.

Does the php build/build.php work for you?

I have a workaround to just copy the fido.jwt from my local system in the build.php but it seems odd that the composer install from the build script does not trigger the post-install-cmd

avatar nikosdion
nikosdion - comment - 2 Sep 2022

This is what I was talking about with @HLeithner and @laoneo. Here's the problem we have.

For the update_fido_cache.php script to work we need the FIDO server to be working. Otherwise it just hangs for 5 seconds (the timeout we have) and tells you it cannot fetch the file.

The FIDO server is not working at the moment.

They way I see it is that the only way to actually build Joomla 4.2.2 is to commit fido.jwt in the repo (for now!), build the release and remove the file from the repo when the FIDO server starts working again.

avatar roland-d
roland-d - comment - 2 Sep 2022

The FIDO server is not working at the moment.

That must have been a fluke on my end then because when I called the URL I was given a jwt file :)

They way I see it is that the only way to actually build Joomla 4.2.2 is to commit fido.jwt in the repo (for now!), build the release and remove the file from the repo when the FIDO server starts working again.

I did not commit the file to the repo, just added a copy statement in the build script to copy it after composer has run. This seems fine to me and less work than committing the file unless you see an issue with that.

avatar nikosdion
nikosdion - comment - 2 Sep 2022

@roland-d No problem from me! Your solution is not just acceptable, it's tested. It's actually how I was running my local test as you can see in my commit history on this PR :)

avatar richard67
richard67 - comment - 2 Sep 2022

I did not commit the file to the repo, just added a copy statement in the build script to copy it after composer has run. This seems fine to me and less work than committing the file unless you see an issue with that.

@roland-d @nikosdion Does that mean the files is fetched from Fido with every run of composer where it has not rune before or the file has been removed or it is older than 10 days (864000 seconds)? In this case: What do we do if Fido is not reachable when we build a release? In my opinion the file has to be in the repository. Otherwise we still have the problem of creating a DDoS, it's just a bit smaller.

Another thing: Maybe Fido currently is not down but blocked certain user agents or IP ranges due to our DDoS?

avatar richard67
richard67 - comment - 2 Sep 2022

P.S.: We have composer install running on quite a bunch of docker images with every single commit of every single PR. If we do not have the file in our sources, we will fetch if every time because the docker images are new for each test. This still might cause them (or their server protection) to block these requests.

avatar nikosdion
nikosdion - comment - 2 Sep 2022

@richard67 We talked about that with Harald yesterday, he told me he'd have drone.yaml to create a dummy, empty plugins/system/webauthn/fido.jwt file. This will indeed prevent the script from trying to get a new file which means the PR builds will work fine without hitting the FIDO server.

avatar richard67
richard67 - comment - 2 Sep 2022

P.P.S.: Ah, wait,. it's only fetched when packages are built ... so less critical ... but I still think it should be in the sources.

avatar nikosdion
nikosdion - comment - 2 Sep 2022

I was told it would be taken care of. I don't use Drone, I am a Luddite who runs his tests locally :) (Between travelling a lot before Covid-19 and having unreliable Internet more often than not when I'm back home it's actually the only way for me to make sure I can run my tests without frustration).

avatar richard67
richard67 - comment - 2 Sep 2022

Well maybe I am wrong and there is no problem, and I just did not understand it on the first look.

Regarding testing: I can test the performance thing, but I can not test if webauthn with Fido is still working (which in my opinion should be tested, too) because I have no Fido key.

avatar nikosdion
nikosdion - comment - 2 Sep 2022

The WebAuthn part still works but it'd be great to have some people with Windows computers to test Windows Hello. Maybe @brianteeman if he's not super busy?

avatar richard67
richard67 - comment - 2 Sep 2022

@nikosdion As we do not have any other deleted files and folders from 4.2.1 to 4.2.2, I've decided to make a PR for you to make that change in script.php here in your PR: nikosdion#11

This will save the release lead one PR to be merged.

If you don't want that, let me know, and I will make a separate PR for that in this repo here.

avatar nikosdion
nikosdion - comment - 2 Sep 2022

Thank you, Richard! I just merged it :)

avatar joomla-cms-bot joomla-cms-bot - change - 2 Sep 2022
Category Administration Language & Strings Repository Front End Plugins External Library Composer Change Repository Administration com_admin Language & Strings External Library Composer Change Front End Plugins
avatar richard67
richard67 - comment - 2 Sep 2022

Thanks Nik.

avatar richard67
richard67 - comment - 2 Sep 2022

I have tested this item successfully on a66514a

I have updated a 4.2.1 site (copy of my 3.10 homepage once updated to J4 for development) on my domain to the custom update URL provided by drone for this PR.

I could successfully verify that the performance problem is fixed by this PR.

In addition I have successfully tested that Webauthn still works as primary login authenticator and as MFA factor and both together. I have used Windows Hello with a PIN on my computer and Firefox latest stable.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38664.

avatar richard67 richard67 - test_item - 2 Sep 2022 - Tested successfully
avatar tecpromotion
tecpromotion - comment - 2 Sep 2022

I have tested this item successfully on a66514a

I was able to test it successfully on multiple websites and different hosts.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38664.

avatar tecpromotion tecpromotion - test_item - 2 Sep 2022 - Tested successfully
avatar richard67 richard67 - change - 2 Sep 2022
Status Pending Ready to Commit
avatar richard67
richard67 - comment - 2 Sep 2022

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38664.

avatar roland-d roland-d - change - 2 Sep 2022
Labels Added: ?
avatar roland-d roland-d - change - 2 Sep 2022
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2022-09-02 19:37:59
Closed_By roland-d
avatar roland-d roland-d - close - 2 Sep 2022
avatar roland-d roland-d - merge - 2 Sep 2022
avatar roland-d
roland-d - comment - 2 Sep 2022

Thank you

avatar herrjemand
herrjemand - comment - 8 Sep 2022

Thanks for fixing this guys lol.

If you ever have deployment issues and help with FIDO in general: ackermann.yuriy@gmail.com.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38664.

Add a Comment

Login with GitHub to post a comment