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

Invalid serial number for shell descriptor during tenant setup. Validate database connection before allowing new tenant to be added #11863

Closed
MikeAlhayek opened this issue Jun 13, 2022 · 0 comments · Fixed by #11822
Milestone

Comments

@MikeAlhayek
Copy link
Member

Is your feature request related to a problem? Please describe.

When a user is trying to add a new tenant while the database already exists, the app throw the following exception

image

The above exception is valid, but I think we should prevent it from happening but adding check to prevent the user from getting to this state.

Here are some scenarios that could lead to this exception

  1. On a local environment
  • The developer adds a new tenant (with or without prefix). For argument sake, we'll use site1 prefix.
  • The developer deletes the App_Data folder from the project
  • The developer adds another tenant with the prefix site1
  • The UI will allow adding the tenant, but the setup of the tenant will fail with the exception listed above. IMO, The tenant should not be added in this case
  1. A user adds a new tenant from the UI
  • The user adds a new tenant (with or without prefix). For argument sake, we'll use site1 prefix.
  • The user adds another tenant (and unintentionally uses the site1 prefix.)
  • Unfortunately, the UI will allow adding the tenant, but the setup of the tenant will fail with the exception listed above. IMO, The tenant should not be added in this case

Additionally, there is no way of telling if the provided database connection is valid when a new tenant is added. Currently, the only way is to attempt to setup the tenant, and hope that all goes well. Otherwise, you'll need to update the connection string and attempt to setup the site all over again. IMO, this isn't a good user experience and is error-prone.

Describe the solution you'd like

I would think some sort connection validation should take place to ensure that the database does not exists before allowing the tenant to be added. Since OrchardCore uses Yessql which require the Document table, I think we should run the following checks against this table prior allowing adding/updating the database connection

  1. Try to open a connection to the provided database connection. If the connection is open successfully, we then would know that the database connection is valid.
  2. When the connection is valid, we can try to execute select query on the Document table, if the select query runs, then we know that the table already exists and we'll prevent the user from using the provided prefix. Even if the user does not provide a prefix and Document table already used, we'll force the user to use a prefix.
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 a pull request may close this issue.

3 participants