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

legacy flag does not work from version v0.0.30 when the login method is spn #331

Closed
bseenu opened this issue Sep 1, 2023 · 7 comments · Fixed by #338
Closed

legacy flag does not work from version v0.0.30 when the login method is spn #331

bseenu opened this issue Sep 1, 2023 · 7 comments · Fixed by #338

Comments

@bseenu
Copy link

bseenu commented Sep 1, 2023

How to reproduce it

kubelogin get-token --tenant-id <> --server-id <> --client-id <> --client-secret <> --login spn --legacy

Expected outcome:

token with audience field has "spn:" prefix

It works fine on version v0.0.29 and lower

@weinong
Copy link
Contributor

weinong commented Sep 1, 2023

hmm. i think that is not supported in the new sdk (azidentity) that we migrated in v0.0.30. Are you managing your own k8s AAD configuration? would it be possible to migrate the server side?

@bseenu
Copy link
Author

bseenu commented Sep 1, 2023

Changing the server side will require api restart and we are seeing if we can avoid that

This is failing only for serviceprincipal flow, the deviceToken flow is still working as expected

@weinong
Copy link
Contributor

weinong commented Sep 1, 2023

are you okay to stay on v0.0.29 and work on your backend migration? I want to avoid reverting that change for an outdated sdk

@bseenu
Copy link
Author

bseenu commented Sep 1, 2023

Any reason why is the device code flow not switched to azidentity, it is still using adal library - https://github.com/Azure/kubelogin/blob/master/pkg/token/devicecode.go

I am not asking you to revert it, but can we use legacy flag to decide to use azidentity or adal for getting the token ?

@weinong
Copy link
Contributor

weinong commented Sep 1, 2023

i might migrate devicelogin to azidentity when i have free resources :)

your proposal is fine, but it still requires us to have outdated adal sdk.

@ldemailly
Copy link

ldemailly commented Sep 8, 2023

Changing the server side will require api restart and we are seeing if we can avoid that

It's worse than that actually, it requires control plane rebuild and takes 30 minutes, so it's not an option

@weinong
Copy link
Contributor

weinong commented Sep 8, 2023

i will see if I can bring back the old code when --legacy is enabled next week

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 a pull request may close this issue.

3 participants