Doesn't work with nextcloud 12

[Tarry91]

When activating the app on nextcloud 12.0.3 I get the error message: "this app cannot be enabled because it makes the server unstable."

PHP: 7.0.23

I get the following error in the logs: Methods with the same name as their class will not be constructors in a future version of PHP; Auth_Yubico has a deprecated constructor at [...]/apps/twofactor_yubikey/vendor/auth_yubico/Yubico.php#35

[sullenx]

I was able to get mine to work in nextcloud 12, but you need to make a bunch of changes:

1. Fix the constructor problem, you need to create a new constructor.

"apps/twofactor_yubikey/vendor/auth_yubico/Yubico.php", add:

function __construct($id, $key = '', $https = 0, $httpsverify = 1)
        {
        $this->_id =  $id;
                $this->_key = base64_decode($key);
                $this->_https = $https; 
                $this->_httpsverify = $httpsverify; 
    }

Then delete the constructor there, and put:

function Auth_Yubico($id, $key = '', $https = 0, $httpsverify = 1)
    {
        self::__construct($id,$key,$https,$httpsverify);
    }

2. Now that's not the only problem here, you need to fix the "apps/twofactor_yubikey/appinfo/info.xml", replace with the following:

<dependencies>

        <owncloud min-version="11" max-version="12" />
 </dependencies>

3. Last one, this should finally work. In nextcloud 12, they've deprecated OC\IDb with OC\IDbConnection, so you need to replace it in apps/twofactor_yubikey/lib/Db/KeyIDMapper.php:

-use OCP\IDb; 
+use OCP\IDbConnection;

- public function __construct(IDb $db)
+ public function __construct(IDbConnection $db)

That's it. Everything should be working again. I'd love to change the code, but i don't have access. Hopefully the original author can fix it.

[sullenx]

I have forked the project and made the changes. Try it out: https://github.com/sullenx/twofactor_yubikey

[Tarry91]

Wow thank you! I did found the first two problems myself, but then I gave up :(

Thank you for the fork. I will try it and give you feedback there.

[Tarry91]

@sullenx I cannot create an issue at your forked project so I answer here:

1) The installation did work just fine, however I could not login with the OTP. I always get the error "Error while authenticating the second factor" (translated from german). The nextcloud log shows the following error: "Undefined index: yubikeyID at /path/to/nextcloud/apps/twofactor_yubikey/templates/settings-personal.php#25". I didn't see that I have to provide my own API keys in the admin panel. It works now

2) When I delete the yubikey ID from the field in the setting, I am still promted for an OTP on login (which of course also fails). I would have thought, that this would disable yubikey OTP for the given user.

I am not sure how involved you want to be with the project. But if you are willing to continue the work it would be great, if more than one yubikey can be set, for example one for the road and one that always stays home.

[sullenx]

@Tarry91 I fixed your #2 problem. Now if you clear the YubiKeyID, it will disable Yubi 2nd Factor for the user. I committed it to the fork. I just put a check for empty() in the Yubiotp.php:

if( !empty($keyID) )
   {
     $dbKeyID = new KeyID();

     $dbKeyID->setUserId($user->getUID());
     $dbKeyID->setYubikeyId($keyID);

     $this->keyIDMapper->insert($dbKeyID);
  }

As far as the continuing the project, I just been fixing things to get it to work on my setup. I like the feature request. I'd say let's see if the original author @jaark can just merge the fork OR find other ways to fix. If the original author @jaark orphans this project, then I'll see if i can study it to maintain it as i find these features useful.

[Tarry91]

@sullenx Thank you for the fix! Works just intendet :) If you give me your paypal I would love to buy you a beer for getting this app up running again!

I believe that @jaark has orphaned the project. He didn't respond here in the last six weeks nor in the nextcloud forum, where I found this app (https://help.nextcloud.com/t/yubikey-2fa-provider-comments-advice-please/3175). I think you should not hold back hoping for an answer. If he gets active again I am all for merging the projects, but I wouldn't hold my breath.

If you don't mind the work I would suggest you open up the your fork to collect feedback and continue the project there. Maybe we can even find other people interested in the app. I am no php developer but maybe I can somehow figure out how to get additional yubikeys enabled, as I need this feature for my setup :)

I think this app is great and has a lot of potential!

[jaark]

@sullenx thanks for this. I've been up to my ears in another project for the past year leaving no time for anything else, I've not even downloaded any version of NC12 yet!

If you are willing, I think that the best course of action would be for your fork to be regarded as the canonical version of the app. I'm not sure if there's any formal way of doing this but I can easily put the info and a link at the top of the readme in my version. Please let me know if you are willing and if you know of a better method of reassignment.

[sullenx]

@Tarry91 I implemented the multi-key support and committed it to the fork. I'd first disable and delete the existing installation, flush out all javascript (hard reload). For multi-key support, i had to remove indexing on user_id instead just on id. I also borrowed yubikey icon from the U2F project. Take a look, i tested it in chrome and IE.

@jaark I think it might be best if we merge my changes to your project and add me as a contributor, delete my fork. I'm thinking more from not having multiple projects of the same thing in git hub. I can update/add as things come up, since i use Yubikeys in my own setup, i don't see why not share it with others.

[jaark]

I'm guessing we're both pretty much newbies when it comes to the underbelly of github.

I've added you as a collaborator. if I understand things right you should have full access to the repository. I've noticed that there is also a transfer ownership button which should solve your duplicated projects concern. If you want we can take a look at clicking that after we get your changes merged into mine. I've created a pull request which I think will do the job.

Edit : It looks to me like the pull request stuff worked.

[sullenx]

@jaark Great! Thanks. I forgot to upload the img folder, I added to this project. I will close down the fork and contribute here.

@Tarry91 Please use this repository, it is up to date. We can now close this issue and the other one.

[sullenx]

Closing issue. Fork deleted. All changes are reflected here.

[Tarry91]

@sullenx Thanks for adding multikey support! works just fine and looks very nice!

EDIT: I did create sume issues with feature request and some ideas I had for the project. But it works just fine now and I installed it on all of my instances. Thanks again for your work!