-
Notifications
You must be signed in to change notification settings - Fork 1
Adding okta and related multi-tenant modifications and docs #24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
90ef15e to
f3b8d2a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds comprehensive multi-tenant SSO support with Okta as the primary use case, enabling B2B SaaS applications to support multiple enterprise customers with their own identity providers.
Key Changes:
- Adds Okta provider with domain-based configuration and security validation
- Implements TenantManager for multi-tenant OAuth configuration
- Introduces
onResolveProviderhook for dynamic provider resolution - Adds security validation utilities (SSRF protection, injection prevention)
- Comprehensive test coverage for new features
Reviewed changes
Copilot reviewed 26 out of 28 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/tenantManager.ts | Core multi-tenant management with tenant registration, email domain routing, and provider configuration |
| src/lib/providers/okta.ts | Okta OAuth provider with domain validation and SSRF protection |
| src/lib/providers/validation.ts | Security validation utilities for domain safety, email domains, tenant IDs, and XSS prevention |
| src/lib/providers/azure.ts | Adds Azure tenant ID validation using new validation utilities |
| src/lib/providers/auth0.ts | Adds domain validation and allowlist checking for Auth0 |
| src/lib/hookManager.ts | Adds onResolveProvider hook for dynamic provider resolution |
| src/lib/resource.ts | Implements dynamic provider resolution with validation and error handling |
| src/lib/config.ts | Adds Okta to provider configuration switch statement |
| src/lib/providers/index.ts | Registers Okta and generic providers |
| src/types.ts | Documents onResolveProvider hook with security requirements and examples |
| src/index.ts | Exports TenantManager, validation utilities, and provider utilities |
| test/lib/tenantManager.test.js | Comprehensive tests for tenant management including validation, bulk operations, and security |
| test/lib/providers/validation.test.js | Security-focused tests for SSRF protection, injection prevention, and validation |
| test/lib/providers/okta.test.js | Tests for Okta provider configuration and domain validation |
| test/lib/providers/azure.test.js | Updates tests to use valid GUID format for Azure tenant IDs |
| test/lib/config.test.js | Updates tests to use valid Azure tenant ID format |
| docs/multi-tenant-sso.md | Complete guide for multi-tenant SSO setup with examples and security considerations |
| docs/providers.md | Okta setup documentation with group-based role mapping instructions |
| docs/lifecycle-hooks.md | Documents onResolveProvider hook with comprehensive security requirements |
| docs/getting-started.md | Clarifies that built-in providers are templates requiring configuration |
| examples/okta-multi-tenant.js | Example showing multi-tenant Okta configuration |
| package.json | Moves harperdb to devDependencies, adds test scripts |
| .bun/preload.js | Bun test environment setup with HarperDB mocking |
| bunfig.toml | Updates preload path to .bun directory |
| assets/test.html | Adds Okta button styling |
| README.md | Updates provider list and clarifies built-in vs active providers |
| .github/workflows/pr-checks.yml | Adds Node 20-specific test script |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
DavidCockerill
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Epic!
kriszyp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, comprehensive, nice work. One note about the example admin endpoint.
docs/multi-tenant-sso.md
Outdated
| Create an admin endpoint to add tenants dynamically: | ||
|
|
||
| ```typescript | ||
| scope.resources.set('admin/tenants', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would you access scope outside creating your own plugin? I think we would want to describe a standard exported Resource.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah...that's a bad partial example. Working on refining this doc right now - it's currently not super useful.
…to ensure the plugin is good.
c6e52e8 to
e0227c5
Compare
No description provided.