{"action":"opened","issue":{"url":"https://api.github.com/repos/anxiousmodernman/verbose-barnacle/issues/14","repository_url":"https://api.github.com/repos/anxiousmodernman/verbose-barnacle","labels_url":"https://api.github.com/repos/anxiousmodernman/verbose-barnacle/issues/14/labels{/name}","comments_url":"https://api.github.com/repos/anxiousmodernman/verbose-barnacle/issues/14/comments","events_url":"https://api.github.com/repos/anxiousmodernman/verbose-barnacle/issues/14/events","html_url":"https://github.com/anxiousmodernman/verbose-barnacle/issues/14","id":177523479,"number":14,"title":"Handler calls to isUserAllowedForObjectACM need to create AppError messages based upon type","user":{"login":"anxiousmodernman","id":396004,"avatar_url":"https://avatars.githubusercontent.com/u/396004?v=3","gravatar_id":"","url":"https://api.github.com/users/anxiousmodernman","html_url":"https://github.com/anxiousmodernman","followers_url":"https://api.github.com/users/anxiousmodernman/followers","following_url":"https://api.github.com/users/anxiousmodernman/following{/other_user}","gists_url":"https://api.github.com/users/anxiousmodernman/gists{/gist_id}","starred_url":"https://api.github.com/users/anxiousmodernman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/anxiousmodernman/subscriptions","organizations_url":"https://api.github.com/users/anxiousmodernman/orgs","repos_url":"https://api.github.com/users/anxiousmodernman/repos","events_url":"https://api.github.com/users/anxiousmodernman/events{/privacy}","received_events_url":"https://api.github.com/users/anxiousmodernman/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2016-09-16T21:22:48Z","updated_at":"2016-09-16T21:22:48Z","closed_at":null,"body":"Previously that method did not return an error if the user didn't have access as long as the server communication succeeded.  Thus, when an error was returned, it was assumed to be an error reaching AAC under this pattern..\n\n```\nhasAACAccess, err := h.isUserAllowedForObjectACM(ctx, &dbObject)\nif err != nil {\n\treturn NewAppError(502, err, \"Error communicating with authorization service\")\n}\n```\n\nNow, if there are messages returned from AAC, then that is formed into an error passed back when HasAccess is false.  The logic in handlers no longer assumes a 502 failure to communicate with AAC, but rather always forces a 403 using this model\n\n```\nhasAACAccess, err := h.isUserAllowedForObjectACM(ctx, &dbObject)\nif err != nil {\n\t// TODO: Isolate different error types\n\t//return NewAppError(502, err, \"Error communicating with authorization service\")\n\treturn NewAppError(403, err, err.Error())\n}\n```\n\nThis will need to be further cleaned up, so that based upon the error returned, an appropriate status code is given.  500 or 502 is appropriate if there is an error reaching or communicating with AAC, while 403s are appropriate if AAC was reached, and indicated that the user does not have access.\n\nThe affected areas are:\n\n- [ ] addObjectShare\n- [ ] createObject\n- [ ] getObject\n- [ ] getObjectStream\n- [ ] removeObjectShare\n- [ ] updateObject (3 instances)\n   - [ ] From clearance perspective on OLD ACM\n   - [ ] Access check against altered ACM as a whole NEW ACM\n   - [ ] If ACM provided differs from what is currently set (may now be redundant?)\n- [ ] updateObjectStream (2 instances)\n   - [ ] From clearance perspective on OLD ACM\n   - [ ] Access check against altered ACM"}}
{"actor":{"display_login":"anxiousmodernman"}}