Skip to content

[Copilot] Configuration and Constants Issues #824

Closed
@mawasile

Description

@mawasile
Contributor

Configuration and Constants Issues

This document consolidates issues related to configuration handling, constants definition, and type safety in configuration management that require fixes to ensure proper provider behavior and maintainability.

ISSUE 1

File: /workspaces/terraform-provider-power-platform/internal/api/client.go

Problem: Magic strings used for CAE challenge detection

The function IsCaeChallengeResponse uses hard-coded substrings "claims=" and "insufficient_claims" to determine if a WWW-Authenticate header is a CAE (Continuous Access Evaluation) challenge. Re-using these string literals directly in code can lead to duplication and errors if the string needs to be updated or checked elsewhere.

Impact: Reduced maintainability and risk of bugs if string requirements change. Severity: low

Location: In function IsCaeChallengeResponse

Code Issue:

if resp.StatusCode == http.StatusUnauthorized {
        wwwAuthenticate := resp.Header.Get("WWW-Authenticate")
        if wwwAuthenticate != "" {
                return strings.Contains(wwwAuthenticate, "claims=") &&
                        strings.Contains(wwwAuthenticate, "insufficient_claims")
        }
}

Fix: Move the string literals to appropriately named const values at the top of the file, e.g.:

const wwwAuthenticateClaimsKey = "claims="
const wwwAuthenticateInsufficientClaims = "insufficient_claims"
if resp.StatusCode == http.StatusUnauthorized {
        wwwAuthenticate := resp.Header.Get("WWW-Authenticate")
        if wwwAuthenticate != "" {
                return strings.Contains(wwwAuthenticate, wwwAuthenticateClaimsKey) &&
                        strings.Contains(wwwAuthenticate, wwwAuthenticateInsufficientClaims)
        }
}

ISSUE 2

File: /workspaces/terraform-provider-power-platform/internal/constants/constants.go

Problem: Suspicious/Incorrect Domain Constants for RX Cloud

The block defining constants for RX_* (presumably related to a national/regional cloud) contains what appear to be copy-paste errors regarding the advisor API domain and scope. The following entries:

RX_POWERAPPS_ADVISOR_API_DOMAIN = "api.advisor.powerapps.eaglex.ic.gov"
RX_POWERAPPS_ADVISOR_API_SCOPE  = "https://advisor.powerapps.eaglex.ic.gov/.default"

use "eaglex.ic.gov"—the domain used for the EX ("Eagle X") region—rather than an "microsoft.scloud"-based value that would be correct for RX.

Impact: Medium severity. Using an incorrect or mismatched domain for Power Apps Advisor API for RX-backed tenants results in misconfiguration, which can prevent API calls from working as intended and could cause outages or security holes if the wrong endpoints or tokens are used.

Location: In the block:

const (
        ...
        RX_POWERAPPS_ADVISOR_API_DOMAIN = "api.advisor.powerapps.eaglex.ic.gov"
        RX_POWERAPPS_ADVISOR_API_SCOPE  = "https://advisor.powerapps.eaglex.ic.gov/.default"
        ...
)

Code Issue:

const (
        ...
        RX_POWERAPPS_ADVISOR_API_DOMAIN = "api.advisor.powerapps.eaglex.ic.gov"
        RX_POWERAPPS_ADVISOR_API_SCOPE  = "https://advisor.powerapps.eaglex.ic.gov/.default"
        ...
)

Fix: Change these values to match microsoft.scloud instead of using the EX region domain. Correct the constants as follows:

const (
        ...
        RX_POWERAPPS_ADVISOR_API_DOMAIN = "api.advisor.powerapps.microsoft.scloud"
        RX_POWERAPPS_ADVISOR_API_SCOPE  = "https://advisor.powerapps.microsoft.scloud/.default"
        ...
)

This will ensure that requests for the RX Power Apps Advisor API are properly routed and securely authenticated.

ISSUE 3

File: /workspaces/terraform-provider-power-platform/internal/provider/provider.go

Problem: Type Safety Issue: AuxiliaryTenantIDs Conversion Ignores Type Safety

In configureUseMsi, the conversion from types.List (auxiliaryTenantIDs) to a slice of strings uses v.String() for each element. However, v.String() may produce a Go representation (not always the actual string value) if the Terraform type is not guaranteed to be a string type or a UUID type, and errors are not handled.

Impact: Could result in invalid or unexpected values being added to p.Config.AuxiliaryTenantIDs, leading to authentication problems or provider misbehavior. Severity: medium.

Location:

auxiliaryTenantIDsList := make([]string, len(auxiliaryTenantIDs.Elements()))
for i, v := range auxiliaryTenantIDs.Elements() {
    auxiliaryTenantIDsList[i] = v.String()
}
p.Config.AuxiliaryTenantIDs = auxiliaryTenantIDsList

Fix: Use type assertions and error checking to extract the correct string value. For a types.String, use .ValueString(). For a custom UUID type, extract accordingly:

auxiliaryTenantIDsList := make([]string, len(auxiliaryTenantIDs.Elements()))
for i, v := range auxiliaryTenantIDs.Elements() {
    if sv, ok := v.(types.String); ok {
        auxiliaryTenantIDsList[i] = sv.ValueString()
    } else if uv, ok := v.(customtypes.UUID); ok {
        auxiliaryTenantIDsList[i] = uv.ValueString()
    } else {
        // handle error or skip invalid type
    }
}
p.Config.AuxiliaryTenantIDs = auxiliaryTenantIDsList

Handle cases where the element is of an unexpected type to avoid silent bugs.


Task Completion Instructions

After implementing these fixes:

  1. Run the linter: make lint to ensure code style compliance
  2. Run unit tests: make unittest to verify functionality
  3. Generate documentation: make userdocs to update auto-generated docs
  4. Add changelog entry: Use changie new to document the changes

Changie Entry Template:

kind: fixed
body: Fixed configuration and constants issues including magic strings, incorrect domain constants, and type safety in AuxiliaryTenantIDs conversion
time: [current-timestamp]
custom:
  Issue: "[ISSUE_NUMBER_IF_APPLICABLE]"

Replace [ISSUE_NUMBER_IF_APPLICABLE] with the relevant GitHub issue number, or remove the custom section if no specific issue exists.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingcopilotfixed using GitHub copilot autonomous agentenhancementNew feature or request

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Development

    Participants

    @mawasile

    Issue actions

      [Copilot] Configuration and Constants Issues · Issue #824 · microsoft/terraform-provider-power-platform