diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 05562c3..2cc1895 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -208,8 +208,8 @@ jobs: { printf '# PastureStack Authentication Service %s\n\n' "$RELEASE_TAG" - printf 'This release makes the common OIDC site-access policy authoritative after the encrypted authentication configuration has been created. Authentication-service startup no longer replays absent legacy OIDC keys over a saved restricted or unrestricted policy, so access mode and allowlist values survive process and Server container restarts. The one-time legacy migration path remains available before the canonical configuration exists.\n\n' - printf 'It also preserves the previous explicit-empty allowlist wire contract: a confirmed unrestricted transition durably clears stale restricted identities. Access-only changes continue to skip discovery and provider reload; access expansion remains protected by a single-use MFA confirmation bound to the operator and canonical request digest.\n\n' + printf 'This release repairs the non-secret control-platform identity contract for an already stored OpenID Connect provider. Startup, reload, and policy-only updates reconcile the OIDC user type, identity separator, provider selection, lookup capability, and external-provider flag in a fail-closed order. It performs no discovery and never reads or rewrites the client secret. Upgraded databases with incomplete common settings therefore no longer fail the final token exchange with `Identity externalIdType is invalid`.\n\n' + printf 'The existing source-versus-policy separation, explicit-empty allowlist wire contract, restart persistence, actor-bound MFA confirmation, and one-time legacy migration boundary remain intact. Reconciliation is idempotent and does not rewrite aligned policy or security settings.\n\n' printf '## Immutable coordinates\n\n' printf -- '- Source commit: `%s`\n' "$SOURCE_SHA" printf -- '- Artifact SHA-256: `%s`\n\n' "$artifact_sha" diff --git a/COMPATIBILITY.md b/COMPATIBILITY.md index 3e55691..cbc9604 100644 --- a/COMPATIBILITY.md +++ b/COMPATIBILITY.md @@ -23,6 +23,13 @@ and successful provider initialization. This includes reload requests emitted by platform setting events after the policy write: an already-live provider adopts the updated access policy in memory, while startup and source changes still initialize the provider. +An existing encrypted OIDC configuration is also authoritative for the +non-secret platform identity contract. Startup, reload, and policy-only saves +reconcile the OIDC user type, separator, provider selection, lookup capability, +and external-provider switch before login can resume. The repair performs no +discovery and never reads or rewrites the provider client secret; publishing +the external-provider switch last prevents a partially repaired configuration +from accepting an identity. Expanding access requires a one-time Engine MFA security confirmation bound to the authenticated operator, purpose `oidcAccessPolicyUpdate`, and the canonical diff --git a/README.md b/README.md index 8a0926c..0669e7a 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ PastureStack is an independent community effort to preserve, audit, and moderniz ## Project status -The current compatibility release is `v0.4.41`. It retains the existing Ubuntu 26.04, +The current compatibility release is `v0.4.42`. It retains the existing Ubuntu 26.04, Go 1.27.0, JWT, cookie, TLS, LDAP, GitHub, Shibboleth, dependency, and build maintenance. It adds a provider-neutral OpenID Connect authorization-code client with discovery, PKCE S256, nonce validation, @@ -21,7 +21,16 @@ single-use signed identity proof. The control platform uses that proof for an explicit account-link or reassignment decision; profile fields are never trusted as implicit account-matching keys. -Release `v0.4.41` separates OIDC identity-source changes from site-access +Release `v0.4.42` retains the source-versus-policy separation introduced in +`v0.4.41` and also repairs the non-secret control-platform identity contract +for an already stored OIDC provider. Startup, reload, and policy-only updates +reconcile the OIDC user type, identity separator, provider selection, lookup +capability, and external-provider flag in a fail-closed order without reading +the client secret or repeating discovery. This lets databases upgraded from an +older release recover from incomplete common settings instead of failing the +final token exchange with `Identity externalIdType is invalid`. + +The release separates OIDC identity-source changes from site-access policy changes. An already-enabled provider can change access mode and its OIDC user/group allowlist without repeating discovery, emitting a provider reload generation, or repeating the five-minute local recovery ceremony. @@ -66,9 +75,9 @@ make build make package ``` -Set `VERSION_OVERRIDE=v0.4.41` for the reviewed identity-security compatibility +Set `VERSION_OVERRIDE=v0.4.42` for the reviewed identity-security compatibility release. Packaging produces the deterministic, versioned -`authentication-service-0.4.41-linux-amd64.tar.xz` asset. The manually +`authentication-service-0.4.42-linux-amd64.tar.xz` asset. The manually dispatched release workflow runs the full test and validation suite twice, requires byte-identical packages, verifies a fixed and attested security scanner, publishes CycloneDX SBOMs and scan evidence, and publishes the diff --git a/server/auth_server.go b/server/auth_server.go index fb988bf..ea7073d 100644 --- a/server/auth_server.go +++ b/server/auth_server.go @@ -485,31 +485,52 @@ func updateSettings(saveConfig map[string]map[string]string, secretSettings []st func updateCommonSettings(settings map[string]string) error { for key, value := range settings { - if !shouldUpdateCommonSetting(key, value) { - continue - } - log.Debugf("Updating platform setting %v", key) - setting, err := PlatformClient.Setting.ById(key) - if err != nil { - log.Errorf("Error getting the setting %v , error: %v", key, err) + if err := updateCommonSetting(key, value); err != nil { return err } + } + return nil +} - // The generated Setting.Value field uses json:",omitempty". A typed - // Setting therefore drops the field when an unrestricted OIDC policy - // intentionally clears the allowlist. Use an explicit wire payload so - // an empty value remains distinguishable from "leave unchanged". - setting, err = PlatformClient.Setting.Update(setting, map[string]interface{}{ - "value": value, - }) - if err != nil { - log.Errorf("Error updating the setting %v: %v", key, err) +type commonSettingUpdate struct { + key string + value string +} + +func updateCommonSettingsInOrder(settings []commonSettingUpdate) error { + for _, setting := range settings { + if err := updateCommonSetting(setting.key, setting.value); err != nil { return err } } return nil } +func updateCommonSetting(key string, value string) error { + if !shouldUpdateCommonSetting(key, value) { + return nil + } + log.Debugf("Updating platform setting %v", key) + setting, err := PlatformClient.Setting.ById(key) + if err != nil { + log.Errorf("Error getting the setting %v , error: %v", key, err) + return err + } + + // The generated Setting.Value field uses json:",omitempty". A typed + // Setting therefore drops the field when an unrestricted OIDC policy + // intentionally clears the allowlist. Use an explicit wire payload so an + // empty value remains distinguishable from "leave unchanged". + _, err = PlatformClient.Setting.Update(setting, map[string]interface{}{ + "value": value, + }) + if err != nil { + log.Errorf("Error updating the setting %v: %v", key, err) + return err + } + return nil +} + func shouldUpdateCommonSetting(key string, value string) bool { // Preserve the historical "empty means unchanged" behavior for all common // settings except the OIDC allowlist. An unrestricted policy must be able @@ -517,6 +538,61 @@ func shouldUpdateCommonSetting(key string, value string) bool { return value != "" || key == allowedIdentitiesSetting } +func oidcCommonSettingUpdates(authConfig model.AuthConfig, oidcProvider providers.IdentityProvider) []commonSettingUpdate { + // Keep the external-provider switch last. The control platform can observe + // each setting update independently; publishing the switch only after the + // provider name, type, separator, and lookup contract prevents a partially + // configured OIDC provider from accepting a login in the middle of repair. + return []commonSettingUpdate{ + {userTypeSetting, oidcProvider.GetUserType()}, + {identitySeparatorSetting, oidcProvider.GetIdentitySeparator()}, + {noIdentityLookupSupportedSetting, strconv.FormatBool(!oidcProvider.IsIdentityLookupSupported())}, + {providerNameSetting, authConfig.Provider}, + {providerSetting, authConfig.Provider}, + {externalProviderSetting, "true"}, + } +} + +// reconcileOIDCCommonSettings repairs the non-secret control-platform +// contract for an already stored OIDC provider. Older installations can have +// a valid encrypted auth.config and still retain missing or stale common +// settings. In that state the identity provider completes successfully, but +// the control platform rejects oidc_user/oidc_group while creating its token. +// Reconciliation performs no discovery and never reads or rewrites the client +// secret. +func reconcileOIDCCommonSettings(authConfig model.AuthConfig) error { + if !strings.EqualFold(authConfig.Provider, oidcProviderName) { + return nil + } + oidcProvider, err := providers.GetProvider(authConfig.Provider) + if err != nil { + return err + } + if oidcProvider == nil { + return fmt.Errorf("Could not get the %s auth provider", authConfig.Provider) + } + + desired := oidcCommonSettingUpdates(authConfig, oidcProvider) + keys := make([]string, 0, len(desired)) + for _, setting := range desired { + keys = append(keys, setting.key) + } + current, err := readCommonSettings(keys) + if err != nil { + return err + } + for _, setting := range desired { + if current[setting.key] == setting.value { + continue + } + log.Warnf("Repairing stale OpenID Connect platform setting %s", setting.key) + if err := updateCommonSetting(setting.key, setting.value); err != nil { + return err + } + } + return nil +} + func getAllowedIDString(allowedIdentities []client.Identity, separator string) string { if len(allowedIdentities) > 0 { var idArray []string @@ -648,26 +724,29 @@ func UpdateConfigWithRequest(authConfig model.AuthConfig, updateRequest ConfigUp return err } - //add the generic settings - commonSettings := make(map[string]string) - commonSettings[accessModeSetting] = authConfig.AccessMode - commonSettings[userTypeSetting] = newProvider.GetUserType() - commonSettings[identitySeparatorSetting] = newProvider.GetIdentitySeparator() - commonSettings[allowedIdentitiesSetting] = getAllowedIDString(authConfig.AllowedIdentities, newProvider.GetIdentitySeparator()) - commonSettings[providerNameSetting] = authConfig.Provider - commonSettings[providerSetting] = authConfig.Provider - commonSettings[externalProviderSetting] = "true" - commonSettings[noIdentityLookupSupportedSetting] = strconv.FormatBool(!newProvider.IsIdentityLookupSupported()) - err = updateCommonSettings(commonSettings) + // Publish the provider contract in a deterministic order, then its access + // policy. The external-provider switch is deliberately the last contract + // setting so the control platform never sees a half-configured provider. + commonSettings := []commonSettingUpdate{ + {userTypeSetting, newProvider.GetUserType()}, + {identitySeparatorSetting, newProvider.GetIdentitySeparator()}, + {noIdentityLookupSupportedSetting, strconv.FormatBool(!newProvider.IsIdentityLookupSupported())}, + {providerNameSetting, authConfig.Provider}, + {providerSetting, authConfig.Provider}, + {externalProviderSetting, "true"}, + {allowedIdentitiesSetting, getAllowedIDString(authConfig.AllowedIdentities, newProvider.GetIdentitySeparator())}, + {accessModeSetting, authConfig.AccessMode}, + } + err = updateCommonSettingsInOrder(commonSettings) if err != nil { return errors.Wrap(err, "UpdateConfig: Error Storing the common settings") } //set the security setting last specifically - commonSettings = make(map[string]string) - commonSettings[securitySetting] = strconv.FormatBool(authConfig.Enabled) - commonSettings[authServiceConfigUpdateTimestamp] = time.Now().String() - err = updateCommonSettings(commonSettings) + err = updateCommonSettingsInOrder([]commonSettingUpdate{ + {securitySetting, strconv.FormatBool(authConfig.Enabled)}, + {authServiceConfigUpdateTimestamp, time.Now().String()}, + }) if err != nil { return errors.Wrap(err, "UpdateConfig: Error Storing the provider securitySetting") } @@ -726,19 +805,17 @@ func updateOIDCConfigWithoutInitialization(currentConfig model.AuthConfig, authC return errors.Wrap(err, "UpdateConfig: Error storing OpenID Connect display settings") } } + if err := reconcileOIDCCommonSettings(authConfig); err != nil { + return errors.Wrap(err, "UpdateConfig: Error repairing the OpenID Connect platform contract") + } - orderedSettings := []struct { - key string - value string - }{ + orderedSettings := []commonSettingUpdate{ {allowedIdentitiesSetting, getAllowedIDString(authConfig.AllowedIdentities, newProvider.GetIdentitySeparator())}, {accessModeSetting, authConfig.AccessMode}, {securitySetting, strconv.FormatBool(authConfig.Enabled)}, } - for _, setting := range orderedSettings { - if err := updateCommonSettings(map[string]string{setting.key: setting.value}); err != nil { - return errors.Wrap(err, "UpdateConfig: Error storing OpenID Connect access policy") - } + if err := updateCommonSettingsInOrder(orderedSettings); err != nil { + return errors.Wrap(err, "UpdateConfig: Error storing OpenID Connect access policy") } updateOIDCConfigInMemory(authConfig) return nil @@ -1075,6 +1152,12 @@ func Reload(fromUpdate bool) (bool, error) { <-*refreshReqChannel return false, nil } + if strings.EqualFold(authConfig.Provider, oidcProviderName) { + if err := reconcileOIDCCommonSettings(authConfig); err != nil { + <-*refreshReqChannel + return false, errors.Wrap(err, "Reload: Could not repair the OpenID Connect platform contract") + } + } if strings.EqualFold(authConfig.Provider, oidcProviderName) && canApplyOIDCReloadWithoutInitialization( diff --git a/server/config_update_policy_test.go b/server/config_update_policy_test.go index 4ce2d2a..8fef262 100644 --- a/server/config_update_policy_test.go +++ b/server/config_update_policy_test.go @@ -352,6 +352,12 @@ func TestPolicyOnlyUpdateClearsStoredAllowlistWithoutDiscovery(t *testing.T) { accessModeSetting: "restricted", securitySetting: "true", authServiceConfigUpdateTimestamp: "unchanged-provider-reload-generation", + userTypeSetting: "legacy_user", + identitySeparatorSetting: "#legacy#", + noIdentityLookupSupportedSetting: "false", + providerNameSetting: "legacyconfig", + providerSetting: "legacyconfig", + externalProviderSetting: "false", } var writes []string var platformServer *httptest.Server @@ -429,8 +435,36 @@ func TestPolicyOnlyUpdateClearsStoredAllowlistWithoutDiscovery(t *testing.T) { if settings[allowedIdentitiesSetting] != "" { t.Fatalf("stored allowlist was not cleared: %q", settings[allowedIdentitiesSetting]) } - if len(writes) < 2 || writes[0] != allowedIdentitiesSetting || writes[1] != accessModeSetting { - t.Fatalf("allowlist was not cleared before the access mode changed: %#v", writes) + expectedPrefix := []string{ + userTypeSetting, + identitySeparatorSetting, + noIdentityLookupSupportedSetting, + providerNameSetting, + providerSetting, + externalProviderSetting, + allowedIdentitiesSetting, + accessModeSetting, + securitySetting, + } + if len(writes) != len(expectedPrefix) { + t.Fatalf("unexpected OIDC repair/policy writes: %#v", writes) + } + for index, expected := range expectedPrefix { + if writes[index] != expected { + t.Fatalf("OIDC settings were not repaired in fail-closed order: got %#v, expected %#v", writes, expectedPrefix) + } + } + for key, expected := range map[string]string{ + userTypeSetting: "oidc_user", + identitySeparatorSetting: "#oidc#", + noIdentityLookupSupportedSetting: "true", + providerNameSetting: "oidcconfig", + providerSetting: "oidcconfig", + externalProviderSetting: "true", + } { + if settings[key] != expected { + t.Fatalf("OIDC platform contract setting %s = %q, expected %q", key, settings[key], expected) + } } for _, setting := range writes { if setting == authServiceConfigUpdateTimestamp { @@ -450,6 +484,67 @@ func TestPolicyOnlyUpdateClearsStoredAllowlistWithoutDiscovery(t *testing.T) { } } +func TestOIDCCommonSettingReconciliationIsIdempotentAndDoesNotTouchPolicy(t *testing.T) { + settings := map[string]string{ + userTypeSetting: "oidc_user", + identitySeparatorSetting: "#oidc#", + noIdentityLookupSupportedSetting: "true", + providerNameSetting: "oidcconfig", + providerSetting: "oidcconfig", + externalProviderSetting: "true", + allowedIdentitiesSetting: "oidc_group:operators", + accessModeSetting: "restricted", + securitySetting: "true", + } + var writes []string + var platformServer *httptest.Server + platformServer = httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { + response.Header().Set("Content-Type", "application/json") + if request.Method == http.MethodGet && request.URL.Path == "/v2-beta" { + response.Header().Set("X-API-Schemas", platformServer.URL+"/v2-beta") + _, _ = fmt.Fprintf(response, `{"data":[{"id":"setting","type":"schema","pluralName":"settings","collectionMethods":["GET"],"resourceMethods":["GET","PUT"],"links":{"collection":%q}}]}`, + platformServer.URL+"/v2-beta/settings") + return + } + const prefix = "/v2-beta/settings/" + if !strings.HasPrefix(request.URL.Path, prefix) { + http.Error(response, "unexpected platform path", http.StatusNotFound) + return + } + name := strings.TrimPrefix(request.URL.Path, prefix) + switch request.Method { + case http.MethodGet: + _, _ = fmt.Fprintf(response, `{"id":%q,"type":"setting","activeValue":%q,"value":%q,"links":{"self":%q}}`, + name, settings[name], settings[name], platformServer.URL+request.URL.Path) + case http.MethodPut: + writes = append(writes, name) + http.Error(response, "an aligned setting must not be rewritten", http.StatusInternalServerError) + default: + http.Error(response, "unexpected platform method", http.StatusMethodNotAllowed) + } + })) + defer platformServer.Close() + + platformClient, err := newPlatformClient(platformServer.URL, "access", "secret") + if err != nil { + t.Fatal(err) + } + previousPlatformClient := PlatformClient + PlatformClient = platformClient + defer func() { PlatformClient = previousPlatformClient }() + + if err := reconcileOIDCCommonSettings(oidcConfigForPolicyTest(true, "restricted")); err != nil { + t.Fatal(err) + } + if len(writes) != 0 { + t.Fatalf("idempotent reconciliation rewrote aligned settings: %#v", writes) + } + if settings[allowedIdentitiesSetting] != "oidc_group:operators" || + settings[accessModeSetting] != "restricted" || settings[securitySetting] != "true" { + t.Fatalf("reconciliation touched access policy: %#v", settings) + } +} + func TestUpgradeSettingsDoesNotReplayLegacyMigrationAfterCanonicalConfigExists(t *testing.T) { genericObjectReads := 0 settingRequests := 0