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

make-auth generates client secrets in a legacy way? #3998

Open
SvenAelterman opened this issue Jun 22, 2024 · 8 comments
Open

make-auth generates client secrets in a legacy way? #3998

SvenAelterman opened this issue Jun 22, 2024 · 8 comments
Labels
bug Something isn't working

Comments

@SvenAelterman
Copy link
Collaborator

Describe the bug

Running make-auth creates application registrations with client secrets, however, these client secrets are not shown in the Entra ID UI for app registrations. This makes me wonder if the process uses a legacy method.

If it uses a legacy method, it should be updated to use a current method, which would make it easier to rotate the secrets.

Azure TRE release version (e.g. v0.14.0 or main): v0.17.0

@SvenAelterman SvenAelterman added the bug Something isn't working label Jun 22, 2024
@tim-p-allen
Copy link
Collaborator

Looks like we use the az rest command to create the app registrations via the graph api. Not sure the reason behind that

@jonnyry
Copy link
Collaborator

jonnyry commented Oct 9, 2024

@SvenAelterman just noticed this as well.

I received a bunch of Azure Security alerts saying that credentials were expiring on the following service principals:

  • mytre API
  • mytre Application Admin
  • mytre Automation Admin

Go to the App Registration in the Azure Portal and there are no credentials present for any of these:

Image

However run az ad sp list --display-name "mytre API" and there are definitely credentials present that are expiring:

Image

@marrobi
Copy link
Member

marrobi commented Oct 9, 2024

I can remember this confusing me at the time and was limited support to do what we needed via az cli or terraform.. @ross-p-smith know this was a LONG time ago, but can you remember why the SP secret's don't appear in the portal? I seem to recall something about it being a SP secret, rather than ap secret, but could be confused.

If you look at ./devops/scripts/create_aad_assets.sh there is an RESET_AAD_PASSWORDS variable. The docs cover it on a per app basis with the --reset-password flag. It should maybe also cover how to do all three at once.

I have just run:

export RESET_AAD_PASSWORDS=true
make auth

And the three apps have their secrets updated. A make deploy-core will need running once any variables have been updated (config.yaml or CI/CD tooling).

I suggest trying this in a dev environment first (we have had issues where make deploy-core has needed running twice to update the secrets, not sure if that is still the case), please do report back.

@jonnyry
Copy link
Collaborator

jonnyry commented Oct 9, 2024

Thanks, will try.

This comment describes the behaviour of the portal for SP secrets vs App secrets: Azure/azure-cli#23566 (comment)

@marrobi
Copy link
Member

marrobi commented Oct 9, 2024

Ok, so we are saying the commands should be moved from az restto az app or maybe the AzureAD terraform provider, but there is also a question around are all the settings we currently configure in the project now supported by these methods.

@jonnyry
Copy link
Collaborator

jonnyry commented Oct 14, 2024

@marrobi thanks, I followed your steps above, though instead of a local make deploy-core I ran a full CI/CD deployment which I can see has updated the secrets in the main TRE Key Vault on first run.

Was there something in particular you tested to ensure the new credentials were being picked up? I'm not 100% sure what will be holding on to cached tokens/credentials.

@marrobi
Copy link
Member

marrobi commented Oct 14, 2024

@jonnyry from what I recall the web app didn't update the new credentials. So deploying a new workspaces failed on creating the app registration. The app registration configuration wasn't reading the new version from the key vault until the second run of make all. It might have been resolved since then, was a while back.

@jonnyry
Copy link
Collaborator

jonnyry commented Oct 16, 2024

Ok, so we are saying the commands should be moved from az restto az app or maybe the AzureAD terraform provider, but there is also a question around are all the settings we currently configure in the project now supported by these methods.

I don't know enough on the specifics of holding secrets against Apps vs Service Principals (or why it was done that way in the first place) to know if it's the right thing to do to switch for the purposes of surfacing it in the UI. The above method has worked OK for us so happy with that :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants