Closed brntbeer closed 12 years ago
After sleeping on this I'd love to see a config var OAUTH_PROVIDER
that explicitly determines which mechanism we're supposed to use, and then using config.rb
to enforce it accordingly. Here's a gist with some fixes for the README if that were to happen.
Thanks for the feedback! good idea.
Due to ea5c100, I'm wondering about the use of some of the session info. session['user']['email']
makes sense to me, but what about session['user']['identity_url']
and first_name, last_name. i couldnt find these fields used anywhere.
Also, do we want to grab the github_token as well (kinda like the identity url)? Maybe there's a better way to unify these two pieces.
Really we just need some sort of unique identifier from either scheme. It would be lovely if they were the same type (e.g. email) but not absolutely necessary. Nothing is really tied to the user yet, other than the ability for other users to see who performed an action.
Word. It's good to look ahead. I think for now setting the github_user.login in place of email is good enough then
Ping. Wondering whats holding up this pull request. =\
edit: as in is there anything i've missed that you want fixed? you mentioned needing just some sort of "unique identifier" in https://github.com/obfuscurity/descartes/pull/62#issuecomment-8891406. and in most cases it is set to email. only time it isnt is in github_auth when the user doesnt have an email address specified, in which case i switch to github username
This looks great. Both mechanisms work as intended and I think using GitHub username is fine for now. :metal:
If
GOOGLE_OAUTH_DOMAIN
is not set, fall back to using github auth.Seems to me that dashboards could be used for companies anyways, and they'd likely have github organizations. :sparkler:
If neither of the auth's are configured properly, the app doesn't work. From what I can tell, this is acceptable. However, the error page is different
I'd think this is still ok?