Fix basic auth with empty username or password

[hardillb]

fixes #3324

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

Proposed changes

Add check for undefined values and replace with empty string

Checklist

• I have read the contribution guidelines
• For non-bugfix PRs, I have discussed this change on the forum/slack team.
• I have run grunt to verify the unit tests pass
• I have added suitable unit tests to cover the new/changed functionality

[coveralls]

Coverage increased (+0.004%) to 67.365% when pulling 44616c6 on hardillb:http-basic-username-only into aaa2b4c on node-red:master.

[dceejay]

looks sensible to me.

[tobiasoort]

undefined is falsey in JS, so if both are undefined this if would be skipped. Afaik it's valid to only have b64(":") as a basic auth cred (Og==). I think that's still not possible to achieve with this fix in place.

[hardillb]

Nah, that's and edge case too far and basically pointless. Anybody depending on that behaviour deserves to have it fail.

[tobiasoort]

First to be very clear: I very much agree with you. People playing with basic-auth with no actual credentials are screwed anyway - we don't have to support them :) This is more of an interesting discussion for me. If you feel at any moment irritated or tired by me, let me know. I would not let this point stop this PR from getting merged.

I just wonder what that if is trying to prevent - you either enter a username or a password or both? Why? Also, Is that clear from the UX that that's the only valid input?

I prefer no code over 'what is that actually doing here' code.

[hardillb]

We need the if because we can't be 100% sure what else is in the credentials object, all we know is that it has 1 or more properties. Also it may hold other authentication methods, either now or in the future.