Skip to content

fix: decouple security group check from domain restriction#1382

Open
damsleth wants to merge 1 commit into
devfrom
fix/security-group-domain-check
Open

fix: decouple security group check from domain restriction#1382
damsleth wants to merge 1 commit into
devfrom
fix/security-group-domain-check

Conversation

@damsleth
Copy link
Copy Markdown
Member

Summary

  • checkSecurityGroupMembership short-circuited before hitting Graph when settings.domainRestriction was unset, because mail.includes('@undefined') always returned false. This blocked every legitimate member when a tenant enabled securityGroupEnabled=true without also enabling domainRestrictionEnabled.
  • The guard now gates security-group membership on securityGroupEnabled + securityGroupId only, and applies domain restriction only when domainRestrictionEnabled is true and domainRestriction has a value - matching the independent admin UI toggles.
  • Adds an AVA test file covering the five branch decisions (group disabled, group id empty, group enabled with domain restriction disabled, group enabled with domain restriction enabled inside/outside the domain). Graph is stubbed via require.cache so the short-circuit branches never touch the network.

Release note

Tenants that relied on the de facto coupling (security group enabled + empty domain restriction acting as a deny-all) will now see Graph membership checks succeed. Configure both securityGroupEnabled and domainRestrictionEnabled explicitly to preserve the old semantics.

Test plan

  • npm run lint
  • npm test (the 5 pre-existing uncaught exceptions on dev are unrelated; new suite passes)
  • Manual: tenant with securityGroupEnabled=true, domainRestrictionEnabled=false can sign in
  • Manual: tenant with both flags true still blocks mail outside the configured domain

The security-group guard short-circuited when `domainRestriction` was
unset, turning `mail.includes('@undefined')` into a deny-all for any
tenant that enabled `securityGroupEnabled` without enabling
`domainRestrictionEnabled`. Gate security-group membership on its own
flag, and apply domain restriction only when both
`domainRestrictionEnabled` and `domainRestriction` are set, matching
the independent toggles in the admin UI.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant