If these classes are exact duplicates then would it be good to remove the same things from FOF and provide a class alias? Or do we want to keep the library as untouched as possible?

The FOF library should remain untouched so as to remove all chance of issues.

Once the core no longer depends on FOF, it may be marked as deprecated for removal from inclusion in core during the next major increment.

If @akeeba would like to remove their classes and add an alias to the core classes, that would be fine. But it's my understanding the library as shipped with the CMS is no longer maintained.

The first comment was enough thankyou

Crazy question considering the class' purposes: Is there UT coverage for
them in the old source repo we could utilize?

On Friday, April 11, 2014, Don Gilbert notifications@github.com wrote:

This PR removes the dependency on the FOF library within the TwoFactor
Auth plugins. It moves a few of the required classes into the core of the

CMS in accordance with the GPL licensing of the code.

You can merge this Pull Request by running

git pull https://github.com/dongilbert/joomla-cms bugfix-2fa

Or view, comment on, or merge it at:

#3431
Commit Summary

• Remove dependency on FOF for the twofactorauth plugins and associated component files.

File Changes

• M administrator/components/com_users/helpers/users.phphttps://github.com/joomla/joomla-cms/pull/3431/files#diff-0(10)
• M administrator/components/com_users/models/user.phphttps://github.com/joomla/joomla-cms/pull/3431/files#diff-1(44)
• M components/com_users/models/profile.phphttps://github.com/joomla/joomla-cms/pull/3431/files#diff-2(14)
• A libraries/cms/encrypt/aes.phphttps://github.com/joomla/joomla-cms/pull/3431/files#diff-3(241)
• A libraries/cms/encrypt/base32.phphttps://github.com/joomla/joomla-cms/pull/3431/files#diff-4(225)
• A libraries/cms/encrypt/index.htmlhttps://github.com/joomla/joomla-cms/pull/3431/files#diff-5(1)
• A libraries/cms/encrypt/totp.phphttps://github.com/joomla/joomla-cms/pull/3431/files#diff-6(184)
• M plugins/authentication/joomla/joomla.phphttps://github.com/joomla/joomla-cms/pull/3431/files#diff-7(10)
• M plugins/twofactorauth/totp/totp.phphttps://github.com/joomla/joomla-cms/pull/3431/files#diff-8(48)
• M plugins/twofactorauth/yubikey/yubikey.phphttps://github.com/joomla/joomla-cms/pull/3431/files#diff-9(50)

Patch Links:

• https://github.com/joomla/joomla-cms/pull/3431.patch
• https://github.com/joomla/joomla-cms/pull/3431.diff

—
Reply to this email directly or view it on GitHub#3431
.

When you say old source repo FOF's still using the same repo. But there is some limited test coverage

Nic had asked for proper attribution in one of his posts, and it's the right thing to do IMO. Can we have a comment along the lines of that found at https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/filter/input.php#L15 please?

I'm not necessarily against doing that, Michael. But he didn't do that when he forked a ton of the platform into FOF. No attribution whatsoever. We can add it here though.

One thing to keep in mind though is it's going to set a precedent for every class, function and method to have an attribution section in it's doc block so the authors can put there names in there.

@wilsonge The first comment wasn't enough. If it was, I wouldn't have added the second. :P :P :P

When it's a pure fork of something, to me it's the proper thing to do. But, it's not something I'm willing to die on the sword over honestly.

@dongilbert May I ask which problem are you trying to solve here ? Neither the forge tracker item nor the description here indicates a bug being solved nor a feature request being solved. So still scratching my head.

Nic forked FOF back to himself and is no longer contributing it to Joomla.

I agree. It's the right thing to do. I'll get it added.

@beat Nic is no longer contributing FOF to Joomla for inclusion in core and as such, we'll be stuck maintaining it. Best to refactor all core code that depends on it, mark it as deprecated, and remove it in the next major version increment.

Can we adapt this code into our crypt/cipher package rather than just copying it into a new package?

@dongilbert Thanks for the reply, I think I have finally found the main public twitter conversations that lead to this, for reference, from start to end in sequence:
https://twitter.com/dilbert4life/status/452701039808892928
https://twitter.com/joovlaki/status/452939383659048961
https://twitter.com/joovlaki/status/452947421665185792

Looks like a bit more than just Nic's "fault" as represented above. Needless to say I am very shocked and sad to read all of that. Nobody should be proud of this and all should seriously rethink their doings. All of this is sending an extremely bad signal out. Joomla lost a major contributor and a good software architect. This is a serious case to discuss for OSM, PLT and CLT. I'm off this thread, so it can go back to purely technical.

@beat while I can see how you'd like to paint me the villain here, that's simply not the case. While the twitter links you posted are a part of the situation, for sure, this discussion was actually started back in Feb by Nic himself in regards to the LGPL licensing debate. See for yourself - https://groups.google.com/forum/m/#!topic/frameworkonframework/VxxIKvTooXU - he decided back then to stop contributing FOF to Joomla.

Also note that after discovering the CMS couldn't be re-licensed to LGPL that he retracted the poll immediately https://groups.google.com/d/msg/frameworkonframework/VxxIKvTooXU/RqB81yPJJH4J

So how many times does he have to contribute, retract, retract the retraction, then say "nope, I really mean it this time" before we say "ok, we'll stop using it in core since it's future is uncertain".

@wilsonge re your question of integrating with the JCrypt code, that's definitely a possibility. What are your thoughts @phproberto, @mbabker, @dbhurley, @bakual, @betweenbrain, @nicksavov?

@beat and for the record
I was aware of Nick's decision, but as I do not tweet, I discover now these tweets. They show an undesired aggressiveness which I had already remarked before from some. I sincerely hope that anyone with responsibilities in J! will take the decision to refrain from such behaviour in the future.

Sorry, wrong button
I'm fine with integrating it in JCrypt and I think it would make sense. We can even check if we need all the stuff. For example I don't think the the base32 thing is used by the core 2FA plugin, but I may miss something.

Base32 is used in one of the other classes. Can't remember which one off hand.

dongilbert#11

So this has a rearrangement of the base64 encoding in the JEncryptAES because I needed to keep to the Interface. But see what you guys think

So I don't have any 2 factor methods nor a localhost to test on for the next week or so. So this may fail seriously badly. So fingers crossed it works and there isn't some embarrassing mistake :P

In #3496 @fastslack pointed out that the FOFEncryptAes class has a dependancy on the PHP mcrypt module.
Currently it's not a requirement to run Joomla.

Thus I think we may have to add a check in JEncryptAes to make sure it's available. Otherwise show an error message. Maybe even prevent enabling TFA if mcrypt isn't available.
Or are there better ideas?

It's addressed in the PR @wilsonge sent to @dongilbert

I hadn't had time to review it yet though, due to buying a house and all. Very busy.

It's addressed in the PR @wilsonge sent to @dongilbert

Awesome

Are the PLT happy with the concept of the Cipher method though? because if so I'll try and find time to move the other 2 classes across to use that. Currently the PR is just for the JAes class.

Closing and re-opening against staging now that 3.3 is released.