feat: handle Authorization_RequestDenied errors with informative message

[bgdnext64]

Detect Authorization_RequestDenied errors from Microsoft Graph / Azure AD early in the authorization error parser and return a specific error message explaining these require admin consent or Global Administrator role and cannot be automatically discovered by MPF. Links to Entra ID docs.

Closes #45

[Copilot]

Pull request overview

Adds early detection for Microsoft Graph / Entra Authorization_RequestDenied authorization failures so MPF can return a clearer, actionable error when required permissions can’t be inferred automatically (admin consent / Global Admin required), aligning with Issue #45.

Changes:

• Detect Authorization_RequestDenied in the authorization error parser and return a dedicated explanatory error message with a docs link.
• Add a unit test covering the new Authorization_RequestDenied handling path.
• Update the Go version directive in go.mod.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
pkg/domain/authorizationErrorParser.go	Adds early check for Authorization_RequestDenied and returns a more informative error.
pkg/domain/authorizationErrorParser_test.go	Adds a unit test validating Authorization_RequestDenied returns an error and nil permissions map.
go.mod	Bumps the go directive patch version.

[Copilot]

Using fmt.Errorf with %w to wrap a freshly-created errors.New value doesn't provide useful wrapping semantics (callers can't reliably errors.Is/As against it) and makes the error chain noisy. Prefer returning a single errors.New / fmt.Errorf without %w, or define a package-level sentinel error (or typed error) and wrap that so callers can detect Authorization_RequestDenied programmatically.

[bgdnext64]

Good catch — switched to a package-level sentinel (ErrAuthorizationRequestDenied) so callers can use errors.Is to detect this case. The double-wrap with a freshly-created errors.New is gone.

[maniSbindra]

Could you also add a minimal e2e test for this? The repo has a well-established pattern for Terraform e2e tests (e.g., e2eTerraformInvalid_test.go, e2eTerraformAuthPermissionMismatch_test.go). A Terraform sample that exercises the Authorization_RequestDenied path (e.g., an azuread_group resource) with a corresponding e2e test would validate the full flow end-to-end.

[maniSbindra]

The early return here could discard parseable permissions from mixed error messages. Terraform errors can contain multiple error types in one payload — e.g., both Authorization_RequestDenied and AuthorizationFailed entries. With this placement, MPF would bail out and lose all extractable scope/permission pairs.

Consider moving this check to after the existing parse attempts, and only returning the Authorization_RequestDenied guidance if no permissions were extracted (e.g., in the len(resMap) == 0 fallback path). That way MPF still extracts what it can and only surfaces this message when there's truly nothing parseable.

[bgdnext64]

Agreed, this was the right call. Moved the check out of the early-return path. Parsing now runs first and the Authorization_RequestDenied guidance is only returned in the len(resMap) == 0 fallback, so mixed payloads with parseable AuthorizationFailed entries still produce the scope/permission pairs.

[maniSbindra]

The linked URL points to the Entra built-in roles reference, but this error is typically about Microsoft Graph API application permissions that require admin consent. More actionable links would be:

• Microsoft Graph permissions reference — explains the Graph API permissions model and admin consent requirements
• Terraform AzureAD provider service principal configuration — already referenced in this repo's own docs/known-issues-and-workarounds.MD

Suggestion:

"See https://learn.microsoft.com/en-us/graph/permissions-reference for Microsoft Graph permissions "+
"and https://registry.terraform.io/providers/hashicorp/azuread/latest/docs/guides/service_principal_configuration for Terraform AzureAD provider setup"))

[bgdnext64]

Updated the message to point at the Microsoft Graph permissions reference and the Terraform AzureAD provider service-principal configuration guide instead of the Entra built-in roles page. Those are the actually-actionable links for this scenario.

[maniSbindra]

Good coverage for the pure Authorization_RequestDenied case. Could you also add a test for a mixed error message that contains both Authorization_RequestDenied and parseable AuthorizationFailed / Authorization failed entries? That would guard against the early-return regression where extractable permissions are silently lost.

[bgdnext64]

Added TestMixedAuthorizationRequestDeniedAndAuthorizationFailedError which covers exactly that — an error blob containing both an Authorization_RequestDenied line and a parseable AuthorizationFailed entry. It asserts the Microsoft.Storage/storageAccounts/write permission is still extracted, so the regression you flagged would be caught.

[bgdnext64]

One thing worth flagging on the CI side: the Lint and CI Build/Unit Test jobs are red on this push, but I'm fairly sure none of it is from the changes in this PR. Every failing job dies at the same step — the go-task/setup-task action's own setup, with ##[error]unable to get latest version — well before any of our code, lint config, or tests run.

A few things that point at this being upstream/infra rather than the PR:

• The same workflow files (unchanged here) were green on main and on other PRs as recently as 2026-04-23.
• The CI Build job calls go-task/setup-task without any token, so it's not an SSO/PAT issue tied to my push.
• The error is identical across all 6 lint matrix tasks plus Build and Unit Tests, and reproduces across three full reruns.
• All workflows that don't use go-task/setup-task (Dependency Review, CodeQL, E2E Tests check) pass on this same commit.

Locally go build ./..., go vet ./... and go test ./pkg/domain/... all pass, including the new mixed-error and errors.Is assertions.

If this keeps reproducing for others too, the cleanest fix is probably to pin go-task/setup-task to a specific version (e.g. version: 3.50.0) in lint.yml and ci.yml instead of version: 3.x, so it doesn't depend on the latest-release lookup that's currently failing. Happy to do that as a separate PR if you'd like.

[bgdnext64]

Just a heads up on testing: in addition to the unit tests, I ran the new TestTerraformAuthorizationRequestDenied e2e against my test tenant end-to-end. Spun up a fresh service principal with no roles, pointed it at the new samples/terraform/authorization-request-denied sample (which creates an azuread_group), and let MPF run.

Result: PASS in about 145 seconds. The Terraform apply hit a real Authorization_RequestDenied: Insufficient privileges 403 from Microsoft Graph (not a synthetic string), the new parser detected it via hasAuthorizationRequestDenied after the existing parse attempts, and MPF stopped cleanly with the dedicated guidance error including the updated docs links (Microsoft Graph permissions reference and the Terraform AzureAD provider configuration guide). Custom role, resource group, Terraform artifacts, and the temp SP were all cleaned up.

So the production path — not just the unit-test simulated string — is verified.