OIDC session routes#5693
Open
Pantus Oleh (zibet27) wants to merge 1 commit into
Open
Conversation
Pantus Oleh (zibet27)
changed the title
Contributor
📝 WalkthroughWalkthroughThis PR adds typed authentication APIs, a new OIDC server plugin, and expanded HTTP auth algorithm support. It also updates build wiring, test-host behavior, and adds coverage for the new flows. ChangesHTTP Auth Algorithms
Typesafe authentication APIs
OIDC server plugin
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Suggested labels
Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Pantus Oleh (zibet27)
requested review from
Bruce Hamilton (bjhham) and
Simon Vergauwen (nomisRev)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (10)
ktor-server/ktor-server-test-host/common/test/TestApplicationTest.kt (1)
177-178: ⚡ Quick winUse a backticked descriptive test name for the new test.
Please rename
testCanAccessExternalServicesWhileStartingApplicationto backticked style (for example,can access external services while starting application) to match repository test naming conventions.As per coding guidelines,
**/*Test.{kt,kts}should “Prefer descriptive test names in backticks:describe what is being tested.”🤖 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-server/ktor-server-test-host/common/test/TestApplicationTest.kt` around lines 177 - 178, Rename the test function testCanAccessExternalServicesWhileStartingApplication to a backticked descriptive name per project convention—e.g. change fun testCanAccessExternalServicesWhileStartingApplication() = testApplication { ... } to fun `can access external services while starting application`() = testApplication { ... }—keeping the `@Test` annotation and the existing testApplication body intact so only the function identifier changes.Source: Coding guidelines
ktor-server/ktor-server-plugins/ktor-server-auth-jwt/jvm/test/io/ktor/server/auth/jwt/TypedJwtAuthTest.kt (1)
5-5: ⚡ Quick winNarrow
@OptInscope from file-level to the smallest required declaration.Line 5 applies
@OptInto the whole file; please move it to the class or only the tests that require it.Suggested change
-@file:OptIn(ExperimentalKtorApi::class) - package io.ktor.server.auth.jwt @@ -class TypedJwtAuthTest { +@OptIn(ExperimentalKtorApi::class) +class TypedJwtAuthTest {As per coding guidelines, keep
@OptIn(...)annotation scope minimal.🤖 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-server/ktor-server-plugins/ktor-server-auth-jwt/jvm/test/io/ktor/server/auth/jwt/TypedJwtAuthTest.kt` at line 5, Remove the file-level `@OptIn`(ExperimentalKtorApi::class) and apply it to the narrowest scope that needs it—either annotate the TypedJwtAuthTest class declaration with `@OptIn`(ExperimentalKtorApi::class) or add the annotation to only the individual test functions that require ExperimentalKtorApi features (e.g., the test methods inside TypedJwtAuthTest); ensure you delete the top-of-file annotation and place the same annotation directly on the class or specific test functions.Source: Coding guidelines
ktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/OAuthFlow.kt (1)
19-26: ⚡ Quick winMake
SessionCreatorOAuth2-specific.
OAuth2SessionFlowonly ever suppliesOAuthAccessTokenResponse.OAuth2, so exposing the sealed supertype here unnecessarily widens this new public API and forces downstream casts. Tightening it now keeps the contract aligned with the flow’s actual behavior.As per coding guidelines, API changes must be intentional and well-documented.
🤖 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-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/OAuthFlow.kt` around lines 19 - 26, The type alias SessionCreator<S> is too broad because OAuth2SessionFlow always provides OAuthAccessTokenResponse.OAuth2; change the alias to only accept OAuthAccessTokenResponse.OAuth2 so consumers don't need casts. Update the declaration of SessionCreator to use OAuthAccessTokenResponse.OAuth2 as the parameter type and adjust any usages in OAuth2SessionFlow and related callers to match the narrowed signature (look for SessionCreator, OAuth2SessionFlow, and places passing/receiving OAuthAccessTokenResponse).Source: Coding guidelines
ktor-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/typesafe/OAuthFlowTest.kt (2)
49-52: 💤 Low valueUnsafe cast to OAuth2 token response type.
The cast
token as OAuthAccessTokenResponse.OAuth2at line 50 will throwClassCastExceptionif the token response is not OAuth2 (e.g., OAuth1a). While this is acceptable in a test helper that only handles OAuth2 flows, consider using a safe cast with error message for better test failure diagnostics.♻️ Optional: add explicit check for clearer test failures
sessionCreator = { token -> - val oauth2 = token as OAuthAccessTokenResponse.OAuth2 + val oauth2 = token as? OAuthAccessTokenResponse.OAuth2 + ?: error("Expected OAuth2 token response, got ${token::class.simpleName}") OAuthSession(accessToken = oauth2.accessToken) }🤖 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-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/typesafe/OAuthFlowTest.kt` around lines 49 - 52, Replace the unsafe cast in the sessionCreator lambda by performing a safe cast of token to OAuthAccessTokenResponse.OAuth2 and failing with a clear error if it is null; specifically update the sessionCreator { token -> val oauth2 = token as? OAuthAccessTokenResponse.OAuth2 ?: error("Expected OAuth2 token response but got ${token?.javaClass}") ; OAuthSession(accessToken = oauth2.accessToken) } so tests fail with a descriptive message rather than a ClassCastException (referencing sessionCreator, OAuthAccessTokenResponse.OAuth2, and OAuthSession).
26-29: ⚡ Quick winPrefer
internalvisibility for test fixture types.
OAuthSessionandOAuthPrincipalare only used within the test package. As per coding guidelines, preferinternalvisibility by default to keep the public surface intentional.♻️ Refactor to use internal visibility
-@Serializable -data class OAuthSession(val accessToken: String) +@Serializable +internal data class OAuthSession(val accessToken: String) -data class OAuthPrincipal(val token: String, val source: String) +internal data class OAuthPrincipal(val token: String, val source: String)🤖 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-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/typesafe/OAuthFlowTest.kt` around lines 26 - 29, Make the two test fixture types non-public by changing their visibility to internal: update the declarations of OAuthSession and OAuthPrincipal so they are declared as internal (keep `@Serializable` on OAuthSession), ensuring they remain accessible within the test module but not exposed publicly.Source: Coding guidelines
ktor-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/typesafe/OptionalAndAnonymousAuthTest.kt (1)
21-23: ⚡ Quick winPrefer
internalvisibility for test fixture types.
AnonTestIdentity,AuthenticatedUser, andGuestUserare only used within the test package. As per coding guidelines, preferinternalvisibility by default to keep the public surface intentional.♻️ Refactor to use internal visibility
-interface AnonTestIdentity -data class AuthenticatedUser(val id: String) : AnonTestIdentity -data class GuestUser(val label: String = "guest") : AnonTestIdentity +internal interface AnonTestIdentity +internal data class AuthenticatedUser(val id: String) : AnonTestIdentity +internal data class GuestUser(val label: String = "guest") : AnonTestIdentity🤖 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-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/typesafe/OptionalAndAnonymousAuthTest.kt` around lines 21 - 23, The test fixture types AnonTestIdentity, AuthenticatedUser, and GuestUser should use internal visibility instead of public; locate the declarations of interface AnonTestIdentity and data classes AuthenticatedUser and GuestUser in OptionalAndAnonymousAuthTest.kt and change their visibility modifier to internal so they are confined to the test module.Source: Coding guidelines
ktor-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/typesafe/MultipleSchemeTest.kt (1)
22-27: ⚡ Quick winPrefer
internalvisibility for test fixture types.The
AppUserinterface andBasicUser/BearerUserdata classes are only used within this test file. As per coding guidelines, preferinternalvisibility by default to keep the public surface intentional.♻️ Refactor to use internal visibility
- interface AppUser { + internal interface AppUser { val email: String } - data class BasicUser(override val email: String) : AppUser - data class BearerUser(override val email: String) : AppUser + internal data class BasicUser(override val email: String) : AppUser + internal data class BearerUser(override val email: String) : AppUser🤖 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-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/typesafe/MultipleSchemeTest.kt` around lines 22 - 27, The test fixture types AppUser, BasicUser, and BearerUser are declared public but only used in this test file; change their visibility to internal by adding the internal modifier to the interface AppUser and the data classes BasicUser and BearerUser so they are package-internal and not part of the public API surface.Source: Coding guidelines
ktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/SessionAuthScheme.kt (1)
57-58: 💤 Low valueConsider adding an error message to
checkNotNull.The
checkNotNullcall at line 57 validates thatconfig.contextFactoryis non-null but provides no message. While this is internal framework wiring and the factory is expected to be set, an error message would make debugging easier if this invariant is ever violated.♻️ Proposed improvement
`@OptIn`(InternalAPI::class) - checkNotNull(config.contextFactory)(default) + checkNotNull(config.contextFactory) { + "Context factory not set for session auth scheme '$name'" + }(default)🤖 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-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/SessionAuthScheme.kt` around lines 57 - 58, checkNotNull(config.contextFactory) lacks an error message; update the call in SessionAuthScheme.kt (the checkNotNull(config.contextFactory)(default) invocation) to include a descriptive message so a thrown IllegalStateException clearly states that config.contextFactory (the session context factory) is required/was not provided for the SessionAuthScheme initialization.ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/utils/OidcTestUtils.kt (1)
41-49: 💤 Low valueConsider whether
openIdHttpClientneeds to be a separate function.Line 49 shows
openIdHttpClientsimply delegates todiscoveryClient()with identical behavior. If they're truly equivalent, consider using a single function or documenting why both names exist.🤖 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-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/utils/OidcTestUtils.kt` around lines 41 - 49, openIdHttpClient currently just delegates to discoveryClient, creating redundant API surface; either delete openIdHttpClient and update all call sites to use discoveryClient directly, or if the separate name is intentionally part of the test API, keep it but add a KDoc above openIdHttpClient explaining its semantic purpose and why it forwards to discoveryClient; locate the functions noRedirectsClient, discoveryClient and openIdHttpClient and apply the chosen change so there's no unused/duplicative wrapper.ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/utils/OidcTestLogging.kt (1)
17-17: 💤 Low valueConsider stopping the appender in
close().The
ListAppenderis started at line 17 but never stopped. While this may not cause issues in tests, it's good practice to stop appenders when they're detached.♻️ Suggested fix
override fun close() { logger.detachAppender(appender) + appender.stop() logger.level = previousLevel }🤖 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-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/utils/OidcTestLogging.kt` at line 17, The ListAppender instance named appender in OidcTestLogging is started but never stopped; update the close() method to call appender.stop() (and then detach it if applicable) so the ListAppender<ILoggingEvent> is properly stopped when OidcTestLogging.close() runs—locate the appender property and the close() method to add the stop() call.
🤖 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.
Inline comments:
In
`@ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingNode.kt`:
- Around line 268-273: Add KDoc for the new public extension
ApplicationCall.toRoutingContext describing what it does: converts an
ApplicationCall into a RoutingContext, supporting RoutingCall and
RoutingPipelineCall (wrapping RoutingPipelineCall into a RoutingCall), and
throwing an error for other call types; reference the involved symbols
(toRoutingContext, RoutingContext, RoutingCall, RoutingPipelineCall) and include
a brief usage note and a link to the report as per public API guidelines. Ensure
the KDoc is placed directly above the toRoutingContext declaration and follows
existing KDoc style conventions.
In `@ktor-server/ktor-server-plugins/ktor-server-auth-api-key/build.gradle.kts`:
- Around line 11-13: The build script is enabling the Kotlin compiler flag via
compilerOptions.freeCompilerArgs.add("-Xcontext-parameters") even though this
module has no context receiver usages (the only related token is
withDefaultContext(...)); remove the
freeCompilerArgs.add("-Xcontext-parameters") line from the module's
build.gradle.kts to avoid an unused flag, or if there is a deliberate
cross-module requirement, add a short justification comment next to the
compilerOptions entry and move the flag to a parent/common gradle file so only
modules that need it (those that declare context(...) receivers) inherit it;
locate the compilerOptions block and either delete the freeCompilerArgs.add(...)
call or replace it with a documented justification and relocate it to a shared
build script.
In
`@ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/Oidc.kt`:
- Around line 545-553: defaultOpenIdHttpClient() creates the managed HttpClient
without timeouts, so calls like fetchOpenIdMetadata(...) can hang indefinitely;
update defaultOpenIdHttpClient() to install the HttpTimeout plugin (or wrap
fetchOpenIdMetadata calls in a coroutine timeout) and configure sensible
defaults (e.g., connect/read/request timeouts) so provider discovery and
periodic refresh cannot block forever — reference defaultOpenIdHttpClient and
fetchOpenIdMetadata when making the change and ensure the timeout values are
configurable or reasonable defaults.
In
`@ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcConfig.kt`:
- Around line 183-194: The validate() logic currently skips the "openid" scope
check when accessTokenConfig is present, which allows session-backed OAuth to
pass validation but fail later in createSessions(); update validation so that
whenever oauthConfig indicates sessions are enabled (e.g., sessions != null or
sessionsEnabled flag) you require the "openid" scope be present on the token
source used to create sessions: add a check in OidcConfig.validate (near where
accessTokenConfig and oauthConfig are validated) to assert that if
oauthConfig.sessions is enabled then either accessTokenConfig.scopes (or the
effective OAuth scopes validated by OidcOAuthConfig.validate) contains "openid";
mirror this change in OidcOAuthConfig.validate (and the other location noted
around lines 567-576) so session-backed flows cannot pass without "openid"
before createSessions() runs.
- Around line 70-77: The validate() function currently only checks
initialDiscoveryAttempts and initialDiscoveryRetryDelay; extend it to also
validate discoveryRefreshInterval and discoveryRefreshFailureDelay so invalid
values are rejected up front (matching how startRefreshingMetadata() later uses
them). Specifically, in validate() add finite/non-negative checks for
discoveryRefreshFailureDelay (isFinite && !isNegative) and for
discoveryRefreshInterval (isFinite && !isNegative) and ensure the error messages
reference the correct symbol names; keep existing checks for
initialDiscoveryAttempts and initialDiscoveryRetryDelay intact so all four
durations/attempts are validated before startRefreshingMetadata() runs.
In
`@ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcSessionRefresh.kt`:
- Around line 54-58: The current branch in OidcSessionRefresh.kt that reads
token.refreshToken and calls clearOidcSession() incorrectly invalidates a
still-valid session when no refresh token is issued; change the behavior so that
when token.refreshToken is null you log the absence (logger.debug(...)) and
return without clearing the session (do not call clearOidcSession()), allowing
the session to remain valid until the ID token's exp; alternatively, if you
prefer to treat "Auto" as invalid without refresh tokens, validate this
configuration earlier and throw/reject at setup time rather than clearing
sessions here (refer to the refreshTokenValue check, token.refreshToken, and
clearOidcSession()).
In
`@ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcTokenModels.kt`:
- Line 1: This file (OidcTokenModels.kt) is missing the repository copyright
header; add the standard project header comment block at the top of the file
before the existing file annotation (`@file`:OptIn(ExperimentalTime::class)) so
the header precedes any package or file annotations, ensure the header text
matches the repo's canonical copyright/license wording used in other Kotlin
sources, and keep the rest of the file (including `@file`:OptIn and any
package/imports) unchanged.
- Around line 110-114: The parseJsonObject function currently decodes JWT
segments using the standard Base64 alphabet, which fails for Base64url-encoded
JWT header/payload and causes silent empty-claim fallbacks; change the decoder
used in parseJsonObject to the URL-safe Base64 variant (preserving the
absent-padding option) so that DecodedJWT.header and DecodedJWT.payload are
decoded correctly and claim(...)/claimString(...)/headerString(...) no longer
return empty JsonObject on valid tokens.
In
`@ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcTokens.kt`:
- Around line 441-443: The code appends postLogoutRedirectUri without validating
it; update buildLogoutUrl (OidcTokens.kt) to validate that postLogoutRedirectUri
is an absolute URI before appending: add a require(...) check right before the
parameters.append("post_logout_redirect_uri", uri) call that parses the string
(e.g., via java.net.URI or Ktor Url parsing) and asserts isAbsolute (or scheme
present), and include the actual value in the require message so callers get an
actionable error referencing the invalid postLogoutRedirectUri.
- Around line 103-110: The fallback branch that handles access-token-only
callbacks (where idToken is null) skips token-type validation; update the logic
in OidcTokens.kt so that requireBearerTokenType(response.tokenType) is invoked
before accepting an access token in the idToken fallback path (i.e., call
requireBearerTokenType either before reading
response.extraParameters["id_token"] or inside the run { ... } fallback before
verifyAccessToken(response.accessToken) returns) so non-Bearer token_type values
are rejected; reference symbols: oauthConfig, idToken, response.extraParameters,
verifyAccessToken, requireBearerTokenType.
In
`@ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/OidcJwkProviderConfigTest.kt`:
- Around line 169-170: The test currently reserves a port with ServerSocket(0)
then closes it before calling embeddedServer(CIO, port = port), introducing a
TOCTOU race; instead pass port = 0 directly to embeddedServer
(embeddedServer(CIO, port = 0) { ... }) so the OS assigns a free port
atomically, remove the ServerSocket allocation and the local port variable, and
if the test needs the actual bound port later retrieve it after starting the
server (e.g. from server.resolvedConnectors.first().port or the server's
connector info).
In
`@ktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/SessionAuth.kt`:
- Around line 65-71: The check in CurrentSession.getSessionByName logs the
actual session instance (`$session`) on type mismatch which may leak sensitive
data; change the error message to avoid including the session value and instead
report only the expected and actual runtime types (e.g., use session::class or
session.javaClass to show the session's type) in the check() call inside
getSessionByName so the message remains actionable without exposing session
contents.
In
`@ktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/OAuthFlow.kt`:
- Around line 379-405: You are persisting the session too early: move the
call.sessions.set(flow.sessions, session) so it only runs after principal
resolution succeeds (after val principal = flow.principalResolver(... ) returns
non-null) and only after any subsequent failing work that must not leave a
session (e.g., checkNotNull(flow.config.contextFactory) and
flow.sessions.capture). Also ensure that if an exception occurs after setting
the session you remove it (call.sessions.clear or equivalent) before
returning/failing; update the block around sessionCreator, principalResolver,
flow.sessions.set, call.attributes.put, flow.sessions.capture,
DefaultSessionAuthenticatedContext and the contextFactory onSuccess/onFailure
handling to enforce atomicity (set session only on final success, clear on
errors).
In
`@ktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/OptionalAuthScheme.kt`:
- Around line 113-114: The principalKey uses TypeInfo(Any::class) which drops
the generic B runtime type; change the constructor of AnonymousAuthScheme (or
the enclosing class that creates principalKey) to accept a TypeInfo<B> parameter
(e.g., principalTypeInfo) and use that when constructing principalKey
(AttributeKey("TypesafeAuth:$name:AnonymousPrincipal", principalTypeInfo)) so
the AttributeKey carries the actual principal type at runtime; alternatively, if
you intentionally want Any, add a concise doc comment on
AnonymousAuthScheme/principalKey explaining why Any::class is used and the
runtime implications.
In
`@ktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/TypedAuthInterceptors.kt`:
- Around line 19-32: The current TypedAuthPluginNameGenerator uses a mutable Int
field nextId and increments it unsafely in next(kind: String, parts:
List<String>), which can race; replace nextId with an AtomicInteger (e.g.,
private val nextId = AtomicInteger(0)) and change the increment to a thread-safe
getAndIncrement() call inside TypedAuthPluginNameGenerator.next to ensure unique
plugin names under concurrent route creation.
In
`@ktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/TypedSessionAuthConfig.kt`:
- Around line 119-123: The session rewrite check in TypedSessionAuthConfig
currently uses structural equality (`!=`) so a new instance with equal values
won't be persisted; change the comparison to referential inequality (`!==`) when
deciding to call CurrentSession.set so the session is rewritten only when a
different instance is returned by the transformation block (refer to the code
handling the block result and the call to
io.ktor.server.sessions.CurrentSession.set).
- Around line 71-81: The KDoc advertises configurable DSL knobs but the
properties are internal, so make the advertised knobs public: change
TypedSessionAuthConfig.description and TypedSessionAuthConfig.onUnauthorized
from internal to public (or provide public setters) while keeping their types
(String? and UnauthorizedHandler?) and existing KDoc; ensure authenticateWith's
route-level onUnauthorized still overrides the now-public config property.
---
Nitpick comments:
In
`@ktor-server/ktor-server-plugins/ktor-server-auth-jwt/jvm/test/io/ktor/server/auth/jwt/TypedJwtAuthTest.kt`:
- Line 5: Remove the file-level `@OptIn`(ExperimentalKtorApi::class) and apply it
to the narrowest scope that needs it—either annotate the TypedJwtAuthTest class
declaration with `@OptIn`(ExperimentalKtorApi::class) or add the annotation to
only the individual test functions that require ExperimentalKtorApi features
(e.g., the test methods inside TypedJwtAuthTest); ensure you delete the
top-of-file annotation and place the same annotation directly on the class or
specific test functions.
In
`@ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/utils/OidcTestLogging.kt`:
- Line 17: The ListAppender instance named appender in OidcTestLogging is
started but never stopped; update the close() method to call appender.stop()
(and then detach it if applicable) so the ListAppender<ILoggingEvent> is
properly stopped when OidcTestLogging.close() runs—locate the appender property
and the close() method to add the stop() call.
In
`@ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/utils/OidcTestUtils.kt`:
- Around line 41-49: openIdHttpClient currently just delegates to
discoveryClient, creating redundant API surface; either delete openIdHttpClient
and update all call sites to use discoveryClient directly, or if the separate
name is intentionally part of the test API, keep it but add a KDoc above
openIdHttpClient explaining its semantic purpose and why it forwards to
discoveryClient; locate the functions noRedirectsClient, discoveryClient and
openIdHttpClient and apply the chosen change so there's no unused/duplicative
wrapper.
In
`@ktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/OAuthFlow.kt`:
- Around line 19-26: The type alias SessionCreator<S> is too broad because
OAuth2SessionFlow always provides OAuthAccessTokenResponse.OAuth2; change the
alias to only accept OAuthAccessTokenResponse.OAuth2 so consumers don't need
casts. Update the declaration of SessionCreator to use
OAuthAccessTokenResponse.OAuth2 as the parameter type and adjust any usages in
OAuth2SessionFlow and related callers to match the narrowed signature (look for
SessionCreator, OAuth2SessionFlow, and places passing/receiving
OAuthAccessTokenResponse).
In
`@ktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/SessionAuthScheme.kt`:
- Around line 57-58: checkNotNull(config.contextFactory) lacks an error message;
update the call in SessionAuthScheme.kt (the
checkNotNull(config.contextFactory)(default) invocation) to include a
descriptive message so a thrown IllegalStateException clearly states that
config.contextFactory (the session context factory) is required/was not provided
for the SessionAuthScheme initialization.
In
`@ktor-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/typesafe/MultipleSchemeTest.kt`:
- Around line 22-27: The test fixture types AppUser, BasicUser, and BearerUser
are declared public but only used in this test file; change their visibility to
internal by adding the internal modifier to the interface AppUser and the data
classes BasicUser and BearerUser so they are package-internal and not part of
the public API surface.
In
`@ktor-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/typesafe/OAuthFlowTest.kt`:
- Around line 49-52: Replace the unsafe cast in the sessionCreator lambda by
performing a safe cast of token to OAuthAccessTokenResponse.OAuth2 and failing
with a clear error if it is null; specifically update the sessionCreator { token
-> val oauth2 = token as? OAuthAccessTokenResponse.OAuth2 ?: error("Expected
OAuth2 token response but got ${token?.javaClass}") ; OAuthSession(accessToken =
oauth2.accessToken) } so tests fail with a descriptive message rather than a
ClassCastException (referencing sessionCreator, OAuthAccessTokenResponse.OAuth2,
and OAuthSession).
- Around line 26-29: Make the two test fixture types non-public by changing
their visibility to internal: update the declarations of OAuthSession and
OAuthPrincipal so they are declared as internal (keep `@Serializable` on
OAuthSession), ensuring they remain accessible within the test module but not
exposed publicly.
In
`@ktor-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/typesafe/OptionalAndAnonymousAuthTest.kt`:
- Around line 21-23: The test fixture types AnonTestIdentity, AuthenticatedUser,
and GuestUser should use internal visibility instead of public; locate the
declarations of interface AnonTestIdentity and data classes AuthenticatedUser
and GuestUser in OptionalAndAnonymousAuthTest.kt and change their visibility
modifier to internal so they are confined to the test module.
In `@ktor-server/ktor-server-test-host/common/test/TestApplicationTest.kt`:
- Around line 177-178: Rename the test function
testCanAccessExternalServicesWhileStartingApplication to a backticked
descriptive name per project convention—e.g. change fun
testCanAccessExternalServicesWhileStartingApplication() = testApplication { ...
} to fun `can access external services while starting application`() =
testApplication { ... }—keeping the `@Test` annotation and the existing
testApplication body intact so only the function identifier changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0e8b7958-041a-4774-8e3a-39a812d43df8
📒 Files selected for processing (106)
gradle/artifacts/publishJvmAndCommonPublications.txtktor-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.ktktor-server/ktor-server-core/api/ktor-server-core.apiktor-server/ktor-server-core/api/ktor-server-core.klib.apiktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingNode.ktktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingPipelineCall.ktktor-server/ktor-server-plugins/ktor-server-auth-api-key/api/ktor-server-auth-api-key.apiktor-server/ktor-server-plugins/ktor-server-auth-api-key/api/ktor-server-auth-api-key.klib.apiktor-server/ktor-server-plugins/ktor-server-auth-api-key/build.gradle.ktsktor-server/ktor-server-plugins/ktor-server-auth-api-key/common/src/io/ktor/server/auth/apikey/ApiKeyAuth.ktktor-server/ktor-server-plugins/ktor-server-auth-api-key/common/src/io/ktor/server/auth/apikey/typesafe/ApiKeyTypedProvider.ktktor-server/ktor-server-plugins/ktor-server-auth-api-key/common/src/io/ktor/server/auth/apikey/typesafe/TypedApiKeyAuthConfig.ktktor-server/ktor-server-plugins/ktor-server-auth-api-key/common/test/io/ktor/server/auth/apikey/ApiKeyAuthTest.ktktor-server/ktor-server-plugins/ktor-server-auth-api-key/common/test/io/ktor/server/auth/apikey/TypedApiKeyAuthTest.ktktor-server/ktor-server-plugins/ktor-server-auth-jwt/api/ktor-server-auth-jwt.apiktor-server/ktor-server-plugins/ktor-server-auth-jwt/build.gradle.ktsktor-server/ktor-server-plugins/ktor-server-auth-jwt/jvm/src/io/ktor/server/auth/jwt/typesafe/JwtTypedProvider.ktktor-server/ktor-server-plugins/ktor-server-auth-jwt/jvm/src/io/ktor/server/auth/jwt/typesafe/TypedJwtAuthConfig.ktktor-server/ktor-server-plugins/ktor-server-auth-jwt/jvm/test/io/ktor/server/auth/jwt/TypedJwtAuthTest.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/api/ktor-server-auth-oidc.apiktor-server/ktor-server-plugins/ktor-server-auth-oidc/build.gradle.ktsktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/Oidc.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcAuthenticatedContext.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcAuthorizationTransaction.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcBearer.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcConfig.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcOAuth.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcProvider.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcSessionRefresh.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcStateCodec.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcStateEncryptionKey.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcToken.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcTokenModels.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcTokenRefreshStrategy.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcTokens.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OpenIdProviderMetadata.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OpenIdTestKeys.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/TokenRefreshResponse.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/FetchOpenIdProviderMetadataTest.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/OidcBearerJwtTest.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/OidcConfigValidationTest.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/OidcDiscoveryTest.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/OidcEnvironmentConfigTest.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/OidcJwkProviderConfigTest.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/OidcJwtKeyAndAlgorithmTest.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/OidcOAuthAccessTokenFallbackTest.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/OidcOAuthCallbackSecurityTest.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/OidcOAuthCallbackTest.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/OidcPluginRegistrationTest.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/OidcSessionRoutesTest.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/OidcStateCodecTest.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/OidcUserInfoTest.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/utils/OidcSessionTestUtils.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/utils/OidcTestKeys.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/utils/OidcTestLogging.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/utils/OidcTestOAuthFlow.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/utils/OidcTestProviders.ktktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/utils/OidcTestUtils.ktktor-server/ktor-server-plugins/ktor-server-auth/api/ktor-server-auth.apiktor-server/ktor-server-plugins/ktor-server-auth/api/ktor-server-auth.klib.apiktor-server/ktor-server-plugins/ktor-server-auth/build.gradle.ktsktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/AuthenticationInterceptors.ktktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/OAuth2.ktktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/OAuthCommon.ktktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/OAuthProcedure.ktktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/SessionAuth.ktktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/AuthScheme.ktktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/BasicTypedProvider.ktktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/OAuthFlow.ktktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/OptionalAuthScheme.ktktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/RoleBasedAuthScheme.ktktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/RouteBuilders.ktktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/Scopes.ktktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/SessionAuthScheme.ktktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/SessionsConfigTypesafeExtensions.ktktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/TypedAuthInterceptors.ktktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/TypedBasicAuthConfig.ktktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/TypedBearerAuthConfig.ktktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/TypedFormAuthConfig.ktktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/TypedSessionAuthConfig.ktktor-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/BasicAuthTest.ktktor-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/CryptoTest.ktktor-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/OAuth2Test.ktktor-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/typesafe/BasicSchemesTest.ktktor-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/typesafe/LegacyApiCoexistenceTest.ktktor-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/typesafe/MultipleSchemeTest.ktktor-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/typesafe/NestedRoutesTest.ktktor-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/typesafe/OAuthFlowTest.ktktor-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/typesafe/OptionalAndAnonymousAuthTest.ktktor-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/typesafe/RoleBasedAuthTest.ktktor-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/typesafe/TestUtils.ktktor-server/ktor-server-plugins/ktor-server-auth/common/test/io/ktor/tests/auth/typesafe/UnauthorizedAndChallengesTest.ktktor-server/ktor-server-plugins/ktor-server-auth/jvm/src/io/ktor/server/auth/typesafe/DigestTypedProvider.ktktor-server/ktor-server-plugins/ktor-server-auth/jvm/src/io/ktor/server/auth/typesafe/TypedDigestAuthConfig.ktktor-server/ktor-server-plugins/ktor-server-auth/jvm/test/io/ktor/tests/auth/DigestTest.ktktor-server/ktor-server-plugins/ktor-server-auth/jvm/test/io/ktor/tests/auth/typesafe/DigestAuthTest.ktktor-server/ktor-server-test-host/common/src/io/ktor/server/testing/client/DelegatingTestClientEngine.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktsettings.gradle.kts
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
🛑 Comments failed to post (17)
ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingNode.kt (1)
268-273:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd KDoc for the new public
toRoutingContextAPI.Line 269 introduces a new public extension but it is undocumented, including its supported call types and failure behavior.
✍️ Suggested patch
+/** + * Converts an [ApplicationCall] into a [RoutingContext] when used inside the routing pipeline. + * + * Supports [RoutingCall] and [RoutingPipelineCall]. Fails for non-routing calls. + * + * [Report a problem](https://ktor.io/feedback/?fqname=io.ktor.server.routing.toRoutingContext) + */ `@OptIn`(InternalAPI::class) public fun ApplicationCall.toRoutingContext(): RoutingContext = when (this) { is RoutingCall -> RoutingContext(call = this) is RoutingPipelineCall -> RoutingContext(call = RoutingCall(this)) else -> error("Expected a routing call, but got ${this::class.simpleName}") }As per coding guidelines, public API changes must be documented with KDoc; based on learnings, public extension KDoc should include a brief description and a report link.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements./** * Converts an [ApplicationCall] into a [RoutingContext] when used inside the routing pipeline. * * Supports [RoutingCall] and [RoutingPipelineCall]. Fails for non-routing calls. * * [Report a problem](https://ktor.io/feedback/?fqname=io.ktor.server.routing.toRoutingContext) */ `@OptIn`(InternalAPI::class) public fun ApplicationCall.toRoutingContext(): RoutingContext = when (this) { is RoutingCall -> RoutingContext(call = this) is RoutingPipelineCall -> RoutingContext(call = RoutingCall(this)) else -> error("Expected a routing call, but got ${this::class.simpleName}") }🤖 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-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingNode.kt` around lines 268 - 273, Add KDoc for the new public extension ApplicationCall.toRoutingContext describing what it does: converts an ApplicationCall into a RoutingContext, supporting RoutingCall and RoutingPipelineCall (wrapping RoutingPipelineCall into a RoutingCall), and throwing an error for other call types; reference the involved symbols (toRoutingContext, RoutingContext, RoutingCall, RoutingPipelineCall) and include a brief usage note and a link to the report as per public API guidelines. Ensure the KDoc is placed directly above the toRoutingContext declaration and follows existing KDoc style conventions.Sources: Coding guidelines, Learnings
ktor-server/ktor-server-plugins/ktor-server-auth-api-key/build.gradle.kts (1)
11-13:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick win🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Search for context parameter usage in the auth-api-key module # Check for context(...) declarations rg -nP 'context\s*\(' ktor-server/ktor-server-plugins/ktor-server-auth-api-key/ # Check for context receiver syntax rg -nP 'context\s*\([^)]+\)\s*(class|fun|val|var)' ktor-server/ktor-server-plugins/ktor-server-auth-api-key/Repository: ktorio/ktor
Length of output: 37
🏁 Script executed:
#!/bin/bash set -euo pipefail MOD="ktor-server/ktor-server-plugins/ktor-server-auth-api-key" echo "== gradle snippet ==" nl -ba "$MOD/build.gradle.kts" | sed -n '1,80p' echo echo "== Kotlin files in module ==" find "$MOD" -type f \( -name "*.kt" -o -name "*.kts" \) | sed 's|^\./||' | sort echo echo "== search: context( ... in Kotlin sources ==" rg -n "context\s*\(" "$MOD" -S --type-add 'kotlin:*.kt;*.kts' --type kotlin || true echo echo "== search: -Xcontext-parameters mentions ==" rg -n --hidden "-Xcontext-parameters" "$MOD" || true echo echo "== search: context receiver docs/strings ==" rg -n -S "context receiver|context-parameters|context parameters" "$MOD" || true echo echo "== repo-wide occurrences of -Xcontext-parameters ==" rg -n --hidden "-Xcontext-parameters" . || trueRepository: ktorio/ktor
Length of output: 137
🏁 Script executed:
#!/bin/bash set -euo pipefail MOD="ktor-server/ktor-server-plugins/ktor-server-auth-api-key" echo "== gradle snippet (build.gradle.kts) ==" # show first 80 lines with line numbers via awk awk 'NR<=80 {printf "%4d\t%s\n", NR, $0}' "$MOD/build.gradle.kts" echo echo "== Kotlin files in module ==" find "$MOD" -type f \( -name "*.kt" -o -name "*.kts" \) | sed 's|^\./||' | sort echo echo "== search: context( in Kotlin sources (broader regex) ==" rg -n --no-heading --color=never "context\s*\(" "$MOD" -S --type-add 'kotlin:*.kt;*.kts' --type kotlin || true echo echo "== search: context receiver / context-parameters mentions ==" rg -n --no-heading --color=never "context receiver|context-parameters|context parameters" "$MOD" -S || true echo echo "== search: -Xcontext-parameters mentions (module) ==" rg -n --no-heading --color=never "-Xcontext-parameters" "$MOD" || true echo echo "== repo-wide occurrences of -Xcontext-parameters ==" rg -n --no-heading --color=never "-Xcontext-parameters" . || trueRepository: ktorio/ktor
Length of output: 3128
🏁 Script executed:
#!/bin/bash set -euo pipefail MOD="ktor-server/ktor-server-plugins/ktor-server-auth-api-key" FILE="$MOD/common/src/io/ktor/server/auth/apikey/typesafe/ApiKeyTypedProvider.kt" echo "== Context keyword matches in module (word boundary) ==" rg -n --no-heading --color=never -S --word-regexp 'context\s*\(' "$MOD" --type-add 'kotlin:*.kt;*.kts' --type kotlin || true echo echo "== Context receiver declaration shape (word boundary) ==" rg -n --no-heading --color=never -S --word-regexp 'context\s*\([^)]+\)\s*(class|interface|fun|val|var|object)\b' "$MOD" --type-add 'kotlin:*.kt;*.kts' --type kotlin || true echo echo "== Literal -Xcontext-parameters mentions (module) ==" rg -n --no-heading --color=never -S -e '-Xcontext-parameters' "$MOD" || true echo echo "== Literal -Xcontext-parameters mentions (repo-wide) ==" rg -n --no-heading --color=never -S -e '-Xcontext-parameters' . || true echo echo "== Inspect matched line vicinity around ApiKeyTypedProvider.kt line 29 ==" awk 'NR>=20 && NR<=40 {printf "%4d\t%s\n", NR, $0}' "$FILE"Repository: ktorio/ktor
Length of output: 1729
Remove or justify unused
-Xcontext-parametersin auth-api-key build.gradle.kts
ktor-server/ktor-server-plugins/ktor-server-auth-api-key/build.gradle.ktsadds-Xcontext-parameters, but this module has no Kotlincontext(...)/ context receiver declarations (the onlycontextoccurrence iswithDefaultContext(...)). Remove the flag from this module or add a justification for why it’s required.🤖 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-server/ktor-server-plugins/ktor-server-auth-api-key/build.gradle.kts` around lines 11 - 13, The build script is enabling the Kotlin compiler flag via compilerOptions.freeCompilerArgs.add("-Xcontext-parameters") even though this module has no context receiver usages (the only related token is withDefaultContext(...)); remove the freeCompilerArgs.add("-Xcontext-parameters") line from the module's build.gradle.kts to avoid an unused flag, or if there is a deliberate cross-module requirement, add a short justification comment next to the compilerOptions entry and move the flag to a parent/common gradle file so only modules that need it (those that declare context(...) receivers) inherit it; locate the compilerOptions block and either delete the freeCompilerArgs.add(...) call or replace it with a documented justification and relocate it to a shared build script.ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/Oidc.kt (1)
545-553:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "Existing timeout patterns in the repo:" rg -n "HttpTimeout|requestTimeout|connectTimeout|socketTimeout" --glob '*.kt' --glob '*.kts'Repository: ktorio/ktor
Length of output: 50367
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE="ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/Oidc.kt" echo "Matches for defaultOpenIdHttpClient():" rg -n "defaultOpenIdHttpClient" "$FILE" echo echo "Matches for HttpTimeout or timeout helpers:" rg -n "HttpTimeout|withTimeout|withTimeoutOrNull|requestTimeout|connectTimeout|socketTimeout|timeoutMillis|TimeoutConfig" "$FILE" echo echo "Matches for retry/discovery refresh usage sites:" rg -n "discovery|refresh|metadata|provider\\(|openid|jwks|retry|HttpRequestRetry" "$FILE"Repository: ktorio/ktor
Length of output: 291
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE="ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/Oidc.kt" echo "=== Oidc.kt: around managedClient assignment (lines ~400-480) ===" sed -n '380,480p' "$FILE" | nl -ba | sed -n '1,120p' echo echo "=== Oidc.kt: around defaultOpenIdHttpClient definition (lines ~520-600) ===" sed -n '510,600p' "$FILE" | nl -ba | sed -n '1,120p' echo echo "=== Oidc.kt: search for discovery/refresh + managedClient usage ===" rg -n "managedClient|discovery|refresh|metadata|provider\\(|jwks|openid|retry" "$FILE" || true echo echo "=== Oidc.kt: search for timeouts/coroutine timeout helpers in-file ===" rg -n "HttpTimeout|withTimeout|withTimeoutOrNull|requestTimeout|connectTimeout|socketTimeout|timeoutMillis|TimeoutConfig|withContext\\s*\\{.*timeout" "$FILE" || trueRepository: ktorio/ktor
Length of output: 182
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE="ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/Oidc.kt" echo "=== Oidc.kt: around managedClient assignment (approx lines 400-480) ===" awk 'NR>=380 && NR<=480 {printf "%6d %s\n", NR, $0}' "$FILE" echo echo "=== Oidc.kt: around defaultOpenIdHttpClient definition (approx lines 520-600) ===" awk 'NR>=510 && NR<=600 {printf "%6d %s\n", NR, $0}' "$FILE" echo echo "=== Oidc.kt: search for discovery/refresh + managedClient usage ===" rg -n "managedClient|discovery|refresh|metadata|provider\\(|jwks|openid|retry" "$FILE" || true echo echo "=== Oidc.kt: search for timeouts/coroutine timeout helpers in-file ===" rg -n "HttpTimeout|withTimeout|withTimeoutOrNull|requestTimeout|connectTimeout|socketTimeout|timeoutMillis|TimeoutConfig" "$FILE" || trueRepository: ktorio/ktor
Length of output: 9981
Add timeouts to the managed OIDC discovery HttpClient.
defaultOpenIdHttpClient()installs onlyContentNegotiation, while both initial discovery and periodic refresh callclient.fetchOpenIdMetadata(...)with noHttpTimeout/withTimeoutguarding—so a stalled IdP request can block provider registration/refresh indefinitely and prevent the retry/failure handling from running. InstallHttpTimeoutwith sensible defaults on the managed client (or wrap the fetch in a coroutine timeout).🤖 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-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/Oidc.kt` around lines 545 - 553, defaultOpenIdHttpClient() creates the managed HttpClient without timeouts, so calls like fetchOpenIdMetadata(...) can hang indefinitely; update defaultOpenIdHttpClient() to install the HttpTimeout plugin (or wrap fetchOpenIdMetadata calls in a coroutine timeout) and configure sensible defaults (e.g., connect/read/request timeouts) so provider discovery and periodic refresh cannot block forever — reference defaultOpenIdHttpClient and fetchOpenIdMetadata when making the change and ensure the timeout values are configurable or reasonable defaults.ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcConfig.kt (2)
70-77:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate both periodic refresh durations up front.
startRefreshingMetadata()assumes both refresh durations are validdelay(...)inputs, butvalidate()only checksinitialDiscoveryRetryDelay. A negative or infinitediscoveryRefreshFailureDelaywill fail later inside the refresh loop, and a negativediscoveryRefreshIntervalcurrently disables refresh instead of rejecting invalid config.Suggested fix
internal fun validate() { require(initialDiscoveryAttempts >= 1) { "initialDiscoveryAttempts must be greater than or equal to 1" } + require(discoveryRefreshInterval.isFinite() && !discoveryRefreshInterval.isNegative()) { + "discoveryRefreshInterval must be finite and non-negative" + } + require(discoveryRefreshFailureDelay.isFinite() && !discoveryRefreshFailureDelay.isNegative()) { + "discoveryRefreshFailureDelay must be finite and non-negative" + } require(initialDiscoveryRetryDelay.isFinite() && !initialDiscoveryRetryDelay.isNegative()) { "initialDiscoveryRetryDelay must be finite and non-negative" } }🤖 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-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcConfig.kt` around lines 70 - 77, The validate() function currently only checks initialDiscoveryAttempts and initialDiscoveryRetryDelay; extend it to also validate discoveryRefreshInterval and discoveryRefreshFailureDelay so invalid values are rejected up front (matching how startRefreshingMetadata() later uses them). Specifically, in validate() add finite/non-negative checks for discoveryRefreshFailureDelay (isFinite && !isNegative) and for discoveryRefreshInterval (isFinite && !isNegative) and ensure the error messages reference the correct symbol names; keep existing checks for initialDiscoveryAttempts and initialDiscoveryRetryDelay intact so all four durations/attempts are validated before startRefreshingMetadata() runs.
183-194:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winRequire
openidwhenever session-backed OAuth is enabled.
OidcOAuthConfig.validate()drops theopenidrequirement as soon asaccessToken {}is present. Withsessions {}enabled, that creates a config that passes validation but can only yield an access token, andcreateSessions()later rejects it because it needs anOidcToken.Idto persist the session.Suggested fix
- oauthConfig?.validate(accessTokenAllowed = accessTokenConfig != null) + oauthConfig?.validate( + accessTokenAllowed = accessTokenConfig != null, + sessionRequired = sessionConfig != null, + ) @@ - internal fun validate(accessTokenAllowed: Boolean) { + internal fun validate(accessTokenAllowed: Boolean, sessionRequired: Boolean) { require(::clientId.isInitialized) { "clientId must be configured" } require(::clientSecret.isInitialized) { "clientSecret must be configured" } - require(accessTokenAllowed || "openid" in scopes) { + require((accessTokenAllowed && !sessionRequired) || "openid" in scopes) { "OAuth scopes for OpenID Connect must include openid unless accessToken { audiences = ... } is configured" } idTokenAudience?.let { audience ->Also applies to: 567-576
🤖 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-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcConfig.kt` around lines 183 - 194, The validate() logic currently skips the "openid" scope check when accessTokenConfig is present, which allows session-backed OAuth to pass validation but fail later in createSessions(); update validation so that whenever oauthConfig indicates sessions are enabled (e.g., sessions != null or sessionsEnabled flag) you require the "openid" scope be present on the token source used to create sessions: add a check in OidcConfig.validate (near where accessTokenConfig and oauthConfig are validated) to assert that if oauthConfig.sessions is enabled then either accessTokenConfig.scopes (or the effective OAuth scopes validated by OidcOAuthConfig.validate) contains "openid"; mirror this change in OidcOAuthConfig.validate (and the other location noted around lines 567-576) so session-backed flows cannot pass without "openid" before createSessions() runs.ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcSessionRefresh.kt (1)
54-58:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't invalidate a still-valid session just because no refresh token was issued.
This branch logs the user out as soon as
beforeExpiryis reached, even though the ID token may still be valid. For providers that never return refresh tokens,Autobecomes an early-expiry policy instead of a refresh policy. Fall back to keeping the current session untilexp, or reject that configuration up front.Proposed fix
val refreshTokenValue = token.refreshToken ?: run { - logger.debug("OpenID Connect session has no refresh token") - clearOidcSession() - return null + logger.debug("OpenID Connect session has no refresh token; keeping current session until it expires") + return keepSessionIfNotExpired(token, now) }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.val refreshTokenValue = token.refreshToken ?: run { logger.debug("OpenID Connect session has no refresh token; keeping current session until it expires") return keepSessionIfNotExpired(token, now) }🤖 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-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcSessionRefresh.kt` around lines 54 - 58, The current branch in OidcSessionRefresh.kt that reads token.refreshToken and calls clearOidcSession() incorrectly invalidates a still-valid session when no refresh token is issued; change the behavior so that when token.refreshToken is null you log the absence (logger.debug(...)) and return without clearing the session (do not call clearOidcSession()), allowing the session to remain valid until the ID token's exp; alternatively, if you prefer to treat "Auto" as invalid without refresh tokens, validate this configuration earlier and throw/reject at setup time rather than clearing sessions here (refer to the refreshTokenValue check, token.refreshToken, and clearOidcSession()).ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcTokenModels.kt (2)
1-1:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd the repository copyright header.
This new Kotlin source file is missing the standard header required for new source files. As per coding guidelines, “New source files must include the repository copyright header.”
Proposed fix
+/* + * Copyright 2014-2026 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license. + */ + `@file`:OptIn(ExperimentalTime::class)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements./* * Copyright 2014-2026 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license. */ `@file`:OptIn(ExperimentalTime::class)🤖 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-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcTokenModels.kt` at line 1, This file (OidcTokenModels.kt) is missing the repository copyright header; add the standard project header comment block at the top of the file before the existing file annotation (`@file`:OptIn(ExperimentalTime::class)) so the header precedes any package or file annotations, ensure the header text matches the repo's canonical copyright/license wording used in other Kotlin sources, and keep the rest of the file (including `@file`:OptIn and any package/imports) unchanged.Source: Coding guidelines
110-114:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🌐 Web query:
Inkotlin.io.encoding, doesBase64use the standard alphabet while JWT header/payload segments require base64url decoding, makingBase64.UrlSafethe correct decoder for JWT segments?💡 Result:
Yes, your assessment is correct. In Kotlin's
kotlin.io.encodingpackage,Base64.Default(often accessed viaBase64) uses the standard Base64 alphabet defined in RFC 4648 Section 4, which employs+and/characters [1][2]. JSON Web Tokens (JWT) require "Base64url" encoding as specified in RFC 7519, which mandates a URL-safe alphabet where+and/are replaced by-and_[2][3]. BecauseBase64.UrlSafeimplements this specific RFC 4648 Section 5 URL-safe alphabet, it is the correct choice for decoding JWT header and payload segments [1][4][3]. UsingBase64.Defaultwould result in decoding errors if the JWT segments contain-or_characters, as the default decoder only accepts characters within its own alphabet [2].Citations:
- 1: https://kotlinlang.org/api/core/kotlin-stdlib/kotlin.io.encoding/-base64/-default/
- 2: https://kotlinlang.org/api/core/kotlin-stdlib/kotlin.io.encoding/-base64/
- 3: https://github.com/JetBrains/kotlin/blob/2.3.0/libraries/stdlib/src/kotlin/io/encoding/Base64.kt
- 4: https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.io.encoding/-base64/-default/-url-safe.html
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE="ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcTokenModels.kt" echo "== Lines around parseJsonObject ==" nl -ba "$FILE" | sed -n '70,160p' echo echo "== Find parseJsonObject usages ==" rg -n "parseJsonObject\\(" "$FILE" echo echo "== Check Base64 usage/imports ==" rg -n "import .*Base64|Base64\\." "$FILE"Repository: ktorio/ktor
Length of output: 151
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE="ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcTokenModels.kt" echo "== Lines around parseJsonObject ==" awk 'NR>=70 && NR<=160 {printf "%6d %s\n", NR, $0}' "$FILE" || true echo echo "== Find parseJsonObject usages ==" rg -n "parseJsonObject\\(" "$FILE" || true echo echo "== Check Base64 usage/imports ==" rg -n "import .*Base64|Base64\\." "$FILE" || trueRepository: ktorio/ktor
Length of output: 4573
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE="ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcTokenModels.kt" echo "== File header/imports ==" awk 'NR>=1 && NR<=80 {printf "%6d %s\n", NR, $0}' "$FILE" | sed -n '1,80p' echo echo "== Definition of header/payload accessors ==" awk 'NR>=1 && NR<=120 { if (NR==1 || NR==10 || NR==20 || NR==25 || NR==27 || NR==28 || NR==31 || NR==32 || NR==33 || NR==34 || NR==40 || NR==50 || NR==60 || NR==70) {} }' "$FILE" || true awk 'NR>=15 && NR<=60 {printf "%6d %s\n", NR, $0}' "$FILE" echo echo "== Quick look for DecodedJWT type usage ==" rg -n "DecodedJWT|jwt\\.header|jwt\\.payload" "$FILE"Repository: ktorio/ktor
Length of output: 4665
Use URL-safe Base64 decoding for JWT header/payload segments (ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcTokenModels.kt:110-114)
DecodedJWT.headerandDecodedJWT.payloadare JWT segments encoded with Base64url (RFC 7515/7519), so they use-/_. Decoding them withBase64(standard alphabet+//) can throw insiderunCatching, and the fallbackgetOrNull() ?: JsonObject(emptyMap())makesclaim(...)/claimString(...)/headerString(...)silently act as if the token has no claims.Proposed fix
private fun parseJsonObject(raw: String): JsonObject { return runCatching { - val decoded = Base64.withPadding(Base64.PaddingOption.ABSENT).decode(raw) + val decoded = Base64.UrlSafe.withPadding(Base64.PaddingOption.ABSENT).decode(raw) Json.parseToJsonElement(decoded.decodeToString()) as? JsonObject }.getOrNull() ?: JsonObject(emptyMap()) }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.private fun parseJsonObject(raw: String): JsonObject { return runCatching { val decoded = Base64.UrlSafe.withPadding(Base64.PaddingOption.ABSENT).decode(raw) Json.parseToJsonElement(decoded.decodeToString()) as? JsonObject }.getOrNull() ?: JsonObject(emptyMap()) }🤖 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-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcTokenModels.kt` around lines 110 - 114, The parseJsonObject function currently decodes JWT segments using the standard Base64 alphabet, which fails for Base64url-encoded JWT header/payload and causes silent empty-claim fallbacks; change the decoder used in parseJsonObject to the URL-safe Base64 variant (preserving the absent-padding option) so that DecodedJWT.header and DecodedJWT.payload are decoded correctly and claim(...)/claimString(...)/headerString(...) no longer return empty JsonObject on valid tokens.ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcTokens.kt (2)
103-110:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
token_typeon the access-token-only callback path.This fallback skips
requireBearerTokenType(...)and accepts the callback purely on access-token verification. A provider response with a non-Bearertoken_typecan still be persisted and later treated like a bearer credential.Proposed fix
internal suspend fun OidcProvider<*>.buildOAuthToken( response: OAuthAccessTokenResponse.OAuth2, expectedNonce: String?, ): OidcToken { val config = oauthConfig + requireBearerTokenType(response.tokenType) val idToken = response.extraParameters["id_token"] ?: run { - return requireNotNull(verifyAccessToken(response.accessToken)) { - "OAuth callback access token was not accepted" - } + return verifyAccessToken(response.accessToken) } - requireBearerTokenType(response.tokenType) val nonce = expectedNonce ?: rejectToken("OIDC state nonce is missing") return buildIdToken(🤖 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-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcTokens.kt` around lines 103 - 110, The fallback branch that handles access-token-only callbacks (where idToken is null) skips token-type validation; update the logic in OidcTokens.kt so that requireBearerTokenType(response.tokenType) is invoked before accepting an access token in the idToken fallback path (i.e., call requireBearerTokenType either before reading response.extraParameters["id_token"] or inside the run { ... } fallback before verifyAccessToken(response.accessToken) returns) so non-Bearer token_type values are rejected; reference symbols: oauthConfig, idToken, response.extraParameters, verifyAccessToken, requireBearerTokenType.
441-443:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEnforce the documented absolute redirect-URI contract.
buildLogoutUrl(...)documentspostLogoutRedirectUrias absolute, but this code appends any string. Invalid or relative values are only rejected later by the provider instead of failing fast at the API boundary. As per coding guidelines, “Userequire(...)for argument validation” and “Make error messages actionable; include the problematic value/context.”Proposed fix
+import java.net.URI + internal fun OidcProvider<*>.buildLogoutUrlInternal( idTokenHint: String, postLogoutRedirectUri: String?, ): String { @@ - postLogoutRedirectUri?.let { uri -> + postLogoutRedirectUri?.let { uri -> + require(URI(uri).isAbsolute) { + "postLogoutRedirectUri must be an absolute URI, but was $uri" + } parameters.append("post_logout_redirect_uri", uri) } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.import java.net.URI internal fun OidcProvider<*>.buildLogoutUrlInternal( idTokenHint: String, postLogoutRedirectUri: String?, ): String { postLogoutRedirectUri?.let { uri -> require(URI(uri).isAbsolute) { "postLogoutRedirectUri must be an absolute URI, but was $uri" } parameters.append("post_logout_redirect_uri", uri) } }🤖 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-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/src/io/ktor/server/auth/oidc/OidcTokens.kt` around lines 441 - 443, The code appends postLogoutRedirectUri without validating it; update buildLogoutUrl (OidcTokens.kt) to validate that postLogoutRedirectUri is an absolute URI before appending: add a require(...) check right before the parameters.append("post_logout_redirect_uri", uri) call that parses the string (e.g., via java.net.URI or Ktor Url parsing) and asserts isAbsolute (or scheme present), and include the actual value in the require message so callers get an actionable error referencing the invalid postLogoutRedirectUri.Source: Coding guidelines
ktor-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/OidcJwkProviderConfigTest.kt (1)
169-170:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid ephemeral-port TOCTOU to prevent flaky binds.
Line 169 reserves a free port and closes the socket before Line 170 binds the server. Under parallel test execution, another process can claim that port, causing intermittent bind failures.
🤖 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-server/ktor-server-plugins/ktor-server-auth-oidc/jvm/test/io/ktor/server/auth/oidc/OidcJwkProviderConfigTest.kt` around lines 169 - 170, The test currently reserves a port with ServerSocket(0) then closes it before calling embeddedServer(CIO, port = port), introducing a TOCTOU race; instead pass port = 0 directly to embeddedServer (embeddedServer(CIO, port = 0) { ... }) so the OS assigns a free port atomically, remove the ServerSocket allocation and the local port variable, and if the test needs the actual bound port later retrieve it after starting the server (e.g. from server.resolvedConnectors.first().port or the server's connector info).ktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/SessionAuth.kt (1)
65-71:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid dumping the full session object in the type-mismatch check.
$sessioncan include tokens or user identifiers in the thrown message. Emitting only the runtime type keeps the configuration error actionable without leaking session contents.🔐 Safer message
- check(type.isInstance(session)) { - "Session provider `$sessionName` returned `$session`, but session type `$type` was expected." - } + check(type.isInstance(session)) { + "Session provider `$sessionName` returned `${session::class}`, but session type `$type` was expected." + }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.private fun CurrentSession.getSessionByName(sessionName: String): T? { val session = get(sessionName) ?: return null check(type.isInstance(session)) { "Session provider `$sessionName` returned `${session::class}`, but session type `$type` was expected." } `@Suppress`("UNCHECKED_CAST") return session as T🤖 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-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/SessionAuth.kt` around lines 65 - 71, The check in CurrentSession.getSessionByName logs the actual session instance (`$session`) on type mismatch which may leak sensitive data; change the error message to avoid including the session value and instead report only the expected and actual runtime types (e.g., use session::class or session.javaClass to show the session's type) in the check() call inside getSessionByName so the message remains actionable without exposing session contents.ktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/OAuthFlow.kt (1)
379-405:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t persist the session before principal resolution succeeds.
call.sessions.set(flow.sessions, session)runs beforeprincipalResolverand before the rest of the callback finishes. If principal resolution returnsnull, or a later exception hits the catch block, the response can still carry a session for a failed login. Defer the write until the principal is resolved, and clear it if later steps can still fail.🤖 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-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/OAuthFlow.kt` around lines 379 - 405, You are persisting the session too early: move the call.sessions.set(flow.sessions, session) so it only runs after principal resolution succeeds (after val principal = flow.principalResolver(... ) returns non-null) and only after any subsequent failing work that must not leave a session (e.g., checkNotNull(flow.config.contextFactory) and flow.sessions.capture). Also ensure that if an exception occurs after setting the session you remove it (call.sessions.clear or equivalent) before returning/failing; update the block around sessionCreator, principalResolver, flow.sessions.set, call.attributes.put, flow.sessions.capture, DefaultSessionAuthenticatedContext and the contextFactory onSuccess/onFailure handling to enforce atomicity (set session only on final success, clear on errors).ktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/OptionalAuthScheme.kt (1)
113-114:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTypeInfo loses runtime type information for the principal.
The
principalKeyis declared asAttributeKey<B>but constructed withTypeInfo(Any::class), losing the actual typeBat runtime. This may cause issues if AttributeKey uses TypeInfo for runtime type checking or serialization.Consider either:
- Accepting a
TypeInfoparameter in theAnonymousAuthSchemeconstructor (passed from a reified context)- Documenting why
Any::classis used and the implications🤖 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-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/OptionalAuthScheme.kt` around lines 113 - 114, The principalKey uses TypeInfo(Any::class) which drops the generic B runtime type; change the constructor of AnonymousAuthScheme (or the enclosing class that creates principalKey) to accept a TypeInfo<B> parameter (e.g., principalTypeInfo) and use that when constructing principalKey (AttributeKey("TypesafeAuth:$name:AnonymousPrincipal", principalTypeInfo)) so the AttributeKey carries the actual principal type at runtime; alternatively, if you intentionally want Any, add a concise doc comment on AnonymousAuthScheme/principalKey explaining why Any::class is used and the runtime implications.ktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/TypedAuthInterceptors.kt (1)
19-32:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPotential race condition in plugin name generation.
TypedAuthPluginNameGenerator.nextIdis mutable and incremented without synchronization (line 30). If routes can be created concurrently at runtime, multiple threads could read and incrementnextIdsimultaneously, leading to duplicate plugin names or lost increments.While route configuration typically occurs on a single thread during startup, consider using
AtomicIntegerfornextIdto guarantee correctness in all scenarios:🔒 Proposed fix using AtomicInteger
+import java.util.concurrent.atomic.AtomicInteger + private class TypedAuthPluginNameGenerator { - private var nextId: Int = 0 + private val nextId: AtomicInteger = AtomicInteger(0) fun next(kind: String, parts: List<String>): String = buildString { append("TypedAuth:") append(kind) parts.forEach { part -> append(":") append(part) } append(":") - append(nextId++) + append(nextId.getAndIncrement()) } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.import java.util.concurrent.atomic.AtomicInteger private class TypedAuthPluginNameGenerator { private val nextId: AtomicInteger = AtomicInteger(0) fun next(kind: String, parts: List<String>): String = buildString { append("TypedAuth:") append(kind) parts.forEach { part -> append(":") append(part) } append(":") append(nextId.getAndIncrement()) } }🤖 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-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/TypedAuthInterceptors.kt` around lines 19 - 32, The current TypedAuthPluginNameGenerator uses a mutable Int field nextId and increments it unsafely in next(kind: String, parts: List<String>), which can race; replace nextId with an AtomicInteger (e.g., private val nextId = AtomicInteger(0)) and change the increment to a thread-safe getAndIncrement() call inside TypedAuthPluginNameGenerator.next to ensure unique plugin names under concurrent route creation.ktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/TypedSessionAuthConfig.kt (2)
71-81:
⚠️ Potential issue | 🟠 Major | ⚡ Quick win
description/onUnauthorizedare documented as DSL knobs but are not publicly settable.Both properties are
internal, so callers insession { ... }cannot configure what the KDoc advertises. This makes the documented API behavior unreachable.Proposed fix
- internal var description: String? = null + public var description: String? = null ... - internal var onUnauthorized: UnauthorizedHandler? = null + public var onUnauthorized: UnauthorizedHandler? = nullAs per coding guidelines: keep KDoc correct when behavior/signatures change.
🤖 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-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/TypedSessionAuthConfig.kt` around lines 71 - 81, The KDoc advertises configurable DSL knobs but the properties are internal, so make the advertised knobs public: change TypedSessionAuthConfig.description and TypedSessionAuthConfig.onUnauthorized from internal to public (or provide public setters) while keeping their types (String? and UnauthorizedHandler?) and existing KDoc; ensure authenticateWith's route-level onUnauthorized still overrides the now-public config property.Source: Coding guidelines
119-123:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winSession rewrite currently checks value equality instead of instance identity.
KDoc promises rewrite when a different instance is returned, but Line 162 uses
!=(structural equality). A new instance with equal values will be skipped and not persisted.Proposed fix
- if (updatedSession != session) { + if (updatedSession !== session) { sessions.set(name, updatedSession) }Also applies to: 162-164
🤖 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-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/typesafe/TypedSessionAuthConfig.kt` around lines 119 - 123, The session rewrite check in TypedSessionAuthConfig currently uses structural equality (`!=`) so a new instance with equal values won't be persisted; change the comparison to referential inequality (`!==`) when deciding to call CurrentSession.set so the session is rewritten only when a different instance is returned by the transformation block (refer to the code handling the block result and the call to io.ktor.server.sessions.CurrentSession.set).
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Subsystem
OIDC Server
Motivation
KTOR-5001 Add OpenID Connect (OIDC) support on client and server
Solution
provider.sessionsauthenticationTypedSessionAuthConfig.transformSessioninktor-server-authfor session refresh during authentication