1141162 - Fix or squelch ValueError: Malformed authorization header

Status: VERIFIED FIXED

(Marketplace Graveyard :: Code Quality, defect, P4)

Description

[Rob Hudson [:robhudson]]

In the logs I consistently see this error pretty often.

z.api:ERROR ValueError on verifying_request :./mkt/api/middleware.py:98
Traceback (most recent call last):
  File "./mkt/api/middleware.py", line 93, in process_request
    method, auth_header)
  File "./mkt/api/middleware.py", line 136, in validate_2legged_oauth
    typ, params, oauth_params = oauth._get_signature_type_and_params(req)
  File "/data/addons-stage/www/marketplace.allizom.org/current/venv/lib/python2.7/site-packages/oauthlib/oauth1/rfc5849/endpoints/base.py", line 35, in _get_signature_type_and_params
    exclude_oauth_signature=False, with_realm=True)
  File "/data/addons-stage/www/marketplace.allizom.org/current/venv/lib/python2.7/site-packages/oauthlib/oauth1/rfc5849/signature.py", line 278, in collect_parameters
    authorization_header) if with_realm or i[0] != 'realm'])
  File "/data/addons-stage/www/marketplace.allizom.org/current/venv/lib/python2.7/site-packages/oauthlib/oauth1/rfc5849/utils.py", line 89, in parse_authorization_header
    raise ValueError('Malformed authorization header')
ValueError: Malformed authorization header


If this error is something we don't care about, let's either remove the log or bump it up to log.INFO or log.WARN.

If we do care about this error, let's fix it.

Comment 1

[Mathieu Pillard [:mat]]

It will happen for every request using shared secret as header instead of as query string parameter. We don't do that in fireplace, but we do that at least in reviewer tools.

RestSharedSecretMiddleware should probably be placed before RestOAuthMiddleware in MIDDLEWARE_CLASSES and then modified to alter the headers on the request if it begins with "mkt-shared-secret" to remove the Authorization header, as it's never going to be a valid oauth Authorization header.

Comment 2

[Rob Hudson [:robhudson]]

PR: https://github.com/mozilla/zamboni/pull/2974

Comment 3

[Rob Hudson [:robhudson]]

https://github.com/mozilla/zamboni/commit/7e03cdb 

This is mostly a fix to avoid logging something as an error that isn't really an error but please verify logins aren't affected, from consumer pages and reviewer tools would be good. Thanks.

Comment 4

[Victor Carciu]

Can you please add some STRs to this bug or mark it as [qa-]?

Comment 5

[Mathieu Pillard [:mat]]

See comment 3 for QA instructions.

Comment 6

[Victor Carciu]

Oh, you are right , in this case I will mark this as verified since everything is working as expected