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

Feature: Allowed domains for chatflows #1765

Merged
merged 14 commits into from
Mar 11, 2024

Conversation

0xi4o
Copy link
Contributor

@0xi4o 0xi4o commented Feb 20, 2024

  • Adds an allowed domains dialog for chatflows where users can whitelist which domains can access the chatflow. By default all domains are allowed. If a list is present, then only those domains can use the chatflow prediction, either from embed or API.

flowise-allowed-domains-settings
flowise-allowed-domains-settings-2
Flowise-Test

@automaton82
Copy link
Contributor

I'm a little confused why you'd use this over CORS or CSP? They do the same thing but at a lower level to ensure no access can be done at all except from the whitelisted domains.

@HenryHengZJ
Copy link
Contributor

the intention is to allow the chatbot to be embedded on specific domain:
image

@automaton82
Copy link
Contributor

Yea I get that but that's the same thing CORS does. Except CORS will pass security tests since the preflight options will also fail when not on an allowed domain.

It's the same functionality just at the level where it's expected. Otherwise they'll be tempted to set CORS to * and use this new config instead which isn't secure, since any domain will be able to access the REST API, regardless of this setting.

If you remember I mentioned this on the other ticket. The very first request xdomain sends is OPTIONS preflight check. This is before any logic can be done about a chatbot ID, thus CORS is in the HTTP pipeline and configured at the env level.

@automaton82
Copy link
Contributor

If your goal is to solve multi tenancy in cloud hosting (1 host per N clients) then better to solve it with actual accounts and tenancy, where you keep the tenancy as the initial request and sticky it somehow through all subsequent requests.

Anyways just my $0.02 😄

@HenryHengZJ
Copy link
Contributor

Yea I get that but that's the same thing CORS does. Except CORS will pass security tests since the preflight options will also fail when not on an allowed domain.

It's the same functionality just at the level where it's expected. Otherwise they'll be tempted to set CORS to * and use this new config instead which isn't secure, since any domain will be able to access the REST API, regardless of this setting.

If you remember I mentioned this on the other ticket. The very first request xdomain sends is OPTIONS preflight check. This is before any logic can be done about a chatbot ID, thus CORS is in the HTTP pipeline and configured at the env level.

yeah I think there are 2 level of users for Flowise, for devs its always welcomed to set the env variables CORS for that. For no-code users they might not be familiar with the CORS and setting env variables, so this is an alternative, although it doesnt solve at the very root level, but still offers some protective mechanism at the very least

@0xi4o 0xi4o requested review from chungyau97, vinodkiran and HenryHengZJ and removed request for vinodkiran February 27, 2024 10:42
@0xi4o
Copy link
Contributor Author

0xi4o commented Mar 5, 2024

@HenryHengZJ I've updated the error message for a non-whitelisted domain. Please review this.

if (chatflow.chatbotConfig) {
const parsedConfig = JSON.parse(chatflow.chatbotConfig)
// check whether the first one is not empty. if it is empty that means the user set a value and then removed it.
const isValidAllowedOrigins = parsedConfig.allowedOrigins[0] !== ''
Copy link
Contributor

Choose a reason for hiding this comment

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

what if parsedConfig doesn't have the property allowedOrigins, will it fails?
shouldn't we do something like parsedConfig.allowedOrigins?.length && parsedConfig.allowedOrigins[0] !== '' ?

Copy link
Contributor

Choose a reason for hiding this comment

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

yep, it does error:
image

const origin = new URL(originHeader).host
isDomainAllowed =
parsedConfig.allowedOrigins.filter((domain: string) => {
const allowedOrigin = new URL(domain).host
Copy link
Contributor

Choose a reason for hiding this comment

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

if domain is invalid, you will get error Invalid URL and the apps shut down due to unhandled rejection

@0xi4o 0xi4o merged commit 13ee0d0 into FlowiseAI:main Mar 11, 2024
2 checks passed
@0xi4o 0xi4o deleted the feature/allowed-domains branch March 12, 2024 14:29
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.

None yet

3 participants