Add constraint and config options #300
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
AlexAvlonitis
changed the title
kevindew
reviewed
Apr 23, 2024
kevindew
reviewed
Apr 23, 2024
AlexAvlonitis
force-pushed
the
add-constraints-and-config-options
branch
6 times, most recently
from
0aa3a1a
to
e33d033
Compare
kevindew
reviewed
Apr 24, 2024
There was a problem hiding this comment.
Great stuff Alex, this looks really close. Once we've got my comments resolved we'll seek final review from the owning team.
The only thing I notice is missing is the CHANGELOG entry - we can do that before we release the gem, it'll just go under the "Unreleased" header.
AlexAvlonitis
force-pushed
the
add-constraints-and-config-options
branch
4 times, most recently
from
4850338
to
8bdc6f6
Compare
|
Great stuff. This all looks good to me. Let's see if the #govuk-publishing-platform team are happy. I'll drop them a ping. |
- This config allows apps to change the default behaviour of intercepting 401 requests, by including an initialiser:
`GDS::SSO.config { |config| config.intercept_401_responses = false }`.
For example apps that also include basic http auth would need this option.
AlexAvlonitis
force-pushed
the
add-constraints-and-config-options
branch
from
8bdc6f6
to
6393469
Compare
- Adds a class so that consumer apps can easily add a permission based constraint.
AlexAvlonitis
force-pushed
the
add-constraints-and-config-options
branch
from
6393469
to
dbaa00a
Compare
JonathanHallam
approved these changes
Apr 26, 2024
AlexAvlonitis
force-pushed
the
add-constraints-and-config-options
branch
from
dbaa00a
to
e128c44
Compare
- Adds a warning log message that the existing PermissionDeniedException should be depracated on a new major release, but it will still work for current apps. The reason is that the new error GDS::SSO::PermissionDeniedError has been created to replace the existing GDS::SSO::ControllerMethods::PermissionDeniedException. Also the new error has been placed on the outer layer of the name-spacing (GDS::SSO), which is more generic than the existing GDS::SSO::ControllerMethods one. And it follows the Rails conventions that the error class names should end with ...Error, which is more indicative that they inherit from StandardError rather than the top level error class Exception. - Updates the Railtie so any rescue errors from PermissionDeniedError will be transformed to :forbidden Rails specific ones.
- All the logic that checks the current_user permissions has been extracted to a separate class, because it will need to be re-used from different parts of the code.
AlexAvlonitis
force-pushed
the
add-constraints-and-config-options
branch
from
e128c44
to
077e35e
Compare
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.
Adds config for warden to not intercept 401 requests
This config allows apps to change the default behaviour of intercepting 401 requests, by including an initialiser:
GDS::SSO.config { |config| config.intercept_401_responses = false }.For example apps that also include basic http auth would probably need this option.
Adds authorised user constraint so that consumer apps can easily add a permission based constraint.
Add PermissionDeniedError error class under GDS::SSO namespace and a warning log message that the existing PermissionDeniedException should be deprecated on a new major release.
Extract the user authorisation logic into a separate class and Re-use authorisation logic in the controller_methods class
more info : https://trello.com/c/qb1qpwnr/1369-additional-functionality-to-gds-sso-gem-for-intercepting-401s-and-providing-a-route-constraint