Add common constants for HTTP auth algorithms#5631
Add common constants for HTTP auth algorithms#5631Pantus Oleh (zibet27) wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughExtends HTTP digest authentication with XML Signature algorithm URI support and introduces two new types: KeyAlgorithm value class for key families and SignatureAlgorithm for signature algorithm metadata with URI/JWA-based lookup and JVM integration. ChangesHTTP Authentication Algorithm Model
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
CodeRabbit (@coderabbitai) review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ktor-http/common/src/io/ktor/http/auth/DigestAlgorithm.kt (1)
79-80:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDecouple name lookup from
DEFAULT_ALGORITHMS.
from(name)still searchesDEFAULT_ALGORITHMS, so the newSHA_384andSHA_512constants can never be resolved by name even thoughfromUri(...)exposes them as supported XML digests. IfDEFAULT_ALGORITHMSis intentionally limited to RFC 7616 values, keep that list as-is and add a separate all-known lookup list here.💡 Suggested fix
public fun from(name: String): DigestAlgorithm? { - return DEFAULT_ALGORITHMS.find { it.name.equals(other = name, ignoreCase = true) } + return ALL_ALGORITHMS.find { it.name.equals(other = name, ignoreCase = true) } } + private val ALL_ALGORITHMS: List<DigestAlgorithm> = + DEFAULT_ALGORITHMS + listOf(SHA_384, SHA_512) + private val XML_DIGEST_ALGORITHMS: List<DigestAlgorithm> = listOf(SHA_256, SHA_384, SHA_512)Also applies to: 88-102
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ktor-http/common/src/io/ktor/http/auth/DigestAlgorithm.kt` around lines 79 - 80, DEFAULT_ALGORITHMS is currently used by from(name) which prevents SHA_384 and SHA_512 from being resolved by name; keep DEFAULT_ALGORITHMS limited to the RFC 7616 set but add a new comprehensive lookup list (e.g. ALL_KNOWN_ALGORITHMS or ALL_ALGORITHMS) containing SHA_384, SHA_512 and the existing constants, then change from(name) to search the new comprehensive list while leaving fromUri(...) and DEFAULT_ALGORITHMS unchanged; update any other lookup usages in the same region (lines around the existing constants and methods) to use the new ALL_KNOWN_ALGORITHMS as appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@ktor-http/common/src/io/ktor/http/auth/DigestAlgorithm.kt`:
- Around line 79-80: DEFAULT_ALGORITHMS is currently used by from(name) which
prevents SHA_384 and SHA_512 from being resolved by name; keep
DEFAULT_ALGORITHMS limited to the RFC 7616 set but add a new comprehensive
lookup list (e.g. ALL_KNOWN_ALGORITHMS or ALL_ALGORITHMS) containing SHA_384,
SHA_512 and the existing constants, then change from(name) to search the new
comprehensive list while leaving fromUri(...) and DEFAULT_ALGORITHMS unchanged;
update any other lookup usages in the same region (lines around the existing
constants and methods) to use the new ALL_KNOWN_ALGORITHMS as appropriate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ad3fe417-c6e9-4fa7-8c1e-1a71e8052f05
📒 Files selected for processing (8)
ktor-http/api/ktor-http.apiktor-http/api/ktor-http.klib.apiktor-http/common/src/io/ktor/http/auth/DigestAlgorithm.ktktor-http/common/src/io/ktor/http/auth/KeyAlgorithm.ktktor-http/common/src/io/ktor/http/auth/SignatureAlgorithm.ktktor-http/common/test/io/ktor/tests/http/AuthAlgorithmsTest.ktktor-http/jvm/src/io/ktor/http/auth/SignatureAlgorithm.jvm.ktktor-http/jvm/test/io/ktor/tests/http/DigestAlgorithmJvmTest.kt
Bruce Hamilton (bjhham)
left a comment
There was a problem hiding this comment.
Good call 👍
| public value class KeyAlgorithm(public val name: String) { | ||
| public companion object { | ||
| /** RSA key algorithm family. */ | ||
| public val RSA: KeyAlgorithm = KeyAlgorithm("RSA") |
There was a problem hiding this comment.
I know we've recently added @JvmField annotation to other companion object fields. Should we use it here as well?
cc Bruce Hamilton (@bjhham)
Subsystem
Common
Motivation
Reuse these constants for SAML, OIDC and digest auth