Skip to content

Refactor 1#3

Merged
levish0 merged 20 commits intomainfrom
refactor-1
Apr 5, 2026
Merged

Refactor 1#3
levish0 merged 20 commits intomainfrom
refactor-1

Conversation

@levish0
Copy link
Copy Markdown
Owner

@levish0 levish0 commented Apr 5, 2026

No description provided.

levish0 added 9 commits March 4, 2026 17:24
Introduce a dedicated Redis session client for persistent cron locks and update cron jobs to use it instead of the cache client. Added REDIS_SESSION_HOST/PORT config fields and WorkerConfig::redis_session_url(), plus a SessionClient type alias. Updated cron scheduler and lock helpers to accept/operate on SessionClient. Adjusted docker-compose.e2e.yml for e2e tests (expose MinIO on :9000, make test bucket anonymous, set R2 public domain to localhost, add redis-session env and dependency). Also includes various dependency version bumps in Cargo.toml and a small marker file (.e2e-build.done).
Introduce rename.py: an interactive script to find-and-replace a target word across file contents, filenames, and directory names. It performs case-preserving replacements (preserves UPPER/Title/lower forms), escapes special characters in the search term, skips common folders (.git, __pycache__, node_modules, .venv), and ignores non-text files by catching UnicodeDecodeError. The script walks the tree bottom-up to safely rename directories, prints progress messages, and defaults to the current directory when no path is provided.
Introduce Role and ModerationResourceType iden enums and reexports in migration common module. Add migrations to create enum types and tables: user role enum, user_roles, user_bans, moderation resource type enum, and moderation_logs, and register them in the Migrator. Remove ActorIp column and its index from action logs schema. Also bump migration crate dependencies (async-std, strum, sea-orm-migration) in Cargo.toml.
Refactor common types into a dedicated common module (action, moderation, oauth_provider, role) and introduce corresponding ActiveEnum definitions. Add new entities: moderation_logs, user_bans, user_roles and wire relations to users. Update action_logs, user_oauth_connections and users to use the new common types, change several column types to Text (and mark handle unique), add totp_secret to users, and remove actor_ip from action_logs. Also apply numerous minor formatting/import reorderings across auth route and service files and add missing error/ServerConfig imports where needed.
Move ActionLogFilter and common filtering logic into a new filter.rs and expose it from the action_logs module. Replace duplicated filter construction in find.rs and exists (newer/older) with a single apply_action_log_filter helper to DRY the code; update imports accordingly. No functional change intended—just refactoring to centralize filtering behavior.
Introduce role-based access control: add AccessLevel enum and expand Role variants. Add permission module with PermissionService, UserContext and Rule trait to evaluate access/roles and ban state. Implement user_roles and user_bans repositories (create/delete/find) and wire them into repository exports. Update OAuth pending signup flow and DTOs to remove display_name from pending signup payloads and adjust related imports. Update migration role enum to match new role variants.
Introduce moderation logging end-to-end: add ModerationAction enum in constants and export it; add DTOs for listing moderation logs (request/response); implement server routes, OpenAPI integration, and route registration for /v0/moderation/logs; add repository layer for moderation logs (create, filter, find_list, exists newer/older); add service to handle listing with pagination and has_newer/has_older logic; and wire everything together.

Also: add display_name to the complete signup DTO and propagate it through server and oauth signup service; remove the AccessLevel enum and related permission helper methods from the permission service (adjusted error handling for self-management); and tidy imports/formatting in Google OAuth sign-in code.
Reformat code across crates: reorder and normalize imports, add missing trailing newlines, and adjust indentation/line breaks for chained calls and let bindings (e.g. moderation logs and GitHub OAuth sign-in). These are stylistic changes only and do not alter program logic.
@levish0 levish0 self-assigned this Apr 5, 2026
levish0 added 8 commits April 5, 2026 17:39
Introduce request and response DTOs for user ban, unban, grant role and revoke role flows (with serde, ToSchema and IntoResponse impls). Add a datetime validator (validate_future_datetime) and expose it via validator mod; include unit tests for expires_at validation. Extend axumkit-errors with new user error variants, protocol string constants and map_response/handler entries to return appropriate HTTP status codes and messages for these cases.
Refactor: translate many inline comments and docstrings from Korean to English across multiple crates to improve readability and consistency. Add user management endpoints and services (ban/grant_role/revoke_role/unban) with new route/service modules and DTO exports, and include permission tests. Miscellaneous tidy-ups: clarify config comments, error handler messages, DTO validation/docs, OAuth/TOTP responses, and minor utility tweaks.
Add stricter Unicode-aware validation for strings: import unicode_general_category and document validate_not_blank; restrict handle names to ASCII alphanumeric + underscore (rejecting invisible/homoglyph/combining/control chars) and keep reserved handles list; overhaul display name validation to use Unicode general categories (reject Cc, Cf, Cs, Co, Zl, Zp, So), limit consecutive non-spacing marks to 2 to prevent Zalgo text, and return clearer error kinds for invalid/combining characters. These changes tighten allowed characters and better handle emoji/invisible/unicode edge cases.
Tighten display name validation by rejecting several Unicode general categories (Control, Format, Surrogate, Private Use, Line/Paragraph Separators and Other Symbol — e.g. emoji). Also add handling for NonspacingMark to track consecutive combining marks and reject names with more than two in a row to mitigate Zalgo-style abuse. Inline comments enumerate the relevant categories for clarity.
Replace separate read/write Postgres connections with a single database connection (via PgDog). Simplify ServerConfig to use unified POSTGRES_* env vars and single max/min connection settings, rename connection initializer to establish_connection, update AppState to hold db, and update all API routes and connection usage to use state.db instead of state.write_db/read_db. Also bump workspace version to 0.7.0 in Cargo.toml.
Introduce a deferred email+password signup flow backed by Redis and worker jobs. Adds POST /auth/signup that stores a pending signup (email, handle, hashed password) and queues a verification email; a Lua script (reserve_pending_signup.lua) atomically reserves email/handle indices and token payload. Resend verification now operates by email and reuses the existing token TTL to avoid enumeration. Verify-email consumes the pending token, creates the user with a pre-hashed password (repository_create_user_with_password_hash), commits the DB transaction, and then best-effort cleans up Redis and enqueues indexing. Removes the old public user creation route and adjusts OpenAPI schemas/routes accordingly. Also adds Google One Tap support: DTO, route handler, service hook and new error codes for invalid ID tokens / JWKS failures. Adds jsonwebtoken and related deps to Cargo, updates constants (email signup keys) and cache helpers, and tweaks some response status codes and logging. Overall implements safe pending-signup lifecycle, endpoint wiring, and documentation updates.
@levish0
Copy link
Copy Markdown
Owner Author

levish0 commented Apr 5, 2026

@copilot

@levish0
Copy link
Copy Markdown
Owner Author

levish0 commented Apr 5, 2026

@copilot review

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 5, 2026

@copilot review

Here's the review of the 17 commits in this PR:


✅ Build & Tests

  • cargo check --workspace passes with zero errors and zero warnings
  • All 20 unit tests pass (DTO validators + server unit tests)

🟢 Strengths

DB consolidation — removing the separate read/write pool in favour of a single PgDog connection is a clean simplification. Removing SeaweedFS/NATS/posts/search also trims a lot of dead weight.

Permission systemPermissionService::require_admin_for_target correctly handles: caller must be Admin, cannot self-target, cannot target another Admin. All four management operations (ban/unban/grant_role/revoke_role) wrap their writes in transactions and log to moderation_logs.

Google One Tap — JWT validation is solid: exp/iss/aud/sub required claims, nbf validated, issuer checked against both accounts.google.com forms, audience bound to google_client_id. JWKS caching respects the Cache-Control: max-age header from Google, with a graceful fallback to 5 minutes.

Cron distributed locking — the Lua release_lock.lua / extend_lock.lua scripts are atomic and correct. Using Uuid::now_v7() as the lock token is a nice choice since it's unique and sortable.

redis_cache utility — good centralization; get_json_and_delete (using GET DEL) avoids a TOCTOU window compared to separate GET + DEL.

String validators — using unicode_general_category to block Cc/Cf/Cs/Co/Zl/Zp/So categories and cap consecutive combining marks is thorough.


🟡 Issues

1. rename.py should not be committed
This is a one-off dev utility. It doesn't belong in the repo and inflates the diff significantly. Recommend removing it (and adding to .gitignore if it'll be reused locally).

2. .e2e-build.done is a build artifact
This file was committed but it's a build sentinel, not source. Should be in .gitignore.

3. ModerationAction::SearchReindex is dead code
Search/reindex was removed in this PR, but this enum variant (and its as_str/FromStr arms) remains. Either remove it or add a #[allow(dead_code)] comment explaining it's reserved for a future feature.

4. moderation_action_to_string / string_to_moderation_action free functions are unused
These are re-exported from axumkit-constants but never called anywhere outside the crate. The Display and FromStr impls already cover the same need — the free functions are redundant and can be removed.

5. JWKS cache stampede (minor)
In get_google_jwks, the pattern is: drop read lock → HTTP fetch → acquire write lock. Under concurrent load, multiple coroutines can all see an expired cache simultaneously and all fire an HTTP request to Google. Since the write lock serializes the final update it's not a correctness bug, but it wastes requests. A tokio::sync::Mutex around the refresh path (or double-checked locking after re-acquiring the read lock) would prevent this.

6. Hard-coded production domain in comment
server_config.rs line: // Cookie Domain (e.g., ".seven.wiki" for cross-subdomain cookies) — minor, but leaks the production hostname into the public repo. The original example value (.example.com) was more appropriate.


Summary

The core changes (DB simplification, moderation system, Google One Tap, cron locking) are well-implemented. The main actionable items are cleaning up rename.py, .e2e-build.done, the dead SearchReindex variant, and the two unused free functions.

…dc03-4f36-b9d6-d5b2657f194c

Co-authored-by: levish0 <61821471+levish0@users.noreply.github.com>
levish0 added 2 commits April 5, 2026 19:07
Prevent cache stampede by acquiring the JWKS cache write lock and double-checking cached entries in get_google_jwks; return cached keys when still fresh unless force_refresh is set. fetch_google_jwks now returns (JwkSet, ttl_seconds) and no longer mutates the cache — the caller sets the cached value with an expires_at computed from the returned TTL. Also update a comment example for cookie_domain to use ".example.com".
@levish0 levish0 merged commit 9ff48be into main Apr 5, 2026
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants