-
Couldn't load subscription status.
- Fork 46
getAllSsoTokens method for Edge, Fixes AB#3361797 #2774
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
Conversation
|
❌ Work item link check failed. Description does not contain AB#{ID}. Click here to Learn more. |
| * The time when the token was acquired, in milliseconds since epoch. | ||
| */ | ||
| @SerializedName("AcquisitionTimeMillis") | ||
| private final @Nullable Long mAcquisitionTimeMillis; |
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.
From my understanding, supposing a client with an older MSAL version calls the getSsoToken method, their version of AcquirePrtSsoTokenResult won't have this mAcquisitionTimeMillis, but GSON should be ignoring any parameters which they don't recognize, and thus shouldn't throw an exception or cause a crash.
Folks can correct me if I'm wrong here (and I'll be sure to test this out)
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.
Pull Request Overview
This PR implements a new getAllSsoTokens method for Edge/OneAuth integration that retrieves multiple PRT (Primary Refresh Token) cookies based on environment/cloud from a given Authority value, capped at the 3 most recent tokens. This addresses the requirement from Edge to complement the existing single-token getSsoToken method.
Key changes:
- Added new
getAllSsoTokensmethod in BrokerMsalController with supporting infrastructure - Created AcquirePrtSsoTokenBatchResult class to handle batch token results
- Updated broker protocol version from 18.0 to 19.0 to support the new functionality
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| SpanName.java | Added GetAllSsoTokens span name for telemetry tracking |
| AcquirePrtSsoTokenResult.java | Added acquisition time field for token metadata |
| AcquirePrtSsoTokenBatchResult.java | New class for handling batch SSO token results |
| MsalBrokerResultAdapter.java | Added adapter method for batch result deserialization |
| IBrokerResultAdapter.java | Added interface method for batch result handling |
| AdalBrokerResultAdapter.java | Added unsupported operation stub for batch results |
| MsalBrokerRequestAdapter.java | Added request bundle creation for batch SSO token requests |
| BrokerMsalController.java | Implemented main getAllSsoTokens method with broker operation logic |
| BrokerOperationBundle.java | Added MSAL_ALL_SSO_TOKENS operation type |
| AuthenticationConstants.java | Updated protocol version and added API paths/constants for batch operations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...on4j/src/main/com/microsoft/identity/common/java/commands/AcquirePrtSsoTokenBatchResult.java
Outdated
Show resolved
Hide resolved
...n/src/main/java/com/microsoft/identity/common/internal/controllers/BrokerMsalController.java
Outdated
Show resolved
Hide resolved
...n/src/main/java/com/microsoft/identity/common/internal/controllers/BrokerMsalController.java
Show resolved
Hide resolved
common4j/src/main/com/microsoft/identity/common/java/commands/AcquirePrtSsoTokenResult.java
Outdated
Show resolved
Hide resolved
...n/src/main/java/com/microsoft/identity/common/internal/controllers/BrokerMsalController.java
Outdated
Show resolved
Hide resolved
common4j/src/main/com/microsoft/identity/common/java/opentelemetry/SpanName.java
Outdated
Show resolved
Hide resolved
...n/src/main/java/com/microsoft/identity/common/internal/request/MsalBrokerRequestAdapter.java
Show resolved
Hide resolved
|
|
||
| public Bundle getRequestBundleForAllSsoTokens( | ||
| @NonNull final AcquirePrtSsoTokenCommandParameters parameters, | ||
| @NonNull final String negotiatedBrokerProtocolVersion) { |
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.
Nonnull
Also do you want to add extra validation on presence of required parameters?
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.
Added the Nonnull; right now the validation is done on the broker side, which matches what is done for the existing getSsoToken method. Though I will consider updating both of these potentially if there's a big difference in validating earlier.
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.
![]()
...on4j/src/main/com/microsoft/identity/common/java/commands/AcquirePrtSsoTokenBatchResult.java
Show resolved
Hide resolved
…AD/microsoft-authentication-library-common-for-android into melissaahn/GetAllSsoTokens merging
Summary
We have a getSsoToken for OneAuth/Edge which returns a PRT cookie corresponding to a given account. They have asked us to also implement a method which gets all (well, we're capping it at the 3 most recent) PRT cookies based on Environment/cloud, which we can get from a passed in Authority value.
The changes in common for this method involve defining the method in BrokerMsalController, updating the appropriate adapter classes, creating the AcquirePrtSsoTokenBatchResult class, and bumping the broker protocol version.
Corresponding Broker PR: https://github.com/AzureAD/ad-accounts-for-android/pull/3233
Fixes AB#3361797