You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Andreas Kohn <an...@gmail.com> on 2013/10/16 12:54:24 UTC
Reversed condition in AuthCodeGrantValidator#validateRequest()?
Hi,
I'm currently stepping through the logic for handling OAuth2 requests, at
the same time reading through RFC 6749 and trying to wrap my head around
what's going on :)
I noticed that in AuthCodeGrantValidator#validateRequest() a condition
states "if servlet request has a redirect_uri, then it must match the one
stored in the authcode"[1], but from my reading of the RFC it should be "if
authcode has a redirect_uri, then the servlet request must specify the same
one" [2][3].
Am I missing something?
Regards,
--
Andreas
[1]
67 if (servletRequest.getRedirectURI() != null
68 &&
!servletRequest.getRedirectURI().equals(authCode.getRedirectURI())) {
69 OAuth2NormalizedResponse response = new
OAuth2NormalizedResponse();
70 response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
71 response.setError(ErrorType.INVALID_GRANT.toString());
72 response
73 .setErrorDescription("The redirect URI does not match the one
used in the authorization request");
74 response.setBodyReturned(true);
75 throw new OAuth2Exception(response);
76 }
[2] Section 4.1.3 Access Token Request says
o ensure that the "redirect_uri" parameter is present if the
"redirect_uri" parameter was included in the initial authorization
request as described in Section 4.1.1
<http://tools.ietf.org/html/rfc6749#section-4.1.1>, and if included
ensure that
their values are identical.
[3] Fix would be to replace lines 67 and 68 with:
if (authCode.getRedirectURI() != null
&&
!authCode.getRedirectURI().equals(servletRequest.getRedirectURI())) {
Re: Reversed condition in AuthCodeGrantValidator#validateRequest()?
Posted by Andreas Kohn <an...@gmail.com>.
Good to know indeed.
Guess it would be nice then to also apply the attached patch, which adds a
package-info.java file for the oauth2 package. :)
I'll file a ticket for the original issue in a few minutes.
Regards,
--
Andreas
On Thu, Oct 17, 2013 at 3:00 PM, A Clarke <cl...@gmail.com> wrote:
> One thing to keep in mind Andreas is that you are looking at the most
> recent OAuth2 specification. When the OAuth2 sample provider was written
> D21 was latest revision. The code you're looking at is supposed to comply
> with D21.
>
> I took a quick look at the diagrams and they don't appear to have changed
> so I don't think it applies to your problem.
>
>
> On Thu, Oct 17, 2013 at 8:14 AM, Ryan Baxter <rb...@apache.org> wrote:
>
> > Thanks for the explanation. Sounds like a bug to me as well.
> >
> > On Thu, Oct 17, 2013 at 4:47 AM, Andreas Kohn <an...@gmail.com>
> > wrote:
> > > Hi,
> > >
> > > On Thu, Oct 17, 2013 at 2:26 AM, Ryan Baxter <rb...@apache.org>
> > wrote:
> > >
> > >> I guess the wording in Section 4.1.3 is a little confusing to me.
> > >>
> > >> "ensure that the "redirect_uri" parameter is present if the
> > >> "redirect_uri" parameter was included in the initial
> authorization
> > >> request as described in Section 4.1.1
> > >> <http://tools.ietf.org/html/rfc6749#section-4.1.1>, and if included
> > >> ensure that
> > >> their values are identical."
> > >>
> > >> What is the "initial authorization request"? Is that referring to the
> > >> servletRequest in the code? If so that to me it seems like the code
> > >> is correct.
> > >>
> > >
> > > The "(initial) authorization request" is the request that was sent
> > earlier
> > > in the code flow with the response_type=code parameter. The "access
> token
> > > request" of section 4.1.3 is the *next* request, with
> > > grant_type=authorization_code. As such, no, this does not refer to the
> > > current servlet request (an implementation detail that also happens to
> be
> > > named 'request' :D).
> > >
> > > Looking at the diagram of section 4.1 (
> > > http://tools.ietf.org/html/rfc6749#section-4.1): The "authorization
> > > request" is the marked with (A), the "authorization response" is (C),
> the
> > > "access token request" is (D), and the "access token response" is (E).
> > >
> > > Regards,
> > > --
> > > Andreas
> > >
> > >
> > >
> > >>
> > >> On Wed, Oct 16, 2013 at 8:52 AM, Andreas Kohn <andreas.kohn@gmail.com
> >
> > >> wrote:
> > >> > Hi,
> > >> >
> > >> > On Wed, Oct 16, 2013 at 2:04 PM, Stanton Sievers <
> ssievers@apache.org
> > >> >wrote:
> > >> >
> > >> >> You're concerned about the use case where
> > >> >> the redirect_uri was omitted from the authorization request (which
> is
> > >> valid
> > >> >> since it is optional per section 4.1.1) yet a redirect_uri was
> > provided
> > >> on
> > >> >> the access token request?
> > >> >
> > >> >
> > >> > No, at that part the redirect_uri is indeed optional, and from what
> I
> > can
> > >> > see the RFC doesn't say anything about providing a redirect_uri in
> the
> > >> > token request if there was none provided in the authorization
> request.
> > >> > However, _iff_ a redirect_uri is given with the authorization
> request,
> > >> > _then_ the access token request must also include it (4.1.3), and it
> > must
> > >> > match (ibd.).
> > >> >
> > >> > If I'm reading the logic for that part correctly though Shindig
> allows
> > >> the
> > >> > access token request to not specify a redirect_uri, and in that case
> > it
> > >> > will not check whether the authorization request had one specified.
> In
> > >> > other words: it will provide a client that has an authcode with a
> > token
> > >> > without validating that the authcode redirect URI is correct.
> > >> >
> > >> > Does that make sense?
> > >> >
> > >> > --
> > >> > Andreas
> > >> >
> > >> > In this particular case, due to the logic you
> > >> >> mentioned, an exception would be thrown because the redirect_uris
> do
> > not
> > >> >> match.
> > >> >>
> > >> >> Is that the case you are running into?
> > >> >>
> > >> >> Thanks,
> > >> >> -Stanton
> > >> >>
> > >> >>
> > >> >> On Wed, Oct 16, 2013 at 6:54 AM, Andreas Kohn <
> > andreas.kohn@gmail.com
> > >> >> >wrote:
> > >> >>
> > >> >> > Hi,
> > >> >> >
> > >> >> > I'm currently stepping through the logic for handling OAuth2
> > >> requests, at
> > >> >> > the same time reading through RFC 6749 and trying to wrap my head
> > >> around
> > >> >> > what's going on :)
> > >> >> >
> > >> >> > I noticed that in AuthCodeGrantValidator#validateRequest() a
> > condition
> > >> >> > states "if servlet request has a redirect_uri, then it must match
> > the
> > >> one
> > >> >> > stored in the authcode"[1], but from my reading of the RFC it
> > should
> > >> be
> > >> >> "if
> > >> >> > authcode has a redirect_uri, then the servlet request must
> specify
> > the
> > >> >> same
> > >> >> > one" [2][3].
> > >> >> >
> > >> >> > Am I missing something?
> > >> >> >
> > >> >> > Regards,
> > >> >> > --
> > >> >> > Andreas
> > >> >> >
> > >> >> > [1]
> > >> >> > 67 if (servletRequest.getRedirectURI() != null
> > >> >> > 68 &&
> > >> >> >
> > !servletRequest.getRedirectURI().equals(authCode.getRedirectURI())) {
> > >> >> > 69 OAuth2NormalizedResponse response = new
> > >> >> > OAuth2NormalizedResponse();
> > >> >> > 70 response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
> > >> >> > 71 response.setError(ErrorType.INVALID_GRANT.toString());
> > >> >> > 72 response
> > >> >> > 73 .setErrorDescription("The redirect URI does not
> match
> > >> the
> > >> >> one
> > >> >> > used in the authorization request");
> > >> >> > 74 response.setBodyReturned(true);
> > >> >> > 75 throw new OAuth2Exception(response);
> > >> >> > 76 }
> > >> >> >
> > >> >> > [2] Section 4.1.3 Access Token Request says
> > >> >> >
> > >> >> > o ensure that the "redirect_uri" parameter is present if the
> > >> >> > "redirect_uri" parameter was included in the initial
> > >> authorization
> > >> >> > request as described in Section 4.1.1
> > >> >> > <http://tools.ietf.org/html/rfc6749#section-4.1.1>, and if
> > included
> > >> >> > ensure that
> > >> >> > their values are identical.
> > >> >> >
> > >> >> >
> > >> >> > [3] Fix would be to replace lines 67 and 68 with:
> > >> >> > if (authCode.getRedirectURI() != null
> > >> >> > &&
> > >> >> >
> > !authCode.getRedirectURI().equals(servletRequest.getRedirectURI())) {
> > >> >> >
> > >> >>
> > >>
> >
>
Re: Reversed condition in AuthCodeGrantValidator#validateRequest()?
Posted by A Clarke <cl...@gmail.com>.
One thing to keep in mind Andreas is that you are looking at the most
recent OAuth2 specification. When the OAuth2 sample provider was written
D21 was latest revision. The code you're looking at is supposed to comply
with D21.
I took a quick look at the diagrams and they don't appear to have changed
so I don't think it applies to your problem.
On Thu, Oct 17, 2013 at 8:14 AM, Ryan Baxter <rb...@apache.org> wrote:
> Thanks for the explanation. Sounds like a bug to me as well.
>
> On Thu, Oct 17, 2013 at 4:47 AM, Andreas Kohn <an...@gmail.com>
> wrote:
> > Hi,
> >
> > On Thu, Oct 17, 2013 at 2:26 AM, Ryan Baxter <rb...@apache.org>
> wrote:
> >
> >> I guess the wording in Section 4.1.3 is a little confusing to me.
> >>
> >> "ensure that the "redirect_uri" parameter is present if the
> >> "redirect_uri" parameter was included in the initial authorization
> >> request as described in Section 4.1.1
> >> <http://tools.ietf.org/html/rfc6749#section-4.1.1>, and if included
> >> ensure that
> >> their values are identical."
> >>
> >> What is the "initial authorization request"? Is that referring to the
> >> servletRequest in the code? If so that to me it seems like the code
> >> is correct.
> >>
> >
> > The "(initial) authorization request" is the request that was sent
> earlier
> > in the code flow with the response_type=code parameter. The "access token
> > request" of section 4.1.3 is the *next* request, with
> > grant_type=authorization_code. As such, no, this does not refer to the
> > current servlet request (an implementation detail that also happens to be
> > named 'request' :D).
> >
> > Looking at the diagram of section 4.1 (
> > http://tools.ietf.org/html/rfc6749#section-4.1): The "authorization
> > request" is the marked with (A), the "authorization response" is (C), the
> > "access token request" is (D), and the "access token response" is (E).
> >
> > Regards,
> > --
> > Andreas
> >
> >
> >
> >>
> >> On Wed, Oct 16, 2013 at 8:52 AM, Andreas Kohn <an...@gmail.com>
> >> wrote:
> >> > Hi,
> >> >
> >> > On Wed, Oct 16, 2013 at 2:04 PM, Stanton Sievers <ssievers@apache.org
> >> >wrote:
> >> >
> >> >> You're concerned about the use case where
> >> >> the redirect_uri was omitted from the authorization request (which is
> >> valid
> >> >> since it is optional per section 4.1.1) yet a redirect_uri was
> provided
> >> on
> >> >> the access token request?
> >> >
> >> >
> >> > No, at that part the redirect_uri is indeed optional, and from what I
> can
> >> > see the RFC doesn't say anything about providing a redirect_uri in the
> >> > token request if there was none provided in the authorization request.
> >> > However, _iff_ a redirect_uri is given with the authorization request,
> >> > _then_ the access token request must also include it (4.1.3), and it
> must
> >> > match (ibd.).
> >> >
> >> > If I'm reading the logic for that part correctly though Shindig allows
> >> the
> >> > access token request to not specify a redirect_uri, and in that case
> it
> >> > will not check whether the authorization request had one specified. In
> >> > other words: it will provide a client that has an authcode with a
> token
> >> > without validating that the authcode redirect URI is correct.
> >> >
> >> > Does that make sense?
> >> >
> >> > --
> >> > Andreas
> >> >
> >> > In this particular case, due to the logic you
> >> >> mentioned, an exception would be thrown because the redirect_uris do
> not
> >> >> match.
> >> >>
> >> >> Is that the case you are running into?
> >> >>
> >> >> Thanks,
> >> >> -Stanton
> >> >>
> >> >>
> >> >> On Wed, Oct 16, 2013 at 6:54 AM, Andreas Kohn <
> andreas.kohn@gmail.com
> >> >> >wrote:
> >> >>
> >> >> > Hi,
> >> >> >
> >> >> > I'm currently stepping through the logic for handling OAuth2
> >> requests, at
> >> >> > the same time reading through RFC 6749 and trying to wrap my head
> >> around
> >> >> > what's going on :)
> >> >> >
> >> >> > I noticed that in AuthCodeGrantValidator#validateRequest() a
> condition
> >> >> > states "if servlet request has a redirect_uri, then it must match
> the
> >> one
> >> >> > stored in the authcode"[1], but from my reading of the RFC it
> should
> >> be
> >> >> "if
> >> >> > authcode has a redirect_uri, then the servlet request must specify
> the
> >> >> same
> >> >> > one" [2][3].
> >> >> >
> >> >> > Am I missing something?
> >> >> >
> >> >> > Regards,
> >> >> > --
> >> >> > Andreas
> >> >> >
> >> >> > [1]
> >> >> > 67 if (servletRequest.getRedirectURI() != null
> >> >> > 68 &&
> >> >> >
> !servletRequest.getRedirectURI().equals(authCode.getRedirectURI())) {
> >> >> > 69 OAuth2NormalizedResponse response = new
> >> >> > OAuth2NormalizedResponse();
> >> >> > 70 response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
> >> >> > 71 response.setError(ErrorType.INVALID_GRANT.toString());
> >> >> > 72 response
> >> >> > 73 .setErrorDescription("The redirect URI does not match
> >> the
> >> >> one
> >> >> > used in the authorization request");
> >> >> > 74 response.setBodyReturned(true);
> >> >> > 75 throw new OAuth2Exception(response);
> >> >> > 76 }
> >> >> >
> >> >> > [2] Section 4.1.3 Access Token Request says
> >> >> >
> >> >> > o ensure that the "redirect_uri" parameter is present if the
> >> >> > "redirect_uri" parameter was included in the initial
> >> authorization
> >> >> > request as described in Section 4.1.1
> >> >> > <http://tools.ietf.org/html/rfc6749#section-4.1.1>, and if
> included
> >> >> > ensure that
> >> >> > their values are identical.
> >> >> >
> >> >> >
> >> >> > [3] Fix would be to replace lines 67 and 68 with:
> >> >> > if (authCode.getRedirectURI() != null
> >> >> > &&
> >> >> >
> !authCode.getRedirectURI().equals(servletRequest.getRedirectURI())) {
> >> >> >
> >> >>
> >>
>
Re: Reversed condition in AuthCodeGrantValidator#validateRequest()?
Posted by Ryan Baxter <rb...@apache.org>.
Thanks for the explanation. Sounds like a bug to me as well.
On Thu, Oct 17, 2013 at 4:47 AM, Andreas Kohn <an...@gmail.com> wrote:
> Hi,
>
> On Thu, Oct 17, 2013 at 2:26 AM, Ryan Baxter <rb...@apache.org> wrote:
>
>> I guess the wording in Section 4.1.3 is a little confusing to me.
>>
>> "ensure that the "redirect_uri" parameter is present if the
>> "redirect_uri" parameter was included in the initial authorization
>> request as described in Section 4.1.1
>> <http://tools.ietf.org/html/rfc6749#section-4.1.1>, and if included
>> ensure that
>> their values are identical."
>>
>> What is the "initial authorization request"? Is that referring to the
>> servletRequest in the code? If so that to me it seems like the code
>> is correct.
>>
>
> The "(initial) authorization request" is the request that was sent earlier
> in the code flow with the response_type=code parameter. The "access token
> request" of section 4.1.3 is the *next* request, with
> grant_type=authorization_code. As such, no, this does not refer to the
> current servlet request (an implementation detail that also happens to be
> named 'request' :D).
>
> Looking at the diagram of section 4.1 (
> http://tools.ietf.org/html/rfc6749#section-4.1): The "authorization
> request" is the marked with (A), the "authorization response" is (C), the
> "access token request" is (D), and the "access token response" is (E).
>
> Regards,
> --
> Andreas
>
>
>
>>
>> On Wed, Oct 16, 2013 at 8:52 AM, Andreas Kohn <an...@gmail.com>
>> wrote:
>> > Hi,
>> >
>> > On Wed, Oct 16, 2013 at 2:04 PM, Stanton Sievers <ssievers@apache.org
>> >wrote:
>> >
>> >> You're concerned about the use case where
>> >> the redirect_uri was omitted from the authorization request (which is
>> valid
>> >> since it is optional per section 4.1.1) yet a redirect_uri was provided
>> on
>> >> the access token request?
>> >
>> >
>> > No, at that part the redirect_uri is indeed optional, and from what I can
>> > see the RFC doesn't say anything about providing a redirect_uri in the
>> > token request if there was none provided in the authorization request.
>> > However, _iff_ a redirect_uri is given with the authorization request,
>> > _then_ the access token request must also include it (4.1.3), and it must
>> > match (ibd.).
>> >
>> > If I'm reading the logic for that part correctly though Shindig allows
>> the
>> > access token request to not specify a redirect_uri, and in that case it
>> > will not check whether the authorization request had one specified. In
>> > other words: it will provide a client that has an authcode with a token
>> > without validating that the authcode redirect URI is correct.
>> >
>> > Does that make sense?
>> >
>> > --
>> > Andreas
>> >
>> > In this particular case, due to the logic you
>> >> mentioned, an exception would be thrown because the redirect_uris do not
>> >> match.
>> >>
>> >> Is that the case you are running into?
>> >>
>> >> Thanks,
>> >> -Stanton
>> >>
>> >>
>> >> On Wed, Oct 16, 2013 at 6:54 AM, Andreas Kohn <andreas.kohn@gmail.com
>> >> >wrote:
>> >>
>> >> > Hi,
>> >> >
>> >> > I'm currently stepping through the logic for handling OAuth2
>> requests, at
>> >> > the same time reading through RFC 6749 and trying to wrap my head
>> around
>> >> > what's going on :)
>> >> >
>> >> > I noticed that in AuthCodeGrantValidator#validateRequest() a condition
>> >> > states "if servlet request has a redirect_uri, then it must match the
>> one
>> >> > stored in the authcode"[1], but from my reading of the RFC it should
>> be
>> >> "if
>> >> > authcode has a redirect_uri, then the servlet request must specify the
>> >> same
>> >> > one" [2][3].
>> >> >
>> >> > Am I missing something?
>> >> >
>> >> > Regards,
>> >> > --
>> >> > Andreas
>> >> >
>> >> > [1]
>> >> > 67 if (servletRequest.getRedirectURI() != null
>> >> > 68 &&
>> >> > !servletRequest.getRedirectURI().equals(authCode.getRedirectURI())) {
>> >> > 69 OAuth2NormalizedResponse response = new
>> >> > OAuth2NormalizedResponse();
>> >> > 70 response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
>> >> > 71 response.setError(ErrorType.INVALID_GRANT.toString());
>> >> > 72 response
>> >> > 73 .setErrorDescription("The redirect URI does not match
>> the
>> >> one
>> >> > used in the authorization request");
>> >> > 74 response.setBodyReturned(true);
>> >> > 75 throw new OAuth2Exception(response);
>> >> > 76 }
>> >> >
>> >> > [2] Section 4.1.3 Access Token Request says
>> >> >
>> >> > o ensure that the "redirect_uri" parameter is present if the
>> >> > "redirect_uri" parameter was included in the initial
>> authorization
>> >> > request as described in Section 4.1.1
>> >> > <http://tools.ietf.org/html/rfc6749#section-4.1.1>, and if included
>> >> > ensure that
>> >> > their values are identical.
>> >> >
>> >> >
>> >> > [3] Fix would be to replace lines 67 and 68 with:
>> >> > if (authCode.getRedirectURI() != null
>> >> > &&
>> >> > !authCode.getRedirectURI().equals(servletRequest.getRedirectURI())) {
>> >> >
>> >>
>>
Re: Reversed condition in AuthCodeGrantValidator#validateRequest()?
Posted by Andreas Kohn <an...@gmail.com>.
Hi,
On Thu, Oct 17, 2013 at 2:26 AM, Ryan Baxter <rb...@apache.org> wrote:
> I guess the wording in Section 4.1.3 is a little confusing to me.
>
> "ensure that the "redirect_uri" parameter is present if the
> "redirect_uri" parameter was included in the initial authorization
> request as described in Section 4.1.1
> <http://tools.ietf.org/html/rfc6749#section-4.1.1>, and if included
> ensure that
> their values are identical."
>
> What is the "initial authorization request"? Is that referring to the
> servletRequest in the code? If so that to me it seems like the code
> is correct.
>
The "(initial) authorization request" is the request that was sent earlier
in the code flow with the response_type=code parameter. The "access token
request" of section 4.1.3 is the *next* request, with
grant_type=authorization_code. As such, no, this does not refer to the
current servlet request (an implementation detail that also happens to be
named 'request' :D).
Looking at the diagram of section 4.1 (
http://tools.ietf.org/html/rfc6749#section-4.1): The "authorization
request" is the marked with (A), the "authorization response" is (C), the
"access token request" is (D), and the "access token response" is (E).
Regards,
--
Andreas
>
> On Wed, Oct 16, 2013 at 8:52 AM, Andreas Kohn <an...@gmail.com>
> wrote:
> > Hi,
> >
> > On Wed, Oct 16, 2013 at 2:04 PM, Stanton Sievers <ssievers@apache.org
> >wrote:
> >
> >> You're concerned about the use case where
> >> the redirect_uri was omitted from the authorization request (which is
> valid
> >> since it is optional per section 4.1.1) yet a redirect_uri was provided
> on
> >> the access token request?
> >
> >
> > No, at that part the redirect_uri is indeed optional, and from what I can
> > see the RFC doesn't say anything about providing a redirect_uri in the
> > token request if there was none provided in the authorization request.
> > However, _iff_ a redirect_uri is given with the authorization request,
> > _then_ the access token request must also include it (4.1.3), and it must
> > match (ibd.).
> >
> > If I'm reading the logic for that part correctly though Shindig allows
> the
> > access token request to not specify a redirect_uri, and in that case it
> > will not check whether the authorization request had one specified. In
> > other words: it will provide a client that has an authcode with a token
> > without validating that the authcode redirect URI is correct.
> >
> > Does that make sense?
> >
> > --
> > Andreas
> >
> > In this particular case, due to the logic you
> >> mentioned, an exception would be thrown because the redirect_uris do not
> >> match.
> >>
> >> Is that the case you are running into?
> >>
> >> Thanks,
> >> -Stanton
> >>
> >>
> >> On Wed, Oct 16, 2013 at 6:54 AM, Andreas Kohn <andreas.kohn@gmail.com
> >> >wrote:
> >>
> >> > Hi,
> >> >
> >> > I'm currently stepping through the logic for handling OAuth2
> requests, at
> >> > the same time reading through RFC 6749 and trying to wrap my head
> around
> >> > what's going on :)
> >> >
> >> > I noticed that in AuthCodeGrantValidator#validateRequest() a condition
> >> > states "if servlet request has a redirect_uri, then it must match the
> one
> >> > stored in the authcode"[1], but from my reading of the RFC it should
> be
> >> "if
> >> > authcode has a redirect_uri, then the servlet request must specify the
> >> same
> >> > one" [2][3].
> >> >
> >> > Am I missing something?
> >> >
> >> > Regards,
> >> > --
> >> > Andreas
> >> >
> >> > [1]
> >> > 67 if (servletRequest.getRedirectURI() != null
> >> > 68 &&
> >> > !servletRequest.getRedirectURI().equals(authCode.getRedirectURI())) {
> >> > 69 OAuth2NormalizedResponse response = new
> >> > OAuth2NormalizedResponse();
> >> > 70 response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
> >> > 71 response.setError(ErrorType.INVALID_GRANT.toString());
> >> > 72 response
> >> > 73 .setErrorDescription("The redirect URI does not match
> the
> >> one
> >> > used in the authorization request");
> >> > 74 response.setBodyReturned(true);
> >> > 75 throw new OAuth2Exception(response);
> >> > 76 }
> >> >
> >> > [2] Section 4.1.3 Access Token Request says
> >> >
> >> > o ensure that the "redirect_uri" parameter is present if the
> >> > "redirect_uri" parameter was included in the initial
> authorization
> >> > request as described in Section 4.1.1
> >> > <http://tools.ietf.org/html/rfc6749#section-4.1.1>, and if included
> >> > ensure that
> >> > their values are identical.
> >> >
> >> >
> >> > [3] Fix would be to replace lines 67 and 68 with:
> >> > if (authCode.getRedirectURI() != null
> >> > &&
> >> > !authCode.getRedirectURI().equals(servletRequest.getRedirectURI())) {
> >> >
> >>
>
Re: Reversed condition in AuthCodeGrantValidator#validateRequest()?
Posted by Ryan Baxter <rb...@apache.org>.
I guess the wording in Section 4.1.3 is a little confusing to me.
"ensure that the "redirect_uri" parameter is present if the
"redirect_uri" parameter was included in the initial authorization
request as described in Section 4.1.1
<http://tools.ietf.org/html/rfc6749#section-4.1.1>, and if included
ensure that
their values are identical."
What is the "initial authorization request"? Is that referring to the
servletRequest in the code? If so that to me it seems like the code
is correct.
On Wed, Oct 16, 2013 at 8:52 AM, Andreas Kohn <an...@gmail.com> wrote:
> Hi,
>
> On Wed, Oct 16, 2013 at 2:04 PM, Stanton Sievers <ss...@apache.org>wrote:
>
>> You're concerned about the use case where
>> the redirect_uri was omitted from the authorization request (which is valid
>> since it is optional per section 4.1.1) yet a redirect_uri was provided on
>> the access token request?
>
>
> No, at that part the redirect_uri is indeed optional, and from what I can
> see the RFC doesn't say anything about providing a redirect_uri in the
> token request if there was none provided in the authorization request.
> However, _iff_ a redirect_uri is given with the authorization request,
> _then_ the access token request must also include it (4.1.3), and it must
> match (ibd.).
>
> If I'm reading the logic for that part correctly though Shindig allows the
> access token request to not specify a redirect_uri, and in that case it
> will not check whether the authorization request had one specified. In
> other words: it will provide a client that has an authcode with a token
> without validating that the authcode redirect URI is correct.
>
> Does that make sense?
>
> --
> Andreas
>
> In this particular case, due to the logic you
>> mentioned, an exception would be thrown because the redirect_uris do not
>> match.
>>
>> Is that the case you are running into?
>>
>> Thanks,
>> -Stanton
>>
>>
>> On Wed, Oct 16, 2013 at 6:54 AM, Andreas Kohn <andreas.kohn@gmail.com
>> >wrote:
>>
>> > Hi,
>> >
>> > I'm currently stepping through the logic for handling OAuth2 requests, at
>> > the same time reading through RFC 6749 and trying to wrap my head around
>> > what's going on :)
>> >
>> > I noticed that in AuthCodeGrantValidator#validateRequest() a condition
>> > states "if servlet request has a redirect_uri, then it must match the one
>> > stored in the authcode"[1], but from my reading of the RFC it should be
>> "if
>> > authcode has a redirect_uri, then the servlet request must specify the
>> same
>> > one" [2][3].
>> >
>> > Am I missing something?
>> >
>> > Regards,
>> > --
>> > Andreas
>> >
>> > [1]
>> > 67 if (servletRequest.getRedirectURI() != null
>> > 68 &&
>> > !servletRequest.getRedirectURI().equals(authCode.getRedirectURI())) {
>> > 69 OAuth2NormalizedResponse response = new
>> > OAuth2NormalizedResponse();
>> > 70 response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
>> > 71 response.setError(ErrorType.INVALID_GRANT.toString());
>> > 72 response
>> > 73 .setErrorDescription("The redirect URI does not match the
>> one
>> > used in the authorization request");
>> > 74 response.setBodyReturned(true);
>> > 75 throw new OAuth2Exception(response);
>> > 76 }
>> >
>> > [2] Section 4.1.3 Access Token Request says
>> >
>> > o ensure that the "redirect_uri" parameter is present if the
>> > "redirect_uri" parameter was included in the initial authorization
>> > request as described in Section 4.1.1
>> > <http://tools.ietf.org/html/rfc6749#section-4.1.1>, and if included
>> > ensure that
>> > their values are identical.
>> >
>> >
>> > [3] Fix would be to replace lines 67 and 68 with:
>> > if (authCode.getRedirectURI() != null
>> > &&
>> > !authCode.getRedirectURI().equals(servletRequest.getRedirectURI())) {
>> >
>>
Re: Reversed condition in AuthCodeGrantValidator#validateRequest()?
Posted by Andreas Kohn <an...@gmail.com>.
Hi,
On Wed, Oct 16, 2013 at 2:04 PM, Stanton Sievers <ss...@apache.org>wrote:
> You're concerned about the use case where
> the redirect_uri was omitted from the authorization request (which is valid
> since it is optional per section 4.1.1) yet a redirect_uri was provided on
> the access token request?
No, at that part the redirect_uri is indeed optional, and from what I can
see the RFC doesn't say anything about providing a redirect_uri in the
token request if there was none provided in the authorization request.
However, _iff_ a redirect_uri is given with the authorization request,
_then_ the access token request must also include it (4.1.3), and it must
match (ibd.).
If I'm reading the logic for that part correctly though Shindig allows the
access token request to not specify a redirect_uri, and in that case it
will not check whether the authorization request had one specified. In
other words: it will provide a client that has an authcode with a token
without validating that the authcode redirect URI is correct.
Does that make sense?
--
Andreas
In this particular case, due to the logic you
> mentioned, an exception would be thrown because the redirect_uris do not
> match.
>
> Is that the case you are running into?
>
> Thanks,
> -Stanton
>
>
> On Wed, Oct 16, 2013 at 6:54 AM, Andreas Kohn <andreas.kohn@gmail.com
> >wrote:
>
> > Hi,
> >
> > I'm currently stepping through the logic for handling OAuth2 requests, at
> > the same time reading through RFC 6749 and trying to wrap my head around
> > what's going on :)
> >
> > I noticed that in AuthCodeGrantValidator#validateRequest() a condition
> > states "if servlet request has a redirect_uri, then it must match the one
> > stored in the authcode"[1], but from my reading of the RFC it should be
> "if
> > authcode has a redirect_uri, then the servlet request must specify the
> same
> > one" [2][3].
> >
> > Am I missing something?
> >
> > Regards,
> > --
> > Andreas
> >
> > [1]
> > 67 if (servletRequest.getRedirectURI() != null
> > 68 &&
> > !servletRequest.getRedirectURI().equals(authCode.getRedirectURI())) {
> > 69 OAuth2NormalizedResponse response = new
> > OAuth2NormalizedResponse();
> > 70 response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
> > 71 response.setError(ErrorType.INVALID_GRANT.toString());
> > 72 response
> > 73 .setErrorDescription("The redirect URI does not match the
> one
> > used in the authorization request");
> > 74 response.setBodyReturned(true);
> > 75 throw new OAuth2Exception(response);
> > 76 }
> >
> > [2] Section 4.1.3 Access Token Request says
> >
> > o ensure that the "redirect_uri" parameter is present if the
> > "redirect_uri" parameter was included in the initial authorization
> > request as described in Section 4.1.1
> > <http://tools.ietf.org/html/rfc6749#section-4.1.1>, and if included
> > ensure that
> > their values are identical.
> >
> >
> > [3] Fix would be to replace lines 67 and 68 with:
> > if (authCode.getRedirectURI() != null
> > &&
> > !authCode.getRedirectURI().equals(servletRequest.getRedirectURI())) {
> >
>
Re: Reversed condition in AuthCodeGrantValidator#validateRequest()?
Posted by Stanton Sievers <ss...@apache.org>.
Hi Andreas,
Let me make sure I understand. You're concerned about the use case where
the redirect_uri was omitted from the authorization request (which is valid
since it is optional per section 4.1.1) yet a redirect_uri was provided on
the access token request? In this particular case, due to the logic you
mentioned, an exception would be thrown because the redirect_uris do not
match.
Is that the case you are running into?
Thanks,
-Stanton
On Wed, Oct 16, 2013 at 6:54 AM, Andreas Kohn <an...@gmail.com>wrote:
> Hi,
>
> I'm currently stepping through the logic for handling OAuth2 requests, at
> the same time reading through RFC 6749 and trying to wrap my head around
> what's going on :)
>
> I noticed that in AuthCodeGrantValidator#validateRequest() a condition
> states "if servlet request has a redirect_uri, then it must match the one
> stored in the authcode"[1], but from my reading of the RFC it should be "if
> authcode has a redirect_uri, then the servlet request must specify the same
> one" [2][3].
>
> Am I missing something?
>
> Regards,
> --
> Andreas
>
> [1]
> 67 if (servletRequest.getRedirectURI() != null
> 68 &&
> !servletRequest.getRedirectURI().equals(authCode.getRedirectURI())) {
> 69 OAuth2NormalizedResponse response = new
> OAuth2NormalizedResponse();
> 70 response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
> 71 response.setError(ErrorType.INVALID_GRANT.toString());
> 72 response
> 73 .setErrorDescription("The redirect URI does not match the one
> used in the authorization request");
> 74 response.setBodyReturned(true);
> 75 throw new OAuth2Exception(response);
> 76 }
>
> [2] Section 4.1.3 Access Token Request says
>
> o ensure that the "redirect_uri" parameter is present if the
> "redirect_uri" parameter was included in the initial authorization
> request as described in Section 4.1.1
> <http://tools.ietf.org/html/rfc6749#section-4.1.1>, and if included
> ensure that
> their values are identical.
>
>
> [3] Fix would be to replace lines 67 and 68 with:
> if (authCode.getRedirectURI() != null
> &&
> !authCode.getRedirectURI().equals(servletRequest.getRedirectURI())) {
>