diff --git a/.changeset/24844-google-env-credentials-save.md b/.changeset/24844-google-env-credentials-save.md new file mode 100644 index 00000000000..081438593c1 --- /dev/null +++ b/.changeset/24844-google-env-credentials-save.md @@ -0,0 +1,13 @@ +--- +'@objectstack/service-settings': patch +--- + +fix(service-settings): Setup → Authentication saves "Enable Google login" ON when the Google credentials come from the server (#24844) + +Clause-②: no + +On a deployment that sets `GOOGLE_CLIENT_ID` and `GOOGLE_CLIENT_SECRET` and leaves the two credential fields empty, saving `auth.google_enabled: true` was refused with `SETTINGS_VALIDATION` (`google_client_id` / `google_client_secret`: `required`). An admin who switched Google off on the page could not switch it back on there, although the server held valid credentials. The settings service reads only `OS_*` names, so it cannot see that pair. + +`auth.google_client_id` and `auth.google_client_secret` are no longer `required`. Google sign-in needs the two credentials to resolve, saved on the page or set on the server; it never needed them stored. The `google_enabled` help already says so: Google is offered once both resolve, and the server's pair wins over saved values. + +Who may sign in, and which providers the login page offers, do not change for any deployment. Saving the toggle ON while neither pair resolves is now accepted; the login page then shows no Google button, as it does with the toggle left at its default. diff --git a/packages/services/service-settings/src/manifests/auth.manifest.test.ts b/packages/services/service-settings/src/manifests/auth.manifest.test.ts index beb65c1a780..099ef83aaa7 100644 --- a/packages/services/service-settings/src/manifests/auth.manifest.test.ts +++ b/packages/services/service-settings/src/manifests/auth.manifest.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from 'vitest'; import { SettingsManifestSchema } from '@objectstack/spec/system'; +import type { CryptoAdapter } from '../crypto-adapter.js'; import { SettingsService } from '../settings-service.js'; import { en } from '../translations/index.js'; import { authSettingsManifest } from './auth.manifest.js'; @@ -281,3 +282,82 @@ describe('authSettingsManifest', () => { } }); }); + +/** + * A confidential stand-in, so the stored-credential control below can hold the + * `password`-typed secret: the write path refuses a secret-valued key on an + * adapter that declares no confidentiality. + */ +class ConfidentialTestAdapter implements CryptoAdapter { + readonly confidential = true; + async encrypt(plaintext: string): Promise { + return 'kms:' + Buffer.from(plaintext, 'utf8').toString('base64'); + } + async decrypt(ciphertext: string): Promise { + return Buffer.from(ciphertext.replace(/^kms:/, ''), 'base64').toString('utf8'); + } + digest(): string { + return 'fnv32:deadbeef'; + } +} + +// plugin-auth offers Google once the toggle is on and a client ID and secret +// both resolve, saved here or from GOOGLE_CLIENT_ID / GOOGLE_CLIENT_SECRET, +// with the env pair winning. The settings service reads only OS_* names, so +// it cannot see that pair; a `required` on the two keys made it refuse +// `google_enabled: true` on every deployment whose credentials come from it. +describe('authSettingsManifest — Google credentials may come from the server (not required)', () => { + // FAKE values. This is the env map such a deployment hands the service. + const ENV_CREDENTIALS = { + GOOGLE_CLIENT_ID: 'fake-env-client-id.apps.googleusercontent.test', + GOOGLE_CLIENT_SECRET: 'fake-env-client-secret', + }; + const serviceWith = (env: Record) => { + const svc = new SettingsService({ env, crypto: new ConfidentialTestAdapter() }); + svc.registerManifest(authSettingsManifest as any); + return svc; + }; + + it('declares neither credential key required', () => { + const specs = authSettingsManifest.specifiers as any[]; + for (const key of ['google_client_id', 'google_client_secret']) { + expect(specs.find((s) => s.key === key).required, key).toBe(false); + } + }); + + it('saves Google ON with both credential fields empty, from unset and after an OFF', async () => { + const svc = serviceWith({ ...ENV_CREDENTIALS }); + + await expect(svc.setMany('auth', { google_enabled: true })).resolves.toBeDefined(); + const { values } = await svc.getNamespace('auth'); + expect(values.google_enabled).toMatchObject({ value: true, source: 'global' }); + // Nothing is stored for the pair, so plugin-auth resolves it from the env. + expect(values.google_client_id?.source).toBe('default'); + expect(values.google_client_secret?.source).toBe('default'); + + // The admin's round trip: OFF, then back ON. + await expect(svc.setMany('auth', { google_enabled: false })).resolves.toBeDefined(); + await expect(svc.setMany('auth', { google_enabled: true })).resolves.toBeDefined(); + expect(await svc.get('auth', 'google_enabled')).toMatchObject({ value: true, source: 'global' }); + }); + + it('control: a stored credential pair still saves and reads back', async () => { + const svc = serviceWith({}); + + await expect( + svc.setMany('auth', { + google_enabled: true, + google_client_id: 'fake-stored-client-id', + google_client_secret: 'fake-stored-client-secret', + }), + ).resolves.toBeDefined(); + expect(await svc.get('auth', 'google_client_id')).toMatchObject({ + value: 'fake-stored-client-id', + source: 'global', + }); + expect(await svc.get('auth', 'google_client_secret')).toMatchObject({ + value: 'fake-stored-client-secret', + source: 'global', + }); + }); +}); diff --git a/packages/services/service-settings/src/manifests/auth.manifest.ts b/packages/services/service-settings/src/manifests/auth.manifest.ts index 1fc310c429e..146f1574445 100644 --- a/packages/services/service-settings/src/manifests/auth.manifest.ts +++ b/packages/services/service-settings/src/manifests/auth.manifest.ts @@ -442,6 +442,15 @@ const manifest = { // is the runtime's own default for an undeclared toggle, and a deployment // signing in with env credentials relies on it; the help text carries the // conjunction, so ON beside two empty fields reads as "not offered yet". + // + // The two credential keys below are therefore NOT `required`. `required` + // is judged by `setMany` against the stored value, and the runtime needs + // the pair to RESOLVE, not to be stored: on a deployment that sets + // GOOGLE_CLIENT_ID / GOOGLE_CLIENT_SECRET, a `required` here refused + // `google_enabled: true` (the save touches both through `visible`), so an + // admin who switched Google off could not switch it back on, although + // the server held valid credentials. Saving ON with nothing resolving is + // the same runtime state as the default: no Google button. { type: 'toggle', key: 'google_enabled', @@ -455,7 +464,7 @@ const manifest = { type: 'text', key: 'google_client_id', label: 'Google client ID', - required: true, + required: false, description: 'OAuth client ID from Google Cloud Console. GOOGLE_CLIENT_ID can also be set on the server.', visible: "${data.google_enabled !== false}", @@ -464,7 +473,7 @@ const manifest = { type: 'password', key: 'google_client_secret', label: 'Google client secret', - required: true, + required: false, encrypted: true, description: 'Stored encrypted at rest. GOOGLE_CLIENT_SECRET can also be set on the server.',