directory-api mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Dobrinic <mark.dobri...@twobotechnologies.com>
Subject Re: ModifyPassword extended operation issue
Date Tue, 04 Oct 2016 14:14:33 GMT
Hi Emmanuel,

Thanks for the quick followup! I was able to test your alternative fix,
and I can confirm that it will work as expected.

Created a JIRA-issue as well, so it's possible to track the issue:
https://issues.apache.org/jira/browse/DIRAPI-285

Thanks again for picking this up, looking forward to the update ;)

Cheers,

Mark


On 03/10/16 23:42, Emmanuel Lécharny wrote:
> Le 03/10/16 à 13:09, Mark Dobrinic a écrit :
>> Hi api @directory.apache.org,
>>
>> When using Apache Directory API to send out a ModifyPassword request to
>> a non-Apache LDAP server, I stumbled upon an issue when processing the
>> response of the request.
>>
>> The specification ( https://tools.ietf.org/html/rfc3062#section-2.2 ;
>> RFC 3062 section 2.2), states
>>
>> " Password Modify response is an ExtendedResponse where the responseName
>> field is absent and the response field is optional."
>>
>> But PasswordModifyFactory#decorate() instead assumes there will always
>> be a response field.
> True !
>>
>> I was able to override PasswordModifyFactory and implement a fix by
>> changing the decorate() method in the subclass, but I think it's
>> actually a bug in the library, so I'd like to propose to change it in
>> Apache Directory API instead.
>>
>> The change in PasswordModifyFactory.java is minimal:
>>
>> Line 133, from
>>         byte[] value = response.getResponseValue();
>>
>>         ByteBuffer buffer = ByteBuffer.wrap( value );
>> to
>>         byte[] value = response.getResponseValue();
>>
>>         if (value == null)
>>         {
>>             value = new byte[0];
>>         }
>>
>>         ByteBuffer buffer = ByteBuffer.wrap(value);
> I come with a patch whch is slightly more complex. Here is the
> replacement for the decorate() method :
>
>     /**
>      * {@inheritDoc}
>      */
>     public PasswordModifyResponseDecorator decorate( ExtendedResponse
> decoratedResponse )
>     {
>         if ( decoratedResponse instanceof PasswordModifyResponseDecorator )
>         {
>             return ( PasswordModifyResponseDecorator ) decoratedResponse;
>         }
>
>         if ( decoratedResponse instanceof PasswordModifyResponse )
>         {
>             return new PasswordModifyResponseDecorator( codec, (
> PasswordModifyResponse ) decoratedResponse );
>         }
>
>         // It's an opaque extended operation
>         @SuppressWarnings("unchecked")
>         ExtendedResponseDecorator<ExtendedResponse> response = (
> ExtendedResponseDecorator<ExtendedResponse> ) decoratedResponse;
>
>         // Decode the response, as it's an opaque operation
>         Asn1Decoder decoder = new Asn1Decoder();
>
>         byte[] value = response.getResponseValue();
>         PasswordModifyResponseContainer container = new
> PasswordModifyResponseContainer();
>        
>         PasswordModifyResponse pwdModifyResponse = null;
>        
>         if ( value != null )
>         {
>             ByteBuffer buffer = ByteBuffer.wrap( value );
>    
>             try
>             {
>                 decoder.decode( buffer, container );
>                 pwdModifyResponse = container.getPwdModifyResponse();
>
>                 // Now, update the created response with what we got
> from the extendedResponse
>                 pwdModifyResponse.getLdapResult().setResultCode(
> response.getLdapResult().getResultCode() );
>                 pwdModifyResponse.getLdapResult().setDiagnosticMessage(
> response.getLdapResult().getDiagnosticMessage() );
>                 pwdModifyResponse.getLdapResult().setMatchedDn(
> response.getLdapResult().getMatchedDn() );
>                 pwdModifyResponse.getLdapResult().setReferral(
> response.getLdapResult().getReferral() );
>             }
>             catch ( DecoderException de )
>             {
>                 StringWriter sw = new StringWriter();
>                 de.printStackTrace( new PrintWriter( sw ) );
>                 String stackTrace = sw.toString();
>    
>                 // Error while decoding the value.
>                 pwdModifyResponse = new PasswordModifyResponseImpl(
>                     decoratedResponse.getMessageId(),
>                     ResultCodeEnum.OPERATIONS_ERROR,
>                     stackTrace );
>             }
>         }
>         else
>         {
>             pwdModifyResponse = new PasswordModifyResponseImpl();
>    
>             // Now, update the created response with what we got from
> the extendedResponse
>             pwdModifyResponse.getLdapResult().setResultCode(
> response.getLdapResult().getResultCode() );
>             pwdModifyResponse.getLdapResult().setDiagnosticMessage(
> response.getLdapResult().getDiagnosticMessage() );
>             pwdModifyResponse.getLdapResult().setMatchedDn(
> response.getLdapResult().getMatchedDn() );
>             pwdModifyResponse.getLdapResult().setReferral(
> response.getLdapResult().getReferral() );
>         }
>
>         PasswordModifyResponseDecorator decorated = new
> PasswordModifyResponseDecorator( codec, pwdModifyResponse );
>
>         Control ppolicyControl = response.getControl( PasswordPolicy.OID );
>
>         if ( ppolicyControl != null )
>         {
>             decorated.addControl( ppolicyControl );
>         }
>
>         return decorated;
>     }
>
> Can you test it on your side (I'm afraid I don't have a test case for
> this atm)
>
> Can you also create a JIRA for the bug you have found, so that we have
> it listed when we will cut a release ?
>
> Many thanks !
>


-- 
Regards,

Mark Dobrinic
Software Engineer and Identity Specialist
Twobo Technologies AB

mark.dobrinic@twobotechnologies.com
www.twobotechnologies.com


Mime
View raw message