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

Fix Null Reference Bug in Application Permissions Details #230

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

Raideeen
Copy link

@Raideeen Raideeen commented May 6, 2024

Overview

This pull request addresses a bug in the application details view within PingCastle Cloud. Specifically, when clicking on an application in the "All Applications" summary to view more details, the application failed to load further information if any permission data was null. This was traced back to a lack of null handling in the permissionFormatter function. This fix ensures that all details are displayed correctly, even if some permission data is missing or null.

Purpose

The purpose of this PR is to improve the stability and reliability of the application details view by ensuring that null permission data does not interrupt the user's ability to view application details. This change helps maintain a smooth and error-free user experience in the administration console.

Changes Made

  • permissionFormatter Function: Added a check to handle null values gracefully. If the function encounters null, it now returns "N/A" instead of causing a failure in rendering the details.
  • UI Feedback: Implemented a placeholder text "N/A" to indicate unavailable data, enhancing the clarity of the information presented to the user.

Additional Context

  • This issue was identified during a security diagnostic of a Azure AD Tenant using PingCastle.
  • The permission ID of the undocumented permission is : afdb422a-4b2a-4e07-a708-8ceed48196bf

Testing and Validation

  • Manual Testing: Conducted thorough manual testing to ensure that the application now handles null values without issues. All affected views now render "N/A" where data is missing.

Feedback Request

I would appreciate a review on the implementation of the null handling in the permissionFormatter function. Additionally, feedback on the user-facing text "N/A" would be valuable—should it perhaps be "Error occurred" or another placeholder? Guidance on this choice would be helpful.

Testing and Validation

  • Reproducibility: To reproduce the original issue:

    1. Associate an app with the "TeamsApp.Read.All" permission, which is known to be undocumented on the official Microsoft Graph permissions reference.
    2. Navigate to the application details view in PingCastle Cloud.
    3. Previously, the application details failed to load successfully due to a null permission value. With the fix, the details page now renders successfully, displaying 'N/A' for any null permissions.
  • Manual Testing: Conducted thorough manual testing to ensure that the application now handles null values without issues. All affected views now render "N/A" where data is missing, instead of causing a rendering failure.

How to Validate This Fix

  • Review the updated permissionFormatter function in template/ReportCloudMain.js to confirm proper null handling.
  • Verify changes in the UI components that display the application permissions, particularly in components/ApplicationDetails.js, to ensure that 'N/A' is displayed as intended.
  • Confirm that all automated and manual tests related to this functionality pass without errors.

Thank you for reviewing this pull request!

This commit fixes a bug where the application details failed to load when encountering a null permission value. The issue was identified with permissions that are not documented on the official Microsoft website, such as "TeamsApp.Read.All" (Permission ID: afdb422a-4b2a-4e07-a708-8ceed48196bf).

To handle this, the `permissionFormatter` function now checks for null values and returns 'N/A' to prevent UI rendering failures. This change ensures that the application details page remains functional regardless of the presence of undocumented or null permissions.

Steps to reproduce:
1. Associate an app with the "TeamsApp.Read.All" permission, which is known to be undocumented on the official Microsoft Graph permissions reference.
2. Navigate to the application details view in PingCastle Cloud.
3. Observe that the application details load successfully, displaying 'N/A' for any null permissions.
@vletoux
Copy link
Contributor

vletoux commented Jul 19, 2024

Thanks for providing this PR and analyzing the problem.
I remind that the source code published here is an extract of the private repository so PR management is a challenge.
Accepting the PR for crediting.

@vletoux vletoux merged commit 67c1e82 into netwrix:master Jul 19, 2024
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.

2 participants