Memcache bypasses access restrictions

[miguel]

With memcache active, the function get_entity($guid) can return objects not viewable by the current user, as long as the entity is found in memcache.

If it is not found, usual access restrictions work, because they are checked inside get_entity_as_row. This is with Elgg 1.7.7.

[mrclay]

For persistent key-value stores like Memcache, the entity and some information about its access should be stored separately. E.g.:

Add a function:

/**
 * Can a user access an entity?
 * @return bool
 */
function elgg_user_can_access_entity($user_guid, $entity_guid) {
  // quick as possible DB check (don't hydrate the entity)
}

Each time an entity is stored in Memcached, this key/value must be stored separately:
"u{$current_user_guid}-sees-e{$entity_guid}" = true

When get_entity() finds an entity in Memcached, it must also check this "sees" key for the current user. If missing, it must call elgg_user_can_access_entity(), populating the "sees" key if it returns true. I don't suggest saving false because it's probably not a common case.

[ewinslow]

You mean like this: ​http://reference.elgg.org/engine_2lib_2access_8php.html#af5f7246dfad0743a6568820d19858188?

[mrclay]

@evan, yep!

The remaining problem is that, when an entity's access changes, I see no way to invalidate the per-user visibility caches.

Perhaps a better design would store the access info more like the DB:

KeyValueStore.set("e123-acl") = 234 // or "public"/"private"/"loggedIn"

...as well as caching the ACL member lists:

KeyValueStore.set("acl234-guids") = [35,235,643]

Now you still have a straightforward way to see if user 643 can see entity 12, and better, when either entities or ACLs change, we can immediately reflect this in the key/value store without managing tons of keys or waiting for values to expire.

[cash]

I prefer the later solution (storing access information in memcache).

[ebogdanov]

We are using Elgg, and have the same issues,
If it is possible, I'll vote to fix that issue in upcoming Elgg release. It is almost critical, since memcache is unusable with that defect.

[coldtrick]

Pull request by Steve

​https://github.com/Elgg/Elgg/pull/399

please apply!!!!!!!!!!!!!!!

[Steve Clay]

Fixes #3018: Checks DB for access before using memcache-stored entity (suggested by Jerôme Bakker)

Changeset: 5c069bbca76fb8519548b2c8df2b9b6f3b3885b0

[Evan Winslow]

Merge pull request #399 from mrclay/3018-memcache-access

Fixes #3018: Checks DB for access before using memcache-stored entity

Changeset: 84bf48937a7b294649701c1d6f4553dc0aeb4cd7