4574 add session check redirection #4636
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
implimented modal in header.html and changed mixin to use session check.
anagradova
added
eng
helpdesk
15 tasks
|
Terraform plan for meta No changes. Your infrastructure matches the configuration.✅ Plan applied in Deploy to Development and Management Environment #927 |
|
Terraform plan for dev Plan: 1 to add, 0 to change, 1 to destroy.Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
-/+ destroy and then create replacement
Terraform will perform the following actions:
# module.dev.module.cors.null_resource.cors_header must be replaced
-/+ resource "null_resource" "cors_header" {
!~ id = "*******************" -> (known after apply)
!~ triggers = { # forces replacement
!~ "always_run" = "2025-01-24T17:30:29Z" -> (known after apply)
}
}
Plan: 1 to add, 0 to change, 1 to destroy.✅ Plan applied in Deploy to Development and Management Environment #927 |
tests to handle new test case
gsa-jrothacker
previously approved these changes
Jan 24, 2025
| if not hasattr(request, "user"): | ||
| raise PermissionDenied(PERMISSION_DENIED_MESSAGE) | ||
| if not request.user: | ||
| if not hasattr(request, "user") or not request.user: |
Comment on lines
+85
to
+86
| self.assertTemplateUsed(response, "home.html") | ||
| self.assertTrue(response.context["session_expired"]) |
There was a problem hiding this comment.
I think this is going to be a much better experience, the user will now see a nice page instead of the 403. Great work.
| @@ -217,7 +218,7 @@ def test_anonymous_raises(self): | |||
|
|
|||
| view = self.ViewStub() | |||
| self.assertRaises( | |||
| PermissionDenied, view.dispatch, request, report_id="not-logged-in" | |||
| SessionExpiredException, view.dispatch, request, report_id="not-logged-in" | |||
There was a problem hiding this comment.
I was worried we didn't have any remaining tests for PermissionDenied... but then realized I forgot to expand the diff. Again, I think giving more specific exceptions between a Permissions problem and a Session Expired is going be nice.
backend/config/context_processors.py
Outdated
Comment on lines
66
to
68
| def format_time(seconds): | ||
| minutes, seconds = divmod(seconds, 60) | ||
| return f"{minutes} minutes, {seconds} seconds" |
There was a problem hiding this comment.
I didn't see this used anywhere else (I did a cmd+f to verify).
Minimum allowed line rate is |
gsa-jrothacker
approved these changes
Jan 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link to issue #4574
This feature/fix is designed to solve a usability issue that certain users were experiencing as referenced in this ticket
To test this feature, the user should begin an audit submission and continue to the upload SAC step. Wait for the session to expire with your current login and then attempt to upload the file.
Upon submission, with an expired session, the user should be presented with this screen allowing them to log back in or proceed to the home page.
If the user goes back through the login cycle, the callback URL from login.gov should place them back on the upload submission step where their session expired.
I have created two modals, one for session expiration (session-expired-modal) and one for session timeout warning (session-warning-modal). Each modal has a javascript snippet with it to trigger the modal due to the USWDS framework design requiring a or tag attribute and action to trigger the modal. The javascript snippet creates and simulates a click on an tag for the modal. Due to the nature of how we manage sessions and interaction, the warning modal should never trigger, but I have left the code in place in the nav_primary.html template should we wish to use it in the future.
The session expiration flag is set in mixin.py during the auth check, but could be triggered by anything that can pass the trigger to the template context.
NOTE: A quick way to trigger this for testing is to login, reach the upload step, open a new browser window from the site, logout, and then attempt to upload the file. This should trigger the modal.
PR Checklist: Reviewer
make docker-clean; make docker-first-run && docker compose up; then rundocker compose exec web /bin/bash -c "python manage.py test"Pre Merge Checklist: Merger
-/+ resource "null_resource" "cors_header"should be destroying and recreating its self and~ resource "cloudfoundry_app" "clamav_api"might be updating itssha256for thefac-file-scannerandfac-av-${ENV}by default.main.