Issue #6346
Remote fields username and password not showing up in REST docs
Description
Remote serializer has fields username and password:
These fields don't seem to show up in the REST api docs:
https://docs.pulpproject.org/en/master/nightly/restapi.html#operation/remotes_file_file_create
Related issues
Associated revisions
History
#1
Updated by fao89 about 1 year ago
- Triaged changed from No to Yes
- Sprint set to Sprint 68
#2
Updated by fao89 about 1 year ago
- Status changed from NEW to ASSIGNED
- Assignee set to fao89
#3
Updated by fao89 about 1 year ago
They do not appear because they are write_only: https://github.com/pulp/pulpcore/pull/361/files it is a drf_yasg limitation: https://github.com/axnsan12/drf-yasg/issues/70#issuecomment-368233077 For fixing this, we would have to apply this patch: https://github.com/axnsan12/drf-yasg/issues/70#issuecomment-485050813 Which would change our bindings
#4
Updated by rchan about 1 year ago
- Sprint changed from Sprint 68 to Sprint 69
#5
Updated by fao89 about 1 year ago
- Status changed from ASSIGNED to POST
#6
Updated by pulpbot about 1 year ago
#7
Updated by daviddavis about 1 year ago
#8
Updated by bmbouter about 1 year ago
I'm +1 on using SecretCharField. What would happen with a bindings user:
- Use the bindings to read() the an endpoint whose serializer is using SecretCharField, say as field
password
- Call an update() on the bindings object to update another attribute
asdf
Is there any possibility that while setting asdf
the value of password
is also changed? Or does it require an explicit update of the password
field itself? What about PUT versus PATCH calls?
#9
Updated by dkliban@redhat.com about 1 year ago
It looks like if you take the FileRemote object returned by the read() or create() method and pass it to the update() method, the client_cert field gets updated to a new value. This will be true for any SecretCharField.
(Pdb) file_remote {'ca_cert': None, 'client_cert': 'b3226466e6d9c43c7058f69e1ff41daaf688cd223c084faa3e28202813ecff28', 'client_key': None, 'download_concurrency': 20, 'name': 'bar25', 'policy': 'immediate', 'proxy_url': None, 'pulp_created': datetime.datetime(2020, 3, 25, 17, 9, 13, 631685, tzinfo=tzlocal()), 'pulp_href': '/pulp/api/v3/remotes/file/file/e42eea0d-e583-45ce-bdca-342169385cb2/', 'pulp_last_updated': datetime.datetime(2020, 3, 25, 17, 9, 13, 631703, tzinfo=tzlocal()), 'tls_validation': True, 'url': 'https://repos.fedorapeople.org/pulp/pulp/demo_repos/test_file_repo/PULP_MANIFEST'} (Pdb) fileremotes.update(file_remote.pulp_href, file_remote) {'task': '/pulp/api/v3/tasks/ec6a2bb6-4ce4-4a0e-ab1c-62d37aecfd27/'} (Pdb) file_updated_remote = fileremotes.read(file_remote.pulp_href) (Pdb) file_updated_remote {'ca_cert': None, 'client_cert': '7fc2a6b69d81c4581eac98454217b173a8b23256eefad0bb9eaabe199d8baae8', 'client_key': None, 'download_concurrency': 20, 'name': 'bar25', 'policy': 'immediate', 'proxy_url': None, 'pulp_created': datetime.datetime(2020, 3, 25, 17, 9, 13, 631685, tzinfo=tzlocal()), 'pulp_href': '/pulp/api/v3/remotes/file/file/e42eea0d-e583-45ce-bdca-342169385cb2/', 'pulp_last_updated': datetime.datetime(2020, 3, 25, 17, 10, 37, 747011, tzinfo=tzlocal()), 'tls_validation': True, 'url': 'https://repos.fedorapeople.org/pulp/pulp/demo_repos/test_file_repo/PULP_MANIFEST'}
#10
Updated by fao89 about 1 year ago
write_only=True
pulp-2to3-migration:
validate = serializers.BooleanField(
help_text=_('If ``True``, migration cannot happen without successful validation '
'of the Migration Plan'),
required=False,
default=False,
write_only=True
)
dry_run = serializers.BooleanField(
help_text=_('If ``True``, performs validation of a Migration Plan only, no migration is '
'run.'),
required=False,
default=False,
write_only=True
)
pulp-certguard:
ca_certificate = serializers.FileField(
help_text=_("The Certificate Authority certificate."),
write_only=True
)
pulp_container:
content_units = serializers.ListField(
help_text=_('A list of content units to operate on.'),
write_only=True,
required=False
)
source_repository = serializers.HyperlinkedRelatedField(
help_text=_('A URI of the repository to copy content from.'),
queryset=models.ContainerRepository.objects.all(),
view_name='repositories-container/container-detail',
label=_('Repository'),
write_only=True,
required=False,
)
source_repository_version = NestedRelatedField(
help_text=_('A URI of the repository version to copy content from.'),
view_name='versions-detail',
lookup_field='number',
parent_lookup_kwargs={'repository_pk': 'repository__pk'},
queryset=RepositoryVersion.objects.all(),
write_only=True,
required=False,
)
containerfile = serializers.FileField(
help_text=_(
"An uploaded Containerfile that should be used to run buildah."
),
required=False,
write_only=True,
)
pulpcore:
relative_path = serializers.CharField(
help_text=_("Path where the artifact is located relative to distributions base_path"),
validators=[fields.relative_path_validator],
write_only=True,
)
username = serializers.CharField(
help_text='The username to be used for authentication when syncing.',
write_only=True,
required=False,
allow_null=True,
)
password = serializers.CharField(
help_text='The password to be used for authentication when syncing.',
write_only=True,
required=False,
allow_null=True,
)
add_content_units = serializers.ListField(
help_text=_('A list of content units to add to a new repository version. This content is '
'added after remove_content_units are removed.'),
write_only=True,
required=False
)
remove_content_units = serializers.ListField(
help_text=_("A list of content units to remove from the latest repository version. "
"You may also specify '*' as an entry to remove all content. This content is "
"removed before add_content_units are added."),
write_only=True,
required=False
)
file = serializers.FileField(
help_text=_("A chunk of the uploaded file."),
write_only=True,
)
sha256 = serializers.CharField(
help_text=_("The SHA-256 checksum of the chunk if available."),
required=False,
allow_null=True,
write_only=True,
)
file = FileField(
help_text=_(
"An uploaded file that should be turned into the artifact of the content unit."
),
required=False,
write_only=True,
)
repository = DetailRelatedField(
help_text=_(
"A URI of a repository the new content unit should be associated with."
),
required=False,
write_only=True,
queryset=Repository.objects.all(),
)
#11
Updated by bmbouter about 1 year ago
I see the write_only
usage falling into two categories.
write_only for secrecy¶
These are fields that are unsafe to be re-shown to the user. These are the ones that should have a solution similar to SecretCharField
that shows you the sha256 when viewed, and refuses to overwrite data if you're submitting the sha256 of the data already stored.
The only ones I think that fall in this category are: Remote.username
, Remote.password
write_only for correctness¶
These are all the others, and I think these should be viewable but only allowed to set once. These would be all other fields.
#12
Updated by daviddavis about 1 year ago
I think there's also a third category: a field that is not actually stored in the database. The fields on MigrationPlanRunSerializer[0] are an example.
In such cases, perhaps we just roll a write only serializer to be used for POST/PUT/PATCH (but not GET) and not set the write_only option on these fields.
#13
Updated by bmbouter about 1 year ago
daviddavis wrote:
I think there's also a third category: a field that is not actually stored in the database. The fields on MigrationPlanRunSerializer[0] are an example.
In such cases, perhaps we just roll a write only serializer to be used for POST/PUT/PATCH (but not GET) and not set the write_only option on these fields.
Agreed, sounds great.
#14
Updated by pulpbot about 1 year ago
#15
Updated by pulpbot about 1 year ago
#16
Updated by pulpbot about 1 year ago
#17
Updated by fao89 about 1 year ago
how about the initial issue that made me hide write_only fields? https://pulp.plan.io/issues/5622
#18
Updated by fao89 about 1 year ago
Two options:
1 - https://github.com/pulp/pulpcore/pull/600
- write_only fields do not show up in the docs
- username and password are not write_only anymore
- browsable API response == httpie response == Rest API docs response
2- https://github.com/pulp/pulpcore/pull/613
- write_only fields show up in the docs
- (browsable API response == httpie response) != Rest API docs response
#19
Updated by daviddavis about 1 year ago
- Blocked by Task #6421: Review of write_only fields in pulp and plugins added
#20
Updated by rchan about 1 year ago
- Sprint changed from Sprint 69 to Sprint 70
#21
Updated by rchan about 1 year ago
- Sprint changed from Sprint 70 to Sprint 71
#24
Updated by Anonymous 12 months ago
- Status changed from POST to MODIFIED
Applied in changeset pulpcore|5688324ff35fb67776f7f0630275cfbfc84ea163.
#26
Updated by daviddavis 11 months ago
@snecklifter, we have a 3.4 release planned for next week. Would that work for you?
#28
Updated by Anonymous 11 months ago
daviddavis yes, thanks
#29
Updated by dkliban@redhat.com 11 months ago
- Sprint/Milestone set to 3.4.0
#30
Updated by dkliban@redhat.com 11 months ago
- Status changed from MODIFIED to CLOSED - CURRENTRELEASE
Please register to edit this issue
Return username & password when requesting Remote
https://pulp.plan.io/issues/6346 closes #6346