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

Non-gov access to information (Challenge Managers) #1390

Closed
32 of 35 tasks
r-bartlett-gsa opened this issue Jul 24, 2024 · 23 comments · Fixed by #1419, #1447 or #1495
Closed
32 of 35 tasks

Non-gov access to information (Challenge Managers) #1390

r-bartlett-gsa opened this issue Jul 24, 2024 · 23 comments · Fixed by #1419, #1447 or #1495

Comments

@r-bartlett-gsa
Copy link
Member

r-bartlett-gsa commented Jul 24, 2024

User story

As a security officer, in order to ensure only users with verified identities have access to controlled or sensitive information, I would like to restrict non-government user access, regardless of assigned user role, to submissions and public solvers data.

Acceptance criteria:

  • The following applies to challenge manager role
    • If user's email address is not ending in .gov or .mil, the user does not have access to any submissions data (PII & CUI)
    • If user's email address is not ending in .gov or .mil, the user does not have access to any public solvers data (PII & CUI)
  • If user is assigned a challenge manager role, but their email address does not end in .gov or .mil, then the originator type designation in security logs is challenge_manager_ng
  • Additional AC added 8/27 - A _ng CM can not submit a challenge for approval
  • Additional AC added 8/27 - a _ng CM can not create a new challenge.

Note: public solvers with non .gov or .mil accounts continue have access to submissions and public solvers data they inputted into challenge.gov

Definition of Done

Doing (dev team)

  • Code complete
  • Code is organized appropriately
  • Any known trade offs are documented in the associated GH issue
  • Code is documented, modules, shared functions, etc.
  • Automated testing has been added or updated in response to changes in this PR
  • The feature is smoke tested to confirm it meets requirements
  • Database changes have been peer reviewed for index changes and performance bottlenecks
  • PR that changes or adds UI
    • include a screenshot of the WAVE report for the altered pages
    • Confirm changes were validated for mobile responsiveness
  • PR approved / Peer reviewed
  • Security scans passed
  • Automate accessibility tests passed
  • Build process and deployment is automated and repeatable
  • Feature toggles if appropriate
  • Deploy to staging
  • Move card to testing column in the board

Staging

  • Accessibility tested (Marni)
    • Keyboard navigation
    • Focus confirmed
    • [ ] Color contrast compliance
    • Screen reader testing
  • Usability testing: mobile and desktop (Tracy or Marni
  • [ ] Cross browser testing - UI rendering is performant on below listed devices/browsers (Tracy or Marni)
    - [ ] Windows/Chrome
    - [ ] Windows/Edge
    - [ ] Mac/Chrome
    - [ ] Mac/Safari
    - [ ] iOS/Safari
  • AC review (Renata)
  • Deploy to production (production-like environment for eval capability) (dev team)
  • Move to production column in the board

Production

  • User and security documentation has been reviewed for necessary updates (Renata/Tracy/Dev team)
  • PO / PM approved (Jarah or Renata)
  • AC is met and it works as expected (Jarah or Renata)
  • Move to done column in the board (Jarah or Renata)
@r-bartlett-gsa
Copy link
Member Author

Challenge.gov PII Types

@r-bartlett-gsa
Copy link
Member Author

Split into two stories

@r-bartlett-gsa r-bartlett-gsa changed the title Non-gov access to information Non-gov access to information (Challenge Managers) Jul 29, 2024
@r-bartlett-gsa r-bartlett-gsa added this to the Sprint 08/13/24 milestone Jul 31, 2024
@kkrug
Copy link
Contributor

kkrug commented Aug 9, 2024

@r-bartlett-gsa @jarahameador Alejandro and I have a question regarding the first piece of Acceptance Criteria. What exactly should the non-gov challenge manager be prevented from seeing? It doesn't look like the the portal shows any PII in each submission outside of what could be attached as part of the submission (which we cannot see the contents of from a coding perspective).

Edit to add answer: Non-gov challenge managers should not be able to view submissions through the portal at all. This includes disabling the 'View Submissions' button and hiding the table of all submissions, as the page with the submissions table is possible to access with a direct URL. These changes will also prevent the challenge manager from downloading submissions.

@jdonis jdonis linked a pull request Aug 27, 2024 that will close this issue
10 tasks
@jdonis
Copy link
Contributor

jdonis commented Sep 10, 2024

New AC's added 8/27 are completed
https://docs.google.com/document/d/1oScGqF9NdV6DLQMqUaqN8tEhnGFSqxi3C8VEt_CPjpc/

The new version is already deployed to Staging

@mhotch24
Copy link
Contributor

@jdonis Please let me know login credentials and URL for testing purposes. Thanks!

@jdonis
Copy link
Contributor

jdonis commented Sep 16, 2024

@jdonis Please let me know login credentials and URL for testing purposes. Thanks!

@mhotch24

https://challenge-portal-staging.app.cloud.gov/sign-in/new

I just activated your GSA account to sign in to Staging you have admin rights, to test as a non-gov you need to sign in using any other email, the next step is to add that account as a challenge manager.

Please let me know if you have any doubts.

@mhotch24
Copy link
Contributor

@jdonis my account is still pending recertification. Can you approve it or do I need to ask someone else?

@jdonis
Copy link
Contributor

jdonis commented Sep 18, 2024

@jdonis my account is still pending recertification. Can you approve it or do I need to ask someone else?

@mhotch24 both accounts done!

@mhotch24
Copy link
Contributor

@jdonis We have a color contrast fail. Please change the table header colors to
'primary-dark' | 'blue-warm-70v' | $theme-color-primary-dark | #1a4480

Image

@TCKapGrp
Copy link

@r-bartlett-gsa , a11y test complete. Per convo, bypassing cross browser testing. Now to you for AC testing.

@r-bartlett-gsa
Copy link
Member Author

r-bartlett-gsa commented Oct 2, 2024

@jdonis / @TCKapGrp
The AC for this user story is not met, and it needs to go back to Doing.
As a non-gov CM I am able to access submissions by entering the URLs of submissions list and details pages:
Image

Image


Can you please share a screenshot of the security log showing the challenge_manager_ng designation.


Does the following AC mean that the non gov challenge manager is not able to submit edits to the challenge?

Additional AC added 8/27 - A _ng CM can not submit a challenge for approval

When I read that AC, I assume it is linked to the AC that does not allow the non-gov CM to create a new challenge and hence is not able to submit the challenge for approval. If that is the case, then the new added AC is met.

However, the message on the portal says this:
Image
And that is not true. As a non-gov CM I was able to edit the challenge, and the challenge edits were submitted. That message is not needed, because it is not accurate.

@r-bartlett-gsa r-bartlett-gsa linked a pull request Oct 15, 2024 that will close this issue
7 tasks
@r-bartlett-gsa
Copy link
Member Author

@kkrug I'm still seeing the same issues on staging:

  • As a non-gov challenge manager, I can access the submissions list
  • As a non-gov challenge manager, I can access submissions details
  • Inaccurate message is displayed on a challenge details page (should be removed)
  • In the security log, there are instances where non-gov challenge manager does not have challenge_manager_ng designation:

Image

@jairoanaya
Copy link
Contributor

@kkrug @r-bartlett-gsa the changes have been implemented to control the access as reported. Please review the following PR #1469

@jairoanaya
Copy link
Contributor

The following message has to be removed as well as discussed on meeting with @r-bartlett-gsa and @kkrug Image

@jairoanaya
Copy link
Contributor

@kkrug PR has been submitted removing the banner.

@r-bartlett-gsa
Copy link
Member Author

@kkrug and @jairoanaya Security logs still show incorrect originator type for the non .gov/.mil email address for the accessing site action. Please update so it would designate challenge_manager_ng for ALL actions completed by a non .gov/.mil challenge manager.

Image

@r-bartlett-gsa
Copy link
Member Author

r-bartlett-gsa commented Nov 21, 2024

Image
@jairoanaya and @kkrug Per Jay's message in slack ^^, the issue with the security logs is now fixed.

However, this introduced a new issue and now the submissions are not accessible to ANY users. Here are the steps:
As a government challenge manager:

  1. View challenge (challenge 1321)
  2. Click on View Submissions
  3. Click on a submission title to view submission details page
  4. An Internal Server Error is displayed
  5. The same happens with ALL challenges

Image

As an admin or super admin:

  1. View challenge (challenge 1321)
  2. Click on View Submissions
  3. The user is sent back to challenge list view and an error "Something went wrong" is displayed
  4. The same happens with ALL challenges

Image

(cc: @jdonis )

@jairoanaya
Copy link
Contributor

jairoanaya commented Nov 25, 2024

HI @r-bartlett-gsa the reported issues have been resolved on this PR #1477 . I tested as super admin and non-gov challenge manager, and everything seems to be working as expected. The solution is available on staging.

@r-bartlett-gsa
Copy link
Member Author

r-bartlett-gsa commented Nov 28, 2024

@jairoanaya / @jdonis I completed my review and everything looks great. Thank you!

Given that this is a significant change, and we ran into several unexpected issues while implementing the changes, I would like to do regression testing. @TCKapGrp Can you please test all user scenarios for all users roles to ensure we are not breaking any previous features before we release this to production.

@TCKapGrp
Copy link

TCKapGrp commented Dec 4, 2024

@r-bartlett-gsa , we are currently in process of regression testing. The spreadsheet is located here: https://docs.google.com/spreadsheets/d/1OPx_HrmYS4AtOjt03eRnwO3GmJ9vWnQEmfcsdtOEVFU/edit?usp=sharing

I'll let you know when we are complete.

@TCKapGrp
Copy link

@r-bartlett-gsa , regression testing complete. https://docs.google.com/spreadsheets/d/1OPx_HrmYS4AtOjt03eRnwO3GmJ9vWnQEmfcsdtOEVFU/edit?usp=sharing

I highlighted two things in this document in yellow for your attention but happy to answer any additional questions:
1 - Remove announcement fails
2 - Send bulletin to followers can NOT be tested in staging

@r-bartlett-gsa
Copy link
Member Author

r-bartlett-gsa commented Dec 13, 2024

@r-bartlett-gsa , regression testing complete. https://docs.google.com/spreadsheets/d/1OPx_HrmYS4AtOjt03eRnwO3GmJ9vWnQEmfcsdtOEVFU/edit?usp=sharing

I highlighted two things in this document in yellow for your attention but happy to answer any additional questions: 1 - Remove announcement fails 2 - Send bulletin to followers can NOT be tested in staging

@TCKapGrp Reviewed and left comments. Let's discuss this.

@TCKapGrp
Copy link

Hi @r-bartlett-gsa , https://docs.google.com/spreadsheets/d/1OPx_HrmYS4AtOjt03eRnwO3GmJ9vWnQEmfcsdtOEVFU/edit?usp=sharing has been updated and I personally retested items to confirm pass. I added more notes AND added a user experience issue I found and tagged you into that cell. Happy to review.

@kkrug kkrug mentioned this issue Dec 30, 2024
11 tasks
@kkrug kkrug linked a pull request Dec 30, 2024 that will close this issue
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment