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

Adjudicate substantive changes to the powerbi baseline markdown following the new format #403

Conversation

amart241
Copy link
Collaborator

@amart241 amart241 commented Jun 26, 2023

🗣 Description

Changed formatting and content of PowerBI baseline to match new formatting structure

💭 Motivation and context

Consolidated format and clearer to read and understand layout

🧪 Testing

N/A

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • These code changes follow cisagov code standards.
  • All new and existing tests pass.

✅ Pre-merge checklist

✅ Post-merge checklist

@amart241 amart241 added the baseline-document Issues relating to the text in the baseline documents themselves label Jun 26, 2023
@amart241 amart241 added this to the Emerald milestone Jun 26, 2023
@amart241 amart241 requested review from buidav and ahuynhMITRE June 26, 2023 18:28
@amart241 amart241 self-assigned this Jun 26, 2023
@amart241 amart241 linked an issue Jun 26, 2023 that may be closed by this pull request
12 tasks
@buidav buidav changed the base branch from main to emerald June 26, 2023 18:49
@buidav buidav changed the title 250 adjudicate substantive changes to the powerbi baseline markdown following the new format Adjudicate substantive changes to the powerbi baseline markdown following the new format Jun 26, 2023
@buidav buidav force-pushed the 250-adjudicate-substantive-changes-to-the-powerbi-baseline-markdown branch 2 times, most recently from 0356862 to 225b2b1 Compare June 27, 2023 17:04
Copy link
Collaborator

@buidav buidav left a comment

Choose a reason for hiding this comment

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

some changes

baselines/powerbi.md Outdated Show resolved Hide resolved
baselines/powerbi.md Outdated Show resolved Hide resolved
baselines/powerbi.md Outdated Show resolved Hide resolved
baselines/powerbi.md Outdated Show resolved Hide resolved
baselines/powerbi.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@ahuynhMITRE ahuynhMITRE left a comment

Choose a reason for hiding this comment

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

agree with David's comments and main comment from me was re-adding the "shall" and "should" language into each of the policies.

@schrolla schrolla linked an issue Jul 6, 2023 that may be closed by this pull request
5 tasks
@schrolla schrolla force-pushed the 250-adjudicate-substantive-changes-to-the-powerbi-baseline-markdown branch from 225b2b1 to 3b080c8 Compare July 7, 2023 17:03
@buidav buidav force-pushed the 250-adjudicate-substantive-changes-to-the-powerbi-baseline-markdown branch from 8efcfcb to 4c565f7 Compare July 19, 2023 17:25
@ahuynhMITRE
Copy link
Collaborator

some comments:

  1. Update text following each policy group to the match the guide below:

This section helps reduce security risks related to [Policy Group]. This includes [Scope of policies ] (This sentence is as needed).
Example: "This section helps reduce security risks related to the sharing of files with users external to the agency. This includes guest users, users who use a verification code and users who access an Anyone link."

  1. remove links to admin centers and re-add them as a generic step. This was recently voted on as the current way forward by by the team.

@buidav
Copy link
Collaborator

buidav commented Jul 19, 2023

  1. Update text following each policy group to the match the guide below:
    This section helps reduce security risks related to [Policy Group]. This includes [Scope of policies ] (This sentence is as needed). Example: "This section helps reduce security risks related to the sharing of files with users external to the agency. This includes guest users, users who use a verification code and users who access an Anyone link."

The current rationale format follows what AAD and Defender are using. Would we have to refactor the rationale format for those baselines as well?

@buidav buidav self-requested a review July 19, 2023 18:32
Copy link
Collaborator

@buidav buidav left a comment

Choose a reason for hiding this comment

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

Reviewed and adjudicated my comments directly with Alex.

@amart241
Copy link
Collaborator Author

I have removed a total of 3 policies from the Power BI baseline. I most recently removed the logging related policy because it is inconsistent across the new "fabric" and current Power BI, it also requires and Azure subscription. #315 were the other 2 policies that I removed.

@buidav buidav requested a review from ahuynhMITRE July 19, 2023 19:17
@ahuynhMITRE
Copy link
Collaborator

This comment is tied to the text after each policy group. See policy group 1 below:

"1. Publish to Web
Power BI has a capability to publish reports and content to the web. This capability creates a publicly accessible web URL that does not require authentication or status as an AAD user to view it. While this may be needed for a specific use case or collaboration scenario, it is a best practice to keep this setting off by default to prevent unintended and potentially sensitive data exposure.

If it is deemed necessary to make an exception and enable the feature, admins should limit the ability to publish to the web to only specific security groups, instead of allowing the entire agency to publish data to the web."

This would be updated to the following:
"1. Publish to Web

This section helps reduce security risks related to Power Bi's capability to publish to the web. "

This was a note by Ted since a lot of the text will be covered by the rationale and is inconsistent across baselines.

@buidav
Copy link
Collaborator

buidav commented Jul 19, 2023

This comment is tied to the text after each policy group. See policy group 1 below:

"1. Publish to Web Power BI has a capability to publish reports and content to the web. This capability creates a publicly accessible web URL that does not require authentication or status as an AAD user to view it. While this may be needed for a specific use case or collaboration scenario, it is a best practice to keep this setting off by default to prevent unintended and potentially sensitive data exposure.

If it is deemed necessary to make an exception and enable the feature, admins should limit the ability to publish to the web to only specific security groups, instead of allowing the entire agency to publish data to the web."

This would be updated to the following: "1. Publish to Web

This section helps reduce security risks related to Power Bi's capability to publish to the web. "

This was a note by Ted since a lot of the text will be covered by the rationale and is inconsistent across baselines.

Will have to bring this up for group discussion then because I don't remember us have an item/task for standardizing the text at the top level of each policy groups.

@ahuynhMITRE
Copy link
Collaborator

good point, will add as a parking lot item for tomorrow. Probably lost in the many conversations / reviews over the past months and open to whatever works best.

Copy link
Collaborator

@ahuynhMITRE ahuynhMITRE left a comment

Choose a reason for hiding this comment

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

all good after the discussion at the stand up!

@buidav
Copy link
Collaborator

buidav commented Jul 20, 2023

@nanda-katikaneni ready to merge

amart241 added 10 commits July 20, 2023 12:08
updated to newest format, removed should and shall wording, new style for implementation
Addressed comments on pull request for this branch.
additional formatting to the baseline
more formatting and removal of one baseline
more updates
More formatting
more formatting
@nanda-katikaneni
Copy link
Collaborator

@amart241 @buidav - seems there are conflicts for this branch to merge into emerald, can you please update/rebase and resolve conflicts.

@buidav buidav force-pushed the 250-adjudicate-substantive-changes-to-the-powerbi-baseline-markdown branch from f880219 to 15d6b79 Compare July 20, 2023 19:09
@buidav
Copy link
Collaborator

buidav commented Jul 20, 2023

@amart241 @buidav - seems there are conflicts for this branch to merge into emerald, can you please update/rebase and resolve conflicts.

Rebased. There were no conflicts to solve.
Addam mentioned before that GitHub isn't smart enough to figure out the diffs sometimes.

Ready again to merge.

@nanda-katikaneni nanda-katikaneni merged commit f9aaa9e into emerald Jul 20, 2023
@nanda-katikaneni nanda-katikaneni deleted the 250-adjudicate-substantive-changes-to-the-powerbi-baseline-markdown branch July 20, 2023 21:21
crutchfield pushed a commit that referenced this pull request Aug 23, 2023
…wing the new format (#403)

* Update powerbi.md

* Update powerbi.md

* Update powerbi.md

updated to newest format, removed should and shall wording, new style for implementation

* Update powerbi.md

Addressed comments on pull request for this branch.

* Update powerbi.md

additional formatting to the baseline

* Update powerbi.md

more formatting and removal of one baseline

* Update powerbi.md

more updates

* Update powerbi.md

More formatting

* Update powerbi.md

one change

* Update powerbi.md

more formatting
schrolla pushed a commit that referenced this pull request Sep 1, 2023
…wing the new format (#403)

* Update powerbi.md

* Update powerbi.md

* Update powerbi.md

updated to newest format, removed should and shall wording, new style for implementation

* Update powerbi.md

Addressed comments on pull request for this branch.

* Update powerbi.md

additional formatting to the baseline

* Update powerbi.md

more formatting and removal of one baseline

* Update powerbi.md

more updates

* Update powerbi.md

More formatting

* Update powerbi.md

one change

* Update powerbi.md

more formatting
schrolla pushed a commit that referenced this pull request Nov 2, 2023
…wing the new format (#403)

* Update powerbi.md

* Update powerbi.md

* Update powerbi.md

updated to newest format, removed should and shall wording, new style for implementation

* Update powerbi.md

Addressed comments on pull request for this branch.

* Update powerbi.md

additional formatting to the baseline

* Update powerbi.md

more formatting and removal of one baseline

* Update powerbi.md

more updates

* Update powerbi.md

More formatting

* Update powerbi.md

one change

* Update powerbi.md

more formatting
schrolla pushed a commit that referenced this pull request Nov 2, 2023
…wing the new format (#403)

* Update powerbi.md

* Update powerbi.md

* Update powerbi.md

updated to newest format, removed should and shall wording, new style for implementation

* Update powerbi.md

Addressed comments on pull request for this branch.

* Update powerbi.md

additional formatting to the baseline

* Update powerbi.md

more formatting and removal of one baseline

* Update powerbi.md

more updates

* Update powerbi.md

More formatting

* Update powerbi.md

one change

* Update powerbi.md

more formatting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
baseline-document Issues relating to the text in the baseline documents themselves
Projects
None yet
4 participants