Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #315 for realz, actually allowing log on with email. #355

Merged
merged 2 commits into from Dec 9, 2011

Conversation

jeffhandley
Copy link
Member

No description provided.

@@ -149,7 +150,7 @@ public string GenerateApiKey(string username)

public bool ChangePassword(string username, string oldPassword, string newPassword)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we change the username param to usernameOrEmail since we're allowing emails now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will use usernameOrEmail as the parameter, to be consistent with others.

@half-ogre
Copy link
Contributor

Some minor comments, but nothing impeding pushing it.

@osbornm
Copy link
Contributor

osbornm commented Dec 9, 2011

When a user logs in with an email address what do we store in the Identity (email or username)?

@half-ogre
Copy link
Contributor

@osbornm It still returns a User, so that shouldn't change.

@jeffhandley
Copy link
Member Author

Yeah, the controller still users User.Username for the auth token.

@osbornm
Copy link
Contributor

osbornm commented Dec 9, 2011

@anglicangeek & @jeffhandley in that case shouldn't everything expect login take only the username? If username is the "authoritative" field.

jeffhandley added a commit that referenced this pull request Dec 9, 2011
Fixed #315 for realz, actually allowing log on with email.
@jeffhandley jeffhandley merged commit 909caa6 into master Dec 9, 2011
@osbornm
Copy link
Contributor

osbornm commented Dec 9, 2011

What why did the ChangeEmail thing change then?????

@half-ogre
Copy link
Contributor

Yeah, actually, why would change password ever use email? It's always done with a user that's logged in.

@jeffhandley
Copy link
Member Author

We just need another method then, that doesn't fall back to email address. I'll add that.

@osbornm
Copy link
Contributor

osbornm commented Dec 9, 2011

wait what? why does change password fail back to email address? login should set the user name and from then on out the whole site uses username not email?

@jeffhandley
Copy link
Member Author

Addressed here: #356

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants