Conversation
WalkthroughIntroduces session-based login and admin dashboard routes, Basic Auth for newsletter publishing, SurrealDB-backed session storage, and related schemas. Adds new handlers (home, login GET/POST, admin dashboard), session extractor, error variants, dependencies, config field, environment variables, workflow env wiring, and comprehensive tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant R as Router
participant L as Login GET
participant LP as Login POST
participant S as Sessions (SurrealDB store)
participant M as ModelManager
participant A as Admin Dashboard
C->>R: GET /login
R->>L: login::get
L-->>C: 200 HTML (form + flash)
C->>R: POST /login (username, password)
R->>LP: login::post
LP->>M: validate_credientials(credentials)
alt valid
LP->>S: renew + insert user_id
LP-->>C: 303 See Other Location: /admin/dashboard
C->>R: GET /admin/dashboard
R->>A: admin_dashboard (extract TypedSession)
A->>S: get_user_id
alt found
A->>M: get_username(user_id)
A-->>C: 200 HTML "Welcome {username}"
else not found
A-->>C: 303 Location: /login
end
else invalid
LP->>S: set message "Authentication Failed"
LP-->>C: 303 Location: /login
end
sequenceDiagram
autonumber
participant C as Client
participant R as Router
participant N as publish_newsletter
participant M as ModelManager
participant E as EmailClient
C->>R: POST /newsletters (Authorization: Basic ...)
R->>N: publish_newsletter(headers, body)
N->>N: basic_authentication(headers)
alt parsed
N->>M: validate_credientials(credentials)
alt valid
N->>M: fetch confirmed subscribers
loop per subscriber
N->>E: send email
end
N-->>C: 200 OK
else invalid
N-->>C: 401 Unauthorized<br/>WWW-Authenticate: Basic realm="publish"
end
else missing/invalid header
N-->>C: 401 Unauthorized<br/>WWW-Authenticate: Basic realm="publish"
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 34
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (12)
.env.example (2)
3-8: Use a strong, non-default HMAC secret (production-blocking if deployed as-is).Never ship with "secret". Use a high-entropy value and document it inline.
Apply:
# Subscriptions Application SUBSCRIPTIONS__PORT=1337 SUBSCRIPTIONS__HOST=0.0.0.0 SUBSCRIPTIONS__BASE_URL=http://localhost:1337 -SUBSCRIPTIONS__HMAC_SECRET=secret +# 32+ random bytes (base64). Example: `head -c 32 /dev/urandom | base64` +SUBSCRIPTIONS__HMAC_SECRET=change-me-to-a-strong-base64-secret
3-8: Satisfy dotenv-linter ordering (optional).Reorder keys to pass UnorderedKey without changing semantics.
# Subscriptions Application SUBSCRIPTIONS__PORT=1337 SUBSCRIPTIONS__HOST=0.0.0.0 -SUBSCRIPTIONS__BASE_URL=http://localhost:1337 -# 32+ random bytes (base64). Example: `head -c 32 /dev/urandom | base64` -SUBSCRIPTIONS__HMAC_SECRET=change-me-to-a-strong-base64-secret +SUBSCRIPTIONS__HMAC_SECRET=change-me-to-a-strong-base64-secret +SUBSCRIPTIONS__BASE_URL=http://localhost:1337.env.test (1)
3-8: OK to keep a weak test secret, but align with linter ordering.No security concern for tests; reorder to appease dotenv-linter.
# Subscriptions Application SUBSCRIPTIONS__PORT=1337 # not used by tests SUBSCRIPTIONS__HOST=0.0.0.0 # not used by tests -SUBSCRIPTIONS__BASE_URL=http://localhost:1337 # not used by tests -SUBSCRIPTIONS__HMAC_SECRET=secret +SUBSCRIPTIONS__HMAC_SECRET=secret +SUBSCRIPTIONS__BASE_URL=http://localhost:1337 # not used by testssrc/config.rs (2)
36-43: Validate hmac_secret on load (reject empty/weak values).Fail fast if the secret is missing or too short.
impl Config { pub fn load() -> Result<Self, ConfigError> { - config::Config::builder() + let cfg: Self = config::Config::builder() .add_source(config::Environment::with_prefix(env!("CARGO_PKG_NAME")).separator("__")) .build()? - .try_deserialize() + .try_deserialize()?; + // Minimal sanity check; tune threshold as needed. + if cfg.hmac_secret.expose_secret().len() < 32 { + return Err(config::ConfigError::Message( + "SUBSCRIPTIONS__HMAC_SECRET must be at least 32 characters".into(), + )); + } + Ok(cfg) } }Add missing import at the top:
-use secrecy::SecretString; +use secrecy::{ExposeSecret, SecretString};
27-34: Unify secrecy types for consistency.Use the same imported SecretString type everywhere.
pub struct DatabaseConfig { pub base_url: Url, pub username: String, - pub password: secrecy::SecretString, + pub password: SecretString, pub namespace: String, pub name: String, }.github/workflows/cd.yml (2)
72-87: Leaking risk: HMAC secret pulled from unencrypted vars; use Actions secrets (minimum) or Secret Manager (preferred).Do not source cryptographic secrets from
${{ vars.* }}.Minimal fix (use Actions secret):
env_vars: |- RUST_LOG=${{ vars.RUST_LOG }} SUBSCRIPTIONS__PORT=${{ vars.SUBSCRIPTIONS__PORT }} SUBSCRIPTIONS__HOST=${{ vars.SUBSCRIPTIONS__HOST }} SUBSCRIPTIONS__BASE_URL=${{ vars.SUBSCRIPTIONS__BASE_URL }} SUBSCRIPTIONS__DATABASE__BASE_URL=${{ vars.SUBSCRIPTIONS__DATABASE__BASE_URL }} SUBSCRIPTIONS__DATABASE__USERNAME=${{ vars.SUBSCRIPTIONS__DATABASE__USERNAME }} SUBSCRIPTIONS__DATABASE__PASSWORD=${{ secrets.SUBSCRIPTIONS__DATABASE__PASSWORD }} SUBSCRIPTIONS__DATABASE__NAMESPACE=${{ vars.SUBSCRIPTIONS__DATABASE__NAMESPACE }} SUBSCRIPTIONS__DATABASE__NAME=${{ vars.SUBSCRIPTIONS__DATABASE__NAME }} SUBSCRIPTIONS__EMAIL_CLIENT__SENDER_EMAIL=${{ vars.SUBSCRIPTIONS__EMAIL_CLIENT__SENDER_EMAIL }} SUBSCRIPTIONS__EMAIL_CLIENT__BASE_URL=${{ vars.SUBSCRIPTIONS__EMAIL_CLIENT__BASE_URL }} SUBSCRIPTIONS__EMAIL_CLIENT__AUTH_TOKEN=${{ secrets.SUBSCRIPTIONS__EMAIL_CLIENT__AUTH_TOKEN }} SUBSCRIPTIONS__EMAIL_CLIENT__TIMEOUT=${{ vars.SUBSCRIPTIONS__EMAIL_CLIENT__TIMEOUT }} - SUBSCRIPTIONS__HMAC_SECRET=${{ vars.SUBSCRIPTIONS__HMAC_SECRET }} + SUBSCRIPTIONS__HMAC_SECRET=${{ secrets.SUBSCRIPTIONS__HMAC_SECRET }}
69-74: Prefer Cloud Run Secret Manager integration for runtime secrets.Bind the secret at deploy-time via
--set-secretsto avoid env var drift and enable rotation.- flags: "--allow-unauthenticated" # This flag grants public access + # Assumes a Secret Manager secret named SUBSCRIPTIONS__HMAC_SECRET exists in the project. + flags: "--allow-unauthenticated --set-secrets=SUBSCRIPTIONS__HMAC_SECRET=${{ vars.GCP_PROJECT_ID }}/secrets/SUBSCRIPTIONS__HMAC_SECRET:latest"If you adopt this, remove SUBSCRIPTIONS__HMAC_SECRET from env_vars.
src/model.rs (1)
60-79: Check query results for update confirmation
confirm_subscriberignores the query result. Call.check()to surface SurrealDB errors (e.g., failed transaction) and to keep behavior consistent withcreate_subscriber.pub async fn confirm_subscriber(&self, token: String) -> Result<()> { self.db() .await? .query( r#" UPDATE subscriptions SET status = 'CONFIRMED' WHERE token = ( SELECT id FROM ONLY subscription_tokens WHERE token = $token_val ).id AND status = 'PENDING'; "#, ) .bind(("token_val", token)) - .await?; + .await? + .check()?; Ok(()) }tests/api/newsletter.rs (3)
140-153: Mismatch: assertion checks 422 but message says 400Align the failure message with the asserted status.
- "The API did not fail with 400 Bad Request when the payload was {}.", + "The API did not fail with 422 Unprocessable Entity when the payload was {}.",
116-119: Typos in test setup messages (“inisilized”)Tidy up assertion messages.
- .expect("Expected the app to be inisilized!"); + .expect("Expected the app to be initialized!");Also applies to: 83-86, 56-59, 160-163, 188-191, 220-223
128-136: Typos in case descriptions (“messing”)Correct copy for clarity.
- "messing title", + "missing title", ... - "messing content", + "missing content",src/handlers/newsletter.rs (1)
21-27: Do not log headers/body via #[instrument]Current instrumentation will capture
headers(Authorization) andbody(content). That’s sensitive.-#[tracing::instrument(skip(mm, email_client))] +#[tracing::instrument(skip(mm, email_client, headers, body))]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
.env.example(1 hunks).env.test(1 hunks).github/workflows/cd.yml(1 hunks)Cargo.toml(1 hunks)src/config.rs(1 hunks)src/errors.rs(3 hunks)src/handlers/admin/mod.rs(1 hunks)src/handlers/home/home.html(1 hunks)src/handlers/home/mod.rs(1 hunks)src/handlers/login/get.rs(1 hunks)src/handlers/login/login.html(1 hunks)src/handlers/login/mod.rs(1 hunks)src/handlers/login/post.rs(1 hunks)src/handlers/mod.rs(1 hunks)src/handlers/newsletter.rs(3 hunks)src/lib.rs(1 hunks)src/model.rs(2 hunks)src/session_state.rs(1 hunks)src/startup.rs(2 hunks)surrealdb/migrations/definitions/_initial.json(1 hunks)surrealdb/schemas/sessions.surql(1 hunks)surrealdb/schemas/users.surql(1 hunks)tests/api/admin_dashboard.rs(1 hunks)tests/api/helpers.rs(3 hunks)tests/api/login.rs(1 hunks)tests/api/main.rs(1 hunks)tests/api/newsletter.rs(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-03T08:52:40.276Z
Learnt from: TN19N
PR: TN19N/subscriptions#19
File: src/errors.rs:8-8
Timestamp: 2025-09-03T08:52:40.276Z
Learning: In the subscriptions codebase, the user TN19N prefers using thiserror's #[from] macro over manual From trait implementations for error conversions, as it keeps the code cleaner and more idiomatic.
Applied to files:
tests/api/helpers.rs
🧬 Code graph analysis (12)
src/handlers/admin/mod.rs (2)
src/handlers/health_check.rs (1)
health(8-12)src/handlers/subscription.rs (2)
confirm(41-48)subscribe(18-33)
tests/api/admin_dashboard.rs (1)
tests/api/helpers.rs (1)
new(29-82)
src/handlers/home/mod.rs (3)
src/handlers/health_check.rs (1)
health(8-12)src/main.rs (1)
main(6-32)src/handlers/subscription.rs (1)
confirm(41-48)
src/handlers/login/get.rs (1)
src/handlers/login/post.rs (1)
login(20-48)
tests/api/login.rs (1)
tests/api/helpers.rs (1)
new(29-82)
src/startup.rs (5)
src/handlers/admin/mod.rs (1)
admin_dashboard(9-37)src/handlers/home/mod.rs (1)
home(5-7)src/handlers/login/get.rs (1)
login(6-49)src/handlers/login/post.rs (1)
login(20-48)src/state.rs (1)
new(17-23)
src/handlers/login/post.rs (3)
src/handlers/login/get.rs (2)
login(6-49)messages(10-13)src/handlers/subscription.rs (2)
FormData(12-15)subscribe(18-33)src/state.rs (2)
AppState(10-14)AppState(16-24)
tests/api/main.rs (3)
src/handlers/admin/mod.rs (1)
admin_dashboard(9-37)src/handlers/login/get.rs (1)
login(6-49)src/handlers/login/post.rs (1)
login(20-48)
src/handlers/mod.rs (3)
src/handlers/home/mod.rs (1)
home(5-7)src/handlers/login/get.rs (1)
login(6-49)src/handlers/login/post.rs (1)
login(20-48)
tests/api/helpers.rs (3)
src/startup.rs (1)
init(24-55)tests/api/subscriptions.rs (1)
subscribe_works(10-48)tests/api/subscriptions_confirm.rs (1)
confirmation_works(24-75)
tests/api/newsletter.rs (1)
tests/api/helpers.rs (1)
new(29-82)
src/handlers/newsletter.rs (1)
tests/api/helpers.rs (1)
new(29-82)
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 7-7: [UnorderedKey] The SUBSCRIPTIONS__HMAC_SECRET key should go before the SUBSCRIPTIONS__HOST key
(UnorderedKey)
.env.test
[warning] 7-7: [UnorderedKey] The SUBSCRIPTIONS__HMAC_SECRET key should go before the SUBSCRIPTIONS__HOST key
(UnorderedKey)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: check
🔇 Additional comments (7)
tests/api/main.rs (1)
1-1: LGTM: test modules wired correctlyNew
admin_dashboardandloginmodules are declared and align with added test files.Also applies to: 4-4
src/handlers/login/mod.rs (1)
1-2: LGTM: clear module splitPublic
get/postsplit is straightforward and matches routing.src/handlers/home/mod.rs (1)
5-7: LGTM — simple, static home handler is correct.src/startup.rs (1)
28-35: LGTM: request-id is set before tracing and propagatedGood ordering: SetRequestIdLayer runs before TraceLayer so spans include the request id.
src/model.rs (1)
81-93: LGTM: lightweight projection for confirmed subscribersQuery shape and deserialization look good; unknown fields are ignored.
tests/api/helpers.rs (2)
20-21: Expose seeded credentials from TestApp is usefulSurfacing
test_usersimplifies login/basic-auth in tests. LGTM.
77-77: LGTM: cookies persisted in tests
save_cookies()is necessary for session-backed flows across requests.
| base64 = "0.22.1" | ||
| urlencoding = "2.1.3" | ||
| htmlescape = "0.3.1" | ||
| hmac = { version = "0.12.1", features = ["std"] } | ||
| sha2 = "0.10.9" | ||
| hex = "0.4.3" | ||
| tower-cookies = "0.11.0" | ||
| axum-messages = "0.8.0" | ||
| tower-sessions = "0.14.0" | ||
| tower-sessions-surrealdb-store = "0.7.0" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Version compatibility and toolchain pinning.
The added auth/session deps look consistent, but please sanity-check cross-crate compatibility (axum 0.8.x, tower-http 0.6.x, tower-sessions 0.14.x, store 0.7.x) and ensure CI uses a toolchain that supports edition = "2024".
- Consider adding
rust-versionto Cargo.toml and pinning toolchain viarust-toolchain.toml. - Consider caret ranges for non-security-critical crates to ease patch updates.
🏁 Script executed:
#!/bin/bash
# Verify toolchain and compatibility hints in-repo.
echo "rust-toolchain files:"; fd -HI --glob 'rust-toolchain{,.toml}' -t f
echo "workflows referencing toolchains:"; fd -HI .github/workflows -t f | xargs -r rg -n "toolchain|rust-version|rustup"Length of output: 650
🌐 Web query:
Is `tower-sessions-surrealdb-store` 0.7.x compatible with `tower-sessions` 0.14.x and SurrealDB 2.x? Any known breaking changes with Axum 0.8.x?
💡 Result:
Short answer: probably not without changes.
- tower-sessions-surrealdb-store appears to be an unmaintained beta (latest published ~0.5.x) and its repo/docs show no explicit support for tower-sessions 0.14 or recent SurrealDB 2.x. [1][2].
- tower-sessions 0.8+ → 0.14 introduced multiple breaking changes that stores must follow (async/lazy sessions, the Record intermediary, new concrete error type, and later a required create method for session creation/id-collision mitigation). A store written for older tower-sessions versions will need updates to implement these new SessionStore APIs. [3][4].
- SurrealDB 2.x made breaking changes (storage/ID semantics and SurrealQL differences); a store depending on the pre-2.x surrealdb crate or older query/ID semantics will likely need code changes to work with SurrealDB 2.x. See the SurrealDB 2.x migration notes. [5].
- Axum 0.8 also contains breaking changes (path parameter syntax and extractor/trait changes). Since tower-sessions exposes an axum extractor and depends on axum-core, you should verify the store + app use axum-compatible versions; tower-sessions 0.14 bumped axum-core previously, but you may still need to adjust routes/extractors when moving to axum 0.8. [3][6].
Recommendation: if you need to run tower-sessions 0.14 + SurrealDB 2.x + axum 0.8, either
- update/patch tower-sessions-surrealdb-store to implement the current SessionStore trait and use a SurrealDB 2.x-compatible client; or
- look for/author a maintained store crate targeting tower-sessions ≥0.12 and surrealdb 2.x.
References:
- tower-sessions-surrealdb-store (repo/docs). [GitHub / docs.rs]. [2]
- tower-sessions release notes / changelog (breaking changes across 0.7→0.8→...→0.14). [3][4]
- tower-sessions crate docs (0.14). [3]
- tower-sessions changelog (notes about async/lazy, Record, create API). [4]
- SurrealDB 2.x upgrade / breaking changes. [5]
- axum 0.8 announcement (breaking changes). [6]
If you want, I can check the exact tower-sessions-surrealdb-store Cargo.toml and open-source code to list the specific API mismatches and a minimal patch.
🏁 Script executed:
#!/bin/bash
echo "== rust-toolchain.toml =="
sed -n '1,200p' rust-toolchain.toml
echo "== CI workflows referencing toolchains =="
grep -RInE "rust-toolchain|rust-version|toolchain|rustup" .github/workflows || echo "⟂ none found"
echo "== Cargo.toml editions =="
grep -RIn "^edition" Cargo.tomlLength of output: 576
Patch session store compatibility & confirm toolchain pinning
- tower-sessions-surrealdb-store v0.7.x isn’t compatible with tower-sessions v0.14.x and SurrealDB 2.x; update or replace it to implement the current SessionStore trait and SurrealDB v2 client.
- rust-toolchain.toml already pins channel 1.89.0 and CI’s setup-toolchain uses it; Cargo.toml is on edition "2024". Optionally add
rust-version = "1.89"to Cargo.toml. - Consider using caret ranges for non-security deps (e.g.
base64 = "^0.22") to allow automatic patch upgrades.
| Self::Auth(_) => { | ||
| tracing::warn!("Unauthorized : - {self:?}"); | ||
| Response::builder() | ||
| .status(StatusCode::UNAUTHORIZED) | ||
| .header("WWW-Authenticate", r#"Basic realm="publish""#) | ||
| .body(Body::empty()) | ||
| .unwrap_or(StatusCode::INTERNAL_SERVER_ERROR.into_response()) | ||
| } |
There was a problem hiding this comment.
Avoid logging secrets; add charset to WWW-Authenticate.
Do not log Auth(String) contents; it risks leaking credentials. Also include charset="UTF-8" per RFC 7617.
Apply:
- Self::Auth(_) => {
- tracing::warn!("Unauthorized : - {self:?}");
- Response::builder()
- .status(StatusCode::UNAUTHORIZED)
- .header("WWW-Authenticate", r#"Basic realm="publish""#)
- .body(Body::empty())
- .unwrap_or(StatusCode::INTERNAL_SERVER_ERROR.into_response())
- }
+ Self::Auth(_) => {
+ // Avoid logging credential material.
+ tracing::warn!("Unauthorized");
+ Response::builder()
+ .status(StatusCode::UNAUTHORIZED)
+ .header(axum::http::header::WWW_AUTHENTICATE, r#"Basic realm="publish", charset="UTF-8""#)
+ .body(Body::empty())
+ .unwrap_or(StatusCode::INTERNAL_SERVER_ERROR.into_response())
+ }📝 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.
| Self::Auth(_) => { | |
| tracing::warn!("Unauthorized : - {self:?}"); | |
| Response::builder() | |
| .status(StatusCode::UNAUTHORIZED) | |
| .header("WWW-Authenticate", r#"Basic realm="publish""#) | |
| .body(Body::empty()) | |
| .unwrap_or(StatusCode::INTERNAL_SERVER_ERROR.into_response()) | |
| } | |
| Self::Auth(_) => { | |
| // Avoid logging credential material. | |
| tracing::warn!("Unauthorized"); | |
| Response::builder() | |
| .status(StatusCode::UNAUTHORIZED) | |
| .header( | |
| axum::http::header::WWW_AUTHENTICATE, | |
| r#"Basic realm="publish", charset="UTF-8""#, | |
| ) | |
| .body(Body::empty()) | |
| .unwrap_or(StatusCode::INTERNAL_SERVER_ERROR.into_response()) | |
| } |
🤖 Prompt for AI Agents
In src/errors.rs around lines 49 to 56, the current branch logs the Auth(String)
contents and sets the WWW-Authenticate header without a charset; remove or
sanitize the logged secret by replacing the tracing::warn! call with a generic
message that does not include the auth string (e.g. "Unauthorized access
attempt"), and update the header value to include charset per RFC 7617 (e.g.
Basic realm="publish", charset="UTF-8"); keep the rest of the response
construction identical and do not expose the inner String anywhere in logs or
responses.
| let username = match session.get_user_id().await { | ||
| Ok(Some(user_id)) => mm.get_username(user_id).await?, | ||
| reason => { | ||
| tracing::error!("Failed to authenticate: {reason:?}"); | ||
| return Ok(Redirect::to("/login").into_response()); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Escape username before embedding in HTML (stored XSS risk).
username comes from the DB and is rendered unescaped into HTML. Escape it before formatting.
use std::sync::Arc;
+use askama_escape::escape;
@@
};
+ let username = escape(&username).to_string();
+
let body = format!(
r#"
<!DOCTYPE html>
<html>
<head>
<meta http-equiv="content-type" content="text/html; charset=utf-8">
<title>Admin dashboard</title>
</head>
<body>
<p>Welcome {username}</p>
</body>
</html>
"#
);Add dependency (Cargo.toml):
askama_escape = "0.10"Also applies to: 21-36
🤖 Prompt for AI Agents
In src/handlers/admin/mod.rs around lines 13-19 (and also apply to 21-36), the
username retrieved from the DB is embedded into HTML without escaping (stored
XSS risk); add the askama_escape = "0.10" dependency to Cargo.toml, import and
use the Askama HTML escape utility (or equivalent escape function) to escape
username before inserting it into any HTML/template or string formatting, and
replace direct uses of username in those lines with the escaped value (ensure
all occurrences in 21-36 are updated similarly).
| <!doctype html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta http-equiv="content-type" content="text/html; charset=utf-8" /> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use modern charset meta
Prefer the shorter, HTML5-standard <meta charset="utf-8">.
- <meta http-equiv="content-type" content="text/html; charset=utf-8" />
+ <meta charset="utf-8">📝 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.
| <meta http-equiv="content-type" content="text/html; charset=utf-8" /> | |
| <meta charset="utf-8"> |
🤖 Prompt for AI Agents
In src/handlers/home/home.html around line 4 the document uses the legacy meta
tag `<meta http-equiv="content-type" content="text/html; charset=utf-8" />`;
replace it with the HTML5-standard shorter form `<meta charset="utf-8">` on that
line, removing the http-equiv/content-type attributes and the self-closing slash
so the page declares charset in the modern, preferred way.
| // Query(query): Option<Query<QueryParams>>, | ||
| ) -> Result<impl IntoResponse> { |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove dead, commented parameter.
- // Query(query): Option<Query<QueryParams>>,📝 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.
| // Query(query): Option<Query<QueryParams>>, | |
| ) -> Result<impl IntoResponse> { | |
| ) -> Result<impl IntoResponse> { |
🤖 Prompt for AI Agents
In src/handlers/login/get.rs around lines 8 to 9, remove the dead commented
parameter "// Query(query): Option<Query<QueryParams>>," from the function
signature so the signature no longer contains commented-out parameters; update
any trailing commas/whitespace to keep formatting consistent and run cargo fmt.
| password: "password".into(), | ||
| username: "username".into(), | ||
| }; | ||
| let (router, state) = subscriptions::init(config).await?; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Seed user deterministically or ensure uniqueness
Repeated runs may insert duplicate users. Consider using a deterministic record id (e.g., users:{username}) or adding a unique index on username in migrations to avoid ambiguity in queries.
Example (if you choose deterministic ids):
- .query(
- r#"
- INSERT INTO users {
- username: $username,
- password: crypto::argon2::generate($password)
- }
- "#,
- )
+ .query(r#"
+ CREATE ONLY users:$username CONTENT {
+ username: $username,
+ password: crypto::argon2::generate($password)
+ }
+ "#)Alternatively, add a migration like:
DEFINE INDEX idx_users_username_unique ON TABLE users COLUMNS username UNIQUE;
Also applies to: 55-75
🤖 Prompt for AI Agents
In tests/api/helpers.rs around lines 49-52 (and also affecting lines 55-75), the
seeded test user can be duplicated across runs; fix by creating the user with a
deterministic id (e.g., "users:{username}") when inserting so repeated seeds
upsert the same record, or alternatively add a DB migration that creates a
unique index on the username column (e.g., DEFINE INDEX ... UNIQUE) and adjust
tests to handle unique constraint errors; ensure subsequent queries rely on the
deterministic id or the unique username index to avoid ambiguity.
| // Arrenge | ||
| let app = TestApp::new() | ||
| .await | ||
| .expect("Expected the app to be inissilized!"); | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Fix typos in comments and panic messages.
- // Arrenge
+ // Arrange
@@
- .expect("Expected the app to be inissilized!");
+ .expect("Expected the app to be initialized!");
@@
- // Arrenge
+ // Arrange
@@
- .expect("Expected the app to be inissilized!");
+ .expect("Expected the app to be initialized!");Also applies to: 43-47
🤖 Prompt for AI Agents
In tests/api/login.rs around lines 7-11 (and likewise lines 43-47), fix multiple
typos in comments and panic messages: change "Arrenge" to "Arrange" and
"inissilized" to "initialized" (and ensure correct grammar/capitalization in the
panic string, e.g., "Expected the app to be initialized!"). Update any other
misspellings in those ranges to be clear and correct.
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn redirect_to_admin_dashboard_after_login_login_success() { |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Rename test for clarity (remove duplicate “login”).
-async fn redirect_to_admin_dashboard_after_login_login_success() {
+async fn redirects_to_admin_dashboard_after_successful_login() {📝 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.
| async fn redirect_to_admin_dashboard_after_login_login_success() { | |
| async fn redirects_to_admin_dashboard_after_successful_login() { |
🤖 Prompt for AI Agents
In tests/api/login.rs around line 42, the test function name contains a
duplicated word ("login_login"); rename the function to remove the duplicate so
it reads e.g. redirect_to_admin_dashboard_after_login_success (or another clear,
single-"login" name) and update any references to this test name accordingly.
| .form(&json!({ | ||
| "username": app.test_user.username, | ||
| "password": app.test_user.password, | ||
| })) | ||
| .await; | ||
|
|
||
| // Assert | ||
| assert_eq!(response.status_code(), StatusCode::SEE_OTHER); | ||
| assert_eq!(response.header(LOCATION), "/admin/dashboard"); | ||
| let response = app.server.get("/admin/dashboard").await; | ||
| assert!( | ||
| response | ||
| .text() | ||
| .contains(&format!("Welcome {}", app.test_user.username)) | ||
| ); |
There was a problem hiding this comment.
Fix moved-value bug: don’t move credentials out of app.test_user.
You move the Strings into json! then try to use them again (Line 65) — this will not compile. Clone (or borrow) before building the form payload and reuse the local vars in the assertion.
@@
- let response = app
+ let username = app.test_user.username.clone();
+ let password = app.test_user.password.clone();
+ let response = app
.server
.post("/login")
.form(&json!({
- "username": app.test_user.username,
- "password": app.test_user.password,
+ "username": &username,
+ "password": &password,
}))
.await;
@@
- assert!(
- response
- .text()
- .contains(&format!("Welcome {}", app.test_user.username))
- );
+ assert!(response.text().contains(&format!("Welcome {}", username)));📝 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.
| .form(&json!({ | |
| "username": app.test_user.username, | |
| "password": app.test_user.password, | |
| })) | |
| .await; | |
| // Assert | |
| assert_eq!(response.status_code(), StatusCode::SEE_OTHER); | |
| assert_eq!(response.header(LOCATION), "/admin/dashboard"); | |
| let response = app.server.get("/admin/dashboard").await; | |
| assert!( | |
| response | |
| .text() | |
| .contains(&format!("Welcome {}", app.test_user.username)) | |
| ); | |
| let username = app.test_user.username.clone(); | |
| let password = app.test_user.password.clone(); | |
| let response = app | |
| .server | |
| .post("/login") | |
| .form(&json!({ | |
| "username": &username, | |
| "password": &password, | |
| })) | |
| .await; | |
| // Assert | |
| assert_eq!(response.status_code(), StatusCode::SEE_OTHER); | |
| assert_eq!(response.header(LOCATION), "/admin/dashboard"); | |
| let response = app.server.get("/admin/dashboard").await; | |
| assert!(response.text().contains(&format!("Welcome {}", username))); |
🤖 Prompt for AI Agents
In tests/api/login.rs around lines 52 to 66, the code moves the
username/password Strings into json! for the form then later reuses
app.test_user.username when asserting the response, causing a moved-value
compile error; fix by creating local references or clones before building the
form payload (e.g., let username = app.test_user.username.clone() and let
password = app.test_user.password.clone() or use &app.test_user.username and
&app.test_user.password inside json!), use those local vars when calling
.form(&json!(...)).await, and then reuse the local username in the final
assertion so app.test_user is not moved from.
| #[tokio::test] | ||
| async fn requests_messing_authorization_are_rejected() { | ||
| // Arrange | ||
| let app = TestApp::new() | ||
| .await | ||
| .expect("Expected the app to be inisilized!"); | ||
|
|
||
| // Act | ||
| let response = app | ||
| .server | ||
| .post("/newsletter") | ||
| .json(&json!({ | ||
| "title": "Newsletter title", | ||
| "content": { | ||
| "text": "Newsletter body as plain text", | ||
| "html": "<p>Newsletter body as html</p>", | ||
| }, | ||
| })) | ||
| .await; | ||
|
|
||
| // Assert | ||
| assert_eq!(response.status_code(), StatusCode::UNAUTHORIZED); | ||
| assert_eq!( | ||
| r#"Basic realm="publish""#, | ||
| response.header("WWW-Authenticate") | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Fix test name: “missing” not “messing”
Improves readability and searchability.
-async fn requests_messing_authorization_are_rejected() {
+async fn requests_missing_authorization_are_rejected() {📝 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.
| #[tokio::test] | |
| async fn requests_messing_authorization_are_rejected() { | |
| // Arrange | |
| let app = TestApp::new() | |
| .await | |
| .expect("Expected the app to be inisilized!"); | |
| // Act | |
| let response = app | |
| .server | |
| .post("/newsletter") | |
| .json(&json!({ | |
| "title": "Newsletter title", | |
| "content": { | |
| "text": "Newsletter body as plain text", | |
| "html": "<p>Newsletter body as html</p>", | |
| }, | |
| })) | |
| .await; | |
| // Assert | |
| assert_eq!(response.status_code(), StatusCode::UNAUTHORIZED); | |
| assert_eq!( | |
| r#"Basic realm="publish""#, | |
| response.header("WWW-Authenticate") | |
| ); | |
| } | |
| #[tokio::test] | |
| async fn requests_missing_authorization_are_rejected() { | |
| // Arrange | |
| let app = TestApp::new() | |
| .await | |
| .expect("Expected the app to be inisilized!"); | |
| // Act | |
| let response = app | |
| .server | |
| .post("/newsletter") | |
| .json(&json!({ | |
| "title": "Newsletter title", | |
| "content": { | |
| "text": "Newsletter body as plain text", | |
| "html": "<p>Newsletter body as html</p>", | |
| }, | |
| })) | |
| .await; | |
| // Assert | |
| assert_eq!(response.status_code(), StatusCode::UNAUTHORIZED); | |
| assert_eq!( | |
| r#"Basic realm="publish""#, | |
| response.header("WWW-Authenticate") | |
| ); | |
| } |
🤖 Prompt for AI Agents
In tests/api/newsletter.rs around lines 157 to 183, the test function name
contains a typo ("messing" instead of "missing"); rename the test function from
requests_messing_authorization_are_rejected to
requests_missing_authorization_are_rejected (keep the #[tokio::test] attribute
and body unchanged) and update any references to this test name if they exist
elsewhere in the repo.
1 participant
Summary by CodeRabbit
New Features
Security
Configuration
Chores