Skip to content

server: fail login fast when the configured issuer is not the proxy-advertised one #314

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.

Sign up for GitHub

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

Closed
qu0b wants to merge 1 commit into from
Closed

server: fail login fast when the configured issuer is not the proxy-advertised one #314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
79 changes: 51 additions & 28 deletions pkg/server/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ import (
// approval when the provider does not advertise an expiry.
const minLoginPollWindow = 5 * time.Minute

// proxyScopeDiscoveryTimeout bounds the /auth/metadata fetch performed at login
// proxyAuthDiscoveryTimeout bounds the /auth/metadata fetch performed at login
// so an unreachable proxy fails the login promptly instead of stalling it.
const proxyScopeDiscoveryTimeout = 5 * time.Second
const proxyAuthDiscoveryTimeout = 5 * time.Second

// credentialTarget is the resolved proxy auth configuration the controller
// operates against. It mirrors the values the proxy client uses to build its
Expand Down Expand Up @@ -51,12 +51,12 @@ type credentialController struct {
newClient func(credentialTarget) authclient.Client
newStore func(credentialTarget, authclient.Client) authstore.Store

// discoverScopes resolves the proxy's advertised login scope set at login
// time (from /auth/metadata) so the proxy — not a possibly-stale local
// config — decides which scopes the token carries. It is injected for tests;
// in production it is always set. A nil hook (tests only) requests the
// configured target scopes verbatim.
discoverScopes func(ctx context.Context) ([]string, error)
// discoverLoginAuth resolves the proxy's advertised login contract (issuer
// plus scope set, from /auth/metadata) at login time so the proxy — not a
// possibly-stale local config — decides which scopes the token carries. It
// is injected for tests; in production it is always set. A nil hook (tests
// only) requests the configured target scopes verbatim.
discoverLoginAuth func(ctx context.Context) (proxy.AuthMetadataResponse, error)

mu sync.Mutex
pending *pendingLogin
Expand Down Expand Up @@ -111,8 +111,8 @@ func newCredentialController(log logrus.FieldLogger, meta *serverapi.ProxyAuthMe
// login so a config written before a feature existed still requests the
// scope it now needs, without a re-init — and without ever consulting the
// local scopes field.
c.discoverScopes = func(ctx context.Context) ([]string, error) {
return fetchProxyLoginScopes(ctx, proxyURL)
c.discoverLoginAuth = func(ctx context.Context) (proxy.AuthMetadataResponse, error) {
return fetchProxyLoginAuth(ctx, proxyURL)
}

return c
Expand Down Expand Up @@ -162,14 +162,28 @@ func (c *credentialController) BeginLogin(ctx context.Context) (serverapi.AuthLo
// the proxy cannot be reached we fail loudly rather than guess. Only the
// scopes come from discovery; issuer/client/resource stay as configured so
// the credential file keys identically.
//
// Issuer and scopes are one contract: a scope name is only meaningful to
// the issuer that advertised it (e.g. "workflows" exists on Authentik but
// Dex rejects it as invalid_scope). When the configured issuer is not the
// advertised one — a config from before an issuer migration — starting the
// flow would mint a device code doomed to die in the browser, so refuse
// up front with the migration path instead.
target := c.target
if c.discoverScopes != nil {
scopes, err := c.discoverScopes(ctx)
if c.discoverLoginAuth != nil {
adv, err := c.discoverLoginAuth(ctx)
if err != nil {
return serverapi.AuthLoginResponse{}, fmt.Errorf("resolving login scopes from proxy: %w", err)
}

target.scopes = scopes
if adv.IssuerURL != "" && !sameIssuer(adv.IssuerURL, target.issuerURL) {
return serverapi.AuthLoginResponse{}, fmt.Errorf(
"the proxy advertises issuer %s but this server is configured for %s; "+
"re-run `panda init` to adopt the advertised issuer, restart panda-server, then retry login",
adv.IssuerURL, target.issuerURL)
}

target.scopes = adv.Scopes
}

client := c.newClient(target)
Expand Down Expand Up @@ -244,44 +258,53 @@ func (c *credentialController) Stop() {
c.pending = nil
}

// fetchProxyLoginScopes reads the proxy's advertised login scope set from its
// public /auth/metadata discovery endpoint. A successful fetch returns the
// advertised scopes; an empty slice is a valid result meaning the proxy needs
// no extra scopes (the auth client then applies its own defaults). Any
// transport, status, or decode failure returns an error so the caller fails the
// login loudly instead of guessing a scope set.
func fetchProxyLoginScopes(ctx context.Context, proxyURL string) ([]string, error) {
// sameIssuer compares issuer URLs up to a trailing slash. Authentik's issuer
// requires the trailing slash while other layers (status output, hand-edited
// configs) may carry it trimmed; that difference must not read as a different
// issuer.
func sameIssuer(a, b string) bool {
return strings.TrimRight(a, "/") == strings.TrimRight(b, "/")
}

// fetchProxyLoginAuth reads the proxy's advertised login contract (issuer and
// scope set) from its public /auth/metadata discovery endpoint. An empty scope
// list is a valid result meaning the proxy needs no extra scopes (the auth
// client then applies its own defaults); an empty issuer means an older proxy
// that does not advertise one. Any transport, status, or decode failure
// returns an error so the caller fails the login loudly instead of guessing.
func fetchProxyLoginAuth(ctx context.Context, proxyURL string) (proxy.AuthMetadataResponse, error) {
var meta proxy.AuthMetadataResponse

base := strings.TrimSpace(proxyURL)
if base == "" {
return nil, fmt.Errorf("proxy URL is not configured")
return meta, fmt.Errorf("proxy URL is not configured")
}

reqCtx, cancel := context.WithTimeout(ctx, proxyScopeDiscoveryTimeout)
reqCtx, cancel := context.WithTimeout(ctx, proxyAuthDiscoveryTimeout)
defer cancel()

metadataURL := strings.TrimRight(base, "/") + "/auth/metadata"

req, err := http.NewRequestWithContext(reqCtx, http.MethodGet, metadataURL, nil)
if err != nil {
return nil, fmt.Errorf("building metadata request: %w", err)
return meta, fmt.Errorf("building metadata request: %w", err)
}

resp, err := http.DefaultClient.Do(req)
if err != nil {
return nil, fmt.Errorf("fetching %s: %w", metadataURL, err)
return meta, fmt.Errorf("fetching %s: %w", metadataURL, err)
}
defer func() { _ = resp.Body.Close() }()

if resp.StatusCode != http.StatusOK {
return nil, fmt.Errorf("fetching %s: unexpected status %s", metadataURL, resp.Status)
return meta, fmt.Errorf("fetching %s: unexpected status %s", metadataURL, resp.Status)
}

var meta proxy.AuthMetadataResponse
if err := json.NewDecoder(resp.Body).Decode(&meta); err != nil {
return nil, fmt.Errorf("decoding %s: %w", metadataURL, err)
return meta, fmt.Errorf("decoding %s: %w", metadataURL, err)
}

return meta.Scopes, nil
return meta, nil
}

// runLogin polls for device approval and persists the resulting tokens. The
Expand Down
119 changes: 104 additions & 15 deletions pkg/server/auth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,8 +389,11 @@ func TestBeginLoginScopeSelection(t *testing.T) {
issuerURL: "https://issuer.example", clientID: "panda", resource: "https://r",
enabled: true, scopes: []string{"stale-config-scope"},
},
discoverScopes: func(context.Context) ([]string, error) {
return []string{"openid", "workflows"}, nil
discoverLoginAuth: func(context.Context) (proxy.AuthMetadataResponse, error) {
return proxy.AuthMetadataResponse{
IssuerURL: "https://issuer.example",
Scopes: []string{"openid", "workflows"},
}, nil
},
newClient: func(tg credentialTarget) authclient.Client {
gotScopes = tg.scopes
Expand Down Expand Up @@ -421,8 +424,8 @@ func TestBeginLoginScopeSelection(t *testing.T) {
issuerURL: "https://issuer.example", clientID: "panda",
enabled: true, scopes: []string{"stale-config-scope"},
},
discoverScopes: func(context.Context) ([]string, error) {
return nil, errors.New("proxy unreachable")
discoverLoginAuth: func(context.Context) (proxy.AuthMetadataResponse, error) {
return proxy.AuthMetadataResponse{}, errors.New("proxy unreachable")
},
newClient: func(credentialTarget) authclient.Client {
clientBuilt = true
Expand All @@ -442,16 +445,98 @@ func TestBeginLoginScopeSelection(t *testing.T) {
})
}

// TestFetchProxyLoginScopes verifies scope discovery reads /auth/metadata,
// returns the advertised scopes (empty is a valid answer), and errors on any
// unreachable or non-200 proxy so the caller can fail the login loudly.
func TestFetchProxyLoginScopes(t *testing.T) {
// TestBeginLoginIssuerGate verifies login refuses to start a device flow when
// the proxy advertises a different issuer than the configured one — the
// advertised scope set is only meaningful there, and the flow would die in the
// browser with the issuer's invalid_scope error. A trailing-slash difference or
// an older proxy that advertises no issuer must not trip the gate.
func TestBeginLoginIssuerGate(t *testing.T) {
t.Parallel()

newCtrl := func(t *testing.T, adv proxy.AuthMetadataResponse, clientBuilt *bool) *credentialController {
t.Helper()

return &credentialController{
log: logrus.New(),
target: credentialTarget{
issuerURL: "https://dex.example", clientID: "panda", enabled: true,
},
discoverLoginAuth: func(context.Context) (proxy.AuthMetadataResponse, error) {
return adv, nil
},
newClient: func(credentialTarget) authclient.Client {
*clientBuilt = true

return &fakeAuthClient{
beginResp: &authclient.DeviceAuth{DeviceCode: "d", UserCode: "U", ExpiresIn: 600},
pollResp: &authclient.Tokens{AccessToken: "at", ExpiresAt: time.Now().Add(time.Hour)},
}
},
newStore: func(_ credentialTarget, c authclient.Client) authstore.Store {
return authstore.New(logrus.New(), authstore.Config{
Path: filepath.Join(t.TempDir(), "c.json"), AuthClient: c,
})
},
}
}

t.Run("advertised issuer mismatch refuses the login", func(t *testing.T) {
t.Parallel()

clientBuilt := false
ctrl := newCtrl(t, proxy.AuthMetadataResponse{
IssuerURL: "https://authentik.example/application/o/panda-proxy/",
Scopes: []string{"openid", "workflows"},
}, &clientBuilt)

_, err := ctrl.BeginLogin(context.Background())
require.Error(t, err)
require.Contains(t, err.Error(), "https://authentik.example/application/o/panda-proxy/")
require.Contains(t, err.Error(), "https://dex.example")
require.Contains(t, err.Error(), "panda init")
require.False(t, clientBuilt, "login must not mint a device code at a mismatched issuer")
require.Equal(t, serverapi.AuthLoginNone, ctrl.LoginState().State)
})

t.Run("trailing slash is not a different issuer", func(t *testing.T) {
t.Parallel()

clientBuilt := false
ctrl := newCtrl(t, proxy.AuthMetadataResponse{
IssuerURL: "https://dex.example/",
Scopes: []string{"openid"},
}, &clientBuilt)

_, err := ctrl.BeginLogin(context.Background())
require.NoError(t, err)
require.True(t, clientBuilt)
})

t.Run("proxy without advertised issuer skips the gate", func(t *testing.T) {
t.Parallel()

clientBuilt := false
ctrl := newCtrl(t, proxy.AuthMetadataResponse{
Scopes: []string{"openid"},
}, &clientBuilt)

_, err := ctrl.BeginLogin(context.Background())
require.NoError(t, err)
require.True(t, clientBuilt)
})
}

// TestFetchProxyLoginAuth verifies login-auth discovery reads /auth/metadata,
// returns the advertised issuer and scopes (empty scopes are a valid answer),
// and errors on any unreachable or non-200 proxy so the caller can fail the
// login loudly.
func TestFetchProxyLoginAuth(t *testing.T) {
t.Parallel()

t.Run("empty url errors", func(t *testing.T) {
t.Parallel()

_, err := fetchProxyLoginScopes(context.Background(), "")
_, err := fetchProxyLoginAuth(context.Background(), "")
require.Error(t, err)
})

Expand All @@ -462,25 +547,28 @@ func TestFetchProxyLoginScopes(t *testing.T) {
url := srv.URL
srv.Close()

_, err := fetchProxyLoginScopes(context.Background(), url)
_, err := fetchProxyLoginAuth(context.Background(), url)
require.Error(t, err)
})

tests := []struct {
name string
handler http.HandlerFunc
wantIssuer string
wantScopes []string
wantErr bool
}{
{
name: "advertised scopes returned",
name: "advertised issuer and scopes returned",
handler: func(w http.ResponseWriter, r *http.Request) {
require.Equal(t, "/auth/metadata", r.URL.Path)
_ = json.NewEncoder(w).Encode(proxy.AuthMetadataResponse{
Enabled: true,
Scopes: []string{"openid", "email", "groups", "offline_access", "workflows"},
Enabled: true,
IssuerURL: "https://issuer.example/",
Scopes: []string{"openid", "email", "groups", "offline_access", "workflows"},
})
},
wantIssuer: "https://issuer.example/",
wantScopes: []string{"openid", "email", "groups", "offline_access", "workflows"},
},
{
Expand All @@ -506,15 +594,16 @@ func TestFetchProxyLoginScopes(t *testing.T) {
srv := httptest.NewServer(tt.handler)
defer srv.Close()

scopes, err := fetchProxyLoginScopes(context.Background(), srv.URL)
meta, err := fetchProxyLoginAuth(context.Background(), srv.URL)
if tt.wantErr {
require.Error(t, err)

return
}

require.NoError(t, err)
require.Equal(t, tt.wantScopes, scopes)
require.Equal(t, tt.wantIssuer, meta.IssuerURL)
require.Equal(t, tt.wantScopes, meta.Scopes)
})
}
}