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.

Sign up for GitHub

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

Jump to bottom

AP-5244: change access_denied status to 403 #7468

Closed
wants to merge 1 commit into from
Closed

AP-5244: change access_denied status to 403 #7468

wants to merge 1 commit into from

Conversation

jsugarman
Copy link
Contributor

What

Change status code for the access_denied page redirect to 403

Link to story

So the response status matches the intended behaviour of being
forbidden/403 for the current user.

Checklist

Before you ask people to review this PR:

  • Tests and rubocop should be passing: bundle exec rake
  • Github should not be reporting conflicts; you should have recently run git rebase main.
  • The standards in the Git Workflow document on Confluence should be followed
  • There should be no unnecessary whitespace changes. These make diffs harder to read and conflicts more likely.
  • The PR description should say what you changed and why, with a link to the JIRA story.
  • You should have looked at the diff against main and ensured that nothing unexpected is included in your changes.
  • You should have checked that the commit messages say why the change was made.

@jsugarman
AP-5524: respond with status 403/forbidden for :access_denied
624e4f1 
Currently this is handle by coding a redirect to the access_denied
page with an ok/200 status. Logs therefore do not represent the expected
status for such a page.

However, ideally we should not be redirecting at all, but capturing the
403 through proper authorization via suing something like
`rescue_from Pundit::NotAuthorizedError`.
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@jsugarman jsugarman changed the base branch from main to ap-5524/status-404-for-page-not-found November 22, 2024 13:30
@jsugarman jsugarman changed the base branch from ap-5524/status-404-for-page-not-found to ap-5524/use-rails-404-handling November 22, 2024 13:30
@jsugarman jsugarman closed this Nov 22, 2024
@jsugarman jsugarman deleted the ap-5244/more-railsey-error-handling branch November 22, 2024 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Discussion
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@jsugarman