Skip to content
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

feat: thirdparty re-work #471

Merged
merged 78 commits into from
Jul 19, 2023
Merged

feat: thirdparty re-work #471

merged 78 commits into from
Jul 19, 2023

Conversation

sattvikc
Copy link
Collaborator

@sattvikc sattvikc commented Jan 4, 2023

Summary of change

(A few sentences about this PR)

Related issues

Test Plan

  • Test tenantId with different valid and invalid values to ensure router does not break
  • Test dashboard with tenantId
  • Test dashboard API matches when using tenantId
  • test for coreConfig in tenants-crud.
  • add test for counting the calls of makeDefaultUserContext to ensure it's called from frameworks only.
  • test that overriding getTenantId works
  • test allowed domains claim
  • test user roles claims that it contains roles / permissions according to the tenant
  • test all email password APIs with tenant id
  • test passwordless recipe functions with phoneNumber for multitenancy
  • test passwordless recipe APIs for multitenancy
  • test passwordless code magic link contains tenantId
  • test resend code from different tenants

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

  • Changelog has been updated
  • coreDriverInterfaceSupported.json file has been updated (if needed)
    • Along with the associated array in lib/ts/version.ts
  • frontendDriverInterfaceSupported.json file has been updated (if needed)
  • Changes to the version if needed
    • In package.json
    • In package-lock.json
    • In lib/ts/version.ts
  • Had run npm run build-pretty
  • Had installed and ran the pre-commit hook
  • Issue this PR against the latest non released version branch.
    • To know which one it is, run find the latest released tag (git tag) in the format vX.Y.Z, and then find the latest branch (git branch --all) whose X.Y is greater than the latest released tag.
    • If no such branch exists, then create one from the latest released branch.
  • If have added a new web framework, update the add-ts-no-check.js file to include that
  • If added a new recipe / api interface, then make sure that the implementation of it uses NON arrow functions only (like someFunc: function () {..}).
  • If added a new recipe, then make sure to expose it inside the recipe folder present in the root of this repo. We also need to expose its types.

Progress tracking

  • FDI changes for multitenancy
  • Router changes to accept optional tenantId - DONE (node)
  • Multitenancy recipe implementation - DONE (node)
  • create userContext in the frameworks and remove from API implementations - DONE (node)
  • call MultitenancyRecipe.getTenantId in the router - DONE (node)
  • update fetchValue in claims to accept tenantId & update allowedDomainsClaim to use it
  • Update querier to take tenantId
  • Update API interfaces and implementations for all recipes
    • dashboard
    • emailpassword
    • emailverification
    • jwt
    • multitenancy
    • openid
    • passwordless
    • session
    • thirdparty
    • thirdpartyemailpassword
    • thirdpartypasswordless
    • usermetadata
    • userroles
  • tests for new thirdparty interface
  • tests for multitenant
  • tests for each recipe agains multitenancy

Remaining TODOs for this PR

  • Check if any extra validations are required for gitlab and bitbucket providers (implement in SDK) while updating the SDK
  • We need to add tenantId to the reset password link as a query param in the backend SDK
  • We need to add tenantId to the email verification link as a query param in the backend SDK
  • We need to add tenantId to the password reset link as a query param in the backend SDK
  • backend sdk should protected prop check for tenantId in the accesstoken payload
  • Pass tenantId into sendEmail and sendSms functions on the backend SDK as well.
  • remove multitenancy claim validator in the backend SDKs
  • Update listThirdPartyConfigsForThirdPartyId response in FDI
  • Update deleteTenant response (didExist) in FDI
  • Fix create or update thirdparty config API in core
  • refresh session and few more APIs will not take tenantId from the URL (it should just ignore from the url and use session.getTenantId)
  • getAllowedDomainsForTenantId should return only string[]
  • getAllowedDomainsForTenantId takes userContext, check why it complained in docs
  • fix naming - export const AllowedDomainsClaim = new MultitenancyDomainsClaimClass(); (also rename the file)
  • if getAllowedDomainsForTenantId is defined and returns undefined, don't add the claim to the session'
  • st-tenant-domains - shorten this and inform Mihaly about the change, and ensure it's done in the frontend
  • IMPORTANT - make PR order and notes
  • check type of createOrUpdateTenant to ensure epEnabled, tpEnabled etc are optional - in interface and index
  • make the input and output type of getTenantId non optional
  • search for _ param in the codebase to see if tenantId is being ignored in any of the place where it is required. especially session claims
  • user count per tenant needs to be added in the multitenancyStats in core
  • mulit tenancy usage stats and feature flag info should be obtained from the base storage and not the app level storage
  • Core: In the telemetry ID, we also need to sent the MAU count for that app since last month.
    path: /st/telemetry
    version: 3
    body: {
     superTokensVersion*: string,
     telemetryId*: string,
     mau*: number (since the last 30 days value),
     appId*: string,
     connectionURIDomain*: string (can be an empty string as well for root CUD)
    }
    
  • add getTenantId function to session object.
  • update email verfy get API in CDI to make it app specific
  • rawUserInfo -> fromIdTokenPayload and fromUserInfoAPI should be optional. check in golang if nil is not being expected
  • Wherever reading the tenant id from the session, we need to do session.getTenantId() and not session.getAccessTokenPayload().tId. Once such instance in in the email verification recipe generateEmailVerifyTokenPOST
  • tenantId should not be added to thirdPartyUserId
  • handle tenantId in dashboard (assigned empty string as of now)
  • Connection URI should allow to have path in it (for app id) - add test for this also. Check if we normalise the domain and that removes any path (connection to core)
  • update CDI spec for session APIs (booleans added to fetch / revoke from all tenants)
  • Check for edge function compatibility in node sdk (refactor: edge function compatibility #599) - do at the end:
    • no use of axios
    • no use of jsonwebtoken lib
    • no import from "url" lib
  • Dashboard related changes:
    • The connection URI added to the dashboard bundle should include the app ID part as well. Need to make sure that on the frontend UI for creating a dashboard user, we show the app id path as well.
    • Need APIs for dashboard recipe:
      • Getting a list of tenants for the app:
        path: /api/tenants/list GET
        return: {status: "OK", tenants: {tenantId: string, emailPassword: {enabled: boolean}, thirdParty: {enabled: boolean, providers: [...]}, passwordless:{enabled: boolean}, coreConfig: {..}}[]}
        
    • User object will now have the tenantIds array in it.
    • APIs that will be specific to a tenant:
      • userEmailVerifyTokenPost -> the frontend gives the first tenant (index 0) belonging to the user
      • userEmailVerifyPut -> the frontend gives the first tenant (index 0) belonging to the user.
      • userPasswordPut -> the frontend gives the first tenant which has emailPasswordEnabled: true.
      • usersCountGet needs to be tenant specific (but not in the analytics API)
      • usersGet needs to be tenant specific
      • userPasswordPut
      • userPut - the frontend gives the first tenant which has the correct recipe enabled, based on the type of "Auth Method" we display on the dashboard
    • All requests from the frontend that will be per tenant will be like {apiDomain}/{apiBasePath}/<tenantId>/api/...
    • functions like createEmailVerificationToken that are being called in userEmailVerifyPut should give what tenantId?
    • in userSessionsGet and userSessionsPost, we will get for all tenants
    • analyticsPost needs changing when querying the core?
    • Type of Response in usersGet will change to have tenantIds: string[] in the user obj
    • Type of CommonUserInformation needs to change to have tenantIds: string[]
  • Do functions like user pagination and getUserCount need to change in the backend SDK as well?
  • in sendEmailVerificationEmail, and similar functions in other recipes, take link that contains tenantId and also in the input body, which is a bad interface. we should not take link in the input. also add separate function for creation of link. But we keep the older functions
  • functions like validate in the formFields config also needs to take in a tenantId by the way (non optional in the func definition).
  • core change: when a new app is created, it should have all login methods enabled by default (update CDI spec description as well).
  • core change: Disallow setting of ip_whitelist and deny list for saas users
  • core change: argon2_parallelism, argon2_memory_kb, argon2_iterations should be config yaml only
  • core change: check if bcrypt_log_rounds should be config yaml only (need to decide if tenant level or core level)
  • core change: The hello withouth /hello should work for with appId and or tenantId as well.
  • All callbacks in all TypeInputs for all recipes should take a tenantId (non optional) unless there is a good reason for it to not take it.
  • Verify session and refresh session and all other APIs that take the access / refresh token as an input should return tenantId from the core in the session object part (also change CDI spec, but not version).
  • getSessionInformation function needs to return tenantId from the core.
  • Frontend SDK provider changes:
    • optional thirdpartyid
    • remove clientId from the input
    • confirm that having two items in the privder list with same thirdPartyId throws error
  • Fix error message in core for 403 related tenant operation. The current message is io.supertokens.multitenancy.exception.BadPermissionException: You must use the public tenantId and, public or same appId to add/update an app
  • io.supertokens.multitenancy.exception.BadPermissionException: Not allowed to modify DB related configs. is coming when updating the public app config via the public app itself - change message
  • core: license should not be required for all ee features if using inmemory database for better developer experience
  • update dashboard spec to include changes to dashboard APIs + new API to get list of all tenants
  • change the config.yaml file for removing the DIFFERENT_ACROSS_APPS for those argon2 configs
  • GET /appid-//recipe/multitenancy/tenant: is wrong. In the output, it doesn’t have coreConfig
  • make tenantid compulsory in index functions and see how it affect scripts in docs
  • There is an issue in the core related to user info fetching - it does it based on userID only and not appId, userId.. Specifically, getUsersInfoUsingIdList function in all the auth recipe queries - why does it not take an app ID? Cause otherwise it will not be using any index.. Also, in getUserInfoForRecipeIdFromUserIds query in the core, if a user ID is shared across 2 tenants, then its information will come twice.. Even in getTenantIdsForUserIds_transaction function.
  • issue with the core - if a test fails, subsequent tests are unable to start the core cause of address already in use.
  • in core, ensure the logs contain the right CUD. for eg. APIKeyUnauthorizedError
  • Fix test: testMultipleInstancesAtTheSameTime - the logging test also has the issue that if you run it more than once, it fails the second time, cause the it also picks up the logs from the previous runs.
  • getGloablClaimValidator - add tenantId
  • remove TenantDoesNotExistErrorHandlerMiddleware & RecipeDisabledForTenantErrorHandlerMiddleware and associated config
  • Change routing for Hapi to account for tenant id in the path
  • Update all framework tests to test for API paths with and without tenant id
  • core: for mysql, check if appId in query makes the deadlock test fail
  • core: for postgres, check why deadlock test is not failing, is it using the index?
  • make sure there is NO FDI that returns the core config.. whatsoever
  • What happens if you query http://localhost:3567/appid- to the core?
  • ensure there are no createAndSendCustomEmail & createAndSendCustomMessage anywhere - in lib and tests
  • update examples and ensure all of them are working
  • Update /apiversion API behaviour when default CDI version is set to return until that version
  • The recipe interface functions for multi tenancy, you need to return undefined in case a tenant does not exist when calling the getTenant function - and not throw a 402 error. Also make sure that wherever we call this in our existing code, it's handled correctly -> you should check for undefined and throw an error where appropriate. Also check this behaviour for other functions in the recipe impl as well.
  • Apple Provider error for SignInUp #580 (comment) - ensure this doesn't happen on the new interface
  • GET //recipe/mulltitenancy/tenant in the code with the older cdi and it gave a 404
  • Null pointer issue in core for email password user that belongs to no tenant
  • check for edge func compatibility

PR breakups

  1. Merge with latest branch - fix: merge with latest #590
    • python
    • golang
  2. Update router to handle optional tenantId - fix: router to handle tenantid #592
    • python
    • golang
  3. Multitenancy recipe - fix: multitenancy recipe #594, fix: multitenancy claims #596 & fix: tenantId compulsory in getAllowedDomainsForTenantId #597
    • python
    • golang
  4. User context refactor - fix: user context refactor #595
    • python
    • golang
  5. User roles recipe - fix: multitenancy user roles #600 & fix: tenantId compulsory in user roles recipe interface #605
    • python
    • golang
  6. Email password recipe - fix: multitenancy emailpassword impl #602
    • python
    • golang
  7. Other auth recipes - fix: multitenancy passwordless implementation #606
    • python
    • golang
  8. Session recipe - fix: session recipe impl for multitenancy #607
    • python
    • golang
  9. Dashboard recipe - fix: dashboard multitenancy updates #623
  • python
  • golang
  1. Email related - fix: updating send email verification interface #625 & fix: password reset email #626
  • python
  • golang
  1. tenantId in callback functions - fix: tenantid in config functions #627 & fix: tenantIdForPasswordPolicy #629
  • python
  • golang
  1. Dashboard router - refactor: Update routing logic for Hapi and dashboard recipe to account for multitenancy #631
  • python
  • golang
  1. Test fixes - fix: test fixes #630
  • python
  • golang
  1. Remove Error handlers in multitenancy recipe - fix: remove error handlers in multitenancy recipe #632
  • python
  • golang
  1. Add tenantId in getGlobalClaimValidators - fix: tenant id in get global claim validators #634
  • python
  • golang
  1. tenantId as first param and compulsory - fix: tenantid compulsory #633
  • python
  • golang
  1. Fixes - refactor: Remove core config from dashboard tenants list response #635 and fix: Fix tenant id being passed in the wrong order for password update functions #636
  • python
  • golang
  1. Handle tenant not found error & type fixes for getProvider - fix: handle tenant not found error #639
  • python
  • golang
  1. Bitbucket and gitlab impl & other fixes - fix: bitbucket gitlab impl and other fixes #642
  • python
  • golang
  1. Allowed domain claim fix - fix: allowed domain claim and some typos #650
  • python
  • golang
  1. Allows empty provider list in third party: allows empty provider list #652
  • python
  • golang
  1. Fixes bug in custom third party provider code - fix: user info map #651
  • python
  • golang
  1. auth-react-server fix - fix: ci test fix #656
  • python
  • golang

@sattvikc sattvikc self-assigned this Jan 4, 2023
lib/ts/recipe/thirdparty/types.ts Outdated Show resolved Hide resolved
lib/ts/recipe/thirdparty/types.ts Outdated Show resolved Hide resolved
lib/ts/recipe/thirdparty/types.ts Show resolved Hide resolved
lib/ts/recipe/thirdparty/types.ts Outdated Show resolved Hide resolved
lib/ts/recipe/thirdparty/types.ts Outdated Show resolved Hide resolved
lib/ts/recipe/thirdparty/api/implementation.ts Outdated Show resolved Hide resolved
lib/ts/recipe/thirdparty/api/implementation.ts Outdated Show resolved Hide resolved
lib/ts/recipe/thirdparty/api/implementation.ts Outdated Show resolved Hide resolved
@rishabhpoddar rishabhpoddar marked this pull request as draft January 10, 2023 08:02
lib/ts/recipe/thirdparty/providers/activeDirectory.ts Outdated Show resolved Hide resolved
lib/ts/recipe/thirdparty/providers/apple.ts Outdated Show resolved Hide resolved
lib/ts/recipe/thirdparty/providers/configUtils.ts Outdated Show resolved Hide resolved
lib/ts/recipe/thirdparty/providers/custom.ts Outdated Show resolved Hide resolved
lib/ts/recipe/thirdparty/providers/utils.ts Outdated Show resolved Hide resolved
lib/ts/recipe/thirdparty/types.ts Outdated Show resolved Hide resolved
lib/ts/recipe/thirdparty/types.ts Outdated Show resolved Hide resolved
lib/ts/recipe/thirdparty/utils.ts Outdated Show resolved Hide resolved
nkshah2 and others added 3 commits July 11, 2023 16:03
* fix: test fixes

* fix: claims build

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: cleanup

* fix: tests

* fix: tests

* fix: pr comments
* fix: test fixes

* fix: claims build

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: cleanup

* fix: tests

* fix: tests

* fix: pr comments

* fix: remove error handlers in multitenancy

* fix: pr comments

* fix: pr comments
@sattvikc sattvikc mentioned this pull request Jul 12, 2023
12 tasks
* fix: tenantid compulsory

* fix: type fix

* fix: type fix
* fix: tenant id in global claim validators

* fix: pr comments
@sattvikc sattvikc mentioned this pull request Jul 13, 2023
12 tasks
nkshah2 and others added 7 commits July 14, 2023 11:00
* Remove core config from dashboard tenants list response

* Update tenant list API logic
…e functions (#636)

* Remove core config from dashboard tenants list response

* Fix tenant id being passed in the wrong order for password update functions

* Add user PUT api to list

* Update tenant list API logic

* Update CHANGELOG
* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: test

* fix: tests

* fix: tests
* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: tests

* fix: test

* fix: tests

* fix: tests

* fix: tests
* fix: handle tenant not found error

* fix: pr comments

* fix: pr comments
* fix: github

* fix: user info

* fix: discord and linkedin

* fix: gitlab impl

* fix: bitbucket impl
@rishabhpoddar rishabhpoddar changed the base branch from 14.1 to 15.0 July 18, 2023 05:55
* fix: tests

* fix: tests

* fix: framework tests with tenantid in path

* fix: tests

* fix: google workspaces
* fix: tests

* fix: tests

* fix: framework tests with tenantid in path

* fix: tests

* fix: google workspaces

* fix: version updates

* chore: changelog

* fix: pr comments

* fix: pr comments

* fix: pr comments

* fix: pr comments

* fix: pr comments
@rishabhpoddar rishabhpoddar marked this pull request as ready for review July 19, 2023 06:53
@rishabhpoddar rishabhpoddar merged commit 183c673 into 15.0 Jul 19, 2023
12 of 13 checks passed
@rishabhpoddar rishabhpoddar deleted the feat/tp-rework branch July 19, 2023 10:13
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.

4 participants