Skip to content

Conversation

rossabaker
Copy link
Member

The heavy lift on this one is getting the new DigestAuth to CE3.

It still has the blocking Random embedded. We'll need to adjust this further in light of #6177.

armanbilge and others added 30 commits March 19, 2022 23:07
Co-authored-by: Arman Bilge <armanbilge@gmail.com>
Resolves http4s#6068

NonceKeeper is package-private, so there's no way to construct one,
meaning there's no way to call `challenge`.

Mirror the same parameters from `apply`, and maintain the previous
`challenge` method for bincompat.
…ion/DigestAuth.scala

Co-authored-by: Arman Bilge <armanbilge@gmail.com>
In order to achieve the goal of F.delay(new NonceKeeper) but maintaining
the original intent of `apply`, just revert everything in this PR, make
the original (unusable) `challenge` `private`, and create a new public
`challenge` with the necessary parameters, intended for human use,
without impacting the existing flow.
…ion/DigestAuth.scala

Co-authored-by: Arman Bilge <armanbilge@gmail.com>
Internals unchanged, in preparation for a pure rewrite of DigestAuth.

Includes a private bincompat method
Internals unchanged, in preparation for a pure rewrite of DigestAuth.

Includes a private bincompat method
@rossabaker
Copy link
Member Author

Whoopsies: typelevel/cats-effect#2902

rossabaker and others added 5 commits March 24, 2022 07:53
…ion/Nonce.scala

Co-authored-by: Daniel Esik <e.danicheg@yandex.ru>
…entication/Nonce.scala

Co-authored-by: Daniel Esik <e.danicheg@yandex.ru>
…ladoc

Clean up `BlazeServerBuilder` scaladoc
current <- F.monotonic
lastCleanupMillis <- Ref[F].of(current)
nonces = new LinkedHashMap[String, NonceF[F]]
random <- Random.javaSecuritySecureRandom[F]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see in CE3 this is implemented as:

  def javaSecuritySecureRandom[F[_]: Sync]: F[Random[F]] =
    Sync[F].delay(new java.security.SecureRandom).flatMap(r => javaUtilRandom(r))

We can do the same thing and unblock JS:

  def javaSecuritySecureRandom[F[_]: Sync]: F[Random[F]] =
    Sync[F].delay(new org.http4s.crypto.unsafe.SecureRandom).flatMap(r => javaUtilRandom(r))

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now fixed in CE, so we can revert.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. I didn't remember making 6753dd0 part of this. But it's a tight spot either way:

  • If we do it, we want a consensus on cats.effect.std.SecureRandom before we release.
  • If we don't do it, we can't easily introduce Random to the API later without another crap overload.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to prune that commit because there's a discussion of it on another draft.

@rossabaker rossabaker marked this pull request as ready for review March 24, 2022 19:47
@@ -329,4 +330,13 @@ package object internal {
}
}
}

private[http4s] def javaMajorVersion[F[_]](implicit F: Sync[F]): F[Option[Int]] =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems unused. Do we want this within 0.23?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's used in 0.22. Something like it will need to be in Cats-Effect if we fix its Random. I left it mainly because I don't want to write it again if we ever need it, but it wasn't hard. I guess we can prune it.

@rossabaker rossabaker merged commit fbb5566 into http4s:series/0.23 Mar 25, 2022
@rossabaker rossabaker deleted the merge-0.22-2022-03-23 branch March 25, 2022 21:17
rossabaker added a commit to http4s/http4s-servlet that referenced this pull request Apr 3, 2022
@rossabaker rossabaker added the behind-the-scenes Appreciated, but not user-facing label May 24, 2022
rossabaker added a commit to http4s/blaze that referenced this pull request May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants