microsoft-authentication-library-for-android icon indicating copy to clipboard operation
microsoft-authentication-library-for-android copied to clipboard

Split MSAL (Global) and PCA-Specific Configuration

Open fadidurah opened this issue 3 years ago • 1 comments

What Adding support for global configuration, where developers designate global fields that they'd want all PCA instances to have through a global configuration file. Refer to the design document PR below. https://identitydivision.visualstudio.com/DevEx/_git/AuthLibrariesApiReview/pullrequest/4763

Relies on: https://github.com/AzureAD/microsoft-authentication-library-common-for-android/pull/1726

Global Fields PCA-Specific Unexposed (Default File)
minimum_required_broker_protocol_version client_id environment
web_view_zoom_controls_enabled redirect_uri http
- read_timeout
- connect_timeout
web_view_zoom_enabled authorization_user_agent
power_opt_check_for_network_request_enabled multiple_clouds_supported
handle_null_taskaffinity broker_redirect_uri_registered
authorization_in_current_task account_mode
telemetry client_capabilities
logging
- pii_enabled
- log_level
- logcat_enabled
authorities
browser_safe_list
- browser_package_name
- browser_signature_hashes
- browser_version_lower_bound
- browser_version_upper_bound

Why At the moment, every PublicClientApplication (PCA) needs a configuration file to explicitly set all of its configuration fields, including fields that are not designed to be different across multiple PCA instances within the same application. A developer would need to create a different configuration file if they wanted to exclude a field from a PCA's configuration, and reapply whatever fields they wanted to keep.

How Creating a GlobalSettings class used to initialize the global configuration from a global configuration file that the developer passes in as a parameter. In this design, the global settings initialization is done before the PCA is created in the developer's app. Once initialization is done, the global settings will be available as a static singleton field, which can be used later by PCA instances to supply global fields as command parameters. These settings are called "Global" because as mentioned above, fields included in these files are not meant to be changed between different PCA instances within the same session.

Testing Added unit testing in this PR through GlobalSettingsPublicClientApplicationTest to check current implementation is not broken and new global field reading is being sent to PCAConfiguration correctly. Testing in common (see other PR).

fadidurah avatar Mar 16 '22 11:03 fadidurah

Codecov Report

Merging #1604 (60f30a5) into dev (eb1e156) will increase coverage by 1.13%. The diff coverage is 63.79%.

:exclamation: Current head 60f30a5 differs from pull request most recent head e2dc115. Consider uploading reports for the commit e2dc115 to get more accurate results

@@             Coverage Diff              @@
##                dev    #1604      +/-   ##
============================================
+ Coverage     49.37%   50.50%   +1.13%     
- Complexity      372      381       +9     
============================================
  Files            59       59              
  Lines          2708     2756      +48     
  Branches        333      344      +11     
============================================
+ Hits           1337     1392      +55     
+ Misses         1227     1209      -18     
- Partials        144      155      +11     
Impacted Files Coverage Δ
.../microsoft/identity/client/internal/MsalUtils.java 8.72% <0.00%> (ø)
...tity/client/configuration/LoggerConfiguration.java 58.82% <50.00%> (-21.18%) :arrow_down:
...y/client/PublicClientApplicationConfiguration.java 32.28% <56.25%> (+3.83%) :arrow_up:
...t/PublicClientApplicationConfigurationFactory.java 66.66% <100.00%> (+3.92%) :arrow_up:
...tity/client/internal/CommandParametersAdapter.java 76.99% <100.00%> (+0.10%) :arrow_up:
...t/internal/configuration/LogLevelDeserializer.java 50.00% <0.00%> (-50.00%) :arrow_down:
...osoft/identity/client/PublicClientApplication.java 47.67% <0.00%> (+4.01%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b613b58...e2dc115. Read the comment docs.

codecov-commenter avatar Mar 25 '22 23:03 codecov-commenter