Multiple value fix

[dayer4b]

This pull request contains a fix for the problem raised in Issue #5

[pencil]

Not so sure about this one, as it breaks backward compatibility. Your thoughts on this, @luxflux? Maybe something for the upcoming major release?

[luxflux]

Sorry for the long delay. I really missed my mention here...

What's the reason to use a string as return value? Wouldn't it be better to always return an array with one or more elements?

CASino will dump the array into the response as yaml.

[dayer4b]

I suppose an array would work. I didn't test that. You're right, it would
be better to return such data as an array.

On Mon, Jul 21, 2014 at 3:27 AM, Raffael Schmid [email protected]
wrote:

Sorry for the long delay. I really missed my mention here...

What's the reason to use a string as return value
https://github.com/rbCAS/casino-ldap_authenticator/pull/6/files#diff-3db56e44a637a7b46d9d4e9d9b0fd974L75?
Wouldn't it be better to always return an array with one or more elements?

CASino will dump the array into the response as yaml
https://github.com/rbCAS/CASino/blob/master/app/builders/casino/ticket_validation_response_builder.rb#L37-L46
.

—
Reply to this email directly or view it on GitHub
#6 (comment)
.

Sunil

[adamcrown]

Glad to see there's a fix being worked on. We'd like to switch form rubycas-server to casino but this is a breaking bug for us.

I'd also like to bring up Jasig CAS extra attributes format compatibility. Just because a LDAP attribute only has one value doesn't mean it should be single value. I believe rubycas-server just spits out everything as an array. Which works, but if everything is an array then everything gets YAML encoded and that's especially bad for Jasig compatibility.

I'm thinking the best path forward could be a configuration option for extra attributes so anybody can configure certain extra attributes an being multi-value, and other as not, according to their needs. The default could be single-value to keep backwards compatibility.

Another, thing to consider might be different compatibility modes for extra attributes: Jasig, rubycas-server and possibly others I'm not aware of.

[pencil]

I'm thinking about just including all the values like this:

<name>John Doe</name>
<phone>123456789</phone>
<phone>321456789</phone>
<phone>213456789</phone>

Would this work? Probably would have to check compatibility with CAS clients out there.

Another, thing to consider might be different compatibility modes for extra attributes: Jasig, rubycas-server and possibly others I'm not aware of.

Actually it is kind of hard to get information on how Jasig CAS does it. There is a schema documented but I'm not sure it is actually used since I didn't find a single CAS client that does support it. Probably would have to look through their source code.

[pencil]

CASino 4.0 supports multiple-values for a single field. Basically we can change this line to:

result[index_result] = value

I currently don't have the infrastructure to test it but if you are still interested, you might want to give it a try.

[twyfordr]

We ran into this bug today. Will this patch (or a similiar one) be included in any upcoming release?

[pencil]

I didn't have the chance to test this change. I'm pretty sure most CAS client implementations can't handle multiple values for the same key but that would have to be tested as well.

[pencil]

Closing this and continuing in #5