cxf-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jason Klapste (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CXF-5569) OAuth AbstractAuthFilter and query parameters used for signing
Date Mon, 31 Mar 2014 20:09:15 GMT

    [ https://issues.apache.org/jira/browse/CXF-5569?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13955626#comment-13955626
] 

Jason Klapste commented on CXF-5569:
------------------------------------

I don't believe filters sitting in front of the OAuth filter have anything do to with the
issue-- although in this context I'm not following the impact given that the interaction here
is with HttpServletRequest.getParameterMap(). In any caseā€¦.

OAuthMessage generation happens here:
{code}
        OAuthMessage oAuthMessage = OAuthServlet.getMessage(new CustomHttpServletWrapper(req),
OAuthServlet.getRequestURL(req));
{code}


And in CustomHttpServletWrapper, containsAll() is called which will return false as params.keySet()
is a superset as it would have contained the additional URL based parameters.

So it then falls through to the iteration over the params-- and anything that is not in ALLOW_OAUTH_PARAMS
is ignored.

{code}
    private static class CustomHttpServletWrapper extends HttpServletRequestWrapper {
...
        public Map<String, String[]> getParameterMap() {
            Map<String, String[]> params = super.getParameterMap();
            
            if (ALLOWED_OAUTH_PARAMETERS.containsAll(params.keySet())) {
                return params;
            }
            
            Map<String, String[]> newParams = new HashMap<String, String[]>();
            for (Map.Entry<String, String[]> entry : params.entrySet()) {
                if (ALLOWED_OAUTH_PARAMETERS.contains(entry.getKey())) {    
                    newParams.put(entry.getKey(), entry.getValue());
                }
            }
            return newParams;
        }
    }
{code}

And then onwards to SimpleOAuthValidator through to OAuthSignatureMethod.validate() which
calls getBaseString().

getBaseString() then uses message.getParameters() to generate the string to sign.

Note too, while getBaseString() *seems* to check the URL for query parameters via it's indexOf('?')
call it will never return true due to OAuthServlet.getMessage() which truncates the URL at
the first '?' when the OAuthMessage is first created. Message.getParameters() is populated
by HttpRequestMessage's getParameters() which checks first any authorization header for parameters,
followed by calling getParameterMap() which ends up being CustomHttpServletWrapper's version--
and hence filtered.

I don't have a unit test handy that is narrow enough that I can share (although do have one
in my code base). If you have something near, i.e. a unit test that tests some level of AbstractAuthFilter
I can take what you have and try to write one.

I do know, from my own unit test, commenting out the if check in getParameterMap() above works.

You can also bypass the above if check by embedding the URL query parameters into the OAuth
authorization header thereby by-passing the if check above as HttpRequestMessage.getParameters()
only filters out "realm".

> OAuth AbstractAuthFilter and query parameters used for signing
> --------------------------------------------------------------
>
>                 Key: CXF-5569
>                 URL: https://issues.apache.org/jira/browse/CXF-5569
>             Project: CXF
>          Issue Type: Improvement
>          Components: JAX-RS Security
>    Affects Versions: 2.7.10
>            Reporter: Jason Klapste
>            Assignee: Sergey Beryozkin
>            Priority: Minor
>             Fix For: 3.0.0-milestone2, 2.7.11
>
>
> In the AbstractAuthFilter the query (or body) parameters used for signing are only those
included in ALLOWED_OAUTH_PARAMETERS.
> But if I'm reading the RFC correctly, it looks are though ALL parameters should be considered
for signature generation.
> To support both backwards compatibility, can I suggest exposing the ALLOWED_OAUTH_PARAMETERS
to subclasses (either directly or via getter/setters) along with a flag that can be set to
automatically include any and all parameters?



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Mime
View raw message