Error message for bad bit length is insufficient on secret create

Bug #1386251 reported by Steve Heyman
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Barbican
Fix Released
Low
Juan Antonio Osorio Robles

Bug Description

When you create a secret with an invalid bit length you get back an error 400 but the text of the message doesn't indicate that the bit_length is the culprit. It simply says that a field is not of type integer but it doesn't indicate what that field is.

It should function like TypeOrders's validation for bit length.

Tags: verified
Steve Heyman (sheyman)
Changed in barbican:
status: New → Confirmed
importance: Undecided → Low
Revision history for this message
John Vrbanac (john.vrbanac) wrote :

This appears to still be an issue.

Changed in barbican:
status: Confirmed → New
status: New → Confirmed
tags: added: verified
N Dillon (sicarie)
Changed in barbican:
assignee: nobody → N Dillon (sicarie)
Revision history for this message
Michael McCune (mimccune) wrote :

i started to investigate this issue and wanted to pass on some info. one issue here is that although the "bit_length" is validated for both secrets and orders, the validation error will only include the actual value for "bit_length" not the key name in the output. i tried a few tests across the secrets and order endpoints and they both return the same error message.

sending the following to the /v1/secrets endpoint:

    {
      "name": "test key",
      "bit_length": "non_integer_value"
    }

produces:

    HTTP/1.1 400 Bad Request
    Connection: close
    Content-Length: 149
    Content-Type: application/json; charset=UTF-8

    {
      "code": 400,
      "description": "Provided object does not match schema 'Secret': u'non_integer_value' is not of type 'integer'",
      "title": "Bad Request"
    }

likewise, sending the following to the /v1/orders endpoint:

    {
      "type": "asymmetric",
      "meta": {
        "bit_length": "non_integer_value"
      }
    }

produces the same result:

    HTTP/1.1 400 Bad Request
    Connection: close
    Content-Length: 164
    Content-Type: application/json; charset=UTF-8

    {
        "code": 400,
        "description": "Provided object does not match schema 'Secret' within 'Order': u'non_integer_value' is not of type 'integer'",
        "title": "Bad Request"
    }

the same result is returned if the order type is "key".

from looking at the validators it seems there might be a deeper issue here with the way that errors are reported back from the validation routines. namely that instead of taking the key name to fill in the error message, the function is taking the key value(in these test cases "non_integer_value").

i'm not sure if the proper fix is to just do a special validation on the "bit_length" field before validating the rest of the input data, but i think it's worth investigating how these errors are being reported in general.

Revision history for this message
Juan Antonio Osorio Robles (juan-osorio-robles) wrote :

Yes, this is indeed handled from the jsonschema library, and the errors produced by the library are not sufficient to have proper usability of our error messages. So there are three options.

1: Override the JSONSchema error messages. This would provide us with more custom-made error messages for the overall API and would be a more general solution. But, this would require some refactoring of how things are made, and a wrapper class for the library.

2. Manually check the bit length. This is very easy to do, as you would only need to call this before the _assert_schema_is_valid call in the code. Another alternative for this is to remove the "integer" constraint from the JSONSchema, and check the bit_length manually after the schema validation. Though, I must say I am not keen of this approach at all, since, if we will do this for this attribute, probably we will need to do something similar for a bunch of others. And in the end, why use JSONSchema in the first place if we will end up validating a bunch of stuff manually? So I think this is not a very code-scalable solution and short-sighted.

3. Leave the server code as-is. This will probably be used clients (such as python-barbicanclient) that don't need to have as refined error messages. Furtherly, a change could be done in the client to actually translate this to something more human-readable.

Revision history for this message
John Wood (john-wood-w) wrote :

Sigh. I wonder if this is really a JSONSchema defect that we are trying to code around? I think trying to provide the proper error message will result in us writing a lot of work around code, or all out replacing JSONSchema with our own validator. IMHO I think it would be sufficient to document behavior so folks are aware of it, perhaps in a 'release notes' section of the sphinx docs?

Revision history for this message
Juan Antonio Osorio Robles (juan-osorio-robles) wrote :

Well, digging a bit more detail into this, there actually are some checks regarding the bitlength that are not handled by the json schema. And we have a class for that. So that could be worked on.

Revision history for this message
Michael McCune (mimccune) wrote :

I'm not sure if this is a deficiency in the jsonschema error reporting or if it's just a side effect of how barbican creates the error messages. It really comes down to passing a "key: value" into jsonschema and then reporting the error as "value is not of type X". If the error reporting coming out of barbican used the "key" instead of "value", it would be much clearer.

N Dillon (sicarie)
Changed in barbican:
assignee: N Dillon (sicarie) → nobody
Changed in barbican:
assignee: nobody → pradeep kumar singh (pradeep-singh-u)
Changed in barbican:
assignee: pradeep kumar singh (pradeep-singh-u) → nobody
Changed in barbican:
assignee: nobody → Juan Antonio Osorio Robles (juan-osorio-robles)
Changed in barbican:
status: Confirmed → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to barbican (master)

Reviewed: https://review.openstack.org/205057
Committed: https://git.openstack.org/cgit/openstack/barbican/commit/?id=758e825a6c1b0bceaa105cbe713ad63d4c780d5a
Submitter: Jenkins
Branch: master

commit 758e825a6c1b0bceaa105cbe713ad63d4c780d5a
Author: Juan Antonio Osorio Robles <email address hidden>
Date: Thu Jul 23 16:20:25 2015 +0300

    Add invalid property info to validation error message

    It seems that the information that's being provided by our validation
    errors is not sufficient for the users to make a sensible judgment about
    what was the problem with their request to the server. This change
    attempts to fix that by adding information about the specific property
    that was problematic to the message that's given to the client.

    Change-Id: I4bb72d2d2050bdd44aad2b89e71d50e4fe3d7f1f
    Closes-bug: #1386251

Changed in barbican:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in barbican:
milestone: none → liberty-3
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in barbican:
milestone: liberty-3 → 1.0.0
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.