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

Azure SDK Review - [Azure Identity] #6448

Closed
azure-sdk opened this issue Aug 4, 2023 · 3 comments
Closed

Azure SDK Review - [Azure Identity] #6448

azure-sdk opened this issue Aug 4, 2023 · 3 comments
Assignees
Labels
SDKArchboard ReviewRequested This should be reviewed by the SDK board in partnership with the service team.

Comments

@azure-sdk
Copy link
Collaborator

New SDK Review meeting has been requested.

Service Name: Azure Identity
Review Created By: Bill Wert
Review Date: 08/08/2023 02:05 PM PT

Hero Scenarios Link: Not Provided
Architecture Diagram Link: Not Provided
Core Concepts Doc Link: Not Provided
APIView Links: Javascript, Java, Python, .NET,

Description:

Detailed meeting information and documents provided can be accessed here

@github-actions github-actions bot added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Aug 4, 2023
@billwert
Copy link

billwert commented Aug 4, 2023

Additional API Views for Core:

@billwert
Copy link

billwert commented Aug 4, 2023

Go and C/C++ are not planning a GA with this release as they have not yet picked up these features.

.NET has already shipped the Core change for enabling CAE on the Core TokenRequestOptions type, so it is not included here.

DAC should continue if a dev-time credential fails

Issue for more context.

In the case SharedTokenCredential found an expired token it would cause ChainedTokenCredential to stop further processing. This is undesirable behavior. In addition to fixing this, we decided to make DefaultAzureCredential also continue on any failure from a dev-time credential (such as the CLIs or IDEs.) This change has no API surface changes; it is behavioral. We do not believe modifying the DAC policy here will break customers.

Continuous Access Evaluation for service principals

Issue for more context.

CAE support is now configurable per request. This is exposed through a new property on TokenRequestContext in Core. Names:
.NET: IsCaeEnabled
Java: setEnableCae(boolean) and isCaeEnabled
Python: enable_cae
JS: enableCae

We now maintain two token caches differentiated by filename. This should not be a visible change to users.

The existing AZURE_IDENTITY_DISABLE_CP1 environment variable is now a no-op and has been removed.

Although disabling CAE by default could be a breaking behavioral change for clients talking to services that support CAE and also are subject to a directory policy that forces it to be enabled, the impact is relatively minor given this is a rare policy and CAE support is rare among RPs. To resolve this issue, clients that support CAE need to add a client option to allow customers to opt in to CAE support. The rationale for the change is that keeping behavior as is could be worse, since clients unprepared for CAE challenges won't be able to properly respond and acquire tokens as their RP start supporting CAE.

EnableSupportLogging in MSAL

Issue for more context.

MSAL has requested we give users the ability to turn on additional MSAL logging. They have had ICMs that would have been much faster to resolve with this capability. For MSAL based credentials we add an option to enable this based on the name EnableSupportLogging.

It should be clear that this feature will cause PII to be logged in an unredacted form. Our docs are clear about this danger, but we wanted to highlight it.

New option for controlling interactive browser options for Azure PowerShell.

Issue for more context.

The Azure PowerShell team is required to customize the web view displayed after authentication. Identity needs to pass options through to MSAL to accomplish this.

@joshfree joshfree added the SDKArchboard ReviewRequested This should be reviewed by the SDK board in partnership with the service team. label Aug 8, 2023
@github-actions github-actions bot removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Aug 8, 2023
@billwert
Copy link

billwert commented Aug 8, 2023

Notes and changes from archboard:

  • Decided to use Unsafe in the name of EnableSupportLogging to signal danger. Becomes EnableUnsafeSupportLogging
  • Changed browser customization options properties to SuccessMessage and ErrorMessage
  • .NET: Rename type to BrowserCustomization
  • .NET: Validate that Use is the right verb for UseEmbeddedWebView property
    • Add that as a property instead of a field.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SDKArchboard ReviewRequested This should be reviewed by the SDK board in partnership with the service team.
Projects
None yet
Development

No branches or pull requests

3 participants