Issue #9464
closedRegression: Syncing username+password authenticated remotes fails
Description
We have issues syncing remotes protected with username and password (using Ansible):
FAILED - RETRYING: Sync vendor-product-develop-8 repository from remote (15 retries left).Result was: {
"attempts": 1,
"changed": false,
"invocation": {
"module_args": {
"mirror": true,
"password": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
"pulp_url": "http://localhost:24817",
"refresh_api_cache": false,
"remote": "vendor-product-develop-8-remote",
"repository": "vendor-product-develop-8",
"username": "admin",
"validate_certs": true
}
},
"msg": "Task failed to complete. (failed; https://username_with_underscores-and-dashes:somepasswordstring%40downloads.vendor.url.com/repo/product/rhel/8/x86_64/current/repodata/repomd.xml)",
"retries": 16
}
Seems that escaping @ to %40 breaks the URL and repo sync.
[root@pulp32:~] pulp rpm remote show --name hp-fwpp-develop-8-remote
{
"pulp_href": "/pulp/api/v3/remotes/rpm/rpm/0e309c91-8e25-4e65-b9f2-73258fd347c9/",
"pulp_created": "2021-05-04T18:34:02.181552Z",
"name": "vendor-product-develop-8-remote",
"url": "https://username_with_underscores-and-dashes:somepasswordstring@downloads.vendor.url.com/repo/product/rhel/8/x86_64/current/",
"ca_cert": null,
"client_cert": null,
"tls_validation": true,
"proxy_url": null,
"pulp_labels": {},
"pulp_last_updated": "2021-05-04T18:34:02.181568Z",
"download_concurrency": 10,
"max_retries": null,
"policy": "immediate",
"total_timeout": null,
"connect_timeout": null,
"sock_connect_timeout": null,
"sock_read_timeout": null,
"headers": null,
"rate_limit": null,
"sles_auth_token": null
}
Related issues
Updated by martin about 3 years ago
- Copied from Issue #9329: Regression: Syncing mirrolist based remotes do fail again since 3.14.1 added
Updated by daviddavis about 3 years ago
Looks like this change broke this functionality:
https://github.com/pulp/pulp_rpm/commit/db55c27a9e78a18b64849915bc33628c8a9456fd
Updated by daviddavis about 3 years ago
- Related to Story #9457: [EPIC] As a user, I am given an error if I try to use a url with basic auth added
Updated by ggainey about 3 years ago
There are a couple of approaches available.
One is to sanitize a remote-url at remote-creation-time. That implies using urlparse() to break up the URL, "do the right thing" with the pieces (ie, strip user/pwd out and put them in the right fields on the remote, urlencode things that need to be url-encoded, etc), and reassemble the URL to be correct. A second is to do this at download-time, and fix the url "on the fly"
The first is better, but will require a migration to inspect every stored-remote-url and fix them up. The second is ad-hoc, but doesn't require any fixing up of existing data.
More thought required :)
Updated by ggainey about 3 years ago
One serious issue with letting username/pwd stay on the URL, is that it means they're not protected by the db-encryption we've added for sensitive fields. That leads us even more to option-1 above.
Updated by ggainey about 3 years ago
- Status changed from NEW to ASSIGNED
- Assignee changed from dalley to ggainey
Updated by ipanova@redhat.com about 3 years ago
ggainey wrote:
There are a couple of approaches available.
One is to sanitize a remote-url at remote-creation-time. That implies using urlparse() to break up the URL, "do the right thing" with the pieces (ie, strip user/pwd out and put them in the right fields on the remote, urlencode things that need to be url-encoded, etc), and reassemble the URL to be correct. A second is to do this at download-time, and fix the url "on the fly"
Should not we just raise a validation error during remote creation instead of this obscure autopsy? :)
The first is better, but will require a migration to inspect every stored-remote-url and fix them up. The second is ad-hoc, but doesn't require any fixing up of existing data.
More thought required :)
Updated by ggainey about 3 years ago
- Sprint changed from Sprint 105 to Sprint 106
Updated by ggainey about 3 years ago
- Copied to Backport #9472: Backport to 3.14.z: Regression: Syncing username+password authenticated remotes fails added
Updated by pulpbot about 3 years ago
- Status changed from ASSIGNED to POST
Added by ggainey about 3 years ago
Updated by ggainey about 3 years ago
- Status changed from POST to MODIFIED
Applied in changeset 6cbae29b4a08e5452f3634c204a4266f344f8b6c.
Updated by pulpbot about 3 years ago
- Status changed from MODIFIED to CLOSED - CURRENTRELEASE
Restore embedded-basic-auth-in-url functionality.
fixes #9464 [nocoverage]