creating comment via API: cryptic error message about missing author

[decathorpe]

I'm working on my rust bindings for the bodhi REST API, but I can't get the creation of comments via POST requests to work. I get the following error message from a session that successfully authenticated via OpenID:

{
    "status": "error",
    "errors": [
        {
            "location": "body",
            "name": "comment",
            "description": "You must provide a comment author"
        }
    ]
}

However, if I add a author field to the JSON body of the comment, I get a KeyError for author - user and username don't work either.

I see some other recent issues mentioning a missing "author", but these are from the bodhi python bindings, and I'm not sure it's related.

[bowlofeggs]

Hi @decathorpe!

I see some tracebacks on the server-side for this that might be helpful:

2019-06-06 16:17:56,645 ERROR [bodhi.server][Dummy-1] Error caught.  Handling JSON response.
Traceback (most recent call last):
  File "/usr/lib/python3.7/site-packages/pyramid/tweens.py", line 39, in excview_tween
    response = handler(request)
  File "/usr/lib/python3.7/site-packages/pyramid/router.py", line 156, in handle_request
    view_name
  File "/usr/lib/python3.7/site-packages/pyramid/view.py", line 642, in _call_view
    response = view_callable(context, request)
  File "/usr/lib/python3.7/site-packages/pyramid/config/views.py", line 181, in __call__
    return view(context, request)
  File "/usr/lib/python3.7/site-packages/pyramid/viewderivers.py", line 390, in attr_view
    return view(context, request)
  File "/usr/lib/python3.7/site-packages/pyramid/viewderivers.py", line 368, in predicate_wrapper
    return view(context, request)
  File "/usr/lib/python3.7/site-packages/pyramid/viewderivers.py", line 439, in rendered_view
    result = view(context, request)
  File "/usr/lib/python3.7/site-packages/pyramid/viewderivers.py", line 148, in _requestonly_view
    response = view(request)
  File "/usr/lib/python3.7/site-packages/cornice/service.py", line 487, in wrapper
    validator(request, **args)
  File "/usr/lib/python3.7/site-packages/cornice/validators/_colander.py", line 70, in _validator
    validator(request, RequestSchema(), deserializer, **kwargs)
  File "/usr/lib/python3.7/site-packages/cornice/validators/_colander.py", line 113, in validator
    deserialized = schema.deserialize(cstruct)
  File "/usr/lib/python3.7/site-packages/colander/__init__.py", line 2073, in deserialize
    appstruct = self.typ.deserialize(self, cstruct)
  File "/usr/lib/python3.7/site-packages/colander/__init__.py", line 724, in deserialize
    return self._impl(node, cstruct, callback)
  File "/usr/lib/python3.7/site-packages/colander/__init__.py", line 683, in _impl
    sub_result = callback(subnode, subval)
  File "/usr/lib/python3.7/site-packages/colander/__init__.py", line 722, in callback
    return subnode.deserialize(subcstruct)
  File "/usr/lib/python3.7/site-packages/bodhi/server/schemas.py", line 145, in deserialize
    appstruct = SaveCommentSchema().unflatten(cstruct)
  File "/usr/lib/python3.7/site-packages/colander/__init__.py", line 2037, in unflatten
    return self.typ.unflatten(self, paths, fstruct)
  File "/usr/lib/python3.7/site-packages/colander/__init__.py", line 744, in unflatten
    return _unflatten_mapping(node, paths, fstruct)
  File "/usr/lib/python3.7/site-packages/colander/__init__.py", line 2348, in _unflatten_mapping
    subnode = get_child(curname)
  File "/usr/lib/python3.7/site-packages/colander/__init__.py", line 2195, in __getitem__
    raise KeyError(name)
KeyError: 'author'

and

2019-06-06 16:41:28,338 ERROR [bodhi.server][Dummy-2] Error caught.  Handling JSON response.
Traceback (most recent call last):
  File "/usr/lib/python3.7/site-packages/pyramid/tweens.py", line 39, in excview_tween
    response = handler(request)
  File "/usr/lib/python3.7/site-packages/pyramid/router.py", line 156, in handle_request
    view_name
  File "/usr/lib/python3.7/site-packages/pyramid/view.py", line 642, in _call_view
    response = view_callable(context, request)
  File "/usr/lib/python3.7/site-packages/pyramid/config/views.py", line 181, in __call__
    return view(context, request)
  File "/usr/lib/python3.7/site-packages/pyramid/viewderivers.py", line 390, in attr_view
    return view(context, request)
  File "/usr/lib/python3.7/site-packages/pyramid/viewderivers.py", line 368, in predicate_wrapper
    return view(context, request)
  File "/usr/lib/python3.7/site-packages/pyramid/viewderivers.py", line 439, in rendered_view
    result = view(context, request)
  File "/usr/lib/python3.7/site-packages/pyramid/viewderivers.py", line 148, in _requestonly_view
    response = view(request)
  File "/usr/lib/python3.7/site-packages/cornice/service.py", line 487, in wrapper
    validator(request, **args)
  File "/usr/lib/python3.7/site-packages/cornice/validators/_colander.py", line 70, in _validator
    validator(request, RequestSchema(), deserializer, **kwargs)
  File "/usr/lib/python3.7/site-packages/cornice/validators/_colander.py", line 113, in validator
    deserialized = schema.deserialize(cstruct)
  File "/usr/lib/python3.7/site-packages/colander/__init__.py", line 2073, in deserialize
    appstruct = self.typ.deserialize(self, cstruct)
  File "/usr/lib/python3.7/site-packages/colander/__init__.py", line 724, in deserialize
    return self._impl(node, cstruct, callback)
  File "/usr/lib/python3.7/site-packages/colander/__init__.py", line 683, in _impl
    sub_result = callback(subnode, subval)
  File "/usr/lib/python3.7/site-packages/colander/__init__.py", line 722, in callback
    return subnode.deserialize(subcstruct)
  File "/usr/lib/python3.7/site-packages/bodhi/server/schemas.py", line 145, in deserialize
    appstruct = SaveCommentSchema().unflatten(cstruct)
  File "/usr/lib/python3.7/site-packages/colander/__init__.py", line 2037, in unflatten
    return self.typ.unflatten(self, paths, fstruct)
  File "/usr/lib/python3.7/site-packages/colander/__init__.py", line 744, in unflatten
    return _unflatten_mapping(node, paths, fstruct)
  File "/usr/lib/python3.7/site-packages/colander/__init__.py", line 2358, in _unflatten_mapping
    subnode = get_child(curname)
  File "/usr/lib/python3.7/site-packages/colander/__init__.py", line 2195, in __getitem__
    raise KeyError(name)
KeyError: 'user'

[bowlofeggs]

It may be helpful to inspect the POST that the Python client is sending as an example.

[decathorpe]

Thanks! I tried running bodhi with the --debug flag, but that doesn't seem to trigger log.debug() to actually print the data as it should ...

[decathorpe]

I've managed to "hack" the bodhi python client bindings to print the session and arguments when it does its POST requests, but now I'm stuck again.

Because the python bindings send exactly what I expect them to. There is no mention of "author" or "user" anywhere, not even in the cookies that are set in the requests session.

PS: The KeyError exceptions above happened when I added a "author" or "user" field to the comment data - to try to placate the bodhi server's "have to provide an author" message, but that's obviously not how it wants to be informed of the username ...

[decathorpe]

Additionally, the documentation also doesn't mention "author" or "user" at all:

https://bodhi.fedoraproject.org/docs/server_api/rest/comments.html#service-1-POST

[bowlofeggs]

@decathorpe I would expect the author to be known from the Pyramid session, though I honestly don't know a lot about how that works. Are you sending the same session info that the Python one is sending?

It is not hard to make a local development environment with Vagrant: https://bodhi.fedoraproject.org/docs/developer/vagrant.html

I recommend doing that so you can control the server side and inspect how it works.

[decathorpe]

Looking at the bodhi code, pyramid is used exclusively on the server-side. The bodhi python client handles this simply via the requests session set up via fedora's OpenIDBaseClient.

The session setup I'm doing works the same as the python OpenIDBaseClient, and should result in the same cookies being set.

However, I found where the "missing author" error comes from: https://github.com/fedora-infra/bodhi/blob/develop/bodhi/server/models.py#L2872

It gets the "author" argument from here: https://github.com/fedora-infra/bodhi/blob/develop/bodhi/server/services/comments.py#L210

Which queries the pyramid request for either user or user.name - which both don't seem to be there for some reason. I'll continue debugging ... in the meantime, thanks for your help!

[bowlofeggs]

Good luck, and let me know what you find! I honestly haven't looked into how this works before, since it was already working when I started working on Bodhi and I've never had to mess with it.

[bowlofeggs]

@decathorpe I just realized I should let you know that we are planning to switch Bodhi to OpenID Connect: https://github.com/fedora-infra/bodhi/issues/1180

I believe that @puiterwijk wants to get that done soon.

[decathorpe]

@bowlofeggs okay, thanks for the heads-up. I guess I'll have to implement an OpenID Connect library in rust next :joy:

[decathorpe]

Side note: This is not specific to my rust bindings.

I just adapted fedora-easy-karma to the new bodhi API and now I'm getting the same "You must provide a comment author" error from fedora-easy-karma, which uses the first-party bodhi python bindings:

bodhi.client.bindings.BodhiClientException: You must provide a comment author

[kparal]

Here's a full traceback from fedora-easy-karma:

Traceback (most recent call last):
  File "./fedora-easy-karma.py", line 850, in <module>
    fek = FedoraEasyKarma()
  File "./fedora-easy-karma.py", line 710, in __init__
    karma)
  File "./fedora-easy-karma.py", line 834, in send_comment
    res = bc.comment(update["alias"], comment, karma=karma)
  File "/usr/lib/python3.7/site-packages/bodhi/client/bindings.py", line 145, in wrapper
    raise BodhiClientException(problems)
bodhi.client.bindings.BodhiClientException: You must provide a comment author

It can be reproduced easily:

$ ipython3
import bodhi.client
bc = bodhi.client.bindings.BodhiClient(username='yourname')
bc.comment('FEDORA-2019-55b2db0893', 'test comment', 0)
---------------------------------------------------------------------------
BodhiClientException                      Traceback (most recent call last)
<ipython-input-7-16fdfa314941> in <module>
----> 1 bc.comment('FEDORA-2019-55b2db0893', 'test comment', 0)

/usr/lib/python3.7/site-packages/bodhi/client/bindings.py in wrapper(*args, **kwargs)
    143         except Exception:
    144             pass
--> 145         raise BodhiClientException(problems)
    146     return wrapper
    147 

BodhiClientException: You must provide a comment author

@bowlofeggs Can you please look into this soon? Fixing fedora-easy-karma is quite high priority for QA, thank you.

[kparal]

Even the official CLI tool doesn't work:

$ bodhi updates comment --user kparal --karma 0 FEDORA-2019-55b2db0893 "test comment, please ignore"
You must provide a comment author

[kparal]

As a heads up, @bowlofeggs is away for 2 months and @abompard for 3 weeks, so we'll need to wait until somebody has time to look into this.

[nphilipp]

Even the official CLI tool doesn't work:

$ bodhi updates comment --user kparal --karma 0 FEDORA-2019-55b2db0893 "test comment, please ignore"
You must provide a comment author

Hmm, this just worked for me:

nils@gibraltar:~> bodhi updates comment --user=nphilipp --karma +1 --debug FEDORA-2019-38fb6b744d "Works for me, including context help."
Password: 
The following comment was added to FEDORA-2019-38fb6b744d
Works for me, including context help.
nils@gibraltar:~> 

Here's the comment it created: https://bodhi.fedoraproject.org/updates/FEDORA-2019-38fb6b744d#comment-968141

NB: this is bodhi-client-4.0.2-2.fc30.noarch, @kparal what's the version you use?

[kparal]

Interesting, after I removed ~/.fedora/openidbaseclient-sessions.cache I was suddenly able to submit comments through bodhi updates comment and fedora-easy-karma (bodhi4 branch). It seems that something in already existing session cache is breaking this functionality, perhaps it fails to refresh some session keys? I have my "broken" cache file saved and can email it to developers if needed.

[decathorpe]

If that's the case, then there these are probably two different issues - since the rust code I'm using for OpenID authentication isn't using the session cache file at all.

[kparal]

Interesting, after I removed ~/.fedora/openidbaseclient-sessions.cache I was suddenly able to submit comments through bodhi updates comment and fedora-easy-karma (bodhi4 branch).

The old session file has 8 items in the list, the new one has 9 items. It seems the structure of the file changed, but no conversion is performed, and the existing code fails to work with the old one.

[AdamWill]

@puiterwijk since this is openID auth stuff, were you involved by any chance?

[frantisekz]

So, is there any plan to move forward? This broke fedora-easy-karma which is quite a complication for updates testing.

@abompard , are you the new bodhi maintainer? Can you look into this?

[AdamWill]

A further note from @kparal on the 'downstream' issue:

"My experience was that I had to clear cache every few days or weeks, the error seemed to pop up at random intervals."

so presumably this isn't the 8 items vs. 9 items thing as you initially thought, but rather some kind of cache staleness issue?

[puiterwijk]

https://github.com/fedora-infra/bodhi/blob/develop/bodhi/server/services/comments.py#L216 doesn’t require the caller of this endpoint to be authenticated (no ‘factory’ argument), but fills ‘author’ based on the current authenticated session. The python-Fedora openidbaseclient only attempts authentication if required by a 403 error code or an OpenID redirect. This would return neither of those, so it won’t know that the auth session expired.

So if you request any other endpoints that require auth, I predict it’ll start auth, and then commenting will work again until that session expires.

[AdamWill]

heh, that's just about what I was figuring out right now, yeah. So how do you reckon we should fix this? Fix it on server end by requiring auth in the comment function?

[AdamWill]

Aha - I think I might see when this broke. The whole 'allow anon comments with captcha' thing that the client goes to some trouble to deal with was removed from the server early this year, and that commit simplifies the bit @puiterwijk pointed to quite a lot, including taking out a check that 'author' is actually set. I think the fix here might simply be to restore that if not author: block that was removed. (Or I guess just add the factory arg that @puiterwijk mentioned if that's simpler and achieves the same result, because we don't need to worry about the anonymous comment case any more).

[puiterwijk]

Well, if we no longer allow anonymous comments, the endpoint requires authentication to function.... so yeah, that’s the correct fix then.

[AdamWill]

right, makes sense. I'll just send a PR to make the endpoint require auth.

edit: hum, actually using an ACL factory isn't trivial because we don't have one for 'any authed user' (only for admins and packagers), and I'm not sure just restoring the if not author: block will do the trick (I think it would set the response code to 400, which doesn't happen currently, but not sure if that'll be enough to make things work). So I'm gonna leave it for today, if @puiterwijk or @abompard comes up with a fix soon great, otherwise I'll poke it some more tomorrow.

edit again: Hum, given how openidbaseclient requires_login works, I think if we make the response a 403 (by using pyramid.httpexceptions.HTTPForbidden) that would probably work...

[AdamWill]

OK, finally managed to get a bodhi dev env running again (this is always a nightmare for some reason) and I'm pretty sure my HTTPForbidden fix works. Just triple checking it now and will send a PR soon.

[AdamWill]

So my change won't necessarily fix @decathorpe 's initial problem exactly, since it's intended to trigger the code the Python client library has to try and reauthenticate if it receives a 403 error from the server, but @decathorpe was not using the Python client library at least initially. Still, I think it is pretty likely @decathorpe had basically the same problem: trying to post a comment while anonymous. Armed with that knowledge I think he ought to be able to handle it in his client code and we don't necessarily need to do anything further server side. I suppose the text of the error message could be tweaked to say more specifically that the cause of the problem might be that the session isn't authenticated...

wdyt, @decathorpe ?

[decathorpe]

@AdamWill I think I was hitting the same issue for a different reason - because I should always be already authenticated when hitting that API, due to a different library design in my fedora/bodhi bindings.

But it turned out that the OpenID authentication isn't working correctly yet client-side (probably my fault) - and I still need to investigate why (the corresponding code in python-fedora reads like hedge maze that hasn't been tended for 10 years, so it's gonna take some time).

But thanks for keeping me in the loop :)

EDIT: I think I've fixed the first issue now (by manually handling the cookies correctly). The "You must provide a comment author" is now only displayed if the user has not successfully authenticated (e.g. if providing a wrong username or password).

Now I'm getting CSRF token mismatches when trying to do stuff like create comments ... let's see if I can fix that next :crying_cat_face:

[decathorpe]

For the record: In the meantime, I managed to get things to work correctly. My rust bindings for the REST API are now working and feature-complete :tada: (and I already built a working fedora-easy-karma replacement and an alternative bodhi CLI client on top of them)

Thanks again for everybody's help with debugging and fixing the original issue.

[AdamWill]

Cool! Can we see them? :)

[decathorpe]

@AdamWill: Cool! Can we see them? :)

Sure, all projects I mentioned are inside this GitHub org :) https://github.com/ironthree

I might look into packaging them for fedora, as well, though I haven't done any rust packaging for fedora yet.

[AdamWill]

Do you mind if I point it up for the Fedora QA mailing list and ask folks to give it a spin?

[decathorpe]

@AdamWill Sure! Also, RFEs and issue reports are welcome. I've been using my tools for a few weeks now and most things should "just work" ©

[AdamWill]

@abompard it seems like this wasn't included in 5.1.1. Can we please include it in a version that will get deployed? fedora-easy-karma is broken until this fix is deployed :(

If it doesn't get out soon I'll have to write a downstream patch for the client or something...