Security prevails

[jdelporte]

6.0.0.RC1 introduced a correction on an inconsistency:

Association loading isn't to be affected by scoping consistently whether preloaded / eager loaded or not, with the exception of unscoped.

Before:

Post.where("1=0").scoping do
  Comment.find(1).post                   # => nil
  Comment.preload(:post).find(1).post    # => #<Post id: 1, ...>
  Comment.eager_load(:post).find(1).post # => #<Post id: 1, ...>
end

After:

Post.where("1=0").scoping do
  Comment.find(1).post                   # => #<Post id: 1, ...>
  Comment.preload(:post).find(1).post    # => #<Post id: 1, ...>
  Comment.eager_load(:post).find(1).post # => #<Post id: 1, ...>
end

Fixes #34638, #35398.

"Scoping" is a corner stone in the management of security: we lock users within scopes of the data they are allowed to see. Scopes are used as a data jail mechanism. While we agree on the initial problem statement (the behavior was inconsistent), we believe that to preserve good security of the data, the consistency should be restored as follows:

Post.where("1=0").scoping do
  Comment.find(1).post                   # => nil
  Comment.preload(:post).find(1).post    # => nil
  Comment.eager_load(:post).find(1).post # => nil
end

As a consequence, applications that were safe before Rails 6 may now leak information to some unprivileged users.

We can also note that some inconsistency are present with the current version:

Comment.where('1=0').scoping do
  Comment.where(post_id: 1).exists?   # => false
  Post.find(1).comments.exists?       # => true
end

We can provide a PR for Rails 6 (based on 1a58db0, maybe with a less naive approach) following these lines (enforcing the strong scoping rules, not the weak ones). While we don't see when the weak model is relevant (we do not have a use for it), it might be useful to some. So possibly the solution should not be to have either, but both. In which case, we suggest to have a global configuration mechanism to select whether the application is running with the weak scoping rules, or the strong scoping rules.

[jdelporte]

This commit should fixed the issue

[akimd]

"Scoping" is a corner stone in the management of security: we lock users within scopes of the data they are allowed to see. Scopes are used as a segregation mechanism. While we agree on the initial problem statement (the behavior was inconsistent), we believe that to preserve good security of the data, the consistency should be restored as follows:

Post.where("1=0").scoping do
  Comment.find(1).post                   # => nil
  Comment.preload(:post).find(1).post    # => nil
  Comment.eager_load(:post).find(1).post # => nil
end

As a consequence, applications that were safe before Rails 6 may now leak information to some unprivileged users.

We can provide a PR for Rails 6 (based on https://github.com/jdelporte/rails/commit/1a58db06689508889a01e96959db84059c7ebb8d) following these lines (enforcing the strong scoping rules, not the weak ones). While we don't see when the weak model is relevant (we do not have a use for it), it might be useful to some. So possibly the solution should not be to have either, but both. In which case, we suggest to have a global configuration mechanism to select whether the application is running with the weak scoping rules, or the strong scoping rules.