Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/arch/11-auth-server-storage.md
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,8 @@ Clients must generate a fresh, unguessable `state` for each request and verify t

The page leads with a plain-language summary and the classified destination of the redirect ("this computer" for loopback, the host plus its punycode form for other http(s) hosts, "an app that opens `<scheme>://` links" for custom schemes, or the full URI when it cannot be classified), followed by the client name, labelled as given by the app and not verified. A collapsed "Technical details" section holds the full redirect URI, client ID, scopes, resource and the first upstream provider's internal name. It does not redirect to the upstream identity provider until a browser-bound, same-origin POST to `/oauth/consent` approves the request. Denial returns `access_denied` to the validated client redirect. The pending consent handle and each upstream callback state are single-use, browser-bound records with a 10-minute absolute lifetime from the initial authorization (not extended by chaining). The device flow uses its separate verification and approval page.

The "You will sign in with … next." line shows the upstream's optional `DisplayName` (`display_name` in `UpstreamRunConfig`), falling back to its `Name` when the display name is unset, empty after sanitising, or when the pending authorization's upstream is no longer configured. The label is resolved from the handler's current upstream list at render time, so a swapped upstream set shows the current value. `DisplayName` is display-only: it is never compared, persisted, or used as a key; upstream token storage, remembered consent (`ProviderID`), pending authorizations (`UpstreamProviderName`) and callback routing all stay keyed by `Name`, so changing a display name leaves every stored record valid. At render, control and format runes (bidi overrides, zero-width characters, newlines) are dropped, surrounding whitespace is trimmed, the result is capped at 80 runes with a trailing ellipsis, and the template HTML-escapes it. A malformed label is cleaned at render and never rejected by validation, so a cosmetic field cannot fail server start or an embedder's rebuild of its upstreams. Technical details keep showing `Name` as the internal name. Sanitising protects layout, not meaning: printable misleading text such as "Acme Okta (verified)" is shown in full, and the page presents the label as trusted text on the anti-phishing gate. An embedder must therefore populate `DisplayName` only from operator-controlled values, never from tenant-, admin- or client-writable input.
Comment thread
tgrunnagle marked this conversation as resolved.
Outdated

The form action uses the configured browser-facing authorization base, including any path prefix. The POST accepts only that base's browser-serialized Origin (including normalized IP addresses, domain names and ports), not a missing, null, or foreign Origin. `Referrer-Policy: same-origin` preserves Origin on native same-origin form submissions without leaking the consent URL to other sites.

The page's CSP blocks scripts, framing and base URL changes. It deliberately omits `form-action 'self'`: browsers also apply that restriction to the POST's redirect to an external IdP or validated client URI, including dynamic loopback ports. The template has only the escaped, configured form target, and the POST additionally requires the single-use browser-binding cookie. No COOP header is set on the consent page, to avoid severing an OAuth popup's opener.
Expand Down
4 changes: 4 additions & 0 deletions docs/server/docs.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions docs/server/swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions docs/server/swagger.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions pkg/authserver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -761,6 +761,13 @@ type UpstreamRunConfig struct {
// If empty when only one upstream is configured, defaults to "default".
Name string `json:"name,omitempty" yaml:"name,omitempty"`

// DisplayName is an optional human-readable label for this upstream, shown to
// end users on the consent page in place of Name. It is display-only: never
// compared, stored, or used to route. It is sanitised at render time and never
Comment thread
tgrunnagle marked this conversation as resolved.
Outdated
// rejected by validation. It is shown as trusted text, so it must come from
// operator-controlled configuration only.
DisplayName string `json:"display_name,omitempty" yaml:"display_name,omitempty"`

// Type specifies the provider type: "oidc" or "oauth2".
Type UpstreamProviderType `json:"type" yaml:"type"`

Expand Down Expand Up @@ -1151,6 +1158,13 @@ type UpstreamConfig struct {
// If empty when only one upstream is configured, defaults to "default".
Name string `json:"name,omitempty" yaml:"name,omitempty"`

// DisplayName is an optional human-readable label for this upstream, shown to
// end users on the consent page in place of Name. It is display-only: never
// compared, stored, or used to route. It is sanitised at render time and never
// rejected by validation. It is shown as trusted text, so it must come from
// operator-controlled configuration only.
DisplayName string `json:"display_name,omitempty" yaml:"display_name,omitempty"`

// Type specifies the provider type: "oidc" or "oauth2".
Type UpstreamProviderType `json:"type" yaml:"type"`

Expand Down
40 changes: 40 additions & 0 deletions pkg/authserver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,46 @@ func TestRunConfigValidate(t *testing.T) {
}
}

func TestUpstreamConfig_DisplayNameNeverFailsValidation(t *testing.T) {
Comment thread
tgrunnagle marked this conversation as resolved.
t.Parallel()

oauth2 := &upstream.OAuth2Config{
CommonOAuthConfig: upstream.CommonOAuthConfig{ClientID: "c", RedirectURI: "https://example.com/cb"},
AuthorizationEndpoint: "https://idp.example.com/authorize",
TokenEndpoint: "https://idp.example.com/token",
}
displayNames := map[string]string{
"empty": "",
"plain": "Acme Okta",
"control runes": "Acme\u202e\u200b\nOkta",
"whitespace only": " \t ",
"html": "<script>alert(1)</script>",
"very long": strings.Repeat("é", 1000),
}
for name, displayName := range displayNames {
t.Run("Config/"+name, func(t *testing.T) {
t.Parallel()
cfg := Config{
Issuer: "https://example.com", KeyProvider: keys.NewGeneratingProvider(keys.DefaultAlgorithm),
HMACSecrets: &servercrypto.HMACSecrets{Current: make([]byte, 32)},
AllowedAudiences: []string{"https://mcp.example.com"},
Upstreams: []UpstreamConfig{{
Name: "ffffffff-ffff-ffff-ffff-ffffffffffff", DisplayName: displayName,
Type: UpstreamProviderTypeOAuth2, OAuth2Config: oauth2,
}},
}
require.NoError(t, cfg.Validate())
cfg.Upstreams[0].Name = "Not_A_Label"
require.ErrorContains(t, cfg.Validate(), "must match")
Comment thread
tgrunnagle marked this conversation as resolved.
Outdated
})
t.Run("RunConfig/"+name, func(t *testing.T) {
t.Parallel()
cfg := RunConfig{Upstreams: []UpstreamRunConfig{{Name: "okta", DisplayName: displayName}}}
require.NoError(t, cfg.Validate())
})
}
}

// TestRunConfigValidate_UpstreamFilter covers RunConfig.Validate()'s
// enforcement of UpstreamFilter static invariants against the configured
// upstream list. Thorough matching/union/claim behavior is covered in
Expand Down
8 changes: 5 additions & 3 deletions pkg/authserver/runner/embeddedauthserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -1010,9 +1010,10 @@ func buildUpstreamConfig(rc *authserver.UpstreamRunConfig, insecureAllowHTTP boo
return nil, err
}
return &authserver.UpstreamConfig{
Name: rc.Name,
Type: authserver.UpstreamProviderTypeOIDC,
OIDCConfig: oidcCfg,
Name: rc.Name,
DisplayName: rc.DisplayName,
Type: authserver.UpstreamProviderTypeOIDC,
OIDCConfig: oidcCfg,
}, nil

case authserver.UpstreamProviderTypeOAuth2:
Expand All @@ -1022,6 +1023,7 @@ func buildUpstreamConfig(rc *authserver.UpstreamRunConfig, insecureAllowHTTP boo
}
return &authserver.UpstreamConfig{
Name: rc.Name,
DisplayName: rc.DisplayName,
Type: authserver.UpstreamProviderTypeOAuth2,
OAuth2Config: oauth2Cfg,
}, nil
Expand Down
28 changes: 28 additions & 0 deletions pkg/authserver/runner/embeddedauthserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1405,6 +1405,34 @@ func TestEmbeddedAuthServer_KeyProvider(t *testing.T) {
})
}

func TestBuildUpstreamConfig_CopiesDisplayName(t *testing.T) {
t.Parallel()

for _, rc := range []authserver.UpstreamRunConfig{
{
Name: "okta", DisplayName: "Acme Okta", Type: authserver.UpstreamProviderTypeOIDC,
OIDCConfig: &authserver.OIDCUpstreamRunConfig{
IssuerURL: "https://okta.example.com", ClientID: "c", RedirectURI: "http://localhost:8080/callback",
},
},
{
Name: "okta", DisplayName: "Acme Okta", Type: authserver.UpstreamProviderTypeOAuth2,
OAuth2Config: &authserver.OAuth2UpstreamRunConfig{
AuthorizationEndpoint: "https://okta.example.com/authorize", TokenEndpoint: "https://okta.example.com/token",
ClientID: "c", RedirectURI: "http://localhost:8080/callback",
},
},
} {
t.Run(string(rc.Type), func(t *testing.T) {
t.Parallel()
cfg, err := buildUpstreamConfig(&rc, false)
require.NoError(t, err)
assert.Equal(t, "okta", cfg.Name)
assert.Equal(t, "Acme Okta", cfg.DisplayName)
})
}
}

func TestBuildUpstreamConfig(t *testing.T) {
t.Parallel()

Expand Down
45 changes: 37 additions & 8 deletions pkg/authserver/server/handlers/consent.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,13 @@ import (
)

type consentPageData struct {
Handle, ActionURL, Destination, Summary, RedirectURI, ClientID, ClientName, Resource, Upstream string
Scopes []string
CanRemember bool
RememberFor string
Handle, ActionURL, Destination, Summary, RedirectURI, ClientID, ClientName, Resource string
// Upstream is the upstream's name (its routing and storage key); UpstreamDisplay
// is the sanitised label shown in the "You will sign in with" line.
Upstream, UpstreamDisplay string
Scopes []string
CanRemember bool
RememberFor string
}

// consentStyles extends pageStyles for the consent page only; it is served as a
Expand Down Expand Up @@ -69,7 +72,7 @@ var consentPageTemplate = template.Must(template.New("consent").Parse(`<!DOCTYPE
<div class="card"><div class="card-body">
<h1>Allow this app to sign you in?</h1>
<p class="subtitle summary">{{.Summary}}</p>
<p class="subtitle">You will sign in with {{.Upstream}} next.</p>
<p class="subtitle">You will sign in with {{.UpstreamDisplay}} next.</p>
<dl><dt>Sends your sign-in to:</dt><dd><strong>{{.Destination}}</strong></dd>
<dt>Name given by the app (not verified):</dt><dd>{{.ClientName}}</dd></dl>
<details><summary>Technical details</summary><dl>
Expand Down Expand Up @@ -132,21 +135,47 @@ func displayClientName(name string) string {
}, name)
}

// truncateConsentText caps s at maxConsentClientNameRunes, marking a cut with an ellipsis.
Comment thread
tgrunnagle marked this conversation as resolved.
Outdated
func truncateConsentText(s string) string {
if r := []rune(s); len(r) > maxConsentClientNameRunes {
return string(r[:maxConsentClientNameRunes]) + "…"
Comment thread
tgrunnagle marked this conversation as resolved.
Outdated
}
return s
}

// upstreamDisplayName returns the label for the named upstream on the consent page:
// its sanitised DisplayName when the upstream is still configured and has one,
// otherwise the name itself. The label is resolved at render time so a swapped
// upstream set shows the current value; it is display-only and never a key.
Comment thread
tgrunnagle marked this conversation as resolved.
Outdated
func (h *Handler) upstreamDisplayName(name string) string {
for i := range h.upstreams {
if h.upstreams[i].Name != name {
continue
}
if label := strings.TrimSpace(displayClientName(h.upstreams[i].DisplayName)); label != "" {
Comment thread
tgrunnagle marked this conversation as resolved.
return truncateConsentText(label)
}
break
Comment thread
tgrunnagle marked this conversation as resolved.
Outdated
}
return name
}

func (h *Handler) renderConsent(
w http.ResponseWriter, handle string, pending *storage.PendingAuthorization, client fosite.Client,
) {
destination, summary := classifyRedirect(pending.RedirectURI)
clientName := displayClientName(registration.ClientName(client))
if clientName == "" {
clientName = "No name given"
} else if r := []rune(clientName); len(r) > maxConsentClientNameRunes {
clientName = string(r[:maxConsentClientNameRunes]) + "…"
} else {
clientName = truncateConsentText(clientName)
}
data := consentPageData{
Handle: handle, ActionURL: h.config.GetAuthorizationEndpointBaseURL() + "/oauth/consent",
Destination: destination, Summary: summary, RedirectURI: pending.RedirectURI,
ClientID: client.GetID(), ClientName: clientName, Resource: pending.Resource,
Upstream: pending.UpstreamProviderName, Scopes: pending.Scopes, CanRemember: h.rememberedStorage != nil,
Upstream: pending.UpstreamProviderName, UpstreamDisplay: h.upstreamDisplayName(pending.UpstreamProviderName),
Scopes: pending.Scopes, CanRemember: h.rememberedStorage != nil,
RememberFor: formatDays(storage.ConsentSessionTTL),
}
pageHash, consentHash := sha256.Sum256([]byte(pageStyles)), sha256.Sum256([]byte(consentStyles))
Expand Down
Loading
Loading