Skip to content

SSO Domain Validation chain: dns_txt, url, meta_tag#5414

Merged
aerosol merged 8 commits intomasterfrom
sso-verification
Jun 3, 2025
Merged

SSO Domain Validation chain: dns_txt, url, meta_tag#5414
aerosol merged 8 commits intomasterfrom
sso-verification

Conversation

@aerosol
Copy link
Copy Markdown
Member

@aerosol aerosol commented May 21, 2025

Changes

This PR implements SSO Domain Validation. It consists of three steps:

  1. DNS TXT {domain} record lookup. Successful expectation contains plausible-sso-verification={domain-identifier} record.
  2. HTTP GET lookup at https://{domain}/plausible-sso-verification - successful expectation contatins {domain-identifier} in the body.
  3. META tag lookup at https://{domain} - successful expectation contains <meta name="plausible-sso-verification" content="{domain-identifier}"> in the body of text/html type.

Domain is marked as validated if any of those checks succeeds.

Tests

  • Automated tests have been added
  • This PR does not require tests

Changelog

  • Entry has been added to changelog
  • This PR does not make a user-facing change

Documentation

  • Docs have been updated
  • This change does not need a documentation update

Dark mode

  • The UI has been tested both in dark and light mode
  • This PR does not change the UI

@aerosol aerosol force-pushed the sso-verification branch 2 times, most recently from 247b16a to d2ff84e Compare June 3, 2025 12:07
@aerosol aerosol changed the title [wip] sso verification SSO Validation chain: dns_txt, url, meta_tag Jun 3, 2025
@aerosol aerosol force-pushed the sso-verification branch from d2ff84e to 495f48e Compare June 3, 2025 12:13
@aerosol aerosol requested a review from zoldar June 3, 2025 12:13
@aerosol aerosol marked this pull request as ready for review June 3, 2025 12:14
Bypass.expect_once(bypass, "GET", "/test", fn conn ->
conn
|> Plug.Conn.resp(200, """
<html>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For a moment I was concerned about HTML in different casing but Floki normalizes everything to downcase, so it's fine 👍

Comment thread test/plausible/auth/sso/domain/validation_test.exs Outdated
Comment thread test/plausible/auth/sso/domain/validation_test.exs Outdated
Comment thread test/plausible/auth/sso/domain/validation_test.exs Outdated
use Plausible.DataCase, async: true
use Plausible

@moduletag :ee_only
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm is that necessary given on_ee below? 🤔

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

not really, purely cosmetic I guess - skip a module vs run an empty module?

@@ -0,0 +1,63 @@
defmodule Plasusible.Test.Support.DNSServer do
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice!

run_request(url_override || "https://#{sso_domain}"),
true <- html?(response),
{:ok, html} <- Floki.parse_document(body),
[_] <- Floki.find(html, ~s|meta[name="#{@prefix}"][content="#{domain_identifier}"]|) do
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's going to fail when there's more than one meta with exact same name and content. Maybe we should just check that it's a non-empty list?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

sso_domain
|> to_charlist()
|> :inet_res.lookup(:in, :txt, opts, timeout)
|> Enum.find_value(false, fn
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just double checked that it's going to work fine in presence of multiple TXT records 👍

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yep, I've verified against real world domains

aerosol and others added 4 commits June 3, 2025 14:37
Co-authored-by: Adrian Gruntkowski <adrian.gruntkowski@gmail.com>
Co-authored-by: Adrian Gruntkowski <adrian.gruntkowski@gmail.com>
Co-authored-by: Adrian Gruntkowski <adrian.gruntkowski@gmail.com>
@aerosol aerosol changed the title SSO Validation chain: dns_txt, url, meta_tag SSO Domain Validation chain: dns_txt, url, meta_tag Jun 3, 2025
@aerosol aerosol added this pull request to the merge queue Jun 3, 2025
Merged via the queue into master with commit 6040bed Jun 3, 2025
9 of 12 checks passed
@zoldar zoldar deleted the sso-verification branch June 4, 2025 09:06
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.

2 participants