search

chaostoolkit-incubator / chaostoolkit-azure

Chaos Toolkit Extension for Azure
https://chaostoolkit.org/
Apache License 2.0
22 stars 28 forks source link

POC for azure based token authentication #75

Closed maximmold closed 4 years ago

maximmold commented 4 years ago

This is a quick pull request to consider adding the ability to use an existing token for Azure authentication. Our organization makes it difficult to use a Service Principal and I only have the token from an az login. It is functional, but obviously I think there could be a cleaner way to do this. Throwing this out there for discussion and consideration. Thank you.

buderre commented 4 years ago

@maximmold I'm curious about the token that you want to introduce. Is this the token that the Azure CLI generates? If yes this token will be timely limited, am I right? What kind of limitation do you have in your company? Maybe we can tackle this problem from another side?

mkaszub commented 4 years ago

Hi @buderre @maximmold So this is a bearer token that you can get in OAuth flow. I think this is a good idea to introduce Token Authentication as OAuth is quite common use case.

I think @buderre questions regarding expiration is valid one. I made quick check on AADTokenCredentials and it seems that AADMixin allows us to provide refresh_token. I have similat changest regarding token auth on my site but I did not check if refresh token functionlity works so we would have to verfiy it.

maximmold commented 4 years ago

Yeah, it is the Azure CLI token and it definitely expires after a period. Right now I am just experimenting so it's not a big deal if I have to manually update it every so often. I like the idea of the refresh token though.

buderre commented 4 years ago

@maximmold Before proceeding with the PR,

maximmold commented 4 years ago

Just adding in the setting of the refresh token didn't seem to make a difference. I suspect some exception handling would be needed to exercise the refresh_session function. Where do you recommend adding this documentation?

buderre commented 4 years ago

@maximmold In the file chaosazure/init.py would be sufficient. Above the method would be nice

buderre commented 4 years ago

@maximmold It would be also nice to document the CHANGELOG.md with the change you made. THen I think you're good to go. @Lawouach What do you think? Any stuff forgotten?

Lawouach commented 4 years ago

I think the changes are sound.

Unfortunately, the previous PR now requires a rebase of this one. Hopefully not too complicated.

maximmold commented 4 years ago

@Lawouach Are you referring to my other PR or some other PR

maximmold commented 4 years ago

Closing in favor of https://github.com/chaostoolkit-incubator/chaostoolkit-azure/pull/83