-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add GDS::SSO::PermissionDeniedError to excluded exceptions list #366
Add GDS::SSO::PermissionDeniedError to excluded exceptions list #366
Conversation
844bfaf
to
805245b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change looks good. It would be good if the commit explained why we don't want this send to error to Sentry.
Would you be able to add an entry to the changelog too?
- We do not need to capture these errors generated from gds-sso. They do not indicate that something went wrong with the application; rather, they indicate that a user does not have permissions to access a resource.
805245b
to
1795912
Compare
Thanks for the review @kevindew , I updated the things you suggested, I followed the pattern for the changelog message and version (I noticed it does not have unreleased section), should I update the gem's version as part of this PR? |
We normally just add an unreleased section each time a new one is added. It's no biggie though to update the gem version as part of this PR since this is a relatively slow moving gem. |
1795912
to
95fd7a5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Lets just check with the owning team: Platform Security Reliability team before merging. Would you mind dropping them a message on Slack (#govuk-platform-security-reliability-team)
Related to: alphagov/gds-sso#300
trello card: https://trello.com/c/qb1qpwnr/1369-additional-functionality-to-gds-sso-gem-for-intercepting-401s-and-providing-a-route-constraint