fix(users) Handle missing email - OAuth #1501
Conversation
|
Coverage decreased (-0.06%) to 72.937% when pulling 5c649459287aee083de43c634eac717f06d9d8f6 on Wuntenn:fixEmptyEmail into b2a5cb5 on meanjs:master. |
|
|
||
| // And save the user | ||
| user.save(function (err) { | ||
| return done(err, user); | ||
| return done(err, user); |
There was a problem hiding this comment.
spacing if off here. Looks like 4 spaces, but should be 2.
|
@Wuntenn Have you seen my comments? Let's try to wrap this up so we can get this in for 0.5.0 |
|
Sorry @mleanos, I wasn't quite sure if I had, but had the feeling that I'd done it. Gimmie a sec |
| // Remove sensitive data before login | ||
| user.password = undefined; | ||
| user.salt = undefined; | ||
| (function () { |
There was a problem hiding this comment.
There's no need for an IIFE on the server-side. Can you remove it?
|
@Wuntenn Looks like there's conflicts. Can you rebase this as well? |
|
I was having a few issues fetching from master. It kept on saying "everything up to date" rebase wouldn't do anything. I must've been doing something wrong. |
|
@Wuntenn make sure your upstream repo is pointing to the correct remote url |
|
Coverage decreased (-0.1%) to 73.187% when pulling 5c910493fbaf82675cacbe151488ed19a2303625 on Wuntenn:fixEmptyEmail into 89e8d13 on meanjs:master. |
- Intentionally omits setting email in constructor to trigger defaults when creating user. Handles cases where email is not authorized/given by provider. Related to issue meanjs#1250
Fixes css lintings warnings of properties not alphabetized.
Uses the passport info object to simplify login and remove the need to temporarily cache the redirect within the session.
…he server configs. Also added a time indicator on failed login attempts to give the user feedback on subsequent failed login attempts.
…down to the client. reverted some of the other changes (regarding the http request).
Modified config to be "shared". This will allow future configurations to be easily passed to the client.
For New Article, delete function no required
Replaces the $http service calls with promise based methods of the client-side UsersService for the following: Users Change Password Users Manage Social Accounts Users Password Forgot Users Password Reset Users Signup Users Signin Modifies tests to reflect changes. Closes meanjs#1479
… date (meanjs#1516) * attempting to update karma and gulp-load-plugins to check build status * updating karma to 1.3.0 * updating other karma related packages to latest versions * updating package dependencies to keep up to date
Wuntenn
force-pushed
the
fixEmptyEmail
branch
from
5c91049
to
8fba182
Compare
1 similar comment
1 similar comment
|
May have to revert this commit commit @mleanos I wanted to ask you about removing the duplicated rebased commits first. |
|
What seems to be the problem? I don't see any duplicate commits on master. |
|
It's that there are extra commits above and when I did a git log. I thought it had duplicated some of the commits (it looked that way on my version). Looks fine here though (besides there being 18 commits). I tried a fetch and merge before trying the reset and my local branch showed that it was level with master so all is good. |
This is done to handle cases where email is not authorised or given by provider.
Related to issue #1250