-
Notifications
You must be signed in to change notification settings - Fork 17
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
chore: Add initial api_spec for Multitenancy APIs #131
chore: Add initial api_spec for Multitenancy APIs #131
Conversation
emailPassword: | ||
type: object | ||
properties: | ||
enabled: | ||
type: boolean | ||
passwordless: | ||
type: object | ||
properties: | ||
enabled: | ||
type: boolean | ||
thirdParty: | ||
type: object | ||
properties: | ||
enabled: | ||
type: boolean |
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.
can this be simplified? if we are taking the firstFactors as the source of truth
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 we are using validFirstFactors as the source of truth, but we still might need them for some edge cases
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.
Please check if we really need or not. if we really need it, add description as to why we need it
- name: tenantId | ||
in: query | ||
required: true | ||
schema: |
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.
What if tenant was not found? check similar issue in other thirdparty config related APIs
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.
We are not handling it separately in the backend SDK and let the core return the error accordingly, should I check if tenant exists or not and then return the status accordingly?
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.
Check what's the backend sdk behaviour, does it return null or throws an error?
api_spec.yaml
Outdated
type: string | ||
enum: | ||
- Unauthorised access | ||
/dashboard/api/multitenancy/core-config/list: |
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.
core config list is unrelated to multitenancy, can we use a different path?
/dashboard/api/core/config/list
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.
Done
- name: Miscellaneous | ||
description: Miscellaneous APIs |
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.
In the common section of this spec file, mention how optional tenantId is represented. (refer CDI)
@@ -1477,145 +1498,30 @@ paths: | |||
properties: | |||
enabled: | |||
type: boolean | |||
/dashboard/api/tenants/user/associate: | |||
/{tenantId}/dashboard/api/tenants/third-party: |
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.
make tenantId optional everywhere
- name: tenantId | ||
in: query | ||
required: true | ||
schema: |
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.
Check what's the backend sdk behaviour, does it return null or throws an error?
@@ -1285,6 +1287,10 @@ paths: | |||
type: array |
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.
rename this to providersFromCore
for clarity? Also add a description regarding why we need both.
emailPassword: | ||
type: object | ||
properties: | ||
enabled: | ||
type: boolean | ||
passwordless: | ||
type: object | ||
properties: | ||
enabled: | ||
type: boolean | ||
thirdParty: | ||
type: object | ||
properties: | ||
enabled: | ||
type: boolean |
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.
Please check if we really need or not. if we really need it, add description as to why we need it
Not required any more |
Summary of change
Problem Statement
Added API spec for certain multitenancy APIs which include getting all tenants with their user counts, getting all the info for a particular tenant and deleting a tenant, creating or updating a tenant and associating or dissociating users with a tenant.
All the APIs can be found in #132
Summary of solution
Added API spec for certain multitenancy APIs which include getting all tenants with their user counts, getting all the info for a particular tenant and deleting a tenant, creating or updating a tenant and associating or dissociating users with a tenant.
Related issues
N/A
Test Plan
Tested on all primary browsers for:
Feature tests:
Dashboard Admin access.
POST
,PUT
andDELETE
endpoints with admins only access enabled for the dashboard recipe.POST
,PUT
andDELETE
endpoints without the admins only access enabled.Search
General UI testing
Multi tenant testing
User Roles and Permissions testing
feature_not_enabled
state on both userDetails page and user roles page.User creation
emailpassword
andthirdpartyemailpassword
together and individually.passwordless
andthirdpartypasswordless
together and individually.contactMethod
's ensure that the frontend displays relevant UI based on thecontactMethod
selected.emailpassword
andpasswordless
user with the same email and make sure that the accounts are linked.Documentation changes
(If relevant, please create a PR in our docs repo, or create a checklist here highlighting the necessary changes)
Checklist for important updates
package.json
package-lock.json
src/version.ts
npm run build
Remaining TODOs for this PR
N/A