MemberJunction / MemberJunction/MJ
Explorer: validateEnvironment() blocks catalog-only auth deployments — compiled environment.ts is treated as the only source
- Dominant language
- TSQL
- Stars
- 29
- Forks
- 6
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 323
Description
Follow-up to #2985 (metadata-driven pluggable authentication providers), agreed as a fast-follow rather than folded into that PR. Original analysis by @MattC-BC in https://github.com/MemberJunction/MJ/pull/2985#issuecomment-5297150046 — recorded here so it isn't lost when the PR merges.
## The gap
`validateEnvironment()` in `packages/Angular/Explorer/explorer-app/src/lib/explorer-app.component.ts` (line ~992) still treats the compiled `environment.ts` as the **only** source of authentication configuration. It hard-requires `AUTH_TYPE`, and when `AUTH_TYPE === 'msal'` it additionally requires compiled `CLIENT_ID` and `TENANT_ID` via `validateMsalFields()`.
It runs early in `ngOnInit` and short-circuits the whole app on failure:
```ts
const envValid = this.validateEnvironment();
if (!envValid) {
this.HasError = true;
this.ErrorMessage = 'Environment configuration is incomplete. See errors above.';
return; // setupAuth() never runs
}
```
So a deployment that configures authentication **purely from the new provider catalog** — blank compiled auth values, everything coming from `MJ: Authentication Providers` — is stopped at the "Environment configuration is incomplete" screen before catalog resolution ever gets a chance to matter.
Two details worth noting:
- **`TENANT_ID` has no runtime consumer.** It is read only by the validator itself (`validateMsalFields`) and declared in `bootstrap.types.ts`. The browser driver reads `CLIENT_AUTHORITY`, and #2985 added `MJMSALProvider.EnvironmentFromCatalog` which derives that authority from the row's `Issuer`. So the validator can block a deployment on a variable nothing actually uses.
- **The validator is checking the wrong layer.** It validates the compiled environment, but the value the drivers ultimately consume is the *merged* result of the compiled environment plus the catalog overlay (`mergeCatalogEnvironment`), which the auth module computes privately inside `AuthServicesModule.forRoot()`.
## Why this was scoped out of #2985
Every existing deployment has a populated `environment.ts`, so behavior is unchanged for all of them — catalog values still win where provided, and that is covered by tests. The blocked scenario is the catalog-only end state that nothing runs yet. This was a deliberate scoping decision, not an oversight.
## Proposed direction
**Preferred — merge the catalog into the environment once, at the composition root.** `packages/MJExplorer/src/main.ts` already awaits `AuthProviderCatalog.Preload()` before the app module is imported, so a single merged environment object could feed the validator, bootstrap, and drivers alike, instead of `AuthServicesModule.forRoot()` keeping a private merged copy. That removes the class of bug rather than one instance of it, and gives the validator something meaningful to validate.
**Lighter alternative — relax the validator when a catalog resolution is present.** `MJExplorerAppComponent` already injects `MJ_AUTH_PROVIDER_RESOLUTION` (`@Optional()`), so the component can see whether a provider resolved from the catalog and skip the compiled-value requirements accordingly. Cheaper, but leaves two sources of truth in place.
Either way, `TENANT_ID` should stop being a hard requirement — nothing reads it.
This touches the boot sequence, so it deserves its own review rather than riding along on #2985.
## Acceptance criteria
- [ ] An Explorer deployment with blank compiled auth values and an Active, client-visible catalog row reaches the login screen and can sign in.
- [ ] A deployment with a populated `environment.ts` and no catalog behaves exactly as it does today, including the existing validation error messages when values really are missing.
- [ ] A deployment with **neither** a catalog nor compiled auth config still fails loudly and legibly — this must not become a silent path to a broken login screen.
- [ ] `TENANT_ID` is no longer required by the validator (or the requirement is justified by an actual runtime consumer).
- [ ] Unit coverage for all three states above.
## References
- PR #2985 — metadata-driven pluggable authentication providers
- `packages/Angular/Explorer/explorer-app/src/lib/explorer-app.component.ts` — `validateEnvironment()`, `validateMsalFields()`
- `packages/Angular/Explorer/auth-services/src/lib/catalog-environment.ts` — `mergeCatalogEnvironment()`
- `packages/MJExplorer/src/main.ts` — the preload/composition root
Contributor guide
Research direction
Start with validateEnvironment() and validateMsalFields() in packages/Angular/Explorer/explorer-app/src/lib/explorer-app.component.ts, then trace AuthProviderCatalog.Preload() in packages/MJExplorer/src/main.ts and mergeCatalogEnvironment() in packages/Angular/Explorer/auth-services/src/lib/catalog-environment.ts. Verify the three acceptance states with unit coverage: catalog-only authentication reaches login, existing compiled configuration is unchanged, and neither source still fails clearly without requiring TENANT_ID.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- angular, typescript
- Domain
- authentication, frontend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100