Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements comprehensive multi-tenant support for BulwarkAuth, adding tenant ID isolation across all layers of the application. The implementation follows a row-level multi-tenancy approach where all user data is scoped to tenant IDs.
Changes:
- Added tenant management system with repository, service, and data models
- Updated all authentication, account, and token operations to accept and filter by tenantID
- Modified database schemas to include tenantID fields with compound unique indexes
- Updated integration tests to use tenantID parameter
- Added comprehensive multi-tenant architecture documentation
Reviewed changes
Copilot reviewed 37 out of 38 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/tenants/tenants.go | New tenant management domain model and service layer |
| internal/tenants/tenants_test.go | Comprehensive unit tests for tenant CRUD operations |
| internal/tokens/tokenizer.go | Added tenantID to JWT token claims for token isolation |
| internal/authentication/*.go | Updated all authentication flows to be tenant-aware |
| internal/accounts/*.go | Updated account management to support multi-tenancy |
| internal/email/*.go | Modified email services to accept tenantID parameter |
| api/**/*.go | Updated all HTTP handlers to include tenantID in requests |
| test/integration/integration_test.go | Updated integration tests with tenantID="default" |
| cmd/bulwarkauth/main.go | Added tenant service initialization and default tenant creation |
| cmd/bulwarkauth/config.go | Added DEFAULT_TENANT_ID configuration |
| go.mod | Updated bulwark-auth-guard client to v0.2.1 |
| README.md | Added multi-tenant documentation and usage examples |
| MULTI_TENANT_PLAN.md | Comprehensive implementation plan and architecture guide |
Comments suppressed due to low confidence (2)
internal/accounts/forgot_repository.go:43
- The $set operation should include tenantId in the forgot password token update to ensure tenant isolation. Currently, the update only sets email, created, and token, but doesn't set tenantId, which could cause issues if the document doesn't already exist.
_, err := collection.UpdateOne(ctx, filter, bson.M{"$set": bson.M{"email": email, "created": time.Now(),
"token": forgotToken.String()}}, opts)
if err != nil {
internal/authentication/logon_code_repository.go:48
- The $setOnInsert operation is missing the tenantId field. When creating a new logon code document, the tenantId should be set, otherwise the document won't be properly scoped to a tenant.
update := bson.D{
{"$set", bson.D{
{"code", code},
{"expires", expires},
{"created", time.Now()},
}},
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 38 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
internal/accounts/forgot_repository.go:44
- The forgot password repository Create method does not set the tenantId field in the document being inserted. The $set operation should include "tenantId": tenantID to ensure the field is properly stored in the database.
_, err := collection.UpdateOne(ctx, filter, bson.M{"$set": bson.M{"email": email, "created": time.Now(),
"token": forgotToken.String()}}, opts)
if err != nil {
return err
internal/authentication/logon_code_repository.go:53
- The logon code Create method does not set the tenantId field in the $setOnInsert operation. This means new logon codes won't have a tenantId populated. The $setOnInsert should include "tenantId": tenantID.
update := bson.D{
{"$set", bson.D{
{"code", code},
{"expires", expires},
{"created", time.Now()},
}},
}
opts := options.Update().SetUpsert(true)
_, err := collection.UpdateOne(ctx, filter, update, opts)
if err != nil {
return err
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 39 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
internal/accounts/forgot_repository.go:42
- The TenantID field is added to the Forgot struct but is not included in the $set operation during Create. This means when creating or updating a forgot password record, the tenantId field won't be saved to the database, which breaks tenant isolation. The $set should include "tenantId": tenantID.
filter := bson.D{{Key: "tenantId", Value: tenantID}, {Key: "email", Value: email}}
opts := options.Update().SetUpsert(true)
_, err := collection.UpdateOne(ctx, filter, bson.M{"$set": bson.M{"email": email, "created": time.Now(),
"token": forgotToken.String()}}, opts)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.