Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarification of user authenticated webhook pings #11733

Closed
ianw opened this issue Oct 31, 2024 · 3 comments
Closed

Clarification of user authenticated webhook pings #11733

ianw opened this issue Oct 31, 2024 · 3 comments
Labels
Needed: more information A reply from issue author is required Support Support question

Comments

@ianw
Copy link

ianw commented Oct 31, 2024

Details

Per https://rtdname.readthedocs.io/en/latest/webhooks.html#authentication

This endpoint requires authentication. If authenticating with an integration token, a check will determine if the token is valid and matches the given project. If instead an authenticated user is used to make this request, a check will be performed to ensure the authenticated user is an owner of the project.

The opendev.org project has been using this second method, where central infrastructure stores the username/password for openstackci user and projects add that user as an owner. Projects then specify their webhook end-point (not secret) and the central CI system looks after pinging this webhook on documentation merges using this username/password to authenticate (https://lists.openstack.org/pipermail/openstack-dev/2018-August/132836.html)

This way projects don't need to worry about keeping a secret, their authorization is essentially to add the openstackci user to their project which they accept can trigger builds.

We think it might be related to #11083? We can try and pull out some job logs if this doesn't make immediate sense :)

It would be really great if this continued to work, because otherwise every project will need to generate and keep it's own secret, which is a big change for them all to do.

@ianw
Copy link
Author

ianw commented Oct 31, 2024

To be completely clear about what we're doing -> https://opendev.org/zuul/zuul-jobs/src/branch/master/roles/trigger-readthedocs/tasks/main.yaml#L1

Users give us their webook in rtd_webhook_id and rtd_credentials come from unshared secrets, we then POST with

 - name: Trigger readthedocs build webhook via authentication
      uri:
        method: POST
        url: 'https://readthedocs.org/api/v2/webhook/{{ rtd_project_name }}/{{ rtd_webhook_id }}/'
        user: '{{ rtd_credentials.username }}'
        password: '{{ rtd_credentials.password }}'
        # NOTE(ianw): testing it seems the API doesn't respond with
        # 401 so this is required
        force_basic_auth: yes
      # avoid logging any credentials
      no_log: true

@stsewd
Copy link
Member

stsewd commented Oct 31, 2024

Hi, by reading the code, I see that we still support this use case. But sending the body is always required, an empty dict should work.

@stsewd stsewd added Support Support question Needed: more information A reply from issue author is required labels Oct 31, 2024
@ianw
Copy link
Author

ianw commented Nov 1, 2024

Thank you for looking over that and confirming.

Based on this I've done a little more investigation on our side with a manual test and I agree that the ping with user authentication does seem to work when simply done with a curl POST.

I think actually the problem might be that projects have missed the requirement for a .readthedocs.yaml file and so their builds fail; because CI doesn't log the results of the ping (to avoid any chance of leaking the password) the failure to build can look like a failure to ping when the user doesn't have access to the build results - the new build isn't published as expected in both cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needed: more information A reply from issue author is required Support Support question
Projects
None yet
Development

No branches or pull requests

2 participants