Skip to content

Conversation

@melissaahn
Copy link
Contributor

@melissaahn melissaahn commented Sep 26, 2025

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

@github-actions
Copy link

❌ Work item link check failed. Description does not contain AB#{ID}.

Click here to Learn more.

@github-actions github-actions bot changed the title getAllSsoTokens method for Edge getAllSsoTokens method for Edge, Fixes AB#3361797 Sep 26, 2025
* The time when the token was acquired, in milliseconds since epoch.
*/
@SerializedName("AcquisitionTimeMillis")
private final @Nullable Long mAcquisitionTimeMillis;
Copy link
Contributor Author

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)

@melissaahn melissaahn marked this pull request as ready for review September 26, 2025 07:10
@melissaahn melissaahn requested a review from a team as a code owner September 26, 2025 07:10
Copilot AI review requested due to automatic review settings September 26, 2025 07:10
Copy link
Contributor

Copilot AI left a 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 getAllSsoTokens method 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.


public Bundle getRequestBundleForAllSsoTokens(
@NonNull final AcquirePrtSsoTokenCommandParameters parameters,
@NonNull final String negotiatedBrokerProtocolVersion) {
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

@mohitc1 mohitc1 left a comment

Choose a reason for hiding this comment

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

:shipit:

@melissaahn melissaahn requested a review from a team as a code owner October 6, 2025 16:23
@melissaahn melissaahn merged commit afbc7b2 into dev Oct 10, 2025
24 of 25 checks passed
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.

5 participants