{"action":"created","issue":{"url":"https://api.github.com/repos/Pylons/pyramid/issues/3268","repository_url":"https://api.github.com/repos/Pylons/pyramid","labels_url":"https://api.github.com/repos/Pylons/pyramid/issues/3268/labels{/name}","comments_url":"https://api.github.com/repos/Pylons/pyramid/issues/3268/comments","events_url":"https://api.github.com/repos/Pylons/pyramid/issues/3268/events","html_url":"https://github.com/Pylons/pyramid/issues/3268","id":317538367,"number":3268,"title":"Allow context to be passed along from the permission check to the auth callback (groupfinder) and the effective_principals method","user":{"login":"robinharms","id":737216,"avatar_url":"https://avatars2.githubusercontent.com/u/737216?v=4","gravatar_id":"","url":"https://api.github.com/users/robinharms","html_url":"https://github.com/robinharms","followers_url":"https://api.github.com/users/robinharms/followers","following_url":"https://api.github.com/users/robinharms/following{/other_user}","gists_url":"https://api.github.com/users/robinharms/gists{/gist_id}","starred_url":"https://api.github.com/users/robinharms/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/robinharms/subscriptions","organizations_url":"https://api.github.com/users/robinharms/orgs","repos_url":"https://api.github.com/users/robinharms/repos","events_url":"https://api.github.com/users/robinharms/events{/privacy}","received_events_url":"https://api.github.com/users/robinharms/received_events","type":"User","site_admin":false},"labels":[{"id":196519,"url":"https://api.github.com/repos/Pylons/pyramid/labels/feature-request","name":"feature-request","color":"02e10c","default":false},{"id":199084469,"url":"https://api.github.com/repos/Pylons/pyramid/labels/security","name":"security","color":"5319e7","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":6,"created_at":"2018-04-25T09:01:51Z","updated_at":"2018-05-06T10:13:09Z","closed_at":null,"author_association":"NONE","body":"This is an issue regarding permission checks in traversal. It's only a problem in more complex setups where you might have groups stored further down the resource tree, and want to check permissions on something outside of your path.\r\n\r\nHere's an example. I know the groupfinder method here doesn't do what's expected, but that's pretty much the way people solve things with traversal and locally assigned roles. For instance SubstanceD does this.\r\n\r\nhttps://gist.github.com/robinharms/226770283995e447b13d2cf250584b15\r\n\r\nA lot of folks have invented clever hacks to get around this, like Kotti by @dnouri and others:\r\n\r\nhttps://github.com/Kotti/Kotti/blob/master/kotti/security.py\r\n\r\nWhile using context managers and replace the request.context attribute does solve the problem, it would be much better if that wasn't needed.\r\n\r\nProposal:\r\n\r\n* Add context=None as kwarg for AuthenticationPolicy method effective_principals.\r\n* Make sure Reqeust.has_permission passes along the context to the effective_principals method: https://github.com/Pylons/pyramid/blob/5fce6fd72960ed0893b1d77c8db9efd64a5a81ab/pyramid/security.py#L438 (if it was specified)\r\n* Pass along the context (if specified) to the callback (a.k.a 'groupfinder') method. https://github.com/Pylons/pyramid/blob/5fce6fd72960ed0893b1d77c8db9efd64a5a81ab/pyramid/authentication.py#L159\r\n\r\nThis would greatly help with permission checks, and also remove the need for all clever hacks in more complex permission implementations. (Kotti has solved this, but for instance SubstanceD would have the same problem)\r\n\r\nIf this would be something you could consider, I'd happily do the work. I think it could be a done in a non-intrusive way that would make all old code work and remove the need for clever hacks.\r\n\r\nAll the best,\r\nRobin\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/Pylons/pyramid/issues/comments/386868569","html_url":"https://github.com/Pylons/pyramid/issues/3268#issuecomment-386868569","issue_url":"https://api.github.com/repos/Pylons/pyramid/issues/3268","id":386868569,"user":{"login":"robinharms","id":737216,"avatar_url":"https://avatars2.githubusercontent.com/u/737216?v=4","gravatar_id":"","url":"https://api.github.com/users/robinharms","html_url":"https://github.com/robinharms","followers_url":"https://api.github.com/users/robinharms/followers","following_url":"https://api.github.com/users/robinharms/following{/other_user}","gists_url":"https://api.github.com/users/robinharms/gists{/gist_id}","starred_url":"https://api.github.com/users/robinharms/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/robinharms/subscriptions","organizations_url":"https://api.github.com/users/robinharms/orgs","repos_url":"https://api.github.com/users/robinharms/repos","events_url":"https://api.github.com/users/robinharms/events{/privacy}","received_events_url":"https://api.github.com/users/robinharms/received_events","type":"User","site_admin":false},"created_at":"2018-05-06T10:13:09Z","updated_at":"2018-05-06T10:13:09Z","author_association":"NONE","body":"@bertjwregeer the problem is that you need to subclass and replace has_permission to get that to work, and replacing the policy too afterwards.\r\n\r\nDynamic ACL still doesn't change the fact that the wrong principas will be passed to the permits-function. (If you're using local roles as a concept where you attach roles to groups, which i very much want to do :D )\r\n\r\n(Run the gist i attached!)\r\n\r\n@mmerickel that's great!\r\n\r\nAbout overriding the authz, we roll our own for our projects, but it would feel weird to override it and not follow the IAuthorizationPolicy interface. (I.e. sneaking in context in effective_principals) And that would still require an override of has_permission.\r\n\r\nThanks a lot for digging into this.\r\n "}}
{"actor":{"display_login":"robinharms"}}