refactor(axum): split web integration and add auth context middleware - #8
Merged
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 47 minutes and 6 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughSplits the prior monolithic Axum web server into a modular web layer (app, handlers, middleware, models, response), adds DTOs and error mapping, updates Cargo/features and example main, and standardizes doc examples and small core API/FromStr changes. ChangesWeb axum modularization
Docs & core tweaks
Sequence Diagram(s)sequenceDiagram
participant Client
participant Middleware as Auth Middleware
participant AuthService
participant UserDb as User Store
participant Handler as Request Handler
Client->>Middleware: HTTP Request + Bearer Token
Middleware->>Middleware: Parse Authorization Header
alt token present
Middleware->>AuthService: get_user_from_token(token)
AuthService->>UserDb: Lookup/Create user
UserDb-->>AuthService: User or Error
AuthService-->>Middleware: User or Error
Middleware->>Handler: Insert AuthenticatedUser or AuthenticationFailure in extensions
Handler-->>Client: 200 OK or 401 Unauthorized
else no token
Middleware->>Handler: continue without user
Handler-->>Client: 401 Unauthorized or 200 (if optional)
end
sequenceDiagram
participant Client
participant OAuthHandler as OAuth Callback
participant AuthService
participant OAuthProvider as OAuth Provider
participant Frontend as Frontend
Client->>OAuthHandler: GET /oauth/{provider}/callback?code=...&state=...
OAuthHandler->>AuthService: login(LoginMethod::OAuth2{provider, code})
AuthService->>OAuthProvider: exchange code for tokens / user info
OAuthProvider-->>AuthService: tokens + user info
AuthService->>AuthService: create/update user, generate TokenPair
AuthService-->>OAuthHandler: TokenPair
OAuthHandler->>Frontend: Redirect with percent-encoded tokens in fragment
Frontend-->>Client: token handling page
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/oauth/manager.rs (1)
594-599:⚠️ Potential issue | 🔴 CriticalInfinite recursion in trait implementation.
The
get_redirect_frontend_uritrait method callsself.get_redirect_frontend_uri(provider), which resolves to the same trait method, causing infinite recursion at runtime. The intent appears to be calling the inherent method defined at lines 617-622.🐛 Proposed fix to call the inherent method
async fn get_redirect_frontend_uri( &self, provider: OAuth2Provider, ) -> Result<String, AuthError> { - self.get_redirect_frontend_uri(provider) + OAuth2Manager::get_redirect_frontend_uri(self, provider) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/oauth/manager.rs` around lines 594 - 599, The trait impl for get_redirect_frontend_uri currently recursively calls itself; change it to call the inherent (concrete) implementation instead — replace the self.get_redirect_frontend_uri(provider) call inside the trait impl with a direct call to the concrete type's inherent method (e.g., MyManagerType::get_redirect_frontend_uri(self, provider) using the actual struct/type name that defines the inherent method) so the trait forwards to the defined implementation rather than recursing.
🧹 Nitpick comments (11)
src/core/hash/argon2.rs (1)
39-42: Keep this doctest executable.
Argon2Hasher::new()is side-effect free, sono_runonly drops cheap runtime coverage here. I’d keep this as a normalrustdoctest and re-runcargo test --doclocally to confirm it still passes.Suggested diff
- /// ```rust,no_run + /// ```rust🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/hash/argon2.rs` around lines 39 - 42, The doctest for Argon2Hasher currently uses `no_run`, which suppresses execution; change the doc comment block in src/core/hash/argon2.rs so the doctest is a normal executable Rust doc test (remove `no_run`) for the `Argon2Hasher::new()` example, then run `cargo test --doc` locally to verify it passes; ensure the example remains side-effect free and compiles when run as a doc test.src/core/user/persistence/in_memory.rs (1)
27-30: Remove theno_rundirective to enable doctest execution.The import path
authen::core::user::persistence::InMemoryUserRepois correct. Theno_rundirective is unnecessary—InMemoryUserRepo::new()is a simple constructor with no side effects or external dependencies, making this example safe to compile and execute as a runnable doctest.Suggested change
- /// ```rust,no_run + /// ```rust /// use authen::core::user::persistence::InMemoryUserRepo; /// let repo = InMemoryUserRepo::new(); /// ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/user/persistence/in_memory.rs` around lines 27 - 30, Update the doctest for InMemoryUserRepo by removing the `no_run` directive so the snippet runs as a normal doctest: change the doc block from "```rust,no_run" to "```rust" for the example that imports `authen::core::user::persistence::InMemoryUserRepo` and calls `InMemoryUserRepo::new()`, ensuring the `InMemoryUserRepo::new` constructor is compiled and executed by cargo test.src/core/policy/mod.rs (1)
64-64: Minor inconsistency in doctest directives.The method-level example uses
no_runwhile the module-level example at line 9 uses plainrust. Both examples are self-contained and should compile/run correctly. Consider using consistent doctest directives across the module unless there's a specific reason for the difference.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/policy/mod.rs` at line 64, The doctest directives are inconsistent: the method-level example uses "```rust,no_run" while the module-level example uses "```rust"; pick one directive and make them consistent across this module (either change the method-level block in mod.rs from "```rust,no_run" to "```rust" or change the module-level example to include ",no_run"), ensuring both examples remain self-contained and compile/run as intended; locate the fenced code blocks in src/core/policy/mod.rs (the module-level example and the method-level example) and update the triple-backtick directives to the chosen consistent form.src/core/oauth/manager.rs (1)
374-388: French comments in English codebase.The comments on lines 374 and 386 are in French ("Dans ton code Rust, assure-toi de dédupliquer les scopes" and "Déduplication des scopes"). Consider translating to English for consistency with the rest of the codebase.
♻️ Suggested translation
- // Dans ton code Rust, assure-toi de dédupliquer les scopes + // Collect all scopes from defaults, additional parameters, and config let mut all_scopes = provider .default_scopes() .into_iter() .map(|s| s.to_string()) .collect::<Vec<_>>(); if let Some(additional_scopes) = scopes { all_scopes.extend(additional_scopes); } all_scopes.extend(config.additional_scopes.clone()); - // Déduplication des scopes + // Deduplicate scopes all_scopes.sort(); all_scopes.dedup();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/oauth/manager.rs` around lines 374 - 388, Replace the two French comments around the scope-collection/dedup block with concise English equivalents; update the comment before building all_scopes (near provider.default_scopes(), scopes, config.additional_scopes) to something like "Collect default and additional scopes" and replace the "Déduplication des scopes" comment above all_scopes.sort() / all_scopes.dedup() with "Deduplicate scopes", ensuring comment style and tone match the surrounding English codebase.src/web_axum/handlers/token.rs (2)
24-27: Expired/invalid refresh tokens mapped to 400 may be better as 401.Similar to login, an expired or invalid refresh token is an authentication failure rather than a malformed request. Consider using
ApiError::unauthorizedfor token-related auth failures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/web_axum/handlers/token.rs` around lines 24 - 27, The refresh token failure is an authentication issue but currently maps to ApiError::bad_request; update the error mapping in the refresh path so failures from auth_service.refresh_access_token(&payload.refresh_token).await are converted to ApiError::unauthorized instead of ApiError::bad_request (i.e., replace the map_err target to ApiError::unauthorized or map the underlying error to an unauthorized ApiError), keeping the same call to auth_service.refresh_access_token and preserving the tokens variable.
32-48: Consider returning{valid: false}for invalid tokens instead of an error response.Currently, an invalid/expired token triggers an error (400 response), while only valid tokens reach the success path with
valid: true. This is technically correct but clients may expect a 200 response with{valid: false}for token inspection use cases. If this is intentional, the API documentation should clarify this behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/web_axum/handlers/token.rs` around lines 32 - 48, The validate function currently converts any token validation failure into an ApiError::bad_request; change it so validate_access_token errors are handled by returning a successful 200 response with ValidateTokenResponse.valid = false instead. Concretely, in validate (and where you currently call AuthService::validate_access_token and map_err(ApiError::bad_request)), match the Result from validate_access_token: on Ok(claims) return the existing Json(ValidateTokenResponse { valid: true, subject: claims.get_subject().to_string(), expiration: claims.get_expiration() }), and on Err(_) return Ok(Json(ValidateTokenResponse { valid: false, subject: /* empty string or appropriate default */, expiration: /* None or default matching the type */ })). Remove mapping to ApiError::bad_request for token validation failures so clients receive a 200 with valid:false (adjust the default subject/expiration values to match ValidateTokenResponse field types).src/web_axum/response.rs (1)
33-38: Theinternalmethod hardcodes a misleading error prefix.The "Configuration error:" prefix is hardcoded but
internal()may be used for non-configuration errors (database failures, network issues, etc.). Consider removing the prefix or providing a more generic one.Proposed fix
pub fn internal(error: impl ToString) -> Self { Self::new( StatusCode::INTERNAL_SERVER_ERROR, - format!("Configuration error: {}", error.to_string()), + format!("Internal server error: {}", error.to_string()), ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/web_axum/response.rs` around lines 33 - 38, The internal method currently prepends a misleading "Configuration error:" prefix; update the implementation of Response::internal (the internal function that calls Self::new with StatusCode::INTERNAL_SERVER_ERROR) to remove that hardcoded prefix and either log the raw error string (format!("{}", error.to_string())) or use a generic prefix like "Internal server error:" so the message is accurate for database/network/other failures; ensure the change is made in the internal function that constructs the Response via Self::new and preserves the StatusCode::INTERNAL_SERVER_ERROR.src/web_axum/app.rs (1)
39-40: Panics on bind/serve failures are documented but consider structured error handling.The
.unwrap()calls are documented in the function's# Panicssection, which is good. For production use, you may want to return aResultto allow callers to handle startup failures gracefully.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/web_axum/app.rs` around lines 39 - 40, The startup code currently panics on failures because it uses TcpListener::bind(...).await.unwrap() and serve(...).await.unwrap(); change the function signature to return a Result (e.g., Result<(), anyhow::Error> or axum-compatible error) and replace the unwrap() calls with the ? operator (or map_err to convert into your chosen error type) so bind and serve errors are propagated to the caller instead of panicking; update any call sites to handle or propagate the returned Result accordingly.src/web_axum/handlers/auth.rs (1)
22-28: Consider distinguishing signup error types for clearer API responses.All signup failures (including "user already exists") are mapped to 400 Bad Request. While acceptable, you may want to distinguish user-exists errors (potentially 409 Conflict) from other validation failures for better client-side handling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/web_axum/handlers/auth.rs` around lines 22 - 28, The signup handler currently maps all signup failures from auth_service.signup(...) (called with SignupMethod::Credentials) to ApiError::bad_request, which hides distinct error cases like "user already exists"; update the error handling of the await call to inspect the specific error returned by auth_service.signup (e.g., match on an AuthError or SignupError variant that indicates user already exists) and map that case to an appropriate API error (e.g., ApiError::conflict or an ApiError::already_exists) while keeping other validation or generic errors mapped to ApiError::bad_request so clients can distinguish 409 Conflict from other 400 errors.src/web_axum/handlers/oauth.rs (1)
122-132: Consider using a standard URL encoding library instead of custom implementation.The custom
url_encodefunction is correct and handles RFC 3986 unreserved characters properly, but using an established library likeurlencodingorpercent_encodingwould eliminate maintenance burden. Both crates are well-tested and handle edge cases comprehensively.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/web_axum/handlers/oauth.rs` around lines 122 - 132, The custom url_encode function implements RFC3986-style percent-encoding but should be replaced with a standard crate to reduce maintenance and edge-case bugs: remove or deprecate the url_encode function in src/web_axum/handlers/oauth.rs and switch callers to use a tested crate such as percent_encoding or urlencoding (e.g., percent_encoding::percent_encode_str with an appropriate ASCII set for unreserved characters), add the chosen crate to Cargo.toml, and ensure the new call produces the same RFC3986 unreserved-character behavior as the original implementation.src/web_axum/models.rs (1)
187-197: Consider derivingFromStronOAuth2Providerfor improved maintainability.The hardcoded provider list requires manual updates when new providers are added to the
OAuth2Providerenum. UsingFromStror a derive crate likestrumwould keep the parse logic automatically aligned with enum variants.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/web_axum/models.rs` around lines 187 - 197, The parse_oauth_provider function duplicates variant matching and should be replaced by implementing/deriving std::str::FromStr for the OAuth2Provider enum (or using the strum derive) and mapping parse errors to your ApiError; add a FromStr impl (or #[derive(EnumString)] via strum) on OAuth2Provider that returns a clear parse error, then change parse_oauth_provider to call value.parse::<OAuth2Provider>().map_err(|e| ApiError::bad_request(AuthError::InvalidInput(format!("Unsupported OAuth2 provider: {value}: {e}")))); this removes the hardcoded list and keeps parse logic in the enum type itself.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/web_axum/handlers/auth.rs`:
- Around line 39-45: The current handler maps all errors from auth_service.login
to ApiError::bad_request, but authentication failures
(AuthError::InvalidCredentials) should return HTTP 401; update the error
handling around auth_service.login (the call to AuthService::login/login method)
to match on the returned AuthError and map AuthError::InvalidCredentials to
ApiError::unauthorized (or the equivalent 401 ApiError), while mapping other
variants to ApiError::bad_request so only actual credential failures produce
401.
In `@src/web_axum/handlers/oauth.rs`:
- Line 69: The handler currently returns a permanent redirect via
Redirect::permanent(&redirect_url) which yields a cached 301; change this to a
temporary redirect to avoid client caching of OAuth callbacks—replace
Redirect::permanent with Redirect::temporary(&redirect_url) (or
Redirect::to(&redirect_url) for a 303) wherever the
Redirect::permanent(&redirect_url) expression appears in the oauth callback
handler (the return expression that produces
Ok(Redirect::permanent(&redirect_url))).
---
Outside diff comments:
In `@src/core/oauth/manager.rs`:
- Around line 594-599: The trait impl for get_redirect_frontend_uri currently
recursively calls itself; change it to call the inherent (concrete)
implementation instead — replace the self.get_redirect_frontend_uri(provider)
call inside the trait impl with a direct call to the concrete type's inherent
method (e.g., MyManagerType::get_redirect_frontend_uri(self, provider) using the
actual struct/type name that defines the inherent method) so the trait forwards
to the defined implementation rather than recursing.
---
Nitpick comments:
In `@src/core/hash/argon2.rs`:
- Around line 39-42: The doctest for Argon2Hasher currently uses `no_run`, which
suppresses execution; change the doc comment block in src/core/hash/argon2.rs so
the doctest is a normal executable Rust doc test (remove `no_run`) for the
`Argon2Hasher::new()` example, then run `cargo test --doc` locally to verify it
passes; ensure the example remains side-effect free and compiles when run as a
doc test.
In `@src/core/oauth/manager.rs`:
- Around line 374-388: Replace the two French comments around the
scope-collection/dedup block with concise English equivalents; update the
comment before building all_scopes (near provider.default_scopes(), scopes,
config.additional_scopes) to something like "Collect default and additional
scopes" and replace the "Déduplication des scopes" comment above
all_scopes.sort() / all_scopes.dedup() with "Deduplicate scopes", ensuring
comment style and tone match the surrounding English codebase.
In `@src/core/policy/mod.rs`:
- Line 64: The doctest directives are inconsistent: the method-level example
uses "```rust,no_run" while the module-level example uses "```rust"; pick one
directive and make them consistent across this module (either change the
method-level block in mod.rs from "```rust,no_run" to "```rust" or change the
module-level example to include ",no_run"), ensuring both examples remain
self-contained and compile/run as intended; locate the fenced code blocks in
src/core/policy/mod.rs (the module-level example and the method-level example)
and update the triple-backtick directives to the chosen consistent form.
In `@src/core/user/persistence/in_memory.rs`:
- Around line 27-30: Update the doctest for InMemoryUserRepo by removing the
`no_run` directive so the snippet runs as a normal doctest: change the doc block
from "```rust,no_run" to "```rust" for the example that imports
`authen::core::user::persistence::InMemoryUserRepo` and calls
`InMemoryUserRepo::new()`, ensuring the `InMemoryUserRepo::new` constructor is
compiled and executed by cargo test.
In `@src/web_axum/app.rs`:
- Around line 39-40: The startup code currently panics on failures because it
uses TcpListener::bind(...).await.unwrap() and serve(...).await.unwrap(); change
the function signature to return a Result (e.g., Result<(), anyhow::Error> or
axum-compatible error) and replace the unwrap() calls with the ? operator (or
map_err to convert into your chosen error type) so bind and serve errors are
propagated to the caller instead of panicking; update any call sites to handle
or propagate the returned Result accordingly.
In `@src/web_axum/handlers/auth.rs`:
- Around line 22-28: The signup handler currently maps all signup failures from
auth_service.signup(...) (called with SignupMethod::Credentials) to
ApiError::bad_request, which hides distinct error cases like "user already
exists"; update the error handling of the await call to inspect the specific
error returned by auth_service.signup (e.g., match on an AuthError or
SignupError variant that indicates user already exists) and map that case to an
appropriate API error (e.g., ApiError::conflict or an ApiError::already_exists)
while keeping other validation or generic errors mapped to ApiError::bad_request
so clients can distinguish 409 Conflict from other 400 errors.
In `@src/web_axum/handlers/oauth.rs`:
- Around line 122-132: The custom url_encode function implements RFC3986-style
percent-encoding but should be replaced with a standard crate to reduce
maintenance and edge-case bugs: remove or deprecate the url_encode function in
src/web_axum/handlers/oauth.rs and switch callers to use a tested crate such as
percent_encoding or urlencoding (e.g., percent_encoding::percent_encode_str with
an appropriate ASCII set for unreserved characters), add the chosen crate to
Cargo.toml, and ensure the new call produces the same RFC3986
unreserved-character behavior as the original implementation.
In `@src/web_axum/handlers/token.rs`:
- Around line 24-27: The refresh token failure is an authentication issue but
currently maps to ApiError::bad_request; update the error mapping in the refresh
path so failures from
auth_service.refresh_access_token(&payload.refresh_token).await are converted to
ApiError::unauthorized instead of ApiError::bad_request (i.e., replace the
map_err target to ApiError::unauthorized or map the underlying error to an
unauthorized ApiError), keeping the same call to
auth_service.refresh_access_token and preserving the tokens variable.
- Around line 32-48: The validate function currently converts any token
validation failure into an ApiError::bad_request; change it so
validate_access_token errors are handled by returning a successful 200 response
with ValidateTokenResponse.valid = false instead. Concretely, in validate (and
where you currently call AuthService::validate_access_token and
map_err(ApiError::bad_request)), match the Result from validate_access_token: on
Ok(claims) return the existing Json(ValidateTokenResponse { valid: true,
subject: claims.get_subject().to_string(), expiration: claims.get_expiration()
}), and on Err(_) return Ok(Json(ValidateTokenResponse { valid: false, subject:
/* empty string or appropriate default */, expiration: /* None or default
matching the type */ })). Remove mapping to ApiError::bad_request for token
validation failures so clients receive a 200 with valid:false (adjust the
default subject/expiration values to match ValidateTokenResponse field types).
In `@src/web_axum/models.rs`:
- Around line 187-197: The parse_oauth_provider function duplicates variant
matching and should be replaced by implementing/deriving std::str::FromStr for
the OAuth2Provider enum (or using the strum derive) and mapping parse errors to
your ApiError; add a FromStr impl (or #[derive(EnumString)] via strum) on
OAuth2Provider that returns a clear parse error, then change
parse_oauth_provider to call value.parse::<OAuth2Provider>().map_err(|e|
ApiError::bad_request(AuthError::InvalidInput(format!("Unsupported OAuth2
provider: {value}: {e}")))); this removes the hardcoded list and keeps parse
logic in the enum type itself.
In `@src/web_axum/response.rs`:
- Around line 33-38: The internal method currently prepends a misleading
"Configuration error:" prefix; update the implementation of Response::internal
(the internal function that calls Self::new with
StatusCode::INTERNAL_SERVER_ERROR) to remove that hardcoded prefix and either
log the raw error string (format!("{}", error.to_string())) or use a generic
prefix like "Internal server error:" so the message is accurate for
database/network/other failures; ensure the change is made in the internal
function that constructs the Response via Self::new and preserves the
StatusCode::INTERNAL_SERVER_ERROR.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bc8d6971-de81-41d5-9249-b84a9f0e554b
📒 Files selected for processing (23)
src/core/credentials/mod.rssrc/core/hash/argon2.rssrc/core/oauth/manager.rssrc/core/oauth/store.rssrc/core/password/argon2.rssrc/core/password/mod.rssrc/core/policy/mod.rssrc/core/token/jwt.rssrc/core/token/mod.rssrc/core/user/mod.rssrc/core/user/persistence/in_memory.rssrc/lib.rssrc/web_axum.rssrc/web_axum/app.rssrc/web_axum/handlers/auth.rssrc/web_axum/handlers/mod.rssrc/web_axum/handlers/oauth.rssrc/web_axum/handlers/system.rssrc/web_axum/handlers/token.rssrc/web_axum/middleware.rssrc/web_axum/mod.rssrc/web_axum/models.rssrc/web_axum/response.rs
💤 Files with no reviewable changes (1)
- src/web_axum.rs
- centralize OAuth provider parsing and fragment encoding - map signup/login/token errors to clearer HTTP responses - return server startup errors instead of panicking
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/web_server/src/main.rs (1)
105-118:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the doctest to match the updated API.
The doctest example has two issues:
- Missing the second parameter to
start_server(should bestart_server(auth_service, None).await)- Not handling the
std::io::Result<()>returned bystart_serverThis will cause the doctest to fail compilation.
🔧 Proposed doctest fix
/// ```rust /// use authen::{AuthService, web_axum::start_server}; /// use std::sync::Arc; /// /// #[tokio::main] -/// async fn main() { +/// async fn main() -> std::io::Result<()> { /// env_logger::init(); /// let auth_service = Arc::new(AuthService::default()); /// #[cfg(feature = "axum")] -/// start_server(auth_service).await; +/// start_server(auth_service, None).await?; /// #[cfg(not(feature = "web"))] /// println!("Please enable the 'web' feature to run the web server example."); +/// Ok(()) /// } /// ```🤖 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 `@examples/web_server/src/main.rs` around lines 105 - 118, Update the doctest to match the new start_server signature and return type: change the async main signature to return std::io::Result<()> and call start_server with the second argument (e.g., None) and propagate its Result with ?; finally return Ok(()) at the end. This affects the doctest lines that reference start_server(auth_service) — replace them with start_server(auth_service, None).await? and change async fn main() to async fn main() -> std::io::Result<()> and add Ok(()) before the function end.
🧹 Nitpick comments (1)
Cargo.toml (1)
38-38: Consider bumpingpercent-encodingto the latest patch; no known CVEs for 2.3.1
percent-encoding = { version = "2.3.1", optional = true }pins an older release—latest is2.3.2.No RustSec advisory is associated with
percent-encoding2.3.1, and GitHub Advisory Database searches don’t show a vulnerability specifically tied to that crate/version.🤖 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 `@Cargo.toml` at line 38, The dependency entry percent-encoding = { version = "2.3.1", optional = true } is pinned to an older patch; update it to percent-encoding = { version = "2.3.2", optional = true } in Cargo.toml and then run cargo update for that package (or regenerate Cargo.lock) so the lockfile reflects the bump; confirm no other dependency constraints prevent the update and run tests/build to verify compatibility.
🤖 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 `@examples/web_server/src/main.rs`:
- Around line 137-138: Update the outdated doc comment for the function that now
returns std::io::Result<()> (the program entry/server startup function, e.g.,
main) so it no longer claims it will panic if the Tokio runtime cannot be
started; instead state that startup errors are propagated as a Result and
returned to the caller. Mention the new return type (std::io::Result<()>) and
that errors from starting the Tokio runtime or server are returned, not
panicked, and remove the earlier "Panics" section.
In `@src/web_axum/handlers/token.rs`:
- Around line 38-49: The handler currently treats all errors from
auth_service.validate_access_token as a simple "valid: false"; change the Err
branch to distinguish validation errors from system errors by pattern-matching
the auth service error (from validate_access_token): for expected validation
failures (e.g., AuthError::InvalidToken, AuthError::Expired or whatever variant
your auth error type uses) return Ok(Json(ValidateTokenResponse { valid: false,
subject: String::new(), expiration: 0 })), but for unexpected/system errors
return Err(ApiError::internal(format!("token validation service error: {}",
err))) so the caller sees a 5xx and operators can detect service issues. Ensure
you reference auth_service.validate_access_token, ValidateTokenResponse and
ApiError::internal when implementing the change.
---
Outside diff comments:
In `@examples/web_server/src/main.rs`:
- Around line 105-118: Update the doctest to match the new start_server
signature and return type: change the async main signature to return
std::io::Result<()> and call start_server with the second argument (e.g., None)
and propagate its Result with ?; finally return Ok(()) at the end. This affects
the doctest lines that reference start_server(auth_service) — replace them with
start_server(auth_service, None).await? and change async fn main() to async fn
main() -> std::io::Result<()> and add Ok(()) before the function end.
---
Nitpick comments:
In `@Cargo.toml`:
- Line 38: The dependency entry percent-encoding = { version = "2.3.1", optional
= true } is pinned to an older patch; update it to percent-encoding = { version
= "2.3.2", optional = true } in Cargo.toml and then run cargo update for that
package (or regenerate Cargo.lock) so the lockfile reflects the bump; confirm no
other dependency constraints prevent the update and run tests/build to verify
compatibility.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f26db2d0-b2f2-46a0-af04-60d5ad18ec0b
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
Cargo.tomlexamples/web_server/src/main.rssrc/core/oauth/manager.rssrc/core/oauth/store.rssrc/web_axum/app.rssrc/web_axum/handlers/auth.rssrc/web_axum/handlers/oauth.rssrc/web_axum/handlers/token.rssrc/web_axum/models.rssrc/web_axum/response.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- src/web_axum/response.rs
- src/web_axum/handlers/auth.rs
- src/web_axum/handlers/oauth.rs
- src/web_axum/app.rs
- src/web_axum/models.rs
- src/core/oauth/manager.rs
- treat invalid, expired, and validation token errors as non-fatal - return web server startup errors from the example instead of panicking - refresh `percent-encoding` and lockfile dependencies
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.
Summary
src/web_axum.rsinto focused modules for routing, handlers, models, middleware, and responsesAuthenticatedUserextractors so routes can access the current user from a bearer tokenDetails
GET /meendpoint backed by the authenticated user contextget_authen_axum_routerandstart_servermaininto this branchVerification
cargo check --features webcargo test --docSummary by CodeRabbit
Breaking Changes
New Features
Improvements
Documentation