Bug 1469345

Summary: JSON API Group creation with very long group name should fail with appropriate message
Product: [Retired] Beaker Reporter: Anwesha Chatterjee <achatter>
Component: command lineAssignee: Anwesha Chatterjee <achatter>
Status: CLOSED CURRENTRELEASE QA Contact: Dan Callaghan <dcallagh>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 24CC: dcallagh, mjia, rjoost
Target Milestone: 24.4Keywords: Patch
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2017-10-03 03:57:47 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Anwesha Chatterjee 2017-07-11 03:57:51 UTC
Description of problem:


Version-Release number of selected component (if applicable):


How reproducible:
100%

Steps to Reproduce:
1. Add the following test to 
   beaker/IntegrationTests/src/bkr/inttest/server/selenium/test_group_edit.py:GroupHTTPTest

    def test_create_new_group_long_name(self):
        s = requests.Session()
        requests_login(s, user=self.user, password=u'password')
        response = post_json(get_server_base() + 'groups/', session=s, data={
            'group_name': 'areallylongname'*20,
            'display_name': 'Group longname',
            'description': 'Group longname description',
            'root_password': 'blapppy7',
        })
        response.raise_for_status()
        with session.begin():
            group = Group.by_name(u'areallylongname'*20)
            self.assert_(group)


2. Run the test
   > ./run-tests.sh -sv bkr.inttest.server.selenium.test_group_edit:GroupHTTPTest.test_create_new_group_long_name


Actual results:

Test fails with Traceback:
Traceback (most recent call last):
  File "/root/beaker/IntegrationTests/src/bkr/inttest/server/selenium/test_group_edit.py", line 866, in test_create_new_group_long_name
    response.raise_for_status()
  File "/usr/lib/python2.6/site-packages/requests/models.py", line 834, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
HTTPError: 400 Client Error: BAD REQUEST



Expected results:
error message thrown should be 'Group <name/display name> must be not more than 255 characters long'


Additional info:
Currently group create when performed through CLI uses the old API, therefore cannot be reproduced through CLI (hence the test)

Comment 1 Anwesha Chatterjee 2017-07-11 07:36:38 UTC
[Edit]
Description of problem:


Version-Release number of selected component (if applicable):


How reproducible:
100%

Steps to Reproduce:
1. create a group by using the POST /groups/ API, with {"group_name": <groupname>} in the request body
where group name length is > 255 characters long

Actual results:
group is created and name is truncated

Expected results:
400 response code with a message indicating the name is too long


Additional info:
Currently group create when performed through CLI uses the old API, therefore cannot be reproduced through CLI . Use httpie/curl to replicate.

Comment 2 Anwesha Chatterjee 2017-07-11 22:57:57 UTC
[Edit]
Actual Results:

group is not created - error thrown: 

HTTP/1.1 400 BAD REQUEST
No row was found for one()

Comment 3 Dan Callaghan 2017-07-11 23:27:51 UTC
(In reply to Anwesha Chatterjee from comment #2)
> [Edit]
> Actual Results:
> 
> group is not created - error thrown: 
> 
> HTTP/1.1 400 BAD REQUEST
> No row was found for one()

Okay, that's not too bad. I guess this is coming from inside lazy_create after it does the INSERT..SELECT and tries to SELECT back the new row, which doesn't exist because MySQL silently truncated the name so it doesn't match what we think we inserted. And then the whole transaction rolls back.

But still, we definitely want the real validation to be enforced so that the API gives back a clean error message.

Comment 4 Dan Callaghan 2017-07-11 23:29:27 UTC
So the underlying problem is that lazy_create bypasses all SQLAlchemy validators.

Can you check if there are any other parts of the code affected by this?

Essentially -- any places where we call .lazy_create() and pass in a field which also has a SQLAlchemy validator defined for it. There are not too many callers of .lazy_create() and not too many @validates functions either, so you could probably just audit them by hand.

Comment 5 Anwesha Chatterjee 2017-07-12 04:22:51 UTC
Other area affected by this:

create_user() method in data_setup.py also calls lazy_create with user_name which has associated validations in server/model/identity.py which are then skipped.  But I believe this is test code and does not affect any APIs.

Comment 6 Anwesha Chatterjee 2017-07-13 04:40:45 UTC
https://gerrit.beaker-project.org/#/c/5737/

Comment 9 Dan Callaghan 2017-10-03 03:57:47 UTC
Beaker 24.4 has been released.