Add HOTP algorithm #106

DavidGregory084 · 2022-06-17T16:37:58Z

I couldn't resist adding this since it's very easy if HMAC is already there 😛

core/shared/src/main/scala/bobcats/Hotp.scala

core/js/src/main/scala/bobcats/HotpPlatform.scala

…econditions for HOTP digits

core/shared/src/main/scala/bobcats/Hotp.scala

DavidGregory084 · 2022-06-17T17:03:15Z

I have a PR for TOTP cooking as well, but I'm still figuring out how to test it as it's implemented using F.monotonic. I've seen TestControl but I don't really get how to set a fixed time with it, or what the initial time will be (I guess 0?).
I found in previous projects that I simply used IO(clock.instant()) with some java.time.Clock in my implementation code, then Clock.fixed for testing (see e.g. this squuid thing in https://github.com/hmrc/transit-movements-guarantee-balance/blob/main/app/models/values/BalanceId.scala#L59= and its tests in https://github.com/hmrc/transit-movements-guarantee-balance/blob/main/test/models/values/BalanceIdSpec.scala#L43=) so I've managed to avoid understanding TestControl so far 😆.

armanbilge · 2022-06-17T17:05:28Z

Yes, the initial time will be 0. If you want to advance it just do an IO.sleep at the beginning 😂

Then you can do TestControl.executeEmbed(...) to run your IO efficiently.

core/shared/src/test/scala/bobcats/HotpSuite.scala

DavidGregory084 · 2022-06-17T17:14:02Z

Does that mean that you cannot test with SyncIO since TestControl expects an IO program?

armanbilge · 2022-06-17T17:18:20Z

Note you can always upgrade a SyncIO to an IO, there is a method for that (to I think). In any case IO is fine.

armanbilge · 2022-06-17T19:44:22Z

core/shared/src/main/scala/bobcats/Hotp.scala

+        require(digits >= 6, s"digits must be at least 6, was $digits")
+        require(digits < 10, s"digits must be less than 10, was $digits")


Let's wrap this in F.catchNonFatal so that errors suspend properly into F.

Actually, why do we need Sync here? Would Functor be enough? In which case you can leave the require as-is.

I think there's a design question here whether we need an Hotp capability trait actually since it's not hiding bigger constraints. We could implement these as methods on an object, that take Functor and Hmac constraints. I'd need to look to other Typelevel libs for inspiration on the right way to do this :)

Should there possibly be effectful equivalents of require, assume and assert in cats-effect Sync? It's the lowest part of the Typelevel hierarchy that has knowledge that E is Throwable and can raiseError I think?

Ah, that's interesting :) We have ApplicativeThrow#catchNonFatal and there may be similar methods in there.

DavidGregory084 · 2022-06-20T11:44:22Z

Hmm I don't really know what to do with this test failure. I don't get the same failure locally and there is no actual output from the failed test in the log. I have not been testing with the Chrome JSEnv locally and that seems to be the one that is failing in these test runs but I know very little about Scala.js.

 bobcats.HmacSuite:
  + SHA1 RFC2022 test case 1 with IO0.02s
  + SHA1 RFC2022 test case 2 with IO0.00s
  + SHA1 RFC2022 test case 3 with IO0.00s
  + SHA1 RFC2022 test case 4 with IO0.01s
  + SHA1 RFC2022 test case 5 with IO0.00s
  + SHA1 RFC2022 test case 6 with IO0.00s
  + SHA1 RFC2022 test case 7 with IO0.01s
  + SHA1 with IO0.01s
  + SHA256 with IO0.01s
  + SHA512 with IO0.00s
  + generate key for SHA1 with IO0.00s
  + generate key for SHA[error] Error: Total 22, Failed 0, Errors 1, Passed 21
[error] Error during tests:
[error] 	bobcats.bobcats.HotpSuite
[error] (coreJS / Test / test) sbt.TestsFailedException: Tests unsuccessful
[error] Total time: 8 s, completed Jun 20, 2022 11:35:01 AM
256 with IO0.01s
  + generate key for SHA512 with IO0.00s
Error: Process completed with exit code 1.

DavidGregory084 · 2022-06-20T11:56:35Z

Oh, in my case it's just that I haven't used chromedriver for ages so it's out of date, never mind - I get the same thing as the CI server, "tests unsuccessful" but with no output related to those tests:

[error] Error: Total 8, Failed 0, Errors 3, Passed 5
[error] Error during tests:
[error]         bobcats.HashSuite
[error]         bobcats.HmacSuite
[error]         bobcats.bobcats.HotpSuite
[error] (coreJS / Test / test) sbt.TestsFailedException: Tests unsuccessful

armanbilge · 2022-06-20T14:28:48Z

Yup, sorry, you are stumbling into scalameta/munit#492.

Anyway, did you mention you are running the tests with SyncIO? Browsers don't support synchronous crypto, only async, so SyncIO will crash.

However, Node.js supports sync and async crypto via different APIs. This is why I was running the tests with both SyncIO and IO depending on the runtime. But I no longer think this was a good idea (see #79); we should just require Async and be done with it.

DavidGregory084 · 2022-06-24T15:48:24Z

@armanbilge I think the tests on this PR were failing for the same reason as #108

armanbilge · 2022-06-24T17:22:49Z

@armanbilge I think the tests on this PR were failing for the same reason as #108

You mean because of the old Node.js? I thought the tests were failing in the browser.

armanbilge

Very nice addition, thank you!

core/shared/src/test/scala/bobcats/HotpSuite.scala

Co-authored-by: Arman Bilge <[email protected]>

armanbilge

Fantastic! 😃

Add HOTP algorithm

5669709

DavidGregory084 force-pushed the hotp branch from de2ef8c to 5669709 Compare June 17, 2022 16:40

DavidGregory084 commented Jun 17, 2022

View reviewed changes

core/shared/src/main/scala/bobcats/Hotp.scala Outdated Show resolved Hide resolved

DavidGregory084 commented Jun 17, 2022

View reviewed changes

core/js/src/main/scala/bobcats/HotpPlatform.scala Outdated Show resolved Hide resolved

Add HOTP#generate overload instead of using default arguments, use pr…

3d6ebfe

…econditions for HOTP digits

armanbilge reviewed Jun 17, 2022

View reviewed changes

core/shared/src/main/scala/bobcats/Hotp.scala Outdated Show resolved Hide resolved

Remove HOTP platform traits for now

e3adb37

Make HOTP#generate overload a default implementation

7ae48fa

DavidGregory084 commented Jun 17, 2022

View reviewed changes

core/shared/src/test/scala/bobcats/HotpSuite.scala Outdated Show resolved Hide resolved

armanbilge reviewed Jun 17, 2022

View reviewed changes

DavidGregory084 added 2 commits June 20, 2022 12:23

Remove Hotp capability trait and use companion object methods instead

04ffc71

Remove the capability trait

35f4564

This comment was marked as resolved.

Sign in to view

DavidGregory084 mentioned this pull request Jun 21, 2022

Add Sync helpers for scala.Predef assertions typelevel/cats-effect#3044

Closed

DavidGregory084 added 3 commits June 24, 2022 16:21

Merge branch 'main' of github.com:typelevel/bobcats into hotp

e8d2446

Remove HOTP SyncIO testing

557cc6f

Remove redundant package declaration

c3ff839

armanbilge approved these changes Jun 24, 2022

View reviewed changes

core/shared/src/test/scala/bobcats/HotpSuite.scala Outdated Show resolved Hide resolved

DavidGregory084 and others added 2 commits July 15, 2022 13:27

Replace another usage of ByteVector.fromHex

e2193f6

Co-authored-by: Arman Bilge <[email protected]>

Add missing scodec hex syntax import

aee956a

armanbilge approved these changes Jul 15, 2022

View reviewed changes

armanbilge merged commit 8d03685 into typelevel:main Jul 15, 2022

DavidGregory084 deleted the hotp branch July 15, 2022 13:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add HOTP algorithm #106

Add HOTP algorithm #106

DavidGregory084 commented Jun 17, 2022 •

edited

Loading

DavidGregory084 commented Jun 17, 2022

armanbilge commented Jun 17, 2022

DavidGregory084 commented Jun 17, 2022

armanbilge commented Jun 17, 2022

armanbilge Jun 17, 2022

armanbilge Jun 17, 2022

armanbilge Jun 17, 2022

DavidGregory084 Jun 20, 2022

armanbilge Jun 20, 2022

DavidGregory084 commented Jun 20, 2022 •

edited

Loading

This comment was marked as resolved.

DavidGregory084 commented Jun 20, 2022 •

edited

Loading

armanbilge commented Jun 20, 2022 •

edited

Loading

DavidGregory084 commented Jun 24, 2022

armanbilge commented Jun 24, 2022

armanbilge left a comment

armanbilge left a comment

		require(digits >= 6, s"digits must be at least 6, was $digits")
		require(digits < 10, s"digits must be less than 10, was $digits")

Add HOTP algorithm #106

Add HOTP algorithm #106

Conversation

DavidGregory084 commented Jun 17, 2022 • edited Loading

DavidGregory084 commented Jun 17, 2022

armanbilge commented Jun 17, 2022

DavidGregory084 commented Jun 17, 2022

armanbilge commented Jun 17, 2022

armanbilge Jun 17, 2022

Choose a reason for hiding this comment

armanbilge Jun 17, 2022

Choose a reason for hiding this comment

armanbilge Jun 17, 2022

Choose a reason for hiding this comment

DavidGregory084 Jun 20, 2022

Choose a reason for hiding this comment

armanbilge Jun 20, 2022

Choose a reason for hiding this comment

DavidGregory084 commented Jun 20, 2022 • edited Loading

This comment was marked as resolved.

DavidGregory084 commented Jun 20, 2022 • edited Loading

armanbilge commented Jun 20, 2022 • edited Loading

DavidGregory084 commented Jun 24, 2022

armanbilge commented Jun 24, 2022

armanbilge left a comment

Choose a reason for hiding this comment

armanbilge left a comment

Choose a reason for hiding this comment

DavidGregory084 commented Jun 17, 2022 •

edited

Loading

DavidGregory084 commented Jun 20, 2022 •

edited

Loading

DavidGregory084 commented Jun 20, 2022 •

edited

Loading

armanbilge commented Jun 20, 2022 •

edited

Loading