Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

MM-14478 Update documentation on AllowUntrustedInternalConnections #2499

Merged
merged 3 commits into from
Mar 14, 2019
Merged

MM-14478 Update documentation on AllowUntrustedInternalConnections #2499

merged 3 commits into from
Mar 14, 2019

Conversation

hmhealey
Copy link
Member

As discussed, this makes the setting sound less immediately scary, but instead of just rewriting the docs, I replaced most of the text with a link to them.

Ticket Link

https://mattermost.atlassian.net/browse/MM-14478

Checklist

  • Has UI changes
  • Includes text changes and localization file (.../i18n/en.json) updates

@hmhealey hmhealey added 1: PM Review Requires review by a product manager 2: Dev Review Requires review by a core commiter CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Mar 13, 2019
@hmhealey hmhealey added this to the v5.9.0 milestone Mar 13, 2019
lieut-data
lieut-data suggested changes Mar 13, 2019
View reviewed changes
Copy link
Member

@lieut-data lieut-data left a comment

Choose a reason for hiding this comment

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

Sounds good to me, but do we need to update i18n/en.json?

@hmhealey
Copy link
Member Author

Whoops. Good catch

lieut-data
lieut-data approved these changes Mar 13, 2019
View reviewed changes
Copy link
Member

@lieut-data lieut-data left a comment

Choose a reason for hiding this comment

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

🎉

@hmhealey hmhealey self-assigned this Mar 13, 2019
jasonblais
jasonblais suggested changes Mar 13, 2019
View reviewed changes
i18n/en.json Outdated
@@ -1224,7 +1224,7 @@
"admin.service.insecureTlsTitle": "Enable Insecure Outgoing Connections: ",
"admin.service.integrationAdmin": "Restrict managing integrations to Admins:",
"admin.service.integrationAdminDesc": "When true, webhooks and slash commands can only be created, edited and viewed by Team and System Admins, and OAuth 2.0 applications by System Admins. Integrations are available to all users after they have been created by the Admin.",
"admin.service.internalConnectionsDesc": "In testing environments, such as when developing integrations locally on a development machine, use this setting to specify domains, IP addresses, or CIDR notations to allow internal connections. Separate two or more domains with spaces. **Not recommended for use in production**, since this can allow a user to extract confidential data from your server or internal network.\n \nBy default, user-supplied URLs such as those used for Open Graph metadata, webhooks, or slash commands will not be allowed to connect to reserved IP addresses including loopback or link-local addresses used for internal networks. Push notification and OAuth 2.0 server URLs are trusted and not affected by this setting.",
"admin.service.internalConnectionsDesc": "A whitelist of local network addresses that can be requested by the Mattermost server on behalf of a client. Care should be used when configuring this setting to prevent unintended access to your local network. See [documentation](https://docs.mattermost.com/administration/config-settings.html#allow-untrusted-internal-connections-to) to learn more.",
Copy link
Contributor

Choose a reason for hiding this comment

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

@hmhealey Can you help update the documentation link to be https://mattermost.com/default-allow-untrusted-internal-connections instead?

I'll ask marketing to help add a redirect to the doc.

(We want to avoid hardcoding doc links in the product, in case they get moved to another location, for instance)

Copy link
Contributor

Choose a reason for hiding this comment

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

PS: Ticket for marketing team https://app.asana.com/0/647304641413388/1114101123295685

Copy link
Member

Choose a reason for hiding this comment

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

@jasonblais, love the idea of a permalink, but can we avoid anchoring this at the root of mattermost.com? I fear a future conflict. Perhaps mattermost.com/pl/default-allow-untrusted-internal-connections or some other suitable prefix.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can touch base on the feasibility. We've used the default prefix to indicate default pages that should not be used as landing pages, but I could see it not being a full-proof "permanent" link

Copy link
Contributor

Choose a reason for hiding this comment

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

@hmhealey @lieut-data Redirect for https://mattermost.com/pl/default-allow-untrusted-internal-connections is now set up

Copy link
Member Author

Choose a reason for hiding this comment

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

@jasonblais These permalinks usually go through about.mattermost.com such as https://about.mattermost.com/platform-notice-txt/ or https://about.mattermost.com/hpns-terms/. We haven't used any sort of /pl/ either.

0/5 on if we change it, but I want to confirm this is correct before adding that.

Copy link
Contributor

Choose a reason for hiding this comment

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

  1. We used about.mattermost.com in the past, because that was our site. We now create pages in mattermost.com.
  2. /pl helps with the permalink portion.

Let me know if there are any concerns with either though, in case there are blindspots.

Copy link
Member Author

Choose a reason for hiding this comment

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

K, I'll update that. I think those are good changes to make, but I wanted to make sure we were changing the standard practice on purpose.

@hmhealey hmhealey removed the 2: Dev Review Requires review by a core commiter label Mar 14, 2019
@jasonblais jasonblais added 4: Reviews Complete All reviewers have approved the pull request and removed 1: PM Review Requires review by a product manager labels Mar 14, 2019
@hmhealey hmhealey merged commit 2345086 into master Mar 14, 2019
@hmhealey hmhealey deleted the mm14478 branch March 14, 2019 16:27
hmhealey added a commit that referenced this pull request Mar 14, 2019
@hmhealey
MM-14478 Update documentation on AllowUntrustedInternalConnections (#…
42a23b7 
…2499)

* MM-14478 Update documentation on AllowUntrustedInternalConnections

* Update en.json

* Update documentation link
@hmhealey hmhealey added CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone and removed CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Mar 14, 2019
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation Docs/Done Required documentation has been written and removed Docs/Not Needed Does not require documentation labels Mar 14, 2019
@lindalumitchell lindalumitchell added the Tests/Not Needed Does not require new release tests label Mar 15, 2019
stevepartridge pushed a commit to stevepartridge/mattermost-webapp that referenced this pull request Mar 30, 2019
@hmhealey @stevepartridge
MM-14478 Update documentation on AllowUntrustedInternalConnections (m…
fec2910 
…attermost#2499)

* MM-14478 Update documentation on AllowUntrustedInternalConnections

* Update en.json

* Update documentation link
tuannguyen041094 pushed a commit to Designveloper/mattermost-webapp that referenced this pull request Apr 9, 2019
@hmhealey @tuannguyen041094
MM-14478 Update documentation on AllowUntrustedInternalConnections (m…
8c07343 
…attermost#2499)

* MM-14478 Update documentation on AllowUntrustedInternalConnections

* Update en.json

* Update documentation link
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lieut-data lieut-data lieut-data approved these changes

@DSchalla DSchalla DSchalla approved these changes

@jasonblais jasonblais jasonblais approved these changes

Assignees

@hmhealey hmhealey

Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone Docs/Done Required documentation has been written Tests/Not Needed Does not require new release tests
Projects
None yet
Milestone
v5.9.0
6 participants
@hmhealey @lieut-data @DSchalla @jasonblais @lindalumitchell @amyblais