Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*Bug* Incorrect Typecasting of RequestDurationInDays in Eidsca CR04 Test Leading to Wrong Comparison Results #653

Open
HCRitter opened this issue Feb 5, 2025 · 0 comments
Assignees

Comments

@HCRitter
Copy link

HCRitter commented Feb 5, 2025

Description:
When testing the Eidsca CR04 in a tenant, the test returns a value of 5 days for RequestDurationInDays, which is clearly below the 30-day threshold. However, the test incorrectly reports that the duration is not 30 days or less. The issue is caused by an unnecessary and incorrect typecasting to a string, leading to a faulty comparison.

Failing code:

$result = Invoke-MtGraphRequest -RelativeUri "policies/adminConsentRequestPolicy" -ApiVersion beta

[string]$tenantValue = $result.requestDurationInDays
$testResult = $tenantValue -le '30'
$tenantValueNotSet = $null -eq $tenantValue -and '30' -notlike '*$null*'

Steps to Reproduce:

  1. Execute the following code snippet:
  [string]$Tenantvalue = 5
  $TestResult = $TenantValue -le '30'
  $tenantValueNotSet = $null -eq $tenantValue -and '30' -notlike '*$null*'
  1. Observe that the comparison is not behaving as expected.
  2. Check the property type using:
(Get-MgPolicyAdminConsentRequestPolicy).RequestDurationInDays.gettype()

The property is already an integer, which means that the cast to a string is unnecessary.


Expected Behavior

  • The test should correctly compare the integer value without converting it to a string.
  • A tenant value of 5 should be recognized as less than or equal to 30.

Actual Behavior

  • The string conversion leads to an incorrect comparison, resulting in the test always failing the condition.

Suggested Fix

  • Remove the explicit string cast for RequestDurationInDays to ensure the comparison uses the native integer type.

Source

Test-MtEidscaCR04

HCRitter added a commit to HCRitter/maester that referenced this issue Feb 5, 2025
fix: remove unnecessary string casting and improve tenant value checks in admin consent policy test

Previously, the test for "policies/adminConsentRequestPolicy" incorrectly cast the RequestDurationInDays value to a string, resulting in faulty comparisons. This change removes the string cast, allowing for proper numeric comparison, and improves the handling of unset tenant values using [string]::IsNullOrEmpty() and a check for a zero value.

- Removed casting of RequestDurationInDays to string.
- Compared tenantValue directly against the integer 30.
- Updated tenantValueNotSet logic to reliably detect unset or default values.
- Ensured the testResultMarkdown messages remain consistent with the new logic.
@Cloud-Architekt Cloud-Architekt self-assigned this Feb 17, 2025
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

No branches or pull requests

2 participants