simplesamlphp / simplesamlphp-module-webauthn

A module implementing FIDO2 / WebAuthn as a second authentication factor
GNU Lesser General Public License v2.1
15 stars 8 forks source link

Fix Psalm-issues #24

Closed tvdijen closed 4 years ago

codecov[bot] commented 4 years ago

Codecov Report

Merging #24 into master will not change coverage. The diff coverage is 0%.

Impacted file tree graph

@@          Coverage Diff           @@
##             master   #24   +/-   ##
======================================
  Coverage         0%    0%           
- Complexity      131   133    +2     
======================================
  Files             7     7           
  Lines           460   461    +1     
======================================
- Misses          460   461    +1
Impacted Files Coverage Δ Complexity Δ
lib/WebAuthn/Store/Database.php 0% <ø> (ø) 19 <0> (ø) :arrow_down:
lib/Store.php 0% <ø> (ø) 6 <0> (ø) :arrow_down:
lib/WebAuthn/AAGUID.php 0% <0%> (ø) 8 <0> (ø) :arrow_down:
lib/WebAuthn/WebAuthnAbstractEvent.php 0% <0%> (ø) 36 <0> (+1) :arrow_up:
lib/WebAuthn/WebAuthnRegistrationEvent.php 0% <0%> (ø) 48 <14> (+1) :arrow_up:
lib/Auth/Process/WebAuthn.php 0% <0%> (ø) 11 <0> (ø) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ae0b84e...874c65f. Read the comment docs.

tvdijen commented 4 years ago

Good catch!

restena-sw commented 4 years ago

So is it okay if the min PHP version for this module is now 7.1 due to the syntax I talked about earlier? If so, I think this PR is ready to merge (and I would make a 1.0 release afterwards as there are no open bugs then).

tvdijen commented 4 years ago

Ah, I missed that comment, sorry! I didn't push the version in this PR.. It's because spomky-labs/cbor-php requires PHP 7.1.. I have investigated alternatives during the hackathon, but my general conclusion was that it would leave us with a lot of work and that it wasn't worth the hassle.

Would the introduced change mean that people deploying the module are forced into a much newer PHP version just because of the module?

Yes, that's the consequence.. SSP is pushing to 7.0 for 1.19 and 7.2 for 2.0, so I think it shouldn't matter that much..

restena-sw commented 4 years ago

Great, merged.