Last Comment Bug 1204683 - Add whoami endpoint
: Add whoami endpoint
Status: RESOLVED FIXED
:
Product: bugzilla.mozilla.org
Classification: Other
Component: API (show other bugs)
: Production
: Unspecified Unspecified
-- normal (vote)
: ---
Assigned To: David Lawrence [:dkl]
:
:
Mentors:
Depends on:
Blocks: 1307003
  Show dependency treegraph
 
Reported: 2015-09-14 15:39 PDT by Mark Côté [:mcote]
Modified: 2016-10-02 14:23 PDT (History)
2 users (show)
See Also:
Due Date:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
1204683_1.patch (4.68 KB, patch)
2015-09-14 17:59 PDT, David Lawrence [:dkl]
glob: review-
Details | Diff | Splinter Review
1204683_2.patch (4.02 KB, patch)
2015-09-23 14:04 PDT, David Lawrence [:dkl]
glob: review+
Details | Diff | Splinter Review

Description User image Mark Côté [:mcote] 2015-09-14 15:39:20 PDT
We need an API to get user information for the currently logged-in user, particularly from an API key.
Comment 1 User image David Lawrence [:dkl] 2015-09-14 17:02:06 PDT
taking so no one else will work on this at the same time as myself
Comment 2 User image David Lawrence [:dkl] 2015-09-14 17:59:31 PDT
Created attachment 8661025 [details] [diff] [review]
1204683_1.patch
Comment 3 User image Byron Jones ‹:glob› 2015-09-21 22:47:34 PDT
Comment on attachment 8661025 [details] [diff] [review]
1204683_1.patch

Review of attachment 8661025 [details] [diff] [review]:
-----------------------------------------------------------------

::: Bugzilla/WebService/User.pm
@@ +437,5 @@
> +    return filter $params, {
> +        id        => $self->type('int', $user->id),
> +        real_name => $self->type('string', $user->name),
> +        name      => $self->type('email', $user->login),
> +        email     => $self->type('email', $user->email),

this should match User.get, and return just id, real_name, and name (ie. return $user->email as 'name').

@@ +1116,5 @@
> +=item C<login> (string) - The user's login name.
> +
> +=item C<password> (string) - The user's password.
> +
> +=item C<token> (string) - The user's login token.

none of these are parameters for this method - they are standard auth params.

just document this as a method that doesn't take any params.

@@ +1128,5 @@
> +=over
> +
> +=item id
> +
> +C<int> The unique integer ID that Bugzilla uses to represent this user. 

remove trailing space

@@ +1142,5 @@
> +
> +=item name
> +
> +C<string> The login name of the user. Note that in some situations this is 
> +different than their email.

it's always their email address on bmo - no need for that 2nd sentence (applies to .rst too)

@@ +1170,5 @@
> +=item B<History>
> +
> +=over
> +
> +=item API call was added in Bugzilla B<4.2>.

remove this - it doesn't make any sense with bmo's versioning scheme.
Comment 4 User image David Lawrence [:dkl] 2015-09-23 14:04:38 PDT
Created attachment 8665117 [details] [diff] [review]
1204683_2.patch
Comment 5 User image Leif Gruenwoldt 2015-09-23 20:16:47 PDT
I see this bug is for BMO, so maybe my comment doesn't apply because, I'm looking at upstream, but perhaps this patch should also update the "Authentication Delegation via API Keys" docs page to suggest using this new 'whoami' endpoint rather than the "valid_login" one. As seen in bullet point #3 here:

http://bugzilla.readthedocs.org/en/latest/integrating/auth-delegation.html#auth-delegation

bugzilla/docs/en/rst/integrating/auth-delegation.rst
Comment 6 User image Byron Jones ‹:glob› 2015-09-23 21:29:00 PDT
Comment on attachment 8665117 [details] [diff] [review]
1204683_2.patch

Review of attachment 8665117 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to leifer from comment #5)
> I see this bug is for BMO, so maybe my comment doesn't apply because, I'm
> looking at upstream, but perhaps this patch should also update the
> "Authentication Delegation via API Keys" docs page to suggest using this new
> 'whoami' endpoint rather than the "valid_login" one. As seen in bullet point
> #3 here:
> 
> http://bugzilla.readthedocs.org/en/latest/integrating/auth-delegation.
> html#auth-delegation

excellent point - our docs are at http://bmo.readthedocs.org/en/latest/integrating/auth-delegation.html#auth-delegation
dkl - while you're updating that file, please change 'bugs.example.org' to 'bugzilla.mozila.org' :)

r=glob with these documentation changes.

::: Bugzilla/WebService/User.pm
@@ +1107,5 @@
> +logged in user.
> +
> +=item B<Params>
> +
> +Standard API authentication parameters.

the auth params apply to all methods, so it doesn't make sense to call them out explicitly here (this method doesn't take any params).

i suspect the point you're trying to make is you need to authenticate somehow to use this method - state that in the description rather than in the params docs.
Comment 7 User image David Lawrence [:dkl] 2015-09-24 07:48:14 PDT
To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
   89e6553..923afd7  master -> master
Comment 8 User image Leif Gruenwoldt 2016-10-02 12:30:42 PDT
Is this feature planned to go into upstream Bugzilla too?

Note You need to log in before you can comment on or make changes to this bug.