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 <cla...@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 <rbaxte...@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 <andreas.k...@gmail.com> > > wrote: > > > Hi, > > > > > > On Thu, Oct 17, 2013 at 2:26 AM, Ryan Baxter <rbaxte...@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.k...@gmail.com > > > > >> wrote: > > >> > Hi, > > >> > > > >> > On Wed, Oct 16, 2013 at 2:04 PM, Stanton Sievers < > ssiev...@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.k...@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())) { > > >> >> > > > >> >> > > >> > > >
