feat: add user invitation system - #104
Conversation
Add a complete user invitation feature that allows existing users to invite new users by email. Key functionality includes: - Invitation creation with secure token generation (SHA256 hashed) - Provisional user creation for invitees (UserStatus::Provisional) - Invitation acceptance, revocation, and expiration handling - Email templates for invitation notifications - Auto-accept pending invitations on password registration - Rate limiting via max_pending_per_email configuration Storage support: - Full PostgreSQL implementation with migrations - SQLite and SeaORM stubs (Postgres-only for initial release) Security considerations: - Tokens stored as hashes, plaintext only available at creation - Email validation on invitation creation - Configurable expiration (default 7 days) - invited_by correctly set to None when no inviter specified
📝 WalkthroughWalkthroughThis PR introduces a comprehensive user invitation system to the torii authentication framework, adding domain types (InvitationId, InvitationStatus, Invitation), service-layer business logic, repository abstraction across multiple storage backends (PostgreSQL, SeaORM, SQLite), email templates, and framework integration. Additionally, it extends the User model with status tracking (Provisional/Active) and invitation linkage. Changes
Sequence Diagram(s)sequenceDiagram
actor Admin as Admin/Inviter
participant Torii
participant InvitationService
participant TokenGen as Token Generator
participant Repo as InvitationRepository
participant DB as Database
participant Mailer
rect rgb(200, 220, 255)
Note over Admin,DB: Create Invitation Flow
Admin->>Torii: create_invitation(email, inviter_id)
Torii->>InvitationService: create_invitation(...)
InvitationService->>InvitationService: validate_email()
InvitationService->>InvitationService: check_pending_limit()
InvitationService->>TokenGen: generate_secure_token()
TokenGen-->>InvitationService: token (plaintext)
InvitationService->>InvitationService: hash_token()
InvitationService->>InvitationService: create Invitation<br/>(with token_hash)
InvitationService->>Repo: create(invitation)
Repo->>DB: INSERT invitations
DB-->>Repo: stored invitation
Repo-->>InvitationService: invitation
opt provisional_user enabled
InvitationService->>Repo: create_user(provisional)
Repo->>DB: INSERT users
DB-->>Repo: user
Repo-->>InvitationService: user
end
InvitationService-->>Torii: (invitation, optional_user)
Torii->>Mailer: send_invitation_email(token)
Mailer-->>Torii: email sent
Torii-->>Admin: invitation created
end
rect rgb(220, 255, 220)
Note over Admin,DB: Accept Invitation Flow (Later)
Admin->>Torii: accept_invitation(token, user_id)
Torii->>InvitationService: accept_invitation(token, user_id)
InvitationService->>InvitationService: hash_token()
InvitationService->>Repo: find_by_token_hash(hash)
Repo->>DB: SELECT invitations WHERE token_hash
DB-->>Repo: invitation
Repo-->>InvitationService: invitation
InvitationService->>InvitationService: verify is_valid()<br/>(pending + not expired)
alt token valid & not expired
InvitationService->>Repo: accept(invitation_id, user_id)
Repo->>DB: UPDATE invitations<br/>SET status=Accepted
DB-->>Repo: updated invitation
Repo-->>InvitationService: invitation
opt provisional user exists
InvitationService->>Repo: activate_user(user_id)
Repo->>DB: UPDATE users<br/>SET status=Active
end
InvitationService-->>Torii: (invitation, user)
Torii-->>Admin: invitation accepted
else token invalid or expired
InvitationService-->>Torii: Error
Torii-->>Admin: acceptance failed
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes The review requires understanding the invitation lifecycle across multiple layers (domain, service, repository, storage), verifying token handling and security practices, checking consistency across three storage backends (PostgreSQL implemented, SeaORM/SQLite stubbed), validating business logic for provisional users and status transitions, and ensuring proper integration with existing authentication flows. Additional attention needed for the duplicate adapter block in Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Comment |
|
@greptile |
|
@cmackenzie1 I'll review the changes in this PR for the user invitation system implementation. ✅ Actions performedReview triggered.
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #104 +/- ##
==========================================
- Coverage 67.78% 66.61% -1.17%
==========================================
Files 92 97 +5
Lines 9004 10082 +1078
==========================================
+ Hits 6103 6716 +613
- Misses 2901 3366 +465 ☔ View full report in Codecov by Sentry. |
![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)
torii-storage-seaorm/src/repositories/user.rs (1)
36-51: Bug:statusandinvited_byfields fromNewUserare not persisted.The
create()method buildsuser::ActiveModelusing..Default::default()but never sets thestatusorinvited_byfields from thenew_userparameter. These values will be silently ignored, and the database defaults will be used instead.🔎 Proposed fix
async fn create(&self, new_user: NewUser) -> Result<User, Error> { let user_model = user::ActiveModel { id: Set(new_user.id.to_string()), email: Set(new_user.email), name: Set(new_user.name), email_verified_at: Set(new_user.email_verified_at), + status: Set(new_user.status.as_str().to_string()), + invited_by: Set(new_user.invited_by.map(|id| id.into_inner())), ..Default::default() };
🧹 Nitpick comments (9)
torii-core/src/services/mailer.rs (1)
237-398: Consider adding test coverage for send_invitation_email.The test module includes tests for other email methods (magic link, welcome, password reset, etc.) but not for the new
send_invitation_emailmethod. Adding a test would improve consistency and provide verification that the email builds correctly with the expected subject and content.🔎 Suggested test
#[tokio::test] async fn test_send_invitation_email() { let service = create_test_service(); let result = service .send_invitation_email( "invitee@example.com", "https://example.com/invite/token123", Some("John Inviter"), 7, ) .await; assert!(result.is_ok()); } #[tokio::test] async fn test_send_invitation_email_without_inviter() { let service = create_test_service(); let result = service .send_invitation_email( "invitee@example.com", "https://example.com/invite/token123", None, 7, ) .await; assert!(result.is_ok()); }torii-storage-postgres/src/lib.rs (1)
145-146: Consider logging when status parsing fails.Line 159 uses
unwrap_or_default()which silently falls back to the defaultUserStatusif parsing fails. While this is a safe approach, it could mask data integrity issues.Consider adding a
tracing::warn!when the parse fails to aid debugging:🔎 Suggested improvement
- .status(user.status.parse().unwrap_or_default()) + .status(user.status.parse().unwrap_or_else(|_| { + tracing::warn!(status = %user.status, user_id = %user.id, "Failed to parse user status, using default"); + UserStatus::default() + }))Also applies to: 152-166
torii-storage-postgres/src/migrations/mod.rs (1)
861-958: LGTM! CreateInvitationsTable migration is comprehensive.The table design is solid:
token_hashwith UNIQUE constraint ensures secure, indexed lookups- Foreign keys with
ON DELETE SET NULLmaintain data integrity while allowing cleanup- JSONB
metadataprovides flexibility for future extensions- Index coverage is thorough for expected query patterns
💡 Optional: Consider a partial index for pending invitations
Since querying pending invitations by email is described as "most common query", a partial index could be more efficient:
CREATE INDEX IF NOT EXISTS idx_invitations_email_pending ON invitations(email) WHERE status = 'pending';This would be smaller and faster than the composite
(email, status)index for this specific case.torii-storage-sqlite/src/repositories/invitation.rs (1)
29-106: Consider using a more semantic error type for unimplemented functionality.All methods return
StorageError::Database(...)which semantically implies a database connection/query failure rather than "not implemented". IfStorageErrorhas aNotImplementedorUnsupportedvariant, that would be more appropriate.However, this is acceptable for a stub that will be replaced with a full implementation.
torii-storage-postgres/src/repositories/invitation.rs (2)
28-45: Consider logging when status parsing falls back to Pending.The fallback
unwrap_or(InvitationStatus::Pending)on line 35 silently converts unknown status values toPending. This could mask data corruption or schema mismatches.🔎 Proposed change
- inv.status.parse().unwrap_or(InvitationStatus::Pending), + inv.status.parse().unwrap_or_else(|_| { + tracing::warn!(status = %inv.status, invitation_id = %inv.id, "Unknown invitation status, defaulting to Pending"); + InvitationStatus::Pending + }),
211-230: Consider adding status guard in SQL for defense in depth.The
acceptmethod updates the invitation regardless of its current status. While the service layer validates this, adding aWHERE status = 'pending'guard would provide defense in depth and prevent accidental double-acceptance at the storage level.🔎 Proposed change
let pg_invitation = sqlx::query_as::<_, PostgresInvitation>( r#" UPDATE invitations SET status = 'accepted', accepted_at = NOW(), accepted_by = $1, updated_at = NOW() - WHERE id = $2 + WHERE id = $2 AND status = 'pending' RETURNING id, email, token_hash, inviter_id, status, metadata, expires_at, accepted_at, accepted_by, revoked_at, created_at, updated_at "#, )Note: You would need to handle the case where no row is returned (invitation not in pending state) and return an appropriate error.
torii/src/lib.rs (1)
1060-1061: Consider clampingexpires_in_daysto a minimum of 1 for better email UX.The calculation
(invitation.expires_at - chrono::Utc::now()).num_days()could return 0 (if expiry is less than 24 hours away) or negative (edge case). For email templates, clamping to at least 1 day may provide better user experience.🔎 Proposed change
// Calculate expiry in days from the invitation's expires_at - let expires_in_days = (invitation.expires_at - chrono::Utc::now()).num_days(); + let expires_in_days = (invitation.expires_at - chrono::Utc::now()).num_days().max(1);torii-core/src/services/invitation.rs (2)
231-262: Redundant token verification after hash lookup.After finding the invitation by
token_hash(line 237-240), theverify(token)call on line 244 is redundant. SHA256 hash lookup is deterministic - if the hash matches, the token is valid. The verification adds computational overhead without security benefit.🔎 Consider simplifying
let token_hash = hash_token(token); let invitation = self .invitation_repository .find_by_token_hash(&token_hash) .await? .ok_or(Error::Auth(AuthError::InvalidCredentials))?; - // Verify token and check if invitation can be accepted - if !invitation.verify(token) { - return Err(Error::Auth(AuthError::InvalidCredentials)); - } - if !invitation.can_accept() {The hash lookup already confirms token validity. If you want to keep defense-in-depth, add a comment explaining the redundancy is intentional.
176-189: Unwrap is safe here but consider documenting the invariant.Line 180 uses
.unwrap()oninvitation.token(). This is safe becauseInvitation::new()always sets the token. However, documenting this invariant would improve code clarity.🔎 Consider adding a comment
// Return invitation with plaintext token + // Safety: token() is always Some for newly created invitations via Invitation::new() Ok(( Invitation::new( stored_invitation.id, stored_invitation.email, - invitation.token().unwrap().to_string(), + invitation.token().expect("newly created invitation always has token").to_string(),Or use a more descriptive expect message to document the invariant.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
torii-core/src/crypto.rstorii-core/src/invitation.rstorii-core/src/lib.rstorii-core/src/repositories/adapter.rstorii-core/src/repositories/invitation.rstorii-core/src/repositories/mod.rstorii-core/src/services/invitation.rstorii-core/src/services/magic_link.rstorii-core/src/services/mailer.rstorii-core/src/services/mod.rstorii-core/src/services/oauth.rstorii-core/src/services/passkey.rstorii-core/src/services/password.rstorii-core/src/services/password_reset.rstorii-core/src/storage.rstorii-core/src/user.rstorii-mailer/src/email_types.rstorii-mailer/src/lib.rstorii-mailer/src/templates/auth_templates.rstorii-mailer/src/templates/engine.rstorii-mailer/src/templates/mod.rstorii-storage-postgres/Cargo.tomltorii-storage-postgres/src/lib.rstorii-storage-postgres/src/migrations/mod.rstorii-storage-postgres/src/repositories/invitation.rstorii-storage-postgres/src/repositories/mod.rstorii-storage-postgres/src/repositories/user.rstorii-storage-seaorm/src/repositories/invitation.rstorii-storage-seaorm/src/repositories/mod.rstorii-storage-seaorm/src/repositories/user.rstorii-storage-seaorm/src/user.rstorii-storage-sqlite/src/repositories/invitation.rstorii-storage-sqlite/src/repositories/mod.rstorii/src/lib.rstorii/tests/invitation.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Usethiserrorwith structured error types and#[from]for conversions
Use PascalCase for types, snake_case for functions/variables, SCREAMING_SNAKE_CASE for constants
Use newtype pattern for type safety (e.g.,UserId,SessionToken)
Group imports by category (std lib first, external crates, then internal modules)
Useasync_traitfor async interfaces; design with composition in mind
Add doc comments to public interfaces and modules
Write unit tests in modules with#[cfg(test)]; use#[tokio::test]for async tests
Use Builder Pattern for complex struct creation with validation at build time
Always usemake fmtto format Rust code
Always usemake lintto lint Rust code
**/*.rs: Never use the sqlx query macrosquery!orquery_as!when writing sqlx queries
Always use the non-macro version when writing sqlx queries
Prefer passing borrowed values when using bind in sqlx queries
**/*.rs: Adhere to existing style in Rust code
Prefer writing out builders instead of deriving them in Rust code
Use Tokio as the async runtime in Rust code
Ensure .await works correctly by ensuring Send + Sync on types that cross the await boundary in Rust code
Files:
torii-core/src/services/mod.rstorii-mailer/src/templates/engine.rstorii/tests/invitation.rstorii-core/src/lib.rstorii-core/src/services/magic_link.rstorii-core/src/services/mailer.rstorii-core/src/user.rstorii-mailer/src/templates/auth_templates.rstorii-mailer/src/email_types.rstorii-core/src/repositories/mod.rstorii-core/src/services/password.rstorii-mailer/src/templates/mod.rstorii-storage-seaorm/src/user.rstorii-storage-sqlite/src/repositories/mod.rstorii-storage-postgres/src/repositories/mod.rstorii-storage-seaorm/src/repositories/user.rstorii-storage-postgres/src/repositories/user.rstorii-core/src/crypto.rstorii-storage-postgres/src/repositories/invitation.rstorii-storage-postgres/src/lib.rstorii-core/src/storage.rstorii-storage-sqlite/src/repositories/invitation.rstorii-mailer/src/lib.rstorii-storage-seaorm/src/repositories/mod.rstorii-core/src/services/oauth.rstorii-core/src/repositories/invitation.rstorii-core/src/services/passkey.rstorii-storage-seaorm/src/repositories/invitation.rstorii-core/src/services/invitation.rstorii-storage-postgres/src/migrations/mod.rstorii-core/src/repositories/adapter.rstorii-core/src/services/password_reset.rstorii-core/src/invitation.rstorii/src/lib.rs
🧠 Learnings (4)
📚 Learning: 2025-12-30T23:46:37.502Z
Learnt from: CR
Repo: cmackenzie1/torii-rs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T23:46:37.502Z
Learning: Organize core functionality in `torii-core` crate
Applied to files:
torii-core/src/services/mod.rstorii-core/src/lib.rstorii-core/src/repositories/mod.rstorii-storage-seaorm/src/user.rstorii-storage-postgres/src/repositories/mod.rstorii-core/src/crypto.rstorii-mailer/src/lib.rstorii-storage-seaorm/src/repositories/mod.rstorii-core/src/repositories/invitation.rstorii-storage-seaorm/src/repositories/invitation.rstorii-core/src/services/invitation.rstorii-core/src/invitation.rstorii/src/lib.rs
📚 Learning: 2025-12-30T23:46:37.502Z
Learnt from: CR
Repo: cmackenzie1/torii-rs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T23:46:37.502Z
Learning: Applies to **/*.rs : Write unit tests in modules with `#[cfg(test)]`; use `#[tokio::test]` for async tests
Applied to files:
torii/tests/invitation.rs
📚 Learning: 2025-12-30T23:46:37.502Z
Learnt from: CR
Repo: cmackenzie1/torii-rs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T23:46:37.502Z
Learning: Applies to **/*.rs : Use `async_trait` for async interfaces; design with composition in mind
Applied to files:
torii-storage-postgres/src/repositories/mod.rstorii-core/src/repositories/invitation.rs
📚 Learning: 2025-12-30T23:46:37.502Z
Learnt from: CR
Repo: cmackenzie1/torii-rs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T23:46:37.502Z
Learning: Organize storage backends in separate crates named `torii-storage-*`
Applied to files:
torii-storage-seaorm/src/repositories/invitation.rstorii/src/lib.rs
🧬 Code graph analysis (15)
torii-core/src/services/mod.rs (4)
torii-core/src/repositories/mod.rs (1)
invitation(136-136)torii-storage-postgres/src/repositories/mod.rs (1)
invitation(136-138)torii-storage-seaorm/src/repositories/mod.rs (1)
invitation(138-140)torii-storage-sqlite/src/repositories/mod.rs (1)
invitation(136-138)
torii-mailer/src/templates/engine.rs (1)
torii-mailer/src/templates/auth_templates.rs (6)
from_data(79-97)from_data(158-169)from_data(229-247)from_data(313-324)from_data(384-402)from_data(472-501)
torii/tests/invitation.rs (5)
torii-core/src/invitation.rs (8)
new(36-38)new(207-233)inviter_id(360-363)is_expired(285-287)can_accept(295-297)token(275-277)metadata(366-369)torii-core/src/services/invitation.rs (3)
new(64-70)new(369-373)new(559-563)torii-core/src/repositories/mod.rs (3)
invitation(136-136)token(114-114)user(59-59)torii-storage-postgres/src/repositories/mod.rs (3)
invitation(136-138)token(120-122)user(80-82)torii-storage-seaorm/src/repositories/mod.rs (3)
invitation(138-140)token(122-124)user(82-84)
torii-core/src/lib.rs (5)
torii-core/src/repositories/mod.rs (2)
invitation(136-136)user(59-59)torii-storage-postgres/src/repositories/mod.rs (2)
invitation(136-138)user(80-82)torii-storage-seaorm/src/repositories/mod.rs (2)
invitation(138-140)user(82-84)torii-storage-sqlite/src/repositories/mod.rs (2)
invitation(136-138)user(80-82)torii/src/lib.rs (1)
repositories(651-653)
torii-core/src/services/mailer.rs (1)
torii-core/src/invitation.rs (2)
build(378-387)
torii-core/src/repositories/mod.rs (3)
torii-storage-postgres/src/repositories/mod.rs (2)
invitation(136-138)brute_force(128-130)torii-storage-seaorm/src/repositories/mod.rs (2)
invitation(138-140)brute_force(130-132)torii-storage-sqlite/src/repositories/mod.rs (2)
invitation(136-138)brute_force(128-130)
torii-storage-sqlite/src/repositories/mod.rs (3)
torii-core/src/repositories/mod.rs (1)
invitation(136-136)torii-storage-postgres/src/repositories/mod.rs (1)
invitation(136-138)torii-storage-seaorm/src/repositories/mod.rs (1)
invitation(138-140)
torii-storage-postgres/src/repositories/user.rs (3)
torii-storage-postgres/src/repositories/mod.rs (1)
user(80-82)torii-core/src/storage.rs (1)
id(196-199)torii-core/src/user.rs (1)
id(294-297)
torii-storage-postgres/src/repositories/invitation.rs (2)
torii-core/src/invitation.rs (5)
from(68-70)from(74-76)from_storage(240-269)inviter_id(360-363)torii-core/src/repositories/invitation.rs (12)
create(16-16)find_by_id(19-19)find_by_token_hash(24-24)find_by_email(30-30)find_pending_by_email(35-35)find_by_inviter(38-38)update_status(43-47)accept(53-53)revoke(58-58)delete(61-61)cleanup_expired(67-67)count_pending_by_email(72-72)
torii-storage-postgres/src/lib.rs (2)
torii-core/src/storage.rs (3)
new(169-174)new(315-335)id(196-199)torii-core/src/user.rs (2)
new(90-92)id(294-297)
torii-storage-sqlite/src/repositories/invitation.rs (3)
torii-core/src/invitation.rs (2)
new(36-38)new(207-233)torii-core/src/repositories/adapter.rs (27)
new(31-33)new(73-75)new(115-117)new(144-146)new(238-240)new(300-302)new(346-348)new(410-412)create(38-40)create(80-82)create(417-419)find_by_id(42-44)find_by_id(421-423)find_by_token_hash(425-430)find_by_email(46-48)find_by_email(432-434)find_pending_by_email(436-441)find_by_inviter(443-445)update_status(447-453)accept(455-457)revoke(459-461)delete(58-60)delete(88-90)delete(463-465)cleanup_expired(96-98)cleanup_expired(467-469)count_pending_by_email(471-476)torii-core/src/repositories/invitation.rs (12)
create(16-16)find_by_id(19-19)find_by_token_hash(24-24)find_by_email(30-30)find_pending_by_email(35-35)find_by_inviter(38-38)update_status(43-47)accept(53-53)revoke(58-58)delete(61-61)cleanup_expired(67-67)count_pending_by_email(72-72)
torii-storage-seaorm/src/repositories/mod.rs (4)
torii-core/src/repositories/mod.rs (1)
invitation(136-136)torii-storage-postgres/src/repositories/mod.rs (1)
invitation(136-138)torii-storage-sqlite/src/repositories/mod.rs (2)
invitation(136-138)new(51-72)torii-storage-seaorm/src/lib.rs (1)
new(119-121)
torii-storage-postgres/src/migrations/mod.rs (1)
torii-migration/src/lib.rs (6)
version(38-38)name(41-41)up(44-44)up(67-67)down(47-47)down(70-70)
torii-core/src/repositories/adapter.rs (4)
torii-core/src/repositories/invitation.rs (12)
create(16-16)find_by_id(19-19)find_by_token_hash(24-24)find_by_email(30-30)find_pending_by_email(35-35)find_by_inviter(38-38)update_status(43-47)accept(53-53)revoke(58-58)delete(61-61)cleanup_expired(67-67)count_pending_by_email(72-72)torii-storage-postgres/src/repositories/invitation.rs (12)
create(61-86)find_by_id(88-105)find_by_token_hash(107-124)find_by_email(126-144)find_pending_by_email(146-164)find_by_inviter(166-184)update_status(186-209)accept(211-230)revoke(232-250)delete(252-265)cleanup_expired(267-285)count_pending_by_email(287-305)torii-core/src/repositories/mod.rs (1)
invitation(136-136)torii-storage-postgres/src/repositories/mod.rs (1)
invitation(136-138)
torii-core/src/invitation.rs (3)
torii-core/src/id.rs (2)
generate_prefixed_id(22-31)validate_prefixed_id(62-76)torii-storage-postgres/src/repositories/invitation.rs (1)
from(29-44)torii-core/src/crypto.rs (1)
verify_token_hash(84-87)
⏰ 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). (3)
- GitHub Check: Greptile Review
- GitHub Check: build
- GitHub Check: Greptile Review
🔇 Additional comments (64)
torii-core/src/services/password.rs (1)
189-190: LGTM!The test mock correctly initializes the new
statusandinvited_byfields. UsingActivestatus is appropriate for test users simulating normal registration flow.torii-core/src/user.rs (4)
28-82: Well-designed UserStatus enum with comprehensive trait implementations.The enum correctly models the user lifecycle states with clear semantics. The
FromStrimplementation properly returns a validation error for unknown status values, and theDefaultimplementation defaulting toActiveis appropriate.
223-233: LGTM!The new
statusandinvited_byfields are well-documented and correctly typed. UsingOption<UserId>forinvited_byproperly handles both invited and self-registered users.
268-277: LGTM!Convenience methods delegate to
UserStatusappropriately, maintaining a clean API surface.
286-287: LGTM!Builder extensions follow the established pattern consistently. The
statusfield correctly defaults viaunwrap_or_default()when not explicitly set.Also applies to: 314-322, 348-349
torii-core/src/services/password_reset.rs (1)
177-178: LGTM!Test mock correctly initializes the new
statusandinvited_byfields, consistent with other test files.torii-core/src/repositories/invitation.rs (1)
1-73: Well-designed repository trait following established patterns.The trait provides a comprehensive API for invitation management with:
- Proper use of
async_traitas per coding guidelines- Borrowed references in method signatures following sqlx best practices
- Clear separation between status queries (
find_pending_by_email) and general queries (find_by_email)- Support for rate limiting via
count_pending_by_email- Cleanup operation returning affected count for observability
torii-core/src/services/magic_link.rs (1)
104-105: LGTM!Test mock correctly initializes the new
statusandinvited_byfields, maintaining consistency with other test files.torii-mailer/src/templates/mod.rs (1)
5-6: LGTM!
InvitationTemplateis correctly added to the public exports alongside other authentication-related templates.torii-core/src/crypto.rs (1)
25-50: Well-implemented secure token generation.The implementation correctly uses 256 bits of entropy with
OsRng, which is the appropriate choice for cryptographic randomness. URL-safe base64 encoding is suitable for tokens appearing in URLs. Therandcrate version 0.9 in use properly exportsTryRngCore, confirming compatibility.torii-core/src/services/mod.rs (1)
8-8: LGTM! Module addition follows established patterns.The invitation module and re-exports are consistent with the existing service layer structure.
Also applies to: 20-20
torii-mailer/src/templates/engine.rs (1)
79-82: LGTM! Template rendering follows established patterns.The invitation template branch is consistent with other template types and properly integrates with the template engine.
torii-core/src/services/passkey.rs (1)
122-123: LGTM! Test mock properly initializes new User fields.The defaults are appropriate for test scenarios:
Activestatus for functional users andNonefor invited_by since these are direct creations.torii-mailer/src/lib.rs (1)
12-13: LGTM! Public API exports follow established patterns.The InvitationEmail type is properly exposed in both the crate root and prelude, consistent with other email types.
Also applies to: 23-23
torii-storage-postgres/src/repositories/user.rs (3)
29-31: LGTM! INSERT statement correctly includes new fields.The status and invited_by fields are properly added to the INSERT statement with correct bindings. The use of
user.status.as_str()anduser.invited_by.as_ref().map(|id| id.as_str())follows the appropriate patterns for serializing domain types to SQL.Also applies to: 38-39
53-53: LGTM! SELECT statements consistently include new fields.Both
find_by_idandfind_by_emailqueries now properly select the status and invited_by columns, ensuring complete domain model reconstruction.Also applies to: 74-74
109-111: LGTM! UPDATE statement correctly handles new fields.The status and invited_by fields are properly included in the UPDATE statement. The binding order matches the parameter positions in the SQL query.
Also applies to: 117-118
torii-storage-postgres/Cargo.toml (1)
16-17: LGTM: JSON support properly added for invitation storage.The addition of
serde_jsonand thejsonfeature for sqlx correctly enables JSON-backed fields needed for the invitation system (e.g., token handling, metadata storage).torii-core/src/lib.rs (3)
15-15: LGTM: Invitation module properly integrated.The new
invitationmodule follows the established organizational pattern for core functionality in the torii-core crate.
25-31: LGTM: Public API properly expanded for invitation feature.The re-exports correctly expose:
- Invitation domain types (Invitation, InvitationId, InvitationStatus, NewInvitation)
- Service layer components (InvitationConfig, InvitationService)
- User status tracking (UserStatus)
This enables cross-crate usage as intended by the PR objectives.
42-42: LGTM: UserStatus export enables invitation linkage.Adding
UserStatusto the public user exports supports the new Provisional/Active user status workflow introduced by the invitation system.torii-core/src/services/oauth.rs (2)
163-177: LGTM: Test mock properly updated for new User fields.The MockUser to User conversion correctly populates the new fields:
status: UserStatus::Activeis appropriate for OAuth-registered usersinvited_by: Noneis correct since OAuth users are not invited
296-322: LGTM: Mock OAuth repository properly updated.The
find_user_by_providermock correctly initializes the newstatusandinvited_byfields with appropriate defaults for OAuth users.torii-storage-seaorm/src/user.rs (2)
3-3: LGTM: Import updated for UserStatus.The import correctly adds
UserStatusto support the new user status field.
14-16: Stub implementation acknowledged: verify migration plan.The code correctly reflects that SeaORM doesn't currently include
statusandinvited_bycolumns, defaulting toUserStatus::ActiveandNonerespectively. This aligns with the PR's approach of including "SeaORM stubs."No documented plan or tracking (TODOs, issues, or roadmap) was found for adding these columns in a future release. Consider adding a tracking issue or updating project documentation to formalize the migration plan for these missing fields.
torii-core/src/repositories/mod.rs (4)
21-21: LGTM: Invitation repository module properly added.The new
invitationmodule follows the established pattern for repository modules in the system.
29-35: LGTM: Adapter and trait exports properly organized.The
InvitationRepositoryAdapterandInvitationRepositoryare correctly added to the public exports, maintaining consistency with the existing repository pattern.
128-137: LGTM: InvitationRepositoryProvider trait properly defined.The trait follows the established provider pattern:
- Proper bounds:
Send + Sync + 'static- Associated type with correct trait bound
- Getter method following naming convention
- Comprehensive documentation
184-184: LGTM: RepositoryProvider supertrait correctly extended.Adding
InvitationRepositoryProviderto theRepositoryProvidersupertrait properly integrates invitation support into the unified repository abstraction layer.torii/tests/invitation.rs (1)
1-362: Comprehensive test coverage for invitation system.The test suite provides excellent coverage of the invitation lifecycle, including creation, verification, acceptance, revocation, listing, and metadata handling. Tests are properly gated with feature flags and marked with
#[ignore]for external dependencies. All tests follow the coding guideline to use#[tokio::test]for async tests.torii-mailer/src/templates/auth_templates.rs (1)
405-502: LGTM! Consistent implementation of invitation email template.The
InvitationTemplatefollows the established pattern of other auth templates in the file. The implementation correctly handles optionalinviter_nameand provides a sensible default (7 days) forexpires_in_days. The HTML template uses conditional rendering to show inviter information when available.torii-core/src/services/mailer.rs (1)
41-47: LGTM! Trait method signature is consistent.The
send_invitation_emailmethod signature follows the established pattern of other email methods in theMailerServicetrait, with appropriate parameters for invitation functionality.torii-mailer/src/email_types.rs (1)
150-201: LGTM! Well-documented and consistent implementation.The
InvitationEmailimplementation follows the established pattern of other email types in the file. The code properly handles the optionalinviter_nameparameter by conditionally adding it to the template data and adjusting the subject line accordingly. The doc comments provide clear guidance on the method's parameters and purpose.torii-core/src/storage.rs (4)
7-7: LGTM! Import updated to include UserStatus.The import statement correctly adds
user::UserStatusto support the new status field in the user storage types.
160-161: LGTM! Fields added to support invitation tracking.The
statusandinvited_byfields extend theNewUserstruct to support user status tracking (Provisional/Active) and invitation linkage, which are essential for the invitation system.
191-192: LGTM! Builder methods follow established patterns.The builder methods for
statusandinvited_byfollow the existing builder pattern in the codebase. The implementation is consistent with other builder methods in the file.Also applies to: 216-224
226-236: LGTM! Appropriate use of default for status.The
buildmethod correctly usesunwrap_or_default()for thestatusfield, which will default toUserStatus::Activewhen not explicitly set. Theinvited_byfield is properly passed through as an optional value.torii-storage-sqlite/src/repositories/mod.rs (2)
4-4: LGTM! Invitation repository wiring follows established patterns.The module declaration, re-export, struct field addition, initialization, and trait implementation are consistent with other repository types in this file.
Also applies to: 13-13, 47-47, 59-59, 70-70, 133-139
145-179: Verify: SQLite invitation migrations are missing.The
migrate()function does not include invitation-related migrations (e.g.,CreateInvitationsTable,AddUserStatusAndInvitedBy), while theSqliteInvitationRepositoryis wired in. Users of the SQLite backend will encounter runtime errors when attempting invitation operations.Per the PR description, this is intentional for the initial Postgres-only release. Consider adding a TODO comment or documenting this limitation for future contributors.
torii-storage-postgres/src/lib.rs (2)
66-69: LGTM! Migration imports and wiring are correct.The new migrations
AddUserStatusAndInvitedByandCreateInvitationsTableare properly imported and added to the migration sequence in the correct order.Also applies to: 122-123
169-183: LGTM! PostgresUser serialization is correct.The
From<User> for PostgresUserimplementation properly converts the domainUserto the database representation, correctly handling the newstatusandinvited_byfields.torii-storage-seaorm/src/repositories/user.rs (4)
5-5: LGTM! UserStatus import and create_user helper updates.The helper method correctly sets the new
statusandinvited_byfields when constructingNewUser.Also applies to: 20-31
102-121: Verify:update()does not persiststatusorinvited_bychanges.The
update()method updatesname,email_verified_at, andlocked_at, but notstatusorinvited_by. If status transitions (e.g., Provisional → Active) need to be persisted via this method, the implementation is incomplete.If status updates are handled through a dedicated method (e.g.,
update_status), this is acceptable.
89-100: LGTM!find_or_create_by_emailcorrectly sets new fields.The method properly initializes
statusandinvited_bywhen creating a new user.
164-185: LGTM! Tests updated with new required fields.All test cases correctly include
status: UserStatus::Activeandinvited_by: NoneinNewUserconstruction.Also applies to: 187-206, 219-238, 249-271, 288-310, 312-333, 335-357
torii-storage-seaorm/src/repositories/mod.rs (2)
4-4: LGTM! SeaORM invitation repository wiring is consistent with other repositories.The module declaration, re-export, import updates, struct field addition, and initialization follow the established patterns used for other repository types.
Also applies to: 13-13, 29-31, 49-49, 52-74
135-141: LGTM! InvitationRepositoryProvider implementation is correct.The trait implementation follows the same pattern as other provider traits in this file.
torii-storage-postgres/src/migrations/mod.rs (1)
799-859: LGTM! AddUserStatusAndInvitedBy migration is well-designed.
- Default
'active'for existing users ensures backward compatibilityON DELETE SET NULLforinvited_byis appropriate — inviter deletion shouldn't cascade to invited users- Indexes on
statusandinvited_bysupport common query patterns- Down migration correctly drops indexes before columns
torii-storage-sqlite/src/repositories/invitation.rs (1)
1-27: LGTM! Stub structure and documentation are clear.The module documentation explicitly states this is a stub implementation, and the
#[allow(dead_code)]annotation onpoolis appropriate since it's reserved for future implementation.torii-storage-seaorm/src/repositories/invitation.rs (2)
1-28: LGTM! SeaORM stub structure mirrors SQLite implementation.The
#[derive(Clone)]is appropriate sinceDatabaseConnectionisClone, and the documentation clearly indicates this is a placeholder for future implementation. Based on learnings, this correctly follows the pattern of organizing storage backends in separatetorii-storage-*crates.
30-107: LGTM! Trait implementation is consistent with SQLite stub.All methods follow the same pattern of returning a descriptive error message. The implementation provides a complete API surface that matches the
InvitationRepositorytrait fromtorii-core.torii-storage-postgres/src/repositories/mod.rs (1)
4-4: LGTM!The invitation repository integration follows the established patterns consistently:
- Module declaration and re-export align with other repositories
Arcwrapping for thread-safe sharing- Trait implementation mirrors other provider implementations
- Migration ordering is correct (user status changes before invitation table creation)
Also applies to: 14-14, 26-28, 46-46, 59-59, 70-70, 133-139, 147-148, 175-176
torii-storage-postgres/src/repositories/invitation.rs (2)
61-86: LGTM!The
createmethod correctly inserts the invitation and returns all fields viaRETURNING. Bindings use borrowed references as per coding guidelines.
267-285: LGTM!The
cleanup_expiredmethod correctly updates expired pending invitations to 'expired' status rather than deleting them, preserving audit trail.torii-core/src/repositories/adapter.rs (1)
403-477: LGTM!The
InvitationRepositoryAdapterfollows the established adapter pattern consistently with other adapters in this file. All methods correctly delegate to the underlying provider's invitation repository.torii/src/lib.rs (2)
1280-1284: LGTM!Silently accepting pending invitations during registration is a good UX choice. The warning log on failure ensures observability without blocking the registration flow.
394-396: LGTM!The
invitation_serviceis properly initialized in bothnew()andfrom_builder()constructors, following the same pattern as other services.Also applies to: 521-524, 638-641
torii-core/src/invitation.rs (4)
28-100: LGTM!
InvitationIdfollows the newtype pattern per coding guidelines, with proper prefix validation using the existinggenerate_prefixed_idandvalidate_prefixed_idutilities.
159-199: LGTM!Excellent security design for the
Invitationstruct:
- Token stored as
SecretStringfor memory protection- Public
token_hashfor storage, privatetokenfor transient use- Clear separation between creation (with token) and storage loading (hash only)
300-318: LGTM!Custom
Debugimplementation correctly redacts the sensitive token field while exposing other fields for debugging.
377-388: LGTM!The builder correctly validates required fields (
expires_at) at build time usingRequiredFieldExt, following the Builder Pattern guidelines.torii-core/src/services/invitation.rs (3)
20-47: LGTM!
InvitationConfighas sensible defaults (7 days expiry, provisional user creation enabled, 5 max pending invitations) and clear documentation for each field.
323-352: LGTM!
accept_pending_invitations_for_usercorrectly accepts all valid pending invitations for a user after signup. The sequential processing is appropriate for typical invitation volumes.
625-772: LGTM!Comprehensive test coverage including:
- Invitation creation with provisional user
- Token verification (valid and invalid)
- Invitation acceptance and user activation
- Revocation
- Max pending invitations limit
- Email validation
- Inviter tracking
| async fn send_invitation_email( | ||
| &self, | ||
| to: &str, | ||
| invitation_link: &str, | ||
| inviter_name: Option<&str>, | ||
| expires_in_days: i64, | ||
| ) -> Result<(), Error> { | ||
| let context = self.create_context(inviter_name, Some(to)); | ||
|
|
||
| let email = InvitationEmail::build( | ||
| &self.engine, | ||
| &self.config.get_from_address(), | ||
| to, | ||
| invitation_link, | ||
| inviter_name, | ||
| expires_in_days, | ||
| context, | ||
| ) | ||
| .await | ||
| .map_err(|e| Error::Storage(crate::error::StorageError::Connection(e.to_string())))?; | ||
|
|
||
| self.transport.send_email(email).await.map_err(|e| { | ||
| Error::Storage(crate::error::StorageError::Connection(e.to_string())) | ||
| })?; | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Fix incorrect parameter in create_context call.
Line 215 passes inviter_name as the first argument to create_context, but the first parameter is user_name (which represents the email recipient's name). This will incorrectly populate the template context with the inviter's name as the invitee's user_name, potentially causing confusion in the email greeting.
The email is being sent to the invitee (to), so the context should reflect the invitee's information. Since we don't have the invitee's name at this point, the first parameter should likely be None.
🔎 Proposed fix
- let context = self.create_context(inviter_name, Some(to));
+ let context = self.create_context(None, Some(to));📝 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 send_invitation_email( | |
| &self, | |
| to: &str, | |
| invitation_link: &str, | |
| inviter_name: Option<&str>, | |
| expires_in_days: i64, | |
| ) -> Result<(), Error> { | |
| let context = self.create_context(inviter_name, Some(to)); | |
| let email = InvitationEmail::build( | |
| &self.engine, | |
| &self.config.get_from_address(), | |
| to, | |
| invitation_link, | |
| inviter_name, | |
| expires_in_days, | |
| context, | |
| ) | |
| .await | |
| .map_err(|e| Error::Storage(crate::error::StorageError::Connection(e.to_string())))?; | |
| self.transport.send_email(email).await.map_err(|e| { | |
| Error::Storage(crate::error::StorageError::Connection(e.to_string())) | |
| })?; | |
| Ok(()) | |
| } | |
| async fn send_invitation_email( | |
| &self, | |
| to: &str, | |
| invitation_link: &str, | |
| inviter_name: Option<&str>, | |
| expires_in_days: i64, | |
| ) -> Result<(), Error> { | |
| let context = self.create_context(None, Some(to)); | |
| let email = InvitationEmail::build( | |
| &self.engine, | |
| &self.config.get_from_address(), | |
| to, | |
| invitation_link, | |
| inviter_name, | |
| expires_in_days, | |
| context, | |
| ) | |
| .await | |
| .map_err(|e| Error::Storage(crate::error::StorageError::Connection(e.to_string())))?; | |
| self.transport.send_email(email).await.map_err(|e| { | |
| Error::Storage(crate::error::StorageError::Connection(e.to_string())) | |
| })?; | |
| Ok(()) | |
| } |
🤖 Prompt for AI Agents
In torii-core/src/services/mailer.rs around lines 208 to 234, change the
create_context call so the first argument is None (invitee's name unknown)
instead of inviter_name; keep Some(to) as the second argument. This will
correctly populate the template context for the invitee (user_name) while still
passing the recipient email; update the single call at line ~215 to use None for
the user_name parameter.
| // Get the plaintext token from the invitation | ||
| let token = invitation | ||
| .token() | ||
| .expect("Token should be available after creation"); | ||
|
|
There was a problem hiding this comment.
Avoid .expect() in public API - handle missing token gracefully.
The create_invitation method uses .expect() on invitation.token() at line 1042. While the token should always be present immediately after creation, using .expect() in a public API can lead to panics if the invariant is ever broken.
🔎 Proposed fix
// Get the plaintext token from the invitation
- let token = invitation
- .token()
- .expect("Token should be available after creation");
+ let Some(token) = invitation.token() else {
+ return Err(ToriiError::AuthError(
+ "Invitation token not available after creation".to_string(),
+ ));
+ };📝 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.
| // Get the plaintext token from the invitation | |
| let token = invitation | |
| .token() | |
| .expect("Token should be available after creation"); | |
| // Get the plaintext token from the invitation | |
| let Some(token) = invitation.token() else { | |
| return Err(ToriiError::AuthError( | |
| "Invitation token not available after creation".to_string(), | |
| )); | |
| }; |
🤖 Prompt for AI Agents
In torii/src/lib.rs around lines 1039 to 1043, the code calls
invitation.token().expect(...) which can panic in a public API; change the
method to handle a missing token without panicking by returning a Result (or
Option) from create_invitation and mapping the None case to a descriptive error
returned to the caller (e.g., Err(InvitationError::MissingToken) or Ok(None)
depending on existing error types), propagate the error upward instead of
calling expect, and update call sites and docs accordingly so the API no longer
panics when the token is absent.
Greptile SummaryThis PR implements a comprehensive user invitation system that allows existing users to invite new users by email. The implementation includes secure token generation using SHA256 hashing, provisional user creation, automatic invitation acceptance on registration, and full PostgreSQL support. Key Features Implemented:
Architecture Highlights:
Security Considerations:
The implementation is well-tested, properly documented, and follows the project's style guidelines. All core files have comprehensive unit and integration tests. Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Inviter as Inviter User
participant API as Torii API
participant InvSvc as InvitationService
participant UserRepo as UserRepository
participant InvRepo as InvitationRepository
participant Mailer as MailerService
participant Invitee as Invited User
%% Create Invitation Flow
Note over Inviter,Mailer: Create Invitation Flow
Inviter->>API: create_invitation(email, inviter_id, url, metadata)
API->>InvSvc: create_invitation(email, inviter_id, metadata)
InvSvc->>InvSvc: validate_email(email)
InvSvc->>InvRepo: count_pending_by_email(email)
InvRepo-->>InvSvc: pending_count
alt Too many pending invitations
InvSvc-->>API: Error: Too many pending
API-->>Inviter: Error
end
InvSvc->>UserRepo: find_by_email(email)
UserRepo-->>InvSvc: existing_user (or None)
alt User doesn't exist
InvSvc->>InvSvc: generate_secure_token()
InvSvc->>InvSvc: hash_token(token)
InvSvc->>UserRepo: create(provisional_user)
UserRepo-->>InvSvc: provisional_user (status=Provisional)
InvSvc->>InvRepo: create(invitation)
InvRepo-->>InvSvc: stored_invitation
end
InvSvc-->>API: (invitation, provisional_user)
API->>Mailer: send_invitation_email(email, link, inviter_name)
Mailer-->>API: Email sent
API-->>Inviter: (invitation, provisional_user)
%% Accept Invitation Flow
Note over Invitee,UserRepo: Accept Invitation Flow
Invitee->>API: accept_invitation(token, user_id)
API->>InvSvc: accept_invitation(token, user_id)
InvSvc->>InvSvc: hash_token(token)
InvSvc->>InvRepo: find_by_token_hash(hash)
InvRepo-->>InvSvc: invitation
InvSvc->>InvSvc: verify(token) & can_accept()
alt Invalid or expired
InvSvc-->>API: Error: Invalid/Expired
API-->>Invitee: Error
end
InvSvc->>InvRepo: accept(invitation_id, user_id)
InvRepo-->>InvSvc: accepted_invitation
InvSvc->>UserRepo: find_by_id(user_id)
UserRepo-->>InvSvc: user
alt User is provisional
InvSvc->>InvSvc: user.status = Active
InvSvc->>UserRepo: update(user)
UserRepo-->>InvSvc: activated_user
end
InvSvc-->>API: (accepted_invitation, user)
API-->>Invitee: Success
%% Auto-Accept Flow
Note over Invitee,InvRepo: Auto-Accept After Registration
Invitee->>API: password.register(email, password)
API->>API: User created
API->>InvSvc: accept_pending_invitations_for_user(user)
InvSvc->>InvRepo: find_pending_by_email(user.email)
InvRepo-->>InvSvc: pending_invitations
loop For each pending invitation
InvSvc->>InvRepo: accept(invitation_id, user_id)
InvRepo-->>InvSvc: accepted_invitation
end
InvSvc-->>API: accepted_invitations[]
API-->>Invitee: Registration complete
|
Greptile found no issues!From now on, if a review finishes and we haven't found any issues, we will not post anything, but you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
|
would love to see this merged! |
Thanks for the nudge @philocalyst! I've been thinking more about the API and not 100% sure this is what I want (at least in its current state). Do you have any feedback on the |
2 participants
Add a complete user invitation feature that allows existing users to invite new users by email. Key functionality includes:
Storage support:
Security considerations: