Don't use String for password parameters

[augi]

To lower the potential attack surface, it's recommended not to use String to store sensitive data. So I propose to use CharSequence for password parameters (e.g in GitLabApi.login method). We could use implementation like this. It allows to clear the underlying char array after the sensitive information is not needed.

I'm not creating PR because I'm not sure if the underlying HTTP library is able to handle this. The point is that the CharSequence cannot be converted to String at any time - it would break the concept.

If you point me to the right direction, I'm ready to create a new PR.

[gmessner]

I have a mechanism for doing this. Two questions though:

1. I'm thinking that a SecureString implementation should be provided so that the consumer can easily create a CharSequence, and then use CharSequence as the type of parameter passed to the login method, or should the type be SecusreString, so that a String (implements CharSequence) cannot be passed in?

2. I will only be implementing this for oauth2Login(), the login() methods will be deprecated as GitLab no longer supports session login, and when login() fails it actually uses oauth2Login() anyways. See any issues with this?

[augi]

1. That's actually great question! And little bit tricky :) Accepting CharSequence (so accepting also plain String) doesn't break backward compatibility and works seamlessly for users that don't care about security. On other hand, accepting SecureString indicates that this library takes care about security and that the value is not leaked internally (never instantiates a String from SecureString). So I would probably prefer to accept SecureString only as the method argument. It expresses more explicitly that the library knows that the argument is a sensitive value.
2. Sure, that's no problem IMHO. It doesn't make sense to invest time to something that is deprecated.

[gmessner]

@augi
I've actually finished the implementation. I ended up not restricting it to the secure String implementation I created (SecretString), and it also accepts char[]. If you would like to take a look I've added a PR:

#157

[augi]

That's great, thanks!

Btw. it would be nice to allow to pass also token as char[], like in this constructor. Of course, it makes sense only if the token is required for connection initialization. If the token is required for all the requests then it wouldn't make sense.

[gmessner]

Unfortunately, the token is required in a header on each and every request.

[gmessner]

@augi
PR #157 for this has been merged.

[augi]

Great, thank you!