Skip to content

Conversation

@devinbinnie
Copy link
Member

Summary

This PR adds an E2E test suite (bad_servers.test.js) that verifies handling of servers with bad configurations. It covers adding servers via the Add Server Modal and pre-configured servers, testing scenarios including:

  • unreachable servers (DNS resolution failure)
  • expired certificates
  • outdated TLS versions (1.0, 1.1)
  • weak cipher suites (RC4).

The tests verify that the appropriate error views are displayed with the correct Chrome error codes. It also checks that trusted certificates in the CertificateStore allow pages to load successfully, and that bad servers don't prevent users from logging in and using working Mattermost servers.

Ticket Link

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

NONE

@devinbinnie devinbinnie requested review from a team, fmartingr and yasserfaraazkhan and removed request for a team December 8, 2025 17:52
@devinbinnie devinbinnie added the 2: Dev Review Requires review by a core committer label Dec 8, 2025
@devinbinnie
Copy link
Member Author

@yasserfaraazkhan This PR will likely require your changes from #3512, so we can wait until that is merged first before running these new ones.

Copy link

@fmartingr fmartingr left a comment

Choose a reason for hiding this comment

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

LGTM

FYI: badssl is not recommended for automated testing, and the repository is archived, so I'm not sure if the site will be long lived as it is. Have we considered launching our own server using testcontainers or services in CI?

@devinbinnie
Copy link
Member Author

LGTM

FYI: badssl is not recommended for automated testing, and the repository is archived, so I'm not sure if the site will be long lived as it is. Have we considered launching our own server using testcontainers or services in CI?

Yeah I had a feeling it might not be the best idea, however it's a lot more work to spin up those boxes in CI. I think that is something we should do instead, but maybe we can do it as a follow-up and just use badssl while it still works.

cc @yasserfaraazkhan I may need your help at some point transitioning this over to our own test instances in CI.

@devinbinnie devinbinnie added 4: Reviews Complete All reviewers have approved the pull request Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) and removed 2: Dev Review Requires review by a core committer labels Dec 10, 2025
@amyblais amyblais modified the milestone: v6.1.0 Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4: Reviews Complete All reviewers have approved the pull request Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) release-note-none

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants