Missing isEmpty check for login form fields

[mikefowler]

Hey all:

I'm seeing some errors when a login form, extended with LoginControllerMixin, is submitted with empty input fields (either "identification" or "password"). I'm also noticing that the check for empty fields was removed here and, after searching around in the code a bit, hasn't made it back in anywhere.

Here's the stack trace (not source-mapped, sorry!). In either case I was able to trace it back to the removal of the Empty.isEmpty checks.

[marcoow]

This probably means that you haven't set the controller's authenticatorFactory property.

[mikefowler]

@marcoow, I assure you I have. Perhaps I am not being specific enough: I have ember-simple-auth setup and working properly, 100%, with the exception of these errors. If both form fields are filled (non-empty), this error does not occur, and furthermore if the login credentials are correct, authentication happens properly. If I wasn't setting up the authenticatorFactory property properly, I do not believe that would be working at all.

Is this not reproducible locally for you? The commit I linked to above seems to show very clearly that the isEmpty checks were removed for this use-case.

Ember 1.5.1
Ember-Simple-Auth 0.5.1

[marcoow]

The error you're seeing means that sth. tries to lookup sth. from Ember's container with a null identifier so it's not (at lest not directly) caused by these 2 values being empty. What's your server's response when you submit both empty fields?

[mikefowler]

Would you clarify what you mean by "sth"? I'm returning a 401 for failed auth, empty fields or not.

On Wed, Jun 4, 2014 at 7:46 AM, Marco Otte-Witte notifications@github.com
wrote:

The error you're seeing means that sth. tries to lookup sth. from Ember's container with a null identifier so it's not (at lest not directly) caused by these 2 values being empty. What's your server's response when you submit both empty fields?

Reply to this email directly or view it on GitHub:
#185 (comment)

[marcoow]

"sth." - something. Some part of the app tries to look up something without specifying a name. Would be great if you could debug and get a stack trace.

[mikefowler]

Here's the full stack trace:

As best as I can tell, submitting the form with empty input fields results in a call to Session.invalidate. The Session object then attempts to look up its authenticatorFactory:

The lookup fails because there is no authenticatorFactory defined, which seems to make sense if the session hasn't been set up in the first place, no?

[marcoow]

You're right - that's a bug. I'm not sure whether 401 is the best status code for that situation thought but still the library shouldn't crash. I'm sure if you change that to e.g. 400 it'll work. I'm going to fix that case though by not triggering invalidation when the session is not actually authenticated.

[mikefowler]

Yea, I think it should certainly handle 401 in addition to 400, the spec is fairly clear that 401 can be used to indicate a failed login: “If the request already included Authorization credentials, then the 401 response indicates that authorization has been refused for those credentials.”

Edit - the error does not occur if the server returns a 400, you are correct.

[mikefowler]

Thanks for the responsiveness, @marcoow.