Skip to content

Replace App Registration secrets with Managed Identity + Federated Credentials - #380

Open
Rudolfs Naumenko (nsftwr) wants to merge 6 commits into
Azure:ipam-3.7.0from
nsftwr:feature/no-more-secrets
Open

Rudolfs Naumenko (nsftwr) wants to merge 6 commits into
Azure:ipam-3.7.0from
nsftwr:feature/no-more-secrets

Conversation

@nsftwr

@nsftwr Rudolfs Naumenko (nsftwr) commented Apr 12, 2026 •

Copy link
Copy Markdown

Overview

This change removes the creation and use of secrets for the engine app registration and instead uses the existing managed identity to establish a federated credential relationship with the app registration. This aligns with best practices, where secrets are discouraged and should only be used when no other option is available.

The implementation continues to rely on the existing functions, so it does not require significant changes to the main codebase.

When the user is an admin, the flow uses ClientAssertionCredential to authenticate against the app registration.
When the user is a non-admin, it uses OnBehalfOfCredential. In this case, client_assertion_func is used instead of a client secret, alongside user_assertion, to preserve the on-behalf-of flow.

Additional Details

Testing Conducted

To verify that the correct auth method was used, additional logging was added temporarily. This was then verified by 2 users - an admin user and a non admin.

Admin user:

Screenshot 2026-04-12 213213 Screenshot 2026-04-12 213202 Screenshot 2026-04-12 213321 image

Non-admin user:

Screenshot 2026-04-12 212142 Screenshot 2026-04-12 212124 image Screenshot 2026-04-12 212339

@DCMattyG
DCMattyG self-requested a review April 12, 2026 21:26
@DCMattyG DCMattyG self-assigned this Apr 12, 2026
@DCMattyG

Copy link
Copy Markdown
Contributor

Hi Rudolfs Naumenko (@nsftwr), and thank you so much for your interest in improving Azure IPAM for everyone!

This topic has been discussed before in #353 and the issue I keep coming back to is that not everyone deployed Azure IPAM in Azure. It could be deployed anywhere like another cloud provider, on-premises, on a laptop....and using this method seems to rule out non-Azure deployments.

I'll be honest that it's been a while since I've spent the appropriate amount of brain cycles on the exact topic, but if you understand things different, please do let me know and I'll be happy to re-evaluate this option.

@nsftwr

Rudolfs Naumenko (nsftwr) commented Apr 12, 2026 •

Copy link
Copy Markdown
Author

Thank you for the speedy response DCMattyG! I've read the conversation, and immediately the first thought was

If it could be running locally, why does Cosmos have Managed Identity Credentials?

Then i remembered that globals.COSMOS_KEY is optional. Could we apply the same to this? Have a similar [Switch] parameter that one can specify if they want to be secret-less. Default to the current approach that is set, but if -UseFederatedCredentials is passed then use the approach I've described in this PR.


Edit: App registration creds are still honoured as its using ClientAssertionCredential with the App Reg Clientid, the same for OBO creds - as its using OnBehalfOfCredential() with user_assertion + client_assertion_func

@DCMattyG

Copy link
Copy Markdown
Contributor

I'm open to making it work similarly to Cosmos. Effectively, the deployment script is specific to Azure, so when deploying with the script it could use the Federated Credentials. I don't see a point in adding a switch since the script only targets Azure deployments. We'd definitely need to add the fallback capability to use the Client Secret which, as you pointed out, we can simply do by the presence of the value similarly to Cosmos does it today.

I'll be focusing on getting v4.0.0 out the door and we'll need to rebase this on that since there are a massive number of changes, but perhaps we can tackle this together next.

@nsftwr

Copy link
Copy Markdown
Author

Rebasing and working on top of your changes now or upcoming isn’t an issue whatsoever. I would be very interested what’s required for you to either combine this with v4, or have it released as a minor version very soon after. I’m aiming to migrate to Azure IPAM relatively soon as per something I’m working on, and would love to have the ability to not deal with secrets.

I can have the functionality that looks for the secret’s existence done by tomorrow. Would it be of interest to set up a call some point the next two weeks to discuss any outstanding work on v4, as I am happy to contribute to a great project.

@nsftwr

Copy link
Copy Markdown
Author

Hi DCMattyG, I have added and tested my changes for having the ability to use either secret, or FIC. in both scenarios it works as expected, and no different than the cosmos credential logic.

As I have built this branch on top of your ipam-3.7.0 branch, there arent any rebases required when compared to your v4.0.0 pull request. And this PR proposes to merge my changes into your v4.0.0 pull request. Based on the fact that there arent a lot of code changes for this implementation, could I ask you to reevaluate and see if we can merge this PR into your v4.0.0 PR so FIC ability gets bundled with v4.0.0?

As mentioned, glad to assist in any other outstanding work, as well as with any testing that needs to be conducted.

@nsftwr

Copy link
Copy Markdown
Author

DCMattyG Hi. Any updates on this? Happy to catch up with you on this matter via Teams if that's better.

@nsftwr

Copy link
Copy Markdown
Author

Hi DCMattyG. It has been months since I have opened this. I am wondering if it would be possible for you to review and potentially merge within v4. The work I am doing using Azure IPAM would significantly benefit of having the option to be secret-less, and I am sure it would help others who want to use it as well.

Thank you,
nsftwr

@DCMattyG

DCMattyG commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Hi Rudolfs Naumenko (@nsftwr), I definitely agree that this is something that should be included, however...

  1. Azure IPAM runs on multiple Azure clouds (Public, Azure Gov, Azure DoD, etc.) This is something that I need to verify is supported across the various clouds (which is no simple task) and test that it works. Otherwise, there needs to be logic to say "for Azure Public, do X, for other clouds, do Y", etc.
  2. I'm not sure if you've seen the change list in v4, but it's HUGE. I'm doing my best to wrap all of that up and finish testing everything to avoid bugs.

So, we're on the same page for sure, however I'm thinking this might be a fast-follow to v4 so I have time to get this tested with some colleagues who actually have IL5/IL6 access as this product is absolutely in use in those places.

Hope that makes sense. Thank you so much for this, I love it!

P.S. Can you open a feature request under GitHub Issue and link it to your PR per chance? It will make things just a bit easier to track on my end. Much appreciated!

@DCMattyG

DCMattyG commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Ok Rudolfs Naumenko (@nsftwr), I took a quick first pass through the changes, and a few clear gaps stood out...

Cloud support

  • The federated credential's issuer (login.microsoftonline.com) and audience (api://AzureADTokenExchange) are hard-coded to Azure Public, both in deploy.ps1 and in helper.py. Azure Government/DoD and China use different values, and I still need to confirm whether USSec/IL6 supports this at all. As written, this only works in Azure Public.

Engine

  • managed_identity_credential isn't defined at module level anymore. It became a local variable inside get_cosmos_client() in a June change, and your latest merge from ipam-3.7.0 pulled that in. The secret-free path now throws a NameError.
  • /api/health still requires CLIENT_SECRET, so a secret-free deployment always reports as unhealthy.

Other parts of the solution not covered

  • update.ps1 and the slot templates (appServiceSlot.bicep / functionAppSlot.bicep) still set ENGINE_APP_SECRET from Key Vault. On a secret-free deployment that reference can't resolve, and the engine ends up trying to use the raw @Microsoft.KeyVault(...) text as a secret.
  • The migrate flow removes ENGINE_APP_SECRET but never creates a federated credential, so migrated deployments end up with no credential at all.
  • Existing deployments have no upgrade path from a secret to a federated credential.

Deployment script

  • New-FederatedCredential has no error handling or retry, fails if run a second time (the credential already exists), and quietly lets the deployment continue if creation fails.
  • The -ParameterFile step now needs Entra rights on the Engine app registration, which goes against the reason for splitting identity and infrastructure deployments.

Housekeeping

  • azure-mgmt-resource-subscriptions was downgraded to 1.0.0b1. 1.0.0 does exist on PyPI, so that change should be reverted.
  • Docs (deployment README, .env.example, docker-compose.yml) haven't been updated.

I'm happy to help work on this at some point soon., but covering all supported clouds, plus the update and migrate paths, will be required before it can merge for sure. Hope that makes sense!

This branch has not been deployed

No deployments
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

Successfully merging this pull request may close these issues.

3 participants