Handle client_id/client_secret nil case.

[ragaskar]

The commit (e351f4d) that introduced UAA url escape encoding compatibility means client_id and client_secret can no longer be nil or an error is thrown.

I am honestly not 100% sure if the nil case should be valid (we have a series of -- I think -- integration tests that are using nil here, so perhaps it is?). In any case, the nil case was previously supported. If the nil case is invalid, maybe this class should throw ArgumentError on instantiation instead?

I made this change targeted within the #request_token method because I'm not sure what the possible implications would be by defaulting it in the initialize, but that might arguably be a better place for it if it does not cause a behavior change.

One other minor note: when reading the code I found the option name basic_auth a little confusing, since both requests are using basic_auth. After reading about the reason for the switch, I wonder if something like "unescaped_basic_auth" (or something similar) might be a better name? If you agree, happy to also create a PR that provides both the old 'basic_auth' (for backwards compatibility) and new option.

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/181076709

The labels on this github issue will be updated when the story is started.

[strehle]

ok thanks. Do you need a new release urgently ?

[strehle]

One other minor note: when reading the code I found the option name basic_auth a little confusing, since both requests are using basic_auth. After reading about the reason for the switch, I wonder if something like "unescaped_basic_auth" (or something similar) might be a better name? If you agree, happy to also create a PR that provides both the old 'basic_auth' (for backwards compatibility) and new option.

sure you can also create a new PR with new options
The meaning should be clear, UAA is an Oauth2 server and it does OAuth2 Based Header Encoding of Authorization and not plain Baisc encoding.
If you have no special characters, then this does not matter but if you have special characters, then it matters.
Details in : cloudfoundry/cf-uaac#50 (comment)

The change itself was in uaac a while ago, but it should have been here before .
Issue cloudfoundry/cf-uaac#50

[strehle]

I am honestly not 100% sure if the nil case should be valid (we have a series of -- I think -- integration tests that are using nil here, so perhaps it is?). In any case, the nil case was previously supported. If the nil case is invalid, maybe this class should throw ArgumentError on instantiation instead?

It is ok from coding to have this, but for UAA there cannot be a nil in client_id or client_secret
In cf oauth client you have an empty secret, e.g. cf and "" -> Y2Y6 , but this is not nil (null). UAA needs always the parameter client_id and client_secret, event if it is empty, it is not null