Repair upgraded OIDC identity contract

.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"

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

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

server/auth_server.go

@@ -485,38 +485,114 @@ 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
 	// to persist an explicit empty allowlist instead of retaining stale entries.
 	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(

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