fix: enable update jwt via callback for workloadidentity #719
Conversation
|
@JustinJudd Could you also help to take a look? Since it's you that bring workload identity to adal ^^ |
|
@jhendrixMSFT Could you please kindly take a look? |
There was a problem hiding this comment.
We cannot accept breaking changes in go-autorest so this will have to be refactored.
In addition, the modules in this repo are reaching end-of-life. Our new authentication module azidentity is the path forward. You can find the migration guide here.
autorest/adal/token.go
Outdated
| // NewServicePrincipalTokenFromFederatedToken creates a ServicePrincipalToken from the supplied federated OIDC JWT. | ||
| func NewServicePrincipalTokenFromFederatedToken(oauthConfig OAuthConfig, clientID string, jwt string, resource string, callbacks ...TokenRefreshCallback) (*ServicePrincipalToken, error) { | ||
| // NewServicePrincipalTokenFromFederatedToken creates a ServicePrincipalToken from the supplied federated OIDC JWTCallback. | ||
| func NewServicePrincipalTokenFromFederatedToken(oauthConfig OAuthConfig, clientID string, jwtCallback JWTCallback, resource string, callbacks ...TokenRefreshCallback) (*ServicePrincipalToken, error) { |
There was a problem hiding this comment.
This is a breaking change.
There was a problem hiding this comment.
I doubt whether there is any user would call this NewServicePrincipalTokenFromFederatedToken since the token would expire after 24 hours, that's more like a bug
There was a problem hiding this comment.
This bug actually only happens in such scenario:
For applications running on AKS, they need to use the JWT, which is generated by apiserver and mount to the Pod by kubelet , to exchange OAuth token from Azure identity platform when there is any request to arm at the first time. Azure identity platform will not only return OAuth token for authenticating, it will also return a refresh_token for the application to refresh Oauth token.
ADAL will use the JWT to request Oauth token and refresh_token when there is a request to arm at the first time, and then it will use refresh_token to refresh the OAuth token if it is expired and the JWT should be useless from then.
However, application may have no request to arm for a very long time after it is up (or after master-slave switch). If the JWT itself is expired, apiserver will refresh the JWT and write to file mounted in the Pod. However, under current implementation, the jwt we used to exchange token from Azure identity platform cannot be refreshed since we cannot read the JWT from the file dynamically. So, if the request happens after the JWT is expired, request to exchange token would be rejected, any other request to arm will fail as well
Sure I can make it without breaking changes. However, the reason why I didn't do that is that I think users will encounter building error once they upgrade adal to the new version, which will enforce them to fix this bug. If there is no breaking changes, then users may not fix it actively until they get into real trouble. |
|
The real fix is to use |
I understand this repo is going to be end-of-life, still some of external services dependent on it, I think it's worth to solve. |
Our service is still using adal and it will take some time to migrate to azidentity and msal, for now we need to use adal to support workload identity. |
|
similar fix in msal-for-go: AzureAD/microsoft-authentication-library-for-go#319 |
Thank you for your contribution to Go-AutoRest! We will triage and review it as soon as we can.
As part of submitting, please make sure you can make the following assertions:
Why make this change
When Using federated workload identity, we need to request Oauth bearer token by JWT provided by external platform(e.g. k8s) at first. Related code snippets:
https://github.com/Azure/go-autorest/blob/main/autorest/authorization.go#L120-L125
https://github.com/Azure/go-autorest/blob/main/autorest/adal/token.go#L1045-L1062
https://github.com/Azure/go-autorest/blob/main/autorest/adal/token.go#L375-L379
When this request happens after the external JWT expired, we will get 401 error:
adal: Refresh request failed. Status Code = '401'. Response body: {"error":"invalid_client","error_description":"AADSTS700024: Client assertion is not within its valid time range.We should provide a callback to get the JWT dynamically, instead of providing it statically.