Wrap ASAuthorizationProviderExtensionAuthorizationRequest methods #2347

antrix1989 · 2024-09-24T17:30:38Z

Proposed changes

Describe what this PR is trying to do.

Type of change

Risk

High – Errors could cause MAJOR regression of many scenarios. (Example: new large features or high level infrastructure changes)
Medium – Errors could cause regression of 1 or more scenarios. (Example: somewhat complex bug fixes, small new features)
Small – No issues are expected. (Example: Very small bug fixes, string changes, or configuration settings changes)

Additional information

MSAL/IdentityCore

ameyapat · 2024-09-24T17:35:05Z

Cloned PR for sedemche/wrap_calling_app_bundle_id

AI description (iteration 1)

PR Classification

New feature

PR Summary

This pull request introduces methods to retrieve and display information about the calling application in the broker operation requests.

MSIDBrokerOperationBrowserNativeMessageRequest.m: Added methods to get caller bundle identifier, team identifier, localized display name, and application info.
MSIDBrowserNativeMessageGetTokenRequest.m: Implemented localizedApplicationInfo method to return client ID and redirect URI or display name.
MSIDBaseBrokerOperationRequest.h: Added properties for caller bundle identifier, team identifier, localized display name, and application info.
MSIDBrokerOperationBrowserNativeMessageRequest.h: Added properties for parent process bundle identifier, team ID, and localized name.
changelog.txt: Updated to include new feature for wrapping ASAuthorizationProviderExtensionAuthorizationRequest methods.

ameyapat · 2024-09-24T17:35:08Z

"### Step-by-Step Plan

Review the Class Definition:
- Check for logical consistency.
- Ensure proper use of Objective-C properties and methods.
- Verify API availability annotations.
Identify Potential Issues:
- Look for any missing or incorrect property attributes.
- Ensure proper memory management practices.
- Check for any redundant or unnecessary code.
Suggest Refactoring:
- Propose improvements for readability and maintainability.
- Suggest modern Objective-C practices if applicable.

Review and Suggestions

Logical and Syntactical Review:
- The class definition and property declarations are syntactically correct.
- The use of @protected for _correlationId is appropriate if subclasses need access.
- The readonly attribute for callerBundleIdentifier and other properties is correct since they are likely set internally and not meant to be modified externally.
Potential Issues:
- The correlationId property is declared twice: once as an instance variable and once as a property. This is not necessary and can be simplified.
- The operation property is declared as class and readonly, which is correct if it is meant to be a class-level constant.
Refactoring Suggestions:
- Remove the redundant instance variable declaration for _correlationId.
- Use modern Objective-C syntax for property declarations.
- Ensure consistency in API availability annotations.

Refactored Code

#import 

NS_ASSUME_NONNULL_BEGIN

@interface MSIDBaseBrokerOperationRequest : NSObject

@property (nonatomic, class, readonly) NSString *operation;
@property (nonatomic, strong) NSUUID *correlationId;

@property (nonatomic, readonly) NSString *callerBundleIdentifier;
@property (nonatomic, readonly) NSString *callerTeamIdentifier API_AVAILABLE(ios(14.0), macos(11.0)) API_UNAVAILABLE(watchos, tvos);
@property (nonatomic, readonly) NSString *localizedCallerDisplayName API_AVAILABLE(ios(14.0), macos(11.0)) API_UNAVAILABLE(watchos, tvos);
@property (nonatomic, readonly) NSString *localizedApplicationInfo API_AVAILABLE(ios(14.0), macos(11.0)) API_UNAVAILABLE(watchos, tvos);

- (nullable NSString *)logInfo;

@end

NS_ASSUME_NONNULL_END

Explanation of Changes

Removed the redundant instance variable _correlationId.
Ensured that correlationId is declared with strong to manage memory correctly.
Kept the readonly properties as they are, assuming they are set internally and not meant to be modified externally.
AI-generated content may be incorrect

ameyapat · 2024-09-24T17:35:09Z

"### Step-by-Step Plan

Review the Class Definition: Check the class definition for any logical or syntactical issues.
Check Property Declarations: Ensure that the property declarations are correct and follow best practices.
Identify Potential Refactoring: Suggest any refactoring that could improve readability or maintainability.

Review and Suggestions

Logical and Syntactical Bugs:
- Property Attributes: The properties are declared with (nonatomic), which is fine for non-thread-safe operations. However, consider if copy should be used for NSString properties to ensure immutability.
- Read-Only Property: The method property is declared as readonly, which is appropriate if it should not be modified outside the class.
Refactoring for Readability:
- Use copy Attribute for NSString Properties: This ensures that the properties are immutable and not affected by changes to the original string.
- Add Documentation: Adding comments to describe the purpose of each property can improve readability.

Refactored Code

#import \"MSIDBrokerOperationRequest.h\"

NS_ASSUME_NONNULL_BEGIN

@interface MSIDBrokerOperationBrowserNativeMessageRequest : MSIDBrokerOperationRequest

@property (nonatomic, copy) NSDictionary *payloadJson; // Consider using copy if the dictionary is mutable
@property (nonatomic, copy) NSString *parentProcessBundleIdentifier; // Use copy for NSString properties
@property (nonatomic, copy) NSString *parentProcessTeamId; // Use copy for NSString properties
@property (nonatomic, copy) NSString *parentProcessLocalizedName; // Use copy for NSString properties

@property (nonatomic, readonly, copy) NSString *method; // Ensure readonly property is also copied

@end

NS_ASSUME_NONNULL_END

Explanation

copy Attribute: Using copy for NSString properties ensures that the property holds an immutable copy of the string, preventing unexpected changes if the original string is mutable.
Comments: Adding comments to describe each property can help future developers understand the purpose and usage of each property.
AI-generated content may be incorrect

ameyapat · 2024-09-24T17:35:10Z

"### Step-by-Step Plan

Review the Class Implementation:
- Check for logical errors in the methods.
- Verify the correctness of the JSON serialization and deserialization.
- Ensure proper error handling.
- Look for any potential memory leaks or misuse of properties.
Identify Potential Refactoring:
- Simplify complex methods.
- Improve readability by breaking down large methods.
- Ensure consistent naming conventions.

Code Review and Suggestions

Logical and Syntactical Bugs

Error Handling in initWithJSONDictionary:
- The method checks for errors but does not always set the error pointer correctly.
JSON Serialization in jsonDictionary:
- The method should handle potential serialization errors.
Use of NSLocalizedString:
- Ensure that the localization keys are defined in the localization files.

Refactoring Suggestions

Simplify initWithJSONDictionary:
- Break down the method into smaller helper methods for readability.
Consistent Error Handling:
- Create a helper method for error creation to avoid repetition.
Use of Constants:
- Define string literals as constants for better maintainability.

Refactored Code

#import \"MSIDBrokerOperationBrowserNativeMessageRequest.h\"
#import \"MSIDJsonSerializableTypes.h\"
#import \"MSIDJsonSerializableFactory.h\"
#import \"NSDictionary+MSIDExtensions.h\"

NSString *const MSID_BROWSER_NATIVE_MESSAGE_REQUEST_PAYLOAD_KEY = @\"payload\";
NSString *const MSID_BROWSER_NATIVE_MESSAGE_REQUEST_METHOD_KEY = @\"method\";

@implementation MSIDBrokerOperationBrowserNativeMessageRequest

+ (void)load
{
    [MSIDJsonSerializableFactory registerClass:self forClassType:self.operation];
}

- (NSString *)method
{
    return self.payloadJson[MSID_BROWSER_NATIVE_MESSAGE_REQUEST_METHOD_KEY];
}

#pragma mark - MSIDBrokerOperationRequest

+ (NSString *)operation
{
    return MSID_JSON_TYPE_OPERATION_REQUEST_BROWSER_NATIVE_MESSAGE;
}

- (NSString *)callerBundleIdentifier
{
    return self.parentProcessBundleIdentifier ?: NSLocalizedString(@\"N/A\", nil);
}

- (NSString *)callerTeamIdentifier
{
    return self.parentProcessTeamId ?: NSLocalizedString(@\"N/A\", nil);
}

- (NSString *)localizedCallerDisplayName
{
    return self.parentProcessLocalizedName ?: NSLocalizedString(@\"N/A\", nil);
}

- (NSString *)localizedApplicationInfo
{
    NSString *method = self.payloadJson[MSID_BROWSER_NATIVE_MESSAGE_REQUEST_METHOD_KEY];
    MSIDBrokerOperationRequest *brokerOperationRequest = [MSIDJsonSerializableFactory createFromJSONDictionary:self.payloadJson
                                                                          classType:method
                                                                  assertKindOfClass:MSIDBaseBrokerOperationRequest.class
                                                                              error:nil];
    
    if (![NSString msidIsStringNilOrBlank:brokerOperationRequest.localizedApplicationInfo])
    {
        return brokerOperationRequest.localizedApplicationInfo;
    }
    
    return NSLocalizedString(@\"N/A\", nil);
}

#pragma mark - MSIDJsonSerializable

- (instancetype)initWithJSONDictionary:(NSDictionary *)json error:(NSError *__autoreleasing*)error
{
    self = [super initWithJSONDictionary:json error:error];
    
    if (self)
    {
        if (![self validatePayloadInJSON:json error:error]) return nil;
        
        _payloadJson = [NSDictionary msidDictionaryFromJSONString:json[MSID_BROWSER_NATIVE_MESSAGE_REQUEST_PAYLOAD_KEY]];
        if (!_payloadJson)
        {
            [self setError:error withMessage:@\"Failed to serialize payload.\"];
            return nil;
        }
        
        if (![self validateMethodInPayload:error]) return nil;
        
        _parentProcessBundleIdentifier = [json msidStringObjectForKey:@\"parent_process_bundle_identifier\"];
        _parentProcessTeamId = [json msidStringObjectForKey:@\"parent_process_teamId\"];
        _parentProcessLocalizedName = [json msidStringObjectForKey:@\"parent_process_localized_name\"];
    }
    
    return self;
}

- (BOOL)validatePayloadInJSON:(NSDictionary *)json error:(NSError *__autoreleasing*)error
{
    return [json msidAssertType:NSString.class ofKey:MSID_BROWSER_NATIVE_MESSAGE_REQUEST_PAYLOAD_KEY required:YES error:error];
}

- (BOOL)validateMethodInPayload:(NSError *__autoreleasing*)error
{
    if (!_payloadJson[MSID_BROWSER_NATIVE_MESSAGE_REQUEST_METHOD_KEY])
    {
        [self setError:error withMessage:@\"Payload is invalid: no 'method' found.\"];
        return NO;
    }
    return YES;
}

- (void)setError:(NSError *__autoreleasing*)error withMessage:(NSString *)message
{
    if (error)
    {
        *error = MSIDCreateError(MSIDErrorDomain, MSIDErrorInternal, message, nil, nil, nil, nil, nil, YES);
    }
}

- (NSDictionary *)jsonDictionary
{
    NSMutableDictionary *json = [[super jsonDictionary] mutableCopy];
    if (!json || !self.payloadJson) return nil;
    
    json[MSID_BROWSER_NATIVE_MESSAGE_REQUEST_PAYLOAD_KEY] = [self.payloadJson msidJSONSerializeWithContext:nil];
    
    return json;
}

@end

Summary of Changes

Error Handling:
- Added helper methods validatePayloadInJSON, validateMethodInPayload, and setError for better error handling and readability.
Refactoring:
- Simplified initWithJSONDictionary by breaking it into smaller methods.
- Ensured consistent error handling throughout the class.
  AI-generated content may be incorrect

ameyapat · 2024-09-24T17:35:11Z

"I can help you with software development-related questions. However, the provided snippet is a changelog entry, not a class or code snippet. If you have a specific class or code snippet you want reviewed, please provide the actual code, and I can assist you with that.
AI-generated content may be incorrect

ameyapat · 2024-09-24T17:35:12Z

"### Plan

Review the class for logical bugs:
- Check for potential issues in the methods and properties.
- Ensure proper error handling and validation.
Review the class for syntactical bugs:
- Ensure correct Objective-C syntax.
Suggest refactoring for readability:
- Identify areas where the code can be made more readable or maintainable.

Review and Suggestions

Logical Bugs

Localized Application Info Method:
- The redirectUri is incorrectly assigned the value of self.clientId instead of self.redirectUri.
Fix:
```
__auto_type redirectUri = self.redirectUri ?: NSLocalizedString(@\"N/A\", nil);
```
Initialization with JSON Dictionary:
- Ensure homeAccountId is validated before using it to create an MSIDAccountIdentifier.
- Properly handle the case where authorityString is invalid.
Fix:
```
if (homeAccountId != nil && ![MSIDAccountIdentifier isAccountIdValid:homeAccountId error:error]) return nil;
```

Syntactical Bugs

No syntactical bugs were found.

Refactoring for Readability

Localized Application Info Method:

Use early returns to reduce nesting.

Refactored Code:

- (NSString *)localizedApplicationInfo
{
    if ([NSString msidIsStringNilOrBlank:self.clientId] && [NSString msidIsStringNilOrBlank:self.redirectUri])
    {
        return self.localizedCallerDisplayName;
    }

    __auto_type clientId = self.clientId ?: NSLocalizedString(@\"N/A\", nil);
    __auto_type redirectUri = self.redirectUri ?: NSLocalizedString(@\"N/A\", nil);

    NSString *clientIdKey = NSLocalizedString(@\"Client ID\", nil);
    NSString *redirectUriKey = NSLocalizedString(@\"Redirect URI\", nil);

    return [NSString stringWithFormat:@\"%@: %@ %@: %@\", clientIdKey, clientId, redirectUriKey, redirectUri];
}

Initialization with JSON Dictionary:

Extract validation logic into helper methods for better readability.

Refactored Code:

- (instancetype)initWithJSONDictionary:(NSDictionary *)json error:(NSError *__autoreleasing*)error
{
    self = [super initWithJSONDictionary:json error:error];
    if (!self) return nil;

    if (![self validateRequestJson:json error:error]) return nil;

    NSDictionary *requestJson = json[MSID_BROWSER_NATIVE_MESSAGE_REQUEST_KEY];
    _loginHint = [requestJson msidStringObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_LOGIN_HINT_KEY];
    NSString *homeAccountId = [requestJson msidStringObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_ACCOUNT_ID_KEY];

    if (homeAccountId || _loginHint)
    {
        _accountId = [[MSIDAccountIdentifier alloc] initWithDisplayableId:_loginHint homeAccountId:homeAccountId];
    }

    _clientId = requestJson[MSID_BROWSER_NATIVE_MESSAGE_CLIENT_ID_KEY];
    _authority = [self createAuthorityFromString:[requestJson msidStringObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_AUTHORITY_KEY] error:error];
    if (!_authority) return nil;

    _scopes = requestJson[MSID_BROWSER_NATIVE_MESSAGE_SCOPE_KEY];
    _redirectUri = requestJson[MSID_BROWSER_NATIVE_MESSAGE_REDIRECT_URI_KEY];
    _prompt = MSIDPromptTypeFromString([requestJson msidStringObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_PROMPT_KEY]);
    _nonce = [requestJson msidStringObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_NONCE_KEY];
    _isSts = [requestJson msidBoolObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_IS_STS_KEY];
    _state = [requestJson msidStringObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_STATE_KEY];
    _instanceAware = [requestJson msidBoolObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_INSTANCE_AWARE_KEY];
    _extraParameters = requestJson[MSID_BROWSER_NATIVE_MESSAGE_EXTRA_PARAMETERS_KEY];
    _correlationId = [self createCorrelationIdFromString:[requestJson msidStringObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_CORRELATION_KEY]];
    _platformSequence = [requestJson msidStringObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_PLATFORM_SEQUENCE_KEY];
    _canShowUI = [requestJson msidBoolObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_CAN_SHOW_UI_KEY] ?: YES;

    return self;
}

- (BOOL)validateRequestJson:(NSDictionary *)json error:(NSError *__autoreleasing*)error
{
    return [json msidAssertType:NSDictionary.class ofKey:MSID_BROWSER_NATIVE_MESSAGE_REQUEST_KEY required:YES error:error];
}

- (MSIDAADAuthority *)createAuthorityFromString:(NSString *)authorityString error:(NSError *__autoreleasing*)error
{
    if (!authorityString) return nil;

    NSError *localError;
    MSIDAADAuthority *authority = [[MSIDAADAuthority alloc] initWithURL:[NSURL URLWithString:authorityString] rawTenant:nil context:nil error:&localError];

    if (!authority && localError)
    {
        MSID_LOG_WITH_CTX_PII(MSIDLogLevelError, nil, @\"Non AAD authorities are not supported in broker - %@\", MSID_PII_LOG_MASKABLE(localError));
        if (error) *error = localError;
    }

    return authority;
}

- (NSUUID *)createCorrelationIdFromString:(NSString *)uuidString
{
    return uuidString ? [[NSUUID alloc] initWithUUIDString:uuidString] : [NSUUID UUID];
}

Final Code

#import \"MSIDBrowserNativeMessageGetTokenRequest.h\"
#import \"MSIDJsonSerializableFactory.h\"
#import \"MSIDAADAuthority.h\"
#import \"MSIDAccountIdentifier.h\"
#import \"MSIDConstants.h\"
#import \"MSIDPromptType_Internal.h\"

NSString *const MSID_BROWSER_NATIVE_MESSAGE_CORRELATION_KEY = @\"correlationId\";
NSString *const MSID_BROWSER_NATIVE_MESSAGE_CLIENT_ID_KEY = @\"clientId\";
NSString *const MSID_BROWSER_NATIVE_MESSAGE_AUTHORITY_KEY = @\"authority\";
NSString *const MSID_BROWSER_NATIVE_MESSAGE_SCOPE_KEY = @\"scope\";
NSString *const MSID_BROWSER_NATIVE_MESSAGE_REDIRECT_URI_KEY = @\"redirectUri\";
NSString *const MSID_BROWSER_NATIVE_MESSAGE_PROMPT_KEY = @\"prompt\";
NSString *const MSID_BROWSER_NATIVE_MESSAGE_NONCE_KEY = @\"nonce\";
NSString *const MSID_BROWSER_NATIVE_MESSAGE_IS_STS_KEY = @\"isSts\";
NSString *const MSID_BROWSER_NATIVE_MESSAGE_STATE_KEY = @\"state\";
NSString *const MSID_BROWSER_NATIVE_MESSAGE_LOGIN_HINT_KEY = @\"loginHint\";
NSString *const MSID_BROWSER_NATIVE_MESSAGE_INSTANCE_AWARE_KEY = @\"instance_aware\";
NSString *const MSID_BROWSER_NATIVE_MESSAGE_EXTRA_PARAMETERS_KEY = @\"extraParameters\";
NSString *const MSID_BROWSER_NATIVE_MESSAGE_REQUEST_KEY = @\"request\";
NSString *const MSID_BROWSER_NATIVE_MESSAGE_PLATFORM_SEQUENCE_KEY = @\"x-client-xtra-sku\";
NSString *const MSID_BROWSER_NATIVE_MESSAGE_CAN_SHOW_UI_KEY = @\"canShowUI\";

@implementation MSIDBrowserNativeMessageGetTokenRequest

+ (void)load
{
    [MSIDJsonSerializableFactory registerClass:self forClassType:self.operation];
}

+ (NSString *)operation
{
    return @\"GetToken\";
}

#pragma mark - MSIDBrokerOperationRequest

- (NSString *)localizedApplicationInfo
{
    if ([NSString msidIsStringNilOrBlank:self.clientId] && [NSString msidIsStringNilOrBlank:self.redirectUri])
    {
        return self.localizedCallerDisplayName;
    }

    __auto_type clientId = self.clientId ?: NSLocalizedString(@\"N/A\", nil);
    __auto_type redirectUri = self.redirectUri ?: NSLocalizedString(@\"N/A\", nil);

    NSString *clientIdKey = NSLocalizedString(@\"Client ID\", nil);
    NSString *redirectUriKey = NSLocalizedString(@\"Redirect URI\", nil);

    return [NSString stringWithFormat:@\"%@: %@ %@: %@\", clientIdKey, clientId, redirectUriKey, redirectUri];
}

#pragma mark - MSIDJsonSerializable

- (instancetype)initWithJSONDictionary:(NSDictionary *)json error:(NSError *__autoreleasing*)error
{
    self = [super initWithJSONDictionary:json error:error];
    if (!self) return nil;

    if (![self validateRequestJson:json error:error]) return nil;

    NSDictionary *requestJson = json[MSID_BROWSER_NATIVE_MESSAGE_REQUEST_KEY];
    _loginHint = [requestJson msidStringObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_LOGIN_HINT_KEY];
    NSString *homeAccountId = [requestJson msidStringObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_ACCOUNT_ID_KEY];

    if (homeAccountId || _loginHint)
    {
        _accountId = [[MSIDAccountIdentifier alloc] initWithDisplayableId:_loginHint homeAccountId:homeAccountId];
    }

    _clientId = requestJson[MSID_BROWSER_NATIVE_MESSAGE_CLIENT_ID_KEY];
    _authority = [self createAuthorityFromString:[requestJson msidStringObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_AUTHORITY_KEY] error:error];
    if (!_authority) return nil;

    _scopes = requestJson[MSID_BROWSER_NATIVE_MESSAGE_SCOPE_KEY];
    _redirectUri = requestJson[MSID_BROWSER_NATIVE_MESSAGE_REDIRECT_URI_KEY];
    _prompt = MSIDPromptTypeFromString([requestJson msidStringObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_PROMPT_KEY]);
    _nonce = [requestJson msidStringObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_NONCE_KEY];
    _isSts = [requestJson msidBoolObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_IS_STS_KEY];
    _state = [requestJson msidStringObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_STATE_KEY];
    _instanceAware = [requestJson msidBoolObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_INSTANCE_AWARE_KEY];
    _extraParameters = requestJson[MSID_BROWSER_NATIVE_MESSAGE_EXTRA_PARAMETERS_KEY];
    _correlationId = [self createCorrelationIdFromString:[requestJson msidStringObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_CORRELATION_KEY]];
    _platformSequence = [requestJson msidStringObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_PLATFORM_SEQUENCE_KEY];
    _canShowUI = [requestJson msidBoolObjectForKey:MSID_BROWSER_NATIVE_MESSAGE_CAN_SHOW_UI_KEY] ?: YES;

    return self;
}

- (BOOL)validateRequestJson:(NSDictionary *)json error:(NSError *__autoreleasing*)error
{
    return [json msidAssertType:NSDictionary.class ofKey:MSID_BROWSER_NATIVE_MESSAGE_REQUEST_KEY required:YES error:error];
}

- (MSIDAADAuthority *)createAuthorityFromString:(NSString *)authorityString error:(NSError *__autoreleasing*)error
{
    if (!authorityString) return nil;

    NSError *localError;
    MSIDAADAuthority *authority = [[MSIDAADAuthority alloc] initWithURL:[NSURL URLWithString:authorityString] rawTenant:nil context:nil error:&localError];

    if (!authority && localError)
    {
        MSID_LOG_WITH_CTX_PII(MSIDLogLevelError, nil, @\"Non AAD authorities are not supported in broker - %@\", MSID_PII_LOG_MASKABLE(localError));
        if (error) *error = localError;
    }

    return authority;
}

- (NSUUID *)createCorrelationIdFromString:(NSString *)uuidString
{
    return uuidString ? [[NSUUID alloc] initWithUUIDString:uuidString] : [NSUUID UUID];
}

- (NSDictionary *)jsonDictionary
{
    @throw MSIDException(MSIDGenericException, @\"Not implemented.\", nil);
}

@end

AI-generated content may be incorrect

ameyapat · 2024-09-26T21:31:02Z

Cloned PR for sedemche/wrap_calling_app_bundle_id

AI description (iteration 1)

PR Classification

New feature

PR Summary

This pull request adds support for handling parent process information in MSIDBrokerOperationBrowserNativeMessageRequest and related classes.

MSIDBrokerOperationBrowserNativeMessageRequest.m: Added properties and methods to handle parent process information.
MSIDBrokerOperationBrowserNativeMessageRequestTests.m: Added tests for new properties and methods.
MSIDBrowserNativeMessageGetTokenRequest.m: Implemented localizedApplicationInfo method.
MSIDBaseBrokerOperationRequest.h: Added new properties for caller information.
Updated changelog.txt to reflect the new feature.

ameyapat · 2024-09-26T21:31:03Z

"Added 1 Inline Code Review comment(s) at the following location(s):

/msal-from-github/MSAL/IdentityCore/IdentityCore/src/broker_operation/request/browser_native_message_request/MSIDBrokerOperationBrowserNativeMessageRequest.h at Lines 33-35

AI-generated content may be incorrect
"

ameyapat · 2024-09-26T21:31:04Z

"AI code review (iteration 1)
It might be better to ensure that the properties parentProcessBundleIdentifier, parentProcessTeamId, and parentProcessLocalizedName are marked as copy instead of strong to prevent potential issues with mutability of NSString objects. When properties of type NSString are declared without copy, they can inadvertently become mutable if assigned a mutable string, which can lead to unexpected behaviors if the string is altered elsewhere in the code. Using copy ensures that a copy of the string is used, maintaining the immutability of the property.

Here is the suggested code:

@property (nonatomic, copy) NSString *parentProcessBundleIdentifier;
@property (nonatomic, copy) NSString *parentProcessTeamId;
@property (nonatomic, copy) NSString *parentProcessLocalizedName;

This comment refers to line 33 to 35 in the new file.
AI-generated content may be incorrect

ameyapat · 2024-09-27T18:27:39Z

Cloned PR for sedemche/wrap_calling_app_bundle_id

AI description (iteration 1)

PR Classification

New feature

PR Summary

This pull request adds support for handling parent process information in broker operation requests.

MSIDBrokerOperationBrowserNativeMessageRequest.m: Added properties and methods to handle parent process bundle identifier, team ID, and localized name.
MSIDBrokerOperationBrowserNativeMessageRequestTests.m: Added tests for initializing with and without parent process information.
MSIDBrowserNativeMessageGetTokenRequest.m: Implemented localizedApplicationInfo to return client ID and redirect URI.
MSIDBaseBrokerOperationRequest.h: Added new properties for caller bundle identifier, team identifier, and localized display name.
changelog.txt: Updated changelog to reflect the new feature.

ameyapat · 2024-09-27T18:27:40Z

"Added 1 Inline Code Review comment(s) at the following location(s):

/msal-from-github/MSAL/IdentityCore/IdentityCore/src/broker_operation/request/browser_native_message_request/MSIDBrokerOperationBrowserNativeMessageRequest.h at Lines 33-35

AI-generated content may be incorrect
"

ameyapat · 2024-09-27T18:27:41Z

"AI code review (iteration 1)
It might be better to ensure that the properties parentProcessBundleIdentifier, parentProcessTeamId, and parentProcessLocalizedName are marked as copy instead of strong (which is the default). This is particularly important for classes like NSString that have mutable subclasses (NSMutableString). Using copy ensures that the strings cannot be altered after they are set, which is crucial for maintaining the integrity of these properties throughout the lifecycle of the object.

Here is the suggested code:

@property (nonatomic, copy) NSString *parentProcessBundleIdentifier;
@property (nonatomic, copy) NSString *parentProcessTeamId;
@property (nonatomic, copy) NSString *parentProcessLocalizedName;

This comment refers to line 33 to 35 in the new file.
AI-generated content may be incorrect

modified: MSAL/IdentityCore

a19d1e6

antrix1989 requested a review from a team as a code owner September 24, 2024 17:30

antrix1989 requested review from Veena11 and antonioalwan September 24, 2024 17:30

azure-pipelines bot reviewed Sep 24, 2024

View reviewed changes

MSAL/IdentityCore Show resolved Hide resolved

modified: MSAL/IdentityCore

a038fa9

modified: MSAL/IdentityCore

c7130ce

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wrap ASAuthorizationProviderExtensionAuthorizationRequest methods #2347

Wrap ASAuthorizationProviderExtensionAuthorizationRequest methods #2347

antrix1989 commented Sep 24, 2024 •

edited

Loading

ameyapat commented Sep 24, 2024

ameyapat commented Sep 24, 2024

ameyapat commented Sep 24, 2024

ameyapat commented Sep 24, 2024

ameyapat commented Sep 24, 2024

ameyapat commented Sep 24, 2024

ameyapat commented Sep 26, 2024

ameyapat commented Sep 26, 2024

ameyapat commented Sep 26, 2024

ameyapat commented Sep 27, 2024

ameyapat commented Sep 27, 2024

ameyapat commented Sep 27, 2024

Wrap ASAuthorizationProviderExtensionAuthorizationRequest methods #2347

Are you sure you want to change the base?

Wrap ASAuthorizationProviderExtensionAuthorizationRequest methods #2347

Conversation

antrix1989 commented Sep 24, 2024 • edited Loading

Proposed changes

Type of change

Risk

Additional information

ameyapat commented Sep 24, 2024

AI description (iteration 1)

PR Classification

PR Summary

ameyapat commented Sep 24, 2024

Review and Suggestions

Refactored Code

Explanation of Changes

ameyapat commented Sep 24, 2024

Review and Suggestions

Refactored Code

Explanation

ameyapat commented Sep 24, 2024

Code Review and Suggestions

Logical and Syntactical Bugs

Refactoring Suggestions

Refactored Code

Summary of Changes

ameyapat commented Sep 24, 2024

ameyapat commented Sep 24, 2024

Review and Suggestions

Logical Bugs

Syntactical Bugs

Refactoring for Readability

Final Code

ameyapat commented Sep 26, 2024

AI description (iteration 1)

PR Classification

PR Summary

ameyapat commented Sep 26, 2024

ameyapat commented Sep 26, 2024

ameyapat commented Sep 27, 2024

AI description (iteration 1)

PR Classification

PR Summary

ameyapat commented Sep 27, 2024

ameyapat commented Sep 27, 2024

antrix1989 commented Sep 24, 2024 •

edited

Loading