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

Improve app authentication #1345

Closed
wants to merge 4 commits into from
Closed

Improve app authentication #1345

wants to merge 4 commits into from

Conversation

csutter
Copy link
Contributor

@csutter csutter commented Jan 23, 2025

This improves various aspects of the authentication logic in this app, including:

  • Adding model tests for the User model
  • Adding request tests for authentication overall
  • Cleaning up how system and request tests deal with setting the GDS SSO test user, and fixing a bug where the test user wasn't reset properly in the after(:all) block
  • Moving authentication into a concern for a tidier ApplicationController
  • Creating and using a Current model with ActiveSupport::CurrentAttributes to help with future auditing features (we have prior art for this pattern in Whitehall)

- Move `ApplicationController#admin_user?` into the model as
  `User#admin?`
- Add some actual tests for `User` model (including the GDS SSO shared
  example suite)
- Instead of setting up authentication based on test type, use RSpec
  shared context
- Ensure test user is actually reset after specs
- Add request spec to ensure all controllers in the app are
  authenticated by default
This uses `ActiveSupport::CurrentAttributes` to track the current user
across the request.
@csutter
Copy link
Contributor Author

csutter commented Jan 27, 2025

Closing in favour of improved PR.

@csutter csutter closed this Jan 27, 2025
@csutter csutter deleted the user-fixes branch January 29, 2025 09:51
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.

1 participant