Open phisch opened 5 years ago
From the perspective of the protocol, the redirect url is indeed optional, but the protocol specification does not give a better implementation. Do you have any better implementation? Let's discuss it.
The ClientStore
currently stores objects of the type Client
, which contains a Domain
field. This field should actually be an array of redirection URIs (https://tools.ietf.org/html/rfc6749#section-2). When a request contains a redirection uri, you should check if the given client has this uri in its redirection uris. When the redirection uri is omitted, you should redirect to the only registered redirection_uri of the client, or provide an option to mark one of the registered redirection uris as the default one.
The part with the omitted redirection uri is a bit vague and up to interpretation. I think the solution where you check for the only registered, or the default redurection uri seems like the most reasonable one.
Ok, let me fix this bug.
The changes have been submitted, you can try to update:
go get -u gopkg.in/oauth2.v3/...
@LyricTian can close this I think
related: https://github.com/go-oauth2/oauth2/issues/99 https://github.com/go-oauth2/oauth2/issues/105
I consider this to be a serious vulnerability. It cannot redirect anywhere other than explicitly permitted urls.
https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2.1
If the request fails due to a missing, invalid, or mismatching redirection URI, or if the client identifier is missing or invalid, the authorization server SHOULD inform the resource owner of the error and MUST NOT automatically redirect the user-agent to the invalid redirection URI.
https://tools.ietf.org/html/rfc6749#section-4.1.1 States that the redirect_uri is OPTIONAL
The server returns an error when
redirect_uri
is omitted. https://github.com/go-oauth2/oauth2/blob/master/server/server.go#L152Could affect multiple authorization and token grants.