search

atmos / warden-github

:lock: warden strategy for github oauth
MIT License
54 stars 41 forks source link

Better organisation membership checks #14

Closed sj26 closed 11 years ago

sj26 commented 11 years ago

This is the way the Github API recommends doing organisation checks, and is more accurate because sometimes you may not have permission to list the members in an organization while still retaining membership.

atmos commented 11 years ago

Is .cover? a built in thing that I've never used?

sj26 commented 11 years ago

Yes, but I just realised it's not available in 1.8.7. If that's important, can change to #include? instead. Looks like there's a stray #in? in there as well. :-/

atmos commented 11 years ago

Are the response codes documented somewhere? I don't quite follow the logic in the range checking.

sj26 commented 11 years ago

GitHub will return a 204 (meaning success without content, i.e. user is an organisation member) but generally 200...300 means success in HTTP parlance. See the GitHub API documentation.

atmos commented 11 years ago

Can we do an explicit check for 204 to match the docs rather than the range? It solves the 1.8.x vs 1.9.x problem in a pretty straightforward manner. :)

On Tue, Nov 27, 2012 at 9:29 PM, Samuel Cochran notifications@github.comwrote:

GitHub will return a 204 (meaning success without content, i.e. user is an organisation member) but generally 200...300 means success in HTTP parlance. See the GitHub API documentationhttp://developer.github.com/v3/orgs/members/#check-membership .

— Reply to this email directly or view it on GitHubhttps://github.com/atmos/warden-github/pull/14#issuecomment-10791266.