#123 Enhancement request: "whoami" extended operation
Closed: wontfix None Opened 12 years ago by mkosek.

https://bugzilla.redhat.com/show_bug.cgi?id=437632

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.12)
Gecko/20080201 Firefox/2.0.0.12

Description of problem:
The directory server should support the "Who am I?" extended operation (OID
1.3.6.1.4.1.4203.1.11.3) described in RFC4532.

Version-Release number of selected component (if applicable):


How reproducible:
Always


Steps to Reproduce:
ldapwhoami -x -h ldap-server.example.com

Actual Results:
Result: Protocol error (2)
Additional info: unsupported extended operation

Expected Results:
shoud suport this extended operation

Additional info:
This is an enhancement request, not a bug.

batch update to FUTURE milestone

set default ticket origin to Community

Added initial screened field value.

good work for an intern

{{{
34 * Copyright (C) 2001 Sun Microsystems, Inc. Used by permission.
35 * Copyright (C) 2005 Red Hat, Inc.
}}}
Remove line 34.
Replace 2005 with 2013.
{{{
81 if ( slapi_pblock_get( pb, SLAPI_EXT_OP_REQ_OID, &oid ) != 0 ||
82 slapi_pblock_get( pb, SLAPI_EXT_OP_REQ_VALUE, &bval ) != 0 ||
83 bval->bv_val != NULL ) {
84 slapi_log_error( SLAPI_LOG_FATAL, PLUGIN_NAME, "Could not get OID and request value from request\n");
}}}
line 84: why bval->bv_val has to be NULL? If it does, the error message does not sound correct.
{{{
96 return(1);
}}}
Why 1 is returned here?
{{{
106 fdn = slapi_ch_smprintf("dn: %s", client_dn);
}}}
You may want to call slapi_create_dn_string instead of slapi_ch_smprintf. slapi_create_dn_string normalizes DN format. (E.g, could you pass "uid=tEsT_UsEr0, dc=eXaMpLe, dc=cOm" and see what's returned?)
{{{
114 if ( slapi_pblock_set( pb, SLAPI_EXT_OP_RET_OID, NULL) != 0 ||
115 slapi_pblock_set( pb, SLAPI_EXT_OP_RET_VALUE, &retbval) != 0 )
116 {
117 slapi_log_error(SLAPI_LOG_FATAL,PLUGIN_NAME,"Could not set return values");
118 slapi_send_ldap_result(pb,LDAP_OPERATIONS_ERROR,NULL,"Could not set return values",0,NULL);
119 return( SLAPI_PLUGIN_EXTENDED_SENT_RESULT );
120 }
}}}
If it returns here, client_dn and fdn leak.

One more question... What's logged in the access log?

Replying to [comment:12 nhosoi]:

{{{
34 * Copyright (C) 2001 Sun Microsystems, Inc. Used by permission.
35 * Copyright (C) 2005 Red Hat, Inc.
}}}
Remove line 34.
Replace 2005 with 2013.
done
{{{
81 if ( slapi_pblock_get( pb, SLAPI_EXT_OP_REQ_OID, &oid ) != 0 ||
82 slapi_pblock_get( pb, SLAPI_EXT_OP_REQ_VALUE, &bval ) != 0 ||
83 bval->bv_val != NULL ) {
84 slapi_log_error( SLAPI_LOG_FATAL, PLUGIN_NAME, "Could not get OID and request value from request\n");
}}}
line 84: why bval->bv_val has to be NULL? If it does, the error message does not sound correct.
The requestValue field should be Null according to the whoami RFC. I have changed the error message.
{{{
96 return(1);
}}}
Why 1 is returned here?
changed it to a more suitable return value
{{{
106 fdn = slapi_ch_smprintf("dn: %s", client_dn);
}}}
You may want to call slapi_create_dn_string instead of slapi_ch_smprintf. slapi_create_dn_string normalizes DN format. (E.g, could you pass "uid=tEsT_UsEr0, dc=eXaMpLe, dc=cOm" and see what's returned?)
Not required. slapi_ch_smprintf returns the result in normalized form
{{{
114 if ( slapi_pblock_set( pb, SLAPI_EXT_OP_RET_OID, NULL) != 0 ||
115 slapi_pblock_set( pb, SLAPI_EXT_OP_RET_VALUE, &retbval) != 0 )
116 {
117 slapi_log_error(SLAPI_LOG_FATAL,PLUGIN_NAME,"Could not set return values");
118 slapi_send_ldap_result(pb,LDAP_OPERATIONS_ERROR,NULL,"Could not set return values",0,NULL);
119 return( SLAPI_PLUGIN_EXTENDED_SENT_RESULT );
120 }
}}}
If it returns here, client_dn and fdn leak.
Added the statements to free memory

Replying to [comment:13 nhosoi]:

One more question... What's logged in the access log?

Here are the log results:

[11/Jun/2013:11:27:14 -0700] conn=1 fd=64 slot=64 connection from 127.0.0.1 to 127.0.0.1
[11/Jun/2013:11:27:14 -0700] conn=1 op=0 BIND dn="cn=Directory Manager" method=128 version=3
[11/Jun/2013:11:27:14 -0700] conn=1 op=0 RESULT err=0 tag=97 nentries=0 etime=0 dn="cn=directory manager"
[11/Jun/2013:11:27:14 -0700] conn=1 op=1 EXT oid="1.3.6.1.4.1.4203.1.11.3" name="whoami-plugin"
[11/Jun/2013:11:27:14 -0700] conn=1 op=1 RESULT err=0 tag=120 nentries=0 etime=0
[11/Jun/2013:11:27:14 -0700] conn=1 op=2 UNBIND
[11/Jun/2013:11:27:14 -0700] conn=1 op=2 fd=64 closed - U1
[11/Jun/2013:11:29:33 -0700] conn=2 fd=64 slot=64 connection from 127.0.0.1 to 127.0.0.1
[11/Jun/2013:11:29:33 -0700] conn=2 op=0 BIND dn="" method=128 version=3
[11/Jun/2013:11:29:33 -0700] conn=2 op=0 RESULT err=0 tag=97 nentries=0 etime=0 dn=""
[11/Jun/2013:11:29:33 -0700] conn=2 op=1 EXT oid="1.3.6.1.4.1.4203.1.11.3" name="whoami-plugin"
[11/Jun/2013:11:29:33 -0700] conn=2 op=1 RESULT err=0 tag=120 nentries=0 etime=0
[11/Jun/2013:11:29:33 -0700] conn=2 op=2 UNBIND
[11/Jun/2013:11:29:33 -0700] conn=2 op=2 fd=64 closed - U1

VENDOR and VERSION are automatically set by the installer. Please change this section

{{{
50 #define PLUGIN_NAME "whoami-plugin"
51 #define PLUGIN_VENDOR "Red Hat"
52 #define PLUGIN_VERSION "0.5"
53 #define PLUGIN_DESC "whoami extended operation plugin"
54 #define WHOAMI_EXOP_REQUEST_OID "1.3.6.1.4.1.4203.1.11.3"
55
56 static Slapi_PluginDesc expdesc = { PLUGIN_NAME, PLUGIN_VENDOR, PLUGIN_VERSION,
57 PLUGIN_DESC };
}}}
to
{{{
50 #define PLUGIN_NAME "whoami-plugin"
53 #define PLUGIN_DESC "whoami extended operation plugin"
54 #define WHOAMI_EXOP_REQUEST_OID "1.3.6.1.4.1.4203.1.11.3"
55
56 static Slapi_PluginDesc expdesc = { PLUGIN_NAME, VENDOR, DS_PACKAGE_VERSION,
57 PLUGIN_DESC };
}}}
And these are small things and I know we are not doing well in this area, but to improve the codes' readability, could you try to keep your coding style and follow them all the time? (Or when you modify the existing code, follow the authors' style as much as possible?)
For instance, comparing the line 64 and 69, line 64 has a white space between * and pb, while line 69 does not have it. On the other hand, line 69 has a white space just after the open parenthesis and before the close one, while lne 64 does not have them.
64 int whoami_init(Slapi_PBlock * pb);
69 int whoami_exop( Slapi_PBlock *pb )
You have a white space after ',' in line 120 and 121, but you have none in line 123.
120 if ( slapi_pblock_set( pb, SLAPI_EXT_OP_RET_OID, NULL) != 0 ||
121 slapi_pblock_set( pb, SLAPI_EXT_OP_RET_VALUE, &retbval) != 0 )
123 slapi_log_error(SLAPI_LOG_FATAL,PLUGIN_NAME,"Could not set return values");

I think the line 132 and 133 can be merged into one line. Comparing to the other long lines, there's no need to be folded?
132 slapi_send_ldap_result( pb, LDAP_SUCCESS, NULL,
133 NULL, 0, NULL );
The same here.
150 if ( slapi_pblock_set( pb, SLAPI_PLUGIN_VERSION,
151 SLAPI_PLUGIN_VERSION_03 ) != 0 ||
152 slapi_pblock_set( pb, SLAPI_PLUGIN_DESCRIPTION,
153 (void )&expdesc ) != 0 ||
154 slapi_pblock_set( pb, SLAPI_PLUGIN_EXT_OP_FN,
155 (void
) whoami_exop ) != 0 ||

These are just examples. Please consider these are applied to all the lines. ;)

Replying to [comment:17 nhosoi]:

VENDOR and VERSION are automatically set by the installer. Please change this section

{{{
50 #define PLUGIN_NAME "whoami-plugin"
51 #define PLUGIN_VENDOR "Red Hat"
52 #define PLUGIN_VERSION "0.5"
53 #define PLUGIN_DESC "whoami extended operation plugin"
54 #define WHOAMI_EXOP_REQUEST_OID "1.3.6.1.4.1.4203.1.11.3"
55
56 static Slapi_PluginDesc expdesc = { PLUGIN_NAME, PLUGIN_VENDOR, PLUGIN_VERSION,
57 PLUGIN_DESC };
}}}
to
{{{
50 #define PLUGIN_NAME "whoami-plugin"
53 #define PLUGIN_DESC "whoami extended operation plugin"
54 #define WHOAMI_EXOP_REQUEST_OID "1.3.6.1.4.1.4203.1.11.3"
55
56 static Slapi_PluginDesc expdesc = { PLUGIN_NAME, VENDOR, DS_PACKAGE_VERSION,
57 PLUGIN_DESC };
}}}
done
And these are small things and I know we are not doing well in this area, but to improve the codes' readability, could you try to keep your coding style and follow them all the time? (Or when you modify the existing code, follow the authors' style as much as possible?)
For instance, comparing the line 64 and 69, line 64 has a white space between * and pb, while line 69 does not have it. On the other hand, line 69 has a white space just after the open parenthesis and before the close one, while lne 64 does not have them.
64 int whoami_init(Slapi_PBlock * pb);
69 int whoami_exop( Slapi_PBlock *pb )
You have a white space after ',' in line 120 and 121, but you have none in line 123.
120 if ( slapi_pblock_set( pb, SLAPI_EXT_OP_RET_OID, NULL) != 0 ||
121 slapi_pblock_set( pb, SLAPI_EXT_OP_RET_VALUE, &retbval) != 0 )
123 slapi_log_error(SLAPI_LOG_FATAL,PLUGIN_NAME,"Could not set return values");

I think the line 132 and 133 can be merged into one line. Comparing to the other long lines, there's no need to be folded?
132 slapi_send_ldap_result( pb, LDAP_SUCCESS, NULL,
133 NULL, 0, NULL );
The same here.
150 if ( slapi_pblock_set( pb, SLAPI_PLUGIN_VERSION,
151 SLAPI_PLUGIN_VERSION_03 ) != 0 ||
152 slapi_pblock_set( pb, SLAPI_PLUGIN_DESCRIPTION,
153 (void )&expdesc ) != 0 ||
154 slapi_pblock_set( pb, SLAPI_PLUGIN_EXT_OP_FN,
155 (void
) whoami_exop ) != 0 ||

These are just examples. Please consider these are applied to all the lines. ;)
done

Your patches look good to me.

Sorry, this is my fault I made when I copied and pasted the section, but could you undo your commit, resurrect the line 1291 "#------------------------" in Makefile.am, rerun autogen.sh, and create the 0002 patch, again?
{{{
diff --git a/Makefile.am b/Makefile.am
1291 #------------------------
1292 1292 # libviews-plugin
1293 1293 #------------------------
}}}
And could you double check the cn=whoami,cn=plugins,cn=config entry in your /etc/dirsrv/slapd-<ID>/dse.ldif? What value the version and vendor attribute have?
{{{
nsslapd-pluginVersion: <VERSION>
nsslapd-pluginVendor: <VENDOR>
}}}

Pushed to master:

commit 35e2b38
commit beaad2c
commit 465fa85

Metadata Update from @nhosoi:
- Issue assigned to anjain
- Issue set to the milestone: 1.3.2 - 08/13 (August)

7 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/123

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)

3 years ago

Login to comment on this ticket.

Metadata