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

fix: set expires_in the proxy token response #630

Merged
merged 1 commit into from
Nov 9, 2022

Conversation

aramase
Copy link
Member

@aramase aramase commented Nov 8, 2022

Signed-off-by: Anish Ramasekar [email protected]

Reason for Change:

  • msal-go returns AuthResult which doesn't contain the expires_in value. Computing that in the proxy based on current time and expires_on. This change is to make the response consistent with the IMDS response and also AAD response for token request.

This is the AAD response:

{
    "token_type": "Bearer",
    "expires_in": 86399,
    "ext_expires_in": 86399,
    "access_token": "<token>"
}

xref: https://learn.microsoft.com/en-us/azure/active-directory/develop/v2-oauth2-auth-code-flow#successful-response-2

Requirements

  • squashed commits
  • included documentation
  • added unit tests and e2e tests (if applicable).

Issue Fixed:

fixes #624

Please answer the following questions with yes/no:

Does this change contain code from or inspired by another project? If so, did you notify the maintainers and provide attribution?

  • yes
  • no

Notes for Reviewers:
Opened AzureAD/microsoft-authentication-library-for-go#358 to request adding this as part of AuthResult, so we don't have to compute it.

@aramase aramase changed the title fix: set expires_in the proxy token response fix: set expires_in the proxy token response Nov 8, 2022
@aramase aramase added this to the v0.15.0 milestone Nov 8, 2022
@sozercan sozercan requested a review from enj November 9, 2022 00:38
@aramase aramase requested a review from sozercan November 9, 2022 00:38
Copy link
Member

@akshaysngupta akshaysngupta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Is there a way to test this with the addon ?

@aramase
Copy link
Member Author

aramase commented Nov 9, 2022

Thanks! Is there a way to test this with the addon ?

Once this is merged, you can use the nightly image build from here in your pod spec by manually editing the pod. This will be part of next release (2 weeks from now) post which AKS will update the add-on to use the new release.

pkg/proxy/proxy.go Outdated Show resolved Hide resolved
pkg/proxy/proxy.go Outdated Show resolved Hide resolved
@aramase aramase force-pushed the expires-in branch 2 times, most recently from 3ad5d47 to 4f5af77 Compare November 9, 2022 17:44
@aramase aramase requested a review from enj November 9, 2022 17:45
pkg/proxy/proxy.go Outdated Show resolved Hide resolved
@aramase aramase merged commit c4a69ec into Azure:main Nov 9, 2022
@aramase aramase deleted the expires-in branch November 9, 2022 20:42
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.

AAD token received via the side car is valid for 0 seconds
4 participants