Skip to content

Modify abilities to prevent link sharing #90

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

Merged

Conversation

create-issue-branch[bot]
Copy link
Contributor

@create-issue-branch create-issue-branch bot commented Nov 25, 2022

What's Changed?

  • Updated ability file to allow users to only read projects with no user_id (i.e. RPF starter/example projects) and projects owned by the user
  • Added ability_spec tests
  • Updated project show_spec tests to reflect the above changes
  • Fix error thrown when a project has no image_list

closes #89

@raspberrypiherokubot raspberrypiherokubot temporarily deployed to editor-api-p-issues-89--xayl0g November 25, 2022 10:37 Inactive
@raspberrypigithubbot
Copy link

@raspberrypigithubbot
Copy link

@raspberrypiherokubot raspberrypiherokubot temporarily deployed to editor-api-p-issues-89--qragio November 25, 2022 10:40 Inactive
@loiswells97 loiswells97 marked this pull request as ready for review December 1, 2022 12:41
@raspberrypigithubbot
Copy link

it 'returns forbidden response' do
get "/api/projects/#{another_project.identifier}"

expect(response).to have_http_status(:forbidden)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd rather this was :not_found (no immediate information leakage about whether a project exists)

Timing attacks would still give it away though, as less work is (probably) being done if no project is found.

Happy for this to be pushed into a new issue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interestingly, we now have separate design for these two cases, so maybe this is part of a wider discussion...

Copy link
Contributor

@patch0 patch0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment about forbidden vs not found, but happy to merge

@loiswells97 loiswells97 merged commit 0c6bd2c into main Dec 5, 2022
@loiswells97 loiswells97 deleted the issues/89-Modify_abilities_to_prevent_link_sharing branch December 5, 2022 17:35
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.

Modify abilities to prevent link sharing
4 participants