-
Notifications
You must be signed in to change notification settings - Fork 68
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: add allowedIpRanges widgets config #833
base: main
Are you sure you want to change the base?
feat: add allowedIpRanges widgets config #833
Conversation
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.
Seems reasonable, thank you!
2853bd0
to
93d8dc9
Compare
Head branch was pushed to by a user without write access
7a6c160
to
f31822e
Compare
f31822e
to
1bf7673
Compare
1bf7673
to
0b52f5d
Compare
Adds `allowedIpRanges` widgets config that can be used specifically allow some ip ranges even if its in `disallowedIpRanges`. Signed-off-by: Iulian Meghea <[email protected]>
Co-authored-by: Will Hunt <[email protected]>
0b52f5d
to
be5e734
Compare
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.
A few things to think about. Generally looks good though :)
this.roomSetupWidget = yaml.roomSetupWidget; | ||
if (yaml.disallowedIpRanges !== undefined && (!Array.isArray(yaml.disallowedIpRanges) || !yaml.disallowedIpRanges.every(s => typeof s === "string"))) { | ||
throw new ConfigError("widgets.disallowedIpRanges", "must be a string array"); | ||
} | ||
if (yaml.allowedIpRanges !== undefined && (!Array.isArray(yaml.allowedIpRanges) || !yaml.allowedIpRanges.every(s => typeof s === "string"))) { |
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.
N.B. An empty array would skip this, would we allow empty values for allowedIpRanges (effectively blocking the feature entirely, seems a bit pointless :) )
@@ -40,6 +40,7 @@ export const DefaultConfigRoot: BridgeConfigRoot = { | |||
roomSetupWidget: { | |||
addOnInvite: false, | |||
}, | |||
allowedIpRanges: [], |
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.
Wondering if we want this to actually show up as a default, because the default value would block everything currently :)
@@ -39,6 +39,7 @@ widgets: | |||
# - 2001:db8::/32 | |||
# - ff00::/8 | |||
# - fec0::/10 | |||
# allowedIpRanges: [] |
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.
Ideally we would put in some examples of values here?
Adds
allowedIpRanges
widgets config that can be used specificallyallow some ip ranges even if its in
disallowedIpRanges
.