Issue #761
closed
Pulp allows "search" as a user's login ID
Description
redux of comments: The fix for this should be to disallow the user search from being created in the API, and adding a doc note in the authentication section about 'search' being disallowed as a username. I think having the API error is enough, the bindings and CLI should just show that error as a normal server side Pulp error.
Original report from rbarlow:
I noticed that we have a problem with our REST interface's URL structure. In our urls.py, the following two lines are present:
url(r'^v2/users/search/$', users.UserSearchView.as_view(),
name='user_search'),
url(r'^v2/users/(?P<login>[^/]+)/$', users.UserResourceView.as_view(), name='user_resource')
If there were a user named "search", it would be impossible to retrieve that user because the first URL would match and it would be assumed that the REST call was requesting a search, rather than a user named "search".
However, I did not see anything in our user creation code preventing a user named "search" from being created. However, I was unable to test this, due to #760.
Related issues
Updated by rbarlow about 9 years ago
- Related to Issue #760: Users cannot be created with pulp-admin added
Updated by rbarlow about 9 years ago
It seems to me that ideally the /users/ URL would accept the GET and POST parameters for searching, and then we wouldn't need to have a /users/search/ URL at all. In fact, this is probably true for all of our URLs. Perhaps this is something we should consider for 3.0.0.
Updated by bmbouter about 9 years ago
There are a lot of <stuff_here>/search/ urls in the Pulp API. The idea that GET and POST parameters should provide search is a good one, and should be applied to all search URLs, not just this one.
Regarding this defect, one possible backwards compatible fix would be to reorder these statements in urls.py. The problem with that is that search will break if the user makes a username called 'search'. I think not having one username that isn't allowed and can be documented is better than having search completely break if the user makes a user named 'search'.
Based on that I recommend we document this as a known limitation at a minimum. We could also put in some validation code at the API and/or CLI level.
Updated by rbarlow about 9 years ago
On 03/16/2015 09:24 AM, bmbouter wrote:
There are a lot of <stuff_here>/search/ urls in the Pulp API. The idea
that GET and POST parameters should provide search is a good one, and
should be applied to all search URLs, not just this one.
Agreed, I hinted at this in my comment.
Regarding this defect, one possible backwards compatible fix would be to
reorder these statements in urls.py. The problem with that is that
search will break if the user makes a username called 'search'.
I believe it's worse than that - if we reorder the URLs, it will be
impossible to search regardless of whether a user named search exists,
as "search" will always match as a user ID, and then the View for
dealing with individual users will always be used.
Updated by bmbouter about 9 years ago
rbarlow wrote:
Agreed, I hinted at this in my comment.
Indeed you did. I created #763 so that we could track that broader effort and set backwards incompatible = True.
I believe it's worse than that - if we reorder the URLs, it will be
impossible to search regardless of whether a user named search exists,
as "search" will always match as a user ID, and then the View for
dealing with individual users will always be used.
You're right it is worse then that. Is the fix for this issue to disallow 'search' as a user's login ID in the API? What about the CLI? Also how does the fix for this similar or different from a fix needed for #760?
Updated by rbarlow about 9 years ago
On 03/16/2015 11:52 AM, bmbouter wrote:
You're right it is worse then that. Is the fix for this issue to
disallow 'search' as a user's login ID in the API? What about the CLI?
Also how does the fix for this similar or different from a fix needed
for #760 <https://pulp.plan.io/issues/760>?
I think the fix is either to change the URL structure (which requires
waiting until 3.0.0), or to disallow "search" as the user's login ID
until then. I don't feel strongly one way or the other, but I wanted to
at least document the issue via a ticket.
#760 is not really related to this. I did try to create a user named
"search" as an example there, but it turns out that no user of any name
can be created. I mentioned it here because I was unable to verify for
sure that it was true that a user named "search" can be created. After
reviewing some code, I do believe it's possible, but I wanted to just
try it to find out.
Updated by dkliban@redhat.com about 9 years ago
- Severity changed from Medium to Low
- Triaged changed from No to Yes
- Tags deleted (
Easy Fix)
Updated by bmbouter about 5 years ago
- Status changed from NEW to CLOSED - WONTFIX
Updated by bmbouter about 5 years ago
Pulp 2 is approaching maintenance mode, and this Pulp 2 ticket is not being actively worked on. As such, it is being closed as WONTFIX. Pulp 2 is still accepting contributions though, so if you want to contribute a fix for this ticket, please reopen or comment on it. If you don't have permissions to reopen this ticket, or you want to discuss an issue, please reach out via the developer mailing list.
Updated by bmbouter almost 4 years ago
- Category deleted (
14)
We are removing the 'API' category per open floor discussion June 16, 2020.
.