Added secure string support for credentials

[rubberduck203]

Issue #1048

I added some extra comments and a helper function in the constants class as well. Obviously I removed my credentials before committing and turned the tests back off, but they passed. You can use this snippet to re-instate them for this change.

        return new SecureUsernamePasswordCredentials() 
        { 
            Username = StringToSecureString("username"), 
            Password = StringToSecureString("swordfish") 
        };

[nulltoken]

@ckuhn203 Thanks! Looks clean to me.

@ethomson @jamill @Therzok @bording Thoughts?

[rubberduck203]

One thing I will say before anyone merges this is that I did consider making the existing credentials and this one share a generic interface, but this was the simplest thing that worked. I figured "Copy paste once: ok. Copy paste twice: stop. Refactor."

[rubberduck203]

I might have been quick on the draw there. I'm thinking maybe Username should be a plain string. That's the approach Microsoft took with NetworkCredential. Thoughts?

[nulltoken]

Thoughts?

Makes sense, indeed. Could you please amend your commit with this change?

[Therzok]

Shouldn't this be chars.Length?

[Therzok]

I'd say keep the Username as a secure string, tbh. The username can be sensitive enough data to brute force the password on some providers. :P

[Therzok]

Actually, nevermind, just saw that NetworkCredential does use a string.

[rubberduck203]

Shouldn't this be chars.Length?

Yes. It should have been. I would have thought something would have failed if the wrong password got sent in... Either way, I corrected that.

[rubberduck203]

Okay... I don't get it. I've run the tests locally several times. Results seem intermittent. Sometimes everything passes. Sometimes I get failures. Failures are never the same tests and if I simply re-run the failed tests, they pass. Is this a known issue?

[nulltoken]

I've restarted the build. And it passed. Giving a look at the previous AppVeyor logs, looked like a GitHub transient issue.

[nulltoken]

🆒

Before we merge this in, could you please squash those commits together into a nice cohesive one? We tend not to merge review commits.

[rubberduck203]

Thanks @nulltoken. Much appreciated.

Keep up the good work @ALL. Love the library. Thanks for all your hard work on it (and for helping me to contribute in my own small way)!

[nulltoken]

@ckuhn203 Every contribution is welcome. Yours was very thoughtful! Thanks a lot for it. ✨ ✨

[nulltoken]

Published as NuGet pre-release package LibGit2Sharp.0.22.0-pre20150516171636

[rubberduck203]

👍 thanks again. I'll keep my eye on the easy fixes.