Skip to content

Commit d7a88df

Browse files
os-zhuangclaude
andauthored
fix(auth): SSO env-flag parsing + register-form empty state (ADR-0024 / cloud#551) (#2400)
- plugin-auth: OS_OIDC_PROVIDER_ENABLED / OS_SSO_ENABLED / OS_SCIM_ENABLED now use the shared readBooleanEnv parser so true/1/yes/on all work (was literal 'true' only — OS_SSO_ENABLED=1 silently disabled the RP). + unit tests. - platform-objects: sys_sso_provider list view gets a per-object empty state pointing at "Register SSO Provider" (the shared identity-object copy wrongly said records can't be added here). Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent dc619af commit d7a88df

4 files changed

Lines changed: 69 additions & 8 deletions

File tree

.changeset/adr-0024-sso-quality.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@objectstack/plugin-auth': patch
3+
'@objectstack/platform-objects': patch
4+
---
5+
6+
Auth: SSO quality polish (ADR-0024 / cloud#551)
7+
8+
- **plugin-auth**: `OS_OIDC_PROVIDER_ENABLED` / `OS_SSO_ENABLED` / `OS_SCIM_ENABLED` now parse with the shared `readBooleanEnv` helper (same as `OS_AUTH_TWO_FACTOR` etc.), so the platform-standard truthy set works (`true`/`1`/`yes`/`on`, case-insensitive) instead of only the literal `'true'` — a repeated operator footgun where `OS_SSO_ENABLED=1` silently parsed as disabled. Added unit tests.
9+
- **platform-objects**: `sys_sso_provider`'s list view gets a per-object empty state ("No SSO providers yet" + a pointer to "Register SSO Provider"), replacing the shared identity-object copy ("records are created automatically … cannot be added here") which is wrong for this object — it HAS a register action.

packages/platform-objects/src/identity/sys-sso-provider.object.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,14 @@ export const SysSsoProvider = ObjectSchema.create({
120120
columns: ['provider_id', 'issuer', 'domain', 'created_at'],
121121
sort: [{ field: 'provider_id', order: 'asc' }],
122122
pagination: { pageSize: 50 },
123+
// Per-object empty state — the shared identity-object copy ("created
124+
// automatically … cannot be added here") is wrong for this object, which
125+
// HAS a "Register SSO Provider" action. Point admins at it instead.
126+
emptyState: {
127+
title: 'No SSO providers yet',
128+
message: 'Register your organization’s external OIDC IdP (Okta, Entra, Auth0, …) with “Register SSO Provider”. Members whose email domain matches can then sign in through it.',
129+
icon: 'log-in',
130+
},
123131
},
124132
},
125133

packages/plugins/plugin-auth/src/auth-manager.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1149,6 +1149,47 @@ describe('AuthManager', () => {
11491149
}
11501150
});
11511151

1152+
// ADR-0024 / cloud#551 — OS_SSO_ENABLED uses the shared `readBooleanEnv`
1153+
// parser, so the platform-standard truthy/falsy set works (not only the
1154+
// literal `'true'`). Operators kept setting `OS_SSO_ENABLED=1` and getting
1155+
// a silently-disabled RP.
1156+
it.each(['1', 'true', 'TRUE', 'yes', 'on'])(
1157+
'should treat OS_SSO_ENABLED=%s as enabled',
1158+
(val) => {
1159+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
1160+
const prev = process.env.OS_SSO_ENABLED;
1161+
process.env.OS_SSO_ENABLED = val;
1162+
try {
1163+
const manager = new AuthManager({ secret: 'test-secret-at-least-32-chars-long' });
1164+
expect(manager.getPublicConfig().features.sso).toBe(true);
1165+
} finally {
1166+
if (prev === undefined) delete process.env.OS_SSO_ENABLED;
1167+
else process.env.OS_SSO_ENABLED = prev;
1168+
warnSpy.mockRestore();
1169+
}
1170+
},
1171+
);
1172+
1173+
it.each(['0', 'false', 'off', 'no'])(
1174+
'should treat OS_SSO_ENABLED=%s as disabled even when plugins.sso=true',
1175+
(val) => {
1176+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
1177+
const prev = process.env.OS_SSO_ENABLED;
1178+
process.env.OS_SSO_ENABLED = val;
1179+
try {
1180+
const manager = new AuthManager({
1181+
secret: 'test-secret-at-least-32-chars-long',
1182+
plugins: { sso: true } as any,
1183+
});
1184+
expect(manager.getPublicConfig().features.sso).toBe(false);
1185+
} finally {
1186+
if (prev === undefined) delete process.env.OS_SSO_ENABLED;
1187+
else process.env.OS_SSO_ENABLED = prev;
1188+
warnSpy.mockRestore();
1189+
}
1190+
},
1191+
);
1192+
11521193
it('should filter out disabled providers', () => {
11531194
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
11541195
const manager = new AuthManager({

packages/plugins/plugin-auth/src/auth-manager.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1057,12 +1057,14 @@ export class AuthManager {
10571057
// `OS_MULTI_ORG_ENABLED` / `OS_DISABLE_SIGNUP` pattern). When set, the
10581058
// env var WINS over the config-file setting so platform operators can
10591059
// override per-environment without touching the application bundle.
1060-
const oidcEnv = (globalThis as any)?.process?.env?.OS_OIDC_PROVIDER_ENABLED;
1061-
const oidcFromEnv = oidcEnv != null ? String(oidcEnv).toLowerCase() === 'true' : undefined;
1062-
const ssoEnv = (globalThis as any)?.process?.env?.OS_SSO_ENABLED;
1063-
const ssoFromEnv = ssoEnv != null ? String(ssoEnv).toLowerCase() === 'true' : undefined;
1064-
const scimEnv = (globalThis as any)?.process?.env?.OS_SCIM_ENABLED;
1065-
const scimFromEnv = scimEnv != null ? String(scimEnv).toLowerCase() === 'true' : undefined;
1060+
// Use the shared `readBooleanEnv` parser (same as OS_AUTH_TWO_FACTOR /
1061+
// OS_AUTH_PASSWORD_REJECT_BREACHED / OS_DISABLE_SIGNUP) so these accept the
1062+
// platform-standard truthy set (`true`/`1`/`yes`/`on`, case-insensitive)
1063+
// instead of only the literal string `'true'` — a repeated operator footgun
1064+
// (`OS_SSO_ENABLED=1` silently parsed as disabled).
1065+
const oidcFromEnv = readBooleanEnv('OS_OIDC_PROVIDER_ENABLED');
1066+
const ssoFromEnv = readBooleanEnv('OS_SSO_ENABLED');
1067+
const scimFromEnv = readBooleanEnv('OS_SCIM_ENABLED');
10661068
// @better-auth/scim's `active:false` → ban runs through the admin plugin,
10671069
// and org-scoped tokens need the organization plugin — so enabling SCIM
10681070
// forces `admin` on (organization already defaults on). See ADR-0071.
@@ -2008,8 +2010,9 @@ export class AuthManager {
20082010
* `planAllowsSso` config, since that arrives via `plugins.sso`).
20092011
*/
20102012
public isSsoWired(): boolean {
2011-
const ssoEnv = (globalThis as any)?.process?.env?.OS_SSO_ENABLED;
2012-
const ssoFromEnv = ssoEnv != null ? String(ssoEnv).toLowerCase() === 'true' : undefined;
2013+
// Same parser as `buildPluginList` (`readBooleanEnv`) so the advertised
2014+
// capability can never disagree with the actually-mounted route.
2015+
const ssoFromEnv = readBooleanEnv('OS_SSO_ENABLED');
20132016
return ssoFromEnv ?? (this.config.plugins as any)?.sso ?? false;
20142017
}
20152018

0 commit comments

Comments
 (0)