AuthSessionBroker: Make SessionMap optional, implement AuthSessionName routing#156
AuthSessionBroker: Make SessionMap optional, implement AuthSessionName routing#156
Conversation
- Made SessionMap optional in New-IdleAuthSessionBroker/New-IdleAuthSession
- Added validation: require DefaultAuthSession when SessionMap is empty/null
- Implemented AuthSessionName-based routing in AcquireAuthSession method
- Support AuthSessionName key in SessionMap keys (e.g., @{ AuthSessionName = 'AD'; Role = 'ADAdm' })
- Prioritize AuthSessionName matches over legacy Options-only matches
- Added ambiguity detection for multiple AuthSessionName-only matches
- Updated Invoke-IdleProviderMethod to acquire default session even without AuthSessionName
- Moved AuthSessionOptions validation to occur before broker check
- Added comprehensive tests for new routing behavior
- All existing tests pass
Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
… requirement - Re-added Server, Domain, and Environment examples to SessionMap documentation - These patterns are used by providers (PSRemoting, multi-forest AD, environment routing) - Clarified that at least one of SessionMap or DefaultAuthSession must be provided - All tests passing (30 tests, 0 failures) Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 203db12f5c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR updates IdLE’s authentication session brokering to be more ergonomic and deterministic by allowing workflows to omit SessionMap in single-credential scenarios and by making AuthSessionName a first-class routing key (instead of a hidden trigger).
Changes:
- Make
SessionMapoptional inNew-IdleAuthSession/New-IdleAuthSessionBroker, requiringDefaultAuthSessionwhenSessionMapis null/empty. - Implement
AuthSessionName+AuthSessionOptionsrouting inAcquireAuthSession, with ambiguity detection and legacy Options-only support. - Update provider invocation to attempt default auth session acquisition when a broker exists even if
With.AuthSessionNameis absent, and add/extend tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Steps/Invoke-IdleStepAuthSession.Tests.ps1 | Adds a test asserting default-session acquisition behavior when AuthSessionName is absent. |
| tests/Core/New-IdleAuthSession.Tests.ps1 | Adds test coverage for optional SessionMap and AuthSessionName-based routing/ambiguity handling. |
| src/IdLE/Public/New-IdleAuthSession.ps1 | Makes SessionMap optional in the public wrapper and only forwards it when bound. |
| src/IdLE.Core/Public/New-IdleAuthSessionBroker.ps1 | Makes SessionMap optional; adds AuthSessionName routing, ambiguity detection, and expanded help/examples. |
| src/IdLE.Core/Public/Invoke-IdleProviderMethod.ps1 | Changes auth session acquisition behavior to attempt default session when broker exists. |
Comments suppressed due to low confidence (1)
src/IdLE/Public/New-IdleAuthSession.ps1:56
New-IdleAuthSessionnow allows omittingSessionMap, but the comment-based help still describesSessionMapas if it were required and doesn’t mention the “DefaultAuthSession required when SessionMap is null/empty” rule. Please update the help text so the public wrapper stays aligned with the core behavior.
[Parameter()]
[AllowNull()]
[AllowEmptyCollection()]
[hashtable] $SessionMap,
[Parameter()]
[AllowNull()]
[object] $DefaultAuthSession,
- Fix example in New-IdleAuthSessionBroker to avoid mixing token with Credential type - Move provider method support check before auth acquisition to avoid unnecessary broker calls - Replace '__default__' magic string with empty string for default session requests - Add fail-fast when AuthSessionName is set but Context lacks AcquireAuthSession method - Fix Name-only pattern matching to only match when Options is null/empty (prevents ambiguity) - Improve error handling: rethrow broker errors when provider method requires AuthSession - Add test for fail-fast behavior when AuthSessionName is set without broker - All 381 tests passing Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
Plan: Make SessionMap optional and implement AuthSessionName-based routing
Understanding Current Behavior
Implementation Tasks
Summary of Changes
Breaking Changes (per agent_instructions):
New-IdleAuthSession/New-IdleAuthSessionBrokerNew Capabilities:
All tests passing: 381 total tests, 0 failures
Original prompt
This section details on the original issue you should resolve
<issue_title>AuthSessionBroker: Make SessionMap optional and implement Name-based routing for AuthSessionName</issue_title>
<issue_description>## Problem Statement
Using authentication sessions with IdLE is currently confusing for users and error-prone in simple scenarios:
AuthSessionBrokerwhenWith.AuthSessionNameis set, effectively makingAuthSessionNamea hidden trigger rather than a routing key.AuthSessionNameis not meaningfully mapped toSessionMapentries (routing currently relies onAuthSessionOptionssuch asRole, if present, or falls back toDefaultAuthSession).SessionMapat all;DefaultAuthSessionshould be sufficient.AuthSessionNameis set) and makes first-time workflows unnecessarily hard.Goal: Make auth session usage intuitive, deterministic, and ergonomic while remaining backward compatible.
Proposed Solution
1) Make
SessionMapoptionalUpdate
New-IdleAuthSession/ broker configuration soSessionMapcan be omitted or empty.-SessionMap $nullor no-SessionMapparameter.SessionMapis empty/null andDefaultAuthSessionis not set: fail fast with a clear error.DefaultAuthSessionis set:SessionMapis optional.2) Implement meaningful
AuthSessionNamerouting (AuthSessionName-based mapping)Extend session mapping keys to support AuthSessionName-based routing and make
AuthSessionNamea real selector:Avoid generic key names like
Namein mapping keys; prefer explicitAuthSessionNameto reduce ambiguity.Allow
SessionMapkeys to includeAuthSessionNameand optional selectors (e.g.,AuthSessionRolevia existing keyRole):@{ AuthSessionName = 'AD'; Role = 'ADAdm' }# Role is the existing key; semantically this is the AuthSessionRoleMatching rules (deterministic):
AuthSessionNameandAuthSessionOptionsare provided:key.AuthSessionName == AuthSessionNameand all key/value pairs fromAuthSessionOptionsmatch.AuthSessionNameis provided:key.AuthSessionName == AuthSessionName.DefaultAuthSessionif set.Backward compatibility:
SessionMapkeys that do not includeAuthSessionNamemust continue to work.AuthSessionNameis set but mappings are AuthSessionRole-only (legacy keys using onlyRole), use the existing behavior (Options-only match → default fallback).3) Improve Step ergonomics (no “hidden trigger”)
Steps should not require
With.AuthSessionNamemerely to use credentials if a broker with a default session exists.Proposed behavior:
AuthSessionBrokeris available:With.AuthSessionNameis missing but the broker hasDefaultAuthSession, the step should still acquire the default session.With.AuthSessionNamebecomes optional and is only needed for selecting non-default sessions.4) Documentation + examples
Update documentation to clearly describe:
DefaultAuthSessionvsSessionMapAuthSessionNameandAuthSessionOptionsare used for routingAlternatives Considered
Keep current behavior and only document the “AuthSessionName as trigger” requirement.
Make
AuthSessionNamemandatory everywhere.Introduce provider-specific auth routing (per provider).
Impact
Does this affect existing workflows?
SessionMapRole-only usage continues to work.AuthSessionNamekeep working.DefaultAuthSessioneven ifAuthSessionNamewas omitted (desired ergonomic fix).Any backward compatibility concerns?
Additional Context
Example: Single credential (no SessionMap)