Skip to content

Updating related resources does not check against the related records #118

@brianswko

Description

@brianswko

When updating resources e.g. PATCH /articles/:id/relationships/comments or PATCH /articles/:id with related resources that the user does not have access to, I would expect either a 403 or 404 response instead of a 2xx.

it do
pending 'TODO: Maybe this actually should be succesful?'
is_expected.to be_not_found
end
is marked as a pending spec with a not found response, but it currently returns a 204 and updates the article with comments that the user does not have access to. edit: this was a problem with the spec, not the code itself

it { is_expected.to be_successful }
explicitly checks that if a user sends a PATCH to an article with comments that the user does not have access to, that a successful response is returned (and though the spec doesn't test it, the article now includes those comments that the user doesn't have access to.

I've confirmed that even when writing a custom ArticlePolicy#replace_comments? method, the comments that are passed in are already limited to those that are in scope by

resource_class.find_by_keys(assoc_value, context: context).map(&:_model)
so the method never has a chance to raise an error or return false.

I believe this is a bug, and that it started with ad9aad2 when related_models_with_context was added and used find_by_keys instead of _model_class.where(...). I'd change it back in a PR but am uncertain what other ramifications that may have.

Thoughts?

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions