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

OKAPI-1081: Reject invalid tenant ids #1347

Merged
merged 7 commits into from
Feb 28, 2024

Conversation

julianladisch
Copy link
Contributor

https://folio-org.atlassian.net/browse/OKAPI-1081

Implement
https://folio-org.atlassian.net/wiki/display/TC/ADR-000002+-+Tenant+Id+and+Module+Name+Restrictions
so that creating a new tenant via POST /_/proxy/tenants API is rejected unless the tenant id matches
^[a-z][a-z0-9]{0,30}$

Legacy tenant ids still work when used in any other API.

https://folio-org.atlassian.net/browse/OKAPI-1081

Implement
https://folio-org.atlassian.net/wiki/display/TC/ADR-000002+-+Tenant+Id+and+Module+Name+Restrictions
so that creating a new tenant via POST /_/proxy/tenants API is rejected unless the tenant id matches
^[a-z][a-z0-9]{0,30}$

Legacy tenant ids still work when used in any other API.
@jakub-id
Copy link
Contributor

@julianladisch while this avoids problems with existing tenant data. it is still a breaking change of the API. If we need to bump Okapi version to 6.0.0, I'd like us to release 5.3.0 first. Platform module release deadline for Quesnelia is March 1.

Copy link
Contributor

@hjiebsco hjiebsco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://folio-org.atlassian.net/wiki/spaces/DD/pages/1779867/Tenant+Id+and+Module+Name+Restrictions mentions Okapi and the modules should provide APIs and/or scripts to do the migration. Is that ready? Can we provide a configuration to allow user to continue use the old ^[a-z0-9_-]+$ validation to avoid this breaking change?

@julianladisch
Copy link
Contributor Author

Legacy tenant IDs can still be used. The restriction only affects new tenant IDs created via the API. There's no need to migrate legacy tenant IDs.
Therefore this breaking change doesn't affect existing tenant IDs. The breaking change only affects tenant IDs that people will create after the upgrade.

@hjiebsco
Copy link
Contributor

Legacy tenant IDs can still be used. The restriction only affects new tenant IDs created via the API. There's no need to migrate legacy tenant IDs. Therefore this breaking change doesn't affect existing tenant IDs. The breaking change only affects tenant IDs that people will create after the upgrade.

From users' perspective, for whatever reason, they might need or prefer to name new tenant id consistently with old tenants, or they might need to recreate old tenants with the same ids. It would be better to provide configuration to allow these.

@hjiebsco
Copy link
Contributor

Legacy tenant IDs can still be used. The restriction only affects new tenant IDs created via the API. There's no need to migrate legacy tenant IDs. Therefore this breaking change doesn't affect existing tenant IDs. The breaking change only affects tenant IDs that people will create after the upgrade.

From users' perspective, for whatever reason, they might need or prefer to name new tenant id consistently with old tenants, or they might need to recreate old tenants with the same ids. It would be better to provide configuration to allow these.

FYI that was just a suggestion. No objection if you want to merge as is.

@julianladisch
Copy link
Contributor Author

The FOLIO security team discussed whether there should be an option to allow creation of legacy tenant ids. However, the security team prefers not to have such an option for security reasons. There are workarounds for people who want to do create legacy tenant ids: They can directly write into the Okapi database, or they can install an old Okapi version.

Copy link

sonarcloud bot commented Feb 28, 2024

@julianladisch julianladisch merged commit 8302119 into master Feb 28, 2024
6 checks passed
@julianladisch julianladisch deleted the OKAPI-1081-restrict-invalid-tenantids branch February 28, 2024 22:08
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