-
Notifications
You must be signed in to change notification settings - Fork 786
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Resolve #6068 digestauth challenge redux #6138
Merged
rossabaker
merged 37 commits into
http4s:series/0.22
from
blast-hardcheese:resolve-6068-digestauth-challenge-redux
Mar 24, 2022
Merged
Changes from all commits
Commits
Show all changes
37 commits
Select commit
Hold shift + click to select a range
a34ffa6
Exposing callable parameters for DigestAuth.challenge
blast-hardcheese 4ee881c
Update server/src/main/scala/org/http4s/server/middleware/authenticat…
blast-hardcheese 1cbfe59
Embedding new NonceKeeper in F.delay
blast-hardcheese b645a3b
Chasing F.delay for NonceKeeper
blast-hardcheese 98a1dcb
Update server/src/main/scala/org/http4s/server/middleware/authenticat…
blast-hardcheese 8196978
quicklint
4819197
Promote DigestAuth.apply to F[_]
blast-hardcheese 7a60bd1
Switching AuthenticationSuite to use DigestAuth.applyF
blast-hardcheese db00ac4
First pass of overhauling NonceKeeper with Ref and Semaphore
blast-hardcheese 7163a30
Remove return
blast-hardcheese e50b67b
Promote DigestAuth.apply to F[_]
blast-hardcheese f5ff20b
Break out NonceKeeperF
blast-hardcheese a4054ee
Switch to NonceKeeperF
blast-hardcheese 46c68a6
Break out Nonce
blast-hardcheese 8a2c072
Switch to NonceF
blast-hardcheese c57ae72
bincompat
blast-hardcheese b5283e6
tailrec -> tailRecM
blast-hardcheese 5c7f4d9
Flesh out AuthenticationStore
blast-hardcheese d241359
Bumping deprecated version number
blast-hardcheese 5fe1648
getRandomData should be F-suspended
blast-hardcheese 0ad0342
Strike fear into the hearts of men
blast-hardcheese f702b42
Moving nonces into NonceKeeperF constructor
blast-hardcheese c69302c
F-suspend getRandomData
blast-hardcheese b748965
Documentation
blast-hardcheese 4345b53
Preparing for more secure authStore test
blast-hardcheese 645d2da
Adding a test for Md5HashedAuthenticationStore
blast-hardcheese 3122229
De-case-ize AuthenticationStore members
blast-hardcheese aa6ab18
Renaming AuthenticationStore to AuthStore
blast-hardcheese 61bcac3
Adding Md5HashedAuthStore.precomputeHash helper
blast-hardcheese 8c6ef83
Swapping ju.Date to Instant
blast-hardcheese 35dbb6e
Switching from Instant and realTime to monotonic millis
blast-hardcheese 4a5f76e
Swapping out Long to Duration
blast-hardcheese a31c5d6
Wiring through Blocking and ContextShift for #6165
blast-hardcheese f84f759
uri: String -> Uri
blast-hardcheese 733dcf7
bincompat
blast-hardcheese e028cf5
Stub out Blocker since this is all just going away in CE3 anyway
blast-hardcheese 11ef78b
Push Blocker out to the user's control
blast-hardcheese File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be sealed? I see we match on them in
checkAuthParams
. Would it be better if those cases were a method on this, to permit more pluggable implementations? Or are plaintext and MD5 the two that are specified?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on how I understand the spec, there are only two sensible implementations:
Plain-text passwords:
Pre-hashed passwords:
The way I see it, by pushing more into subclasses of
AuthStore
, we'd need to expose all these parameters for dubious benefit.ha1
is the only secret that needs to be exposed to user codeThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing the spec again for another comment, this is actually called out explicitly by the spec:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like it can be
md5
,md5-sess
, or extensions that probably nobody understands. I think it would be unpleasant to register and understand more for little gain.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The need to add support for this stuff may motivate new contributors 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Additionally, adding the required helper methods, similar to
Md5HashedAuthStore.precomputeHash
)