-
Notifications
You must be signed in to change notification settings - Fork 143
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
Cache improvements #708
base: dev
Are you sure you want to change the base?
Cache improvements #708
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT License. | ||
|
||
package com.microsoft.aad.msal4j; | ||
|
||
import lombok.AccessLevel; | ||
import lombok.Getter; | ||
import lombok.Setter; | ||
import lombok.experimental.Accessors; | ||
|
||
import java.io.Serializable; | ||
|
||
/** | ||
* Contains metadata and additional context for the contents of an AuthenticationResult | ||
*/ | ||
@Accessors(fluent = true) | ||
@Getter | ||
@Setter(AccessLevel.PACKAGE) | ||
public class AuthenticationResultMetadata implements Serializable { | ||
|
||
private TokenSource tokenSource; | ||
private CacheRefreshReason cacheRefreshReason; | ||
|
||
/** | ||
* Sets default metadata values. Used when creating an {@link IAuthenticationResult} before the values are known. | ||
*/ | ||
AuthenticationResultMetadata() { | ||
this.tokenSource = TokenSource.UNKNOWN; | ||
this.cacheRefreshReason = CacheRefreshReason.NOT_APPLICABLE; | ||
} | ||
|
||
AuthenticationResultMetadata(TokenSource tokenSource, CacheRefreshReason refreshReason) { | ||
this.tokenSource = tokenSource; | ||
this.cacheRefreshReason = refreshReason; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT License. | ||
|
||
package com.microsoft.aad.msal4j; | ||
|
||
/** | ||
* List of possible reasons the tokens in an {@link IAuthenticationResult} were refreshed. | ||
*/ | ||
public enum CacheRefreshReason { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are missing a case here when "Claims" are specified. When this happens, MSAL has to bypass the token cache and to ESTS. But not for client capabilities. Client capabilities are internally still claims. But we should not bypass the cache. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still open comment :) |
||
|
||
/** | ||
* Token did not need to be refreshed, or was retrieved in a non-silent call | ||
*/ | ||
NOT_APPLICABLE, | ||
/** | ||
* Silent call was made with the force refresh option | ||
*/ | ||
FORCE_REFRESH, | ||
/** | ||
* Access token was missing from the cache, but a valid refresh token was used to retrieve a new access token | ||
*/ | ||
NO_CACHED_ACCESS_TOKEN, | ||
/** | ||
* Cached access token was expired and successfully refreshed | ||
*/ | ||
EXPIRED, | ||
/** | ||
* Cached access token was not expired but was after the 'refresh_in' value, and was proactively refreshed before the expiration date | ||
*/ | ||
PROACTIVE_REFRESH | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT License. | ||
|
||
package com.microsoft.aad.msal4j; | ||
|
||
/** | ||
* A list of possible sources for the tokens found in an {@link IAuthenticationResult} | ||
*/ | ||
public enum TokenSource { | ||
|
||
/** | ||
* A default value, likely indicates tokens could not be retrieved | ||
*/ | ||
UNKNOWN, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. MSAL throws exception when token cannot be retried. I don't think you can get this value. Also, there is no unit test covering this path. |
||
|
||
/** | ||
* Indicates tokens came from an identity provider, such as Azure AD | ||
*/ | ||
IDENTITY_PROVIDER, | ||
|
||
/** | ||
* Indicates tokens came from MSAL's cache | ||
*/ | ||
CACHE | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add more tests. Consider adding these assertions to all integrationt tests, not just public client ones. Client_credentials is of highest priority.
In particular:
client_credentials + all combos of CacheRefreshReason (especially PROACTIVE_REFRESH)
client_credentials + claims
obo and auth code