Repository navigation
ogar-ad v7: groupType and security-enabled groups; dir-sim plan V19 - #347
Merged
Merged
Conversation
…ed closures A group is security-enabled or not and mail-enabled or not, read from groupType / securityEnabled and mailEnabled / the recipient attributes. Nested groups are expanded once in lance-graph, as two closures: delivery walks mail-enabled groups, security walks only security-enabled ones, as AD's token does. Permissions (V17, V18, RBAC roles) use the security closure; an unread security flag is excluded and fails closed. V17's trustee must be a security-enabled group; a grant to a distribution group confers nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G22yT6htkcdyXsihxxXdrg
groupType is read into the last numeric slot, bit-cast like the other signed LDAP integers. is_security_enabled reads its high bit (ADS_GROUP_TYPE_SECURITY_ENABLED); mail-enabled stays a separate question. A record of an older schema did not read it. V19 now matches the implementation: mail-enabled is a primary SMTP address, and a group created in a simulated version carries no security flag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G22yT6htkcdyXsihxxXdrg
|
Warning Review limit reachedYour organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Next included review available in 26 minutes. View limit details
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Comment |
AdaWorldAPI
marked this pull request as ready for review
October 10, 2026 16:10
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…Behalf source, link scope and expiry Review findings left open when #344 merged: - FullAccess and SendAs need a user or a security-enabled group; SendOnBehalf also accepts mail-enabled distribution groups and is reached through the delivery closure. - Denies follow Windows canonical ACE order with IsInherited kept; an explicit allow outranks an inherited deny; an unreported level is treated as explicit. - SendOnBehalf is read from the GrantSendOnBehalfTo property via Get-Mailbox / Get-DistributionGroup / Get-DynamicDistributionGroup. - SendAs on a distribution group has no impersonation target; the consumer refuses it until a group-sending path exists. - existingAccess links grant nothing; expired grants are refused; caller-scoped Graph permission reads never decide others' access. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G22yT6htkcdyXsihxxXdrg
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A group has two independent properties: whether it is security-enabled (it can hold permissions) and whether it is mail-enabled. That gives three kinds in practice: a security group, a mail-enabled security group, and a distribution group. This PR records the first property, which nothing read before, and plans how nested groups expand.
Code:
ogar-adschema v7groupTypeis read into the last free numeric slot (NUM_SLOTSis 4; slot 3), bit-cast like the other signed LDAP integers.-2147483646is stored as0x80000002.GROUP_TYPE_SECURITY_ENABLEDandis_security_enabled(group_type), which read the high bit.groupType; that is not the same as a cleared bit.group_type_is_read_and_says_security:Plan:
docs/DIRECTORY-SIMULATION-POC.mdV19 (new): security-enabled and mail-enabled are two separate properties. Nested groups are expanded once, in lance-graph, as two closures:
An unread security flag, or a group created in a simulated version, is excluded, so it fails closed.
V17: a trustee must be a user or a security-enabled group. A grant to a distribution group confers nothing. Exchange Online additionally needs a group trustee to be mail-enabled.
This also carries the V19 plan commit (
b331226), which was pushed after #344 had already merged and so never reachedmain.Order
The lance-graph side (dir-sim
ObservedNode::security,View::members_transitive) readsogar_ad::is_security_enabled. Its CI checks out OGARmain, so this PR merges first.Checks
cargo fmt --all --checkandcargo clippy --workspace --all-targets -- -D warningspass.cargo test --workspacepasses exceptogar-from-schema, whose tests need anAdaWorldAPI/OGITsibling checkout this container does not have. They don't touchogar-ad.🤖 Generated with Claude Code
https://claude.ai/code/session_01G22yT6htkcdyXsihxxXdrg
Generated by Claude Code