Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

consent: Use query parameters for challenges #1351

Merged
merged 4 commits into from
Apr 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,11 @@ The old Go SDK is still available but moved to a new path. To use it, change:
Previously, login and consent requests were accepted/rejected by doing one of:

```
GET /oauth2/auth/requests/login/{challenge}
PUT /oauth2/auth/requests/login/{challenge}/accept
PUT /oauth2/auth/requests/login/{challenge}/reject

GET /oauth2/auth/requests/consent/{challenge}
PUT /oauth2/auth/requests/consent/{challenge}/accept
PUT /oauth2/auth/requests/consent/{challenge}/reject
```
Expand All @@ -164,8 +167,11 @@ causing the login/consent app to execute a request it is not supposed to be maki
From now on, the challenge has to be sent using a query parameter instead:

```
GET /oauth2/auth/requests/login?challenge={challenge}
PUT /oauth2/auth/requests/login/accept?challenge={challenge}
PUT /oauth2/auth/requests/login/reject?challenge={challenge}

GET /oauth2/auth/requests/consent?challenge={challenge}
PUT /oauth2/auth/requests/consent/accept?challenge={challenge}
PUT /oauth2/auth/requests/consent/reject?challenge={challenge}
```
Expand Down
8 changes: 4 additions & 4 deletions consent/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ package consent

// swagger:parameters getLoginRequest getConsentRequest
type swaggerGetRequestByChallenge struct {
// in: path
// in: query
// required: true
Challenge string `json:"challenge"`
}
Expand Down Expand Up @@ -61,7 +61,7 @@ type swaggerRevokeAuthenticationSessionPayload struct {

// swagger:parameters acceptLoginRequest
type swaggerAcceptAuthenticationRequest struct {
// in: path
// in: query
// required: true
Challenge string `json:"challenge"`

Expand All @@ -71,7 +71,7 @@ type swaggerAcceptAuthenticationRequest struct {

// swagger:parameters acceptConsentRequest
type swaggerAcceptConsentRequest struct {
// in: path
// in: query
// required: true
Challenge string `json:"challenge"`

Expand All @@ -81,7 +81,7 @@ type swaggerAcceptConsentRequest struct {

// swagger:parameters rejectLoginRequest rejectConsentRequest
type swaggerRejectRequest struct {
// in: path
// in: query
// required: true
Challenge string `json:"challenge"`

Expand Down
63 changes: 37 additions & 26 deletions consent/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,13 @@ func NewHandler(
}

func (h *Handler) SetRoutes(admin *x.RouterAdmin, public *x.RouterPublic) {
admin.GET(LoginPath+"/:challenge", h.GetLoginRequest)
admin.PUT(LoginPath+"/:challenge/accept", h.AcceptLoginRequest)
admin.PUT(LoginPath+"/:challenge/reject", h.RejectLoginRequest)
admin.GET(LoginPath, h.GetLoginRequest)
admin.PUT(LoginPath+"/accept", h.AcceptLoginRequest)
admin.PUT(LoginPath+"/reject", h.RejectLoginRequest)

admin.GET(ConsentPath+"/:challenge", h.GetConsentRequest)
admin.PUT(ConsentPath+"/:challenge/accept", h.AcceptConsentRequest)
admin.PUT(ConsentPath+"/:challenge/reject", h.RejectConsentRequest)
admin.GET(ConsentPath, h.GetConsentRequest)
admin.PUT(ConsentPath+"/accept", h.AcceptConsentRequest)
admin.PUT(ConsentPath+"/reject", h.RejectConsentRequest)

admin.DELETE(SessionsPath+"/login/:user", h.DeleteLoginSession)
admin.GET(SessionsPath+"/consent/:user", h.GetConsentSessions)
Expand Down Expand Up @@ -218,7 +218,7 @@ func (h *Handler) DeleteLoginSession(w http.ResponseWriter, r *http.Request, ps
w.WriteHeader(http.StatusNoContent)
}

// swagger:route GET /oauth2/auth/requests/login/{challenge} admin getLoginRequest
// swagger:route GET /oauth2/auth/requests/login admin getLoginRequest
//
// Get an login request
//
Expand All @@ -245,7 +245,8 @@ func (h *Handler) DeleteLoginSession(w http.ResponseWriter, r *http.Request, ps
// 409: genericError
// 500: genericError
func (h *Handler) GetLoginRequest(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
request, err := h.r.ConsentManager().GetAuthenticationRequest(r.Context(), ps.ByName("challenge"))
challenge := r.URL.Query().Get("challenge")
request, err := h.r.ConsentManager().GetAuthenticationRequest(r.Context(), challenge)
if err != nil {
h.r.Writer().WriteError(w, r, err)
return
Expand All @@ -259,7 +260,7 @@ func (h *Handler) GetLoginRequest(w http.ResponseWriter, r *http.Request, ps htt
h.r.Writer().Write(w, r, request)
}

// swagger:route PUT /oauth2/auth/requests/login/{challenge}/accept admin acceptLoginRequest
// swagger:route PUT /oauth2/auth/requests/login/accept admin acceptLoginRequest
//
// Accept an login request
//
Expand Down Expand Up @@ -291,6 +292,8 @@ func (h *Handler) GetLoginRequest(w http.ResponseWriter, r *http.Request, ps htt
// 401: genericError
// 500: genericError
func (h *Handler) AcceptLoginRequest(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
challenge := r.URL.Query().Get("challenge")

var p HandledAuthenticationRequest
d := json.NewDecoder(r.Body)
d.DisallowUnknownFields()
Expand All @@ -302,8 +305,8 @@ func (h *Handler) AcceptLoginRequest(w http.ResponseWriter, r *http.Request, ps
h.r.Writer().WriteErrorCode(w, r, http.StatusBadRequest, errors.New("Subject from payload can not be empty"))
}

p.Challenge = ps.ByName("challenge")
ar, err := h.r.ConsentManager().GetAuthenticationRequest(r.Context(), ps.ByName("challenge"))
p.Challenge = challenge
ar, err := h.r.ConsentManager().GetAuthenticationRequest(r.Context(), challenge)
if err != nil {
h.r.Writer().WriteError(w, r, err)
return
Expand All @@ -320,7 +323,7 @@ func (h *Handler) AcceptLoginRequest(w http.ResponseWriter, r *http.Request, ps
}
p.RequestedAt = ar.RequestedAt

request, err := h.r.ConsentManager().HandleAuthenticationRequest(r.Context(), ps.ByName("challenge"), &p)
request, err := h.r.ConsentManager().HandleAuthenticationRequest(r.Context(), challenge, &p)
if err != nil {
h.r.Writer().WriteError(w, r, errors.WithStack(err))
return
Expand All @@ -337,7 +340,7 @@ func (h *Handler) AcceptLoginRequest(w http.ResponseWriter, r *http.Request, ps
})
}

// swagger:route PUT /oauth2/auth/requests/login/{challenge}/reject admin rejectLoginRequest
// swagger:route PUT /oauth2/auth/requests/login/reject admin rejectLoginRequest
//
// Reject a login request
//
Expand Down Expand Up @@ -368,6 +371,8 @@ func (h *Handler) AcceptLoginRequest(w http.ResponseWriter, r *http.Request, ps
// 404: genericError
// 500: genericError
func (h *Handler) RejectLoginRequest(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
challenge := r.URL.Query().Get("challenge")

var p RequestDeniedError
d := json.NewDecoder(r.Body)
d.DisallowUnknownFields()
Expand All @@ -376,15 +381,15 @@ func (h *Handler) RejectLoginRequest(w http.ResponseWriter, r *http.Request, ps
return
}

ar, err := h.r.ConsentManager().GetAuthenticationRequest(r.Context(), ps.ByName("challenge"))
ar, err := h.r.ConsentManager().GetAuthenticationRequest(r.Context(), challenge)
if err != nil {
h.r.Writer().WriteError(w, r, err)
return
}

request, err := h.r.ConsentManager().HandleAuthenticationRequest(r.Context(), ps.ByName("challenge"), &HandledAuthenticationRequest{
request, err := h.r.ConsentManager().HandleAuthenticationRequest(r.Context(), challenge, &HandledAuthenticationRequest{
Error: &p,
Challenge: ps.ByName("challenge"),
Challenge: challenge,
RequestedAt: ar.RequestedAt,
})
if err != nil {
Expand All @@ -403,7 +408,7 @@ func (h *Handler) RejectLoginRequest(w http.ResponseWriter, r *http.Request, ps
})
}

// swagger:route GET /oauth2/auth/requests/consent/{challenge} admin getConsentRequest
// swagger:route GET /oauth2/auth/requests/consent admin getConsentRequest
//
// Get consent request information
//
Expand Down Expand Up @@ -432,7 +437,9 @@ func (h *Handler) RejectLoginRequest(w http.ResponseWriter, r *http.Request, ps
// 409: genericError
// 500: genericError
func (h *Handler) GetConsentRequest(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
request, err := h.r.ConsentManager().GetConsentRequest(r.Context(), ps.ByName("challenge"))
challenge := r.URL.Query().Get("challenge")

request, err := h.r.ConsentManager().GetConsentRequest(r.Context(), challenge)
if err != nil {
h.r.Writer().WriteError(w, r, err)
return
Expand All @@ -446,7 +453,7 @@ func (h *Handler) GetConsentRequest(w http.ResponseWriter, r *http.Request, ps h
h.r.Writer().Write(w, r, request)
}

// swagger:route PUT /oauth2/auth/requests/consent/{challenge}/accept admin acceptConsentRequest
// swagger:route PUT /oauth2/auth/requests/consent/accept admin acceptConsentRequest
//
// Accept an consent request
//
Expand Down Expand Up @@ -480,6 +487,8 @@ func (h *Handler) GetConsentRequest(w http.ResponseWriter, r *http.Request, ps h
// 404: genericError
// 500: genericError
func (h *Handler) AcceptConsentRequest(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
challenge := r.URL.Query().Get("challenge")

var p HandledConsentRequest
d := json.NewDecoder(r.Body)
d.DisallowUnknownFields()
Expand All @@ -488,16 +497,16 @@ func (h *Handler) AcceptConsentRequest(w http.ResponseWriter, r *http.Request, p
return
}

cr, err := h.r.ConsentManager().GetConsentRequest(r.Context(), ps.ByName("challenge"))
cr, err := h.r.ConsentManager().GetConsentRequest(r.Context(), challenge)
if err != nil {
h.r.Writer().WriteError(w, r, errors.WithStack(err))
return
}

p.Challenge = ps.ByName("challenge")
p.Challenge = challenge
p.RequestedAt = cr.RequestedAt

hr, err := h.r.ConsentManager().HandleConsentRequest(r.Context(), ps.ByName("challenge"), &p)
hr, err := h.r.ConsentManager().HandleConsentRequest(r.Context(), challenge, &p)
if err != nil {
h.r.Writer().WriteError(w, r, errors.WithStack(err))
return
Expand All @@ -516,7 +525,7 @@ func (h *Handler) AcceptConsentRequest(w http.ResponseWriter, r *http.Request, p
})
}

// swagger:route PUT /oauth2/auth/requests/consent/{challenge}/reject admin rejectConsentRequest
// swagger:route PUT /oauth2/auth/requests/consent/reject admin rejectConsentRequest
//
// Reject an consent request
//
Expand Down Expand Up @@ -549,6 +558,8 @@ func (h *Handler) AcceptConsentRequest(w http.ResponseWriter, r *http.Request, p
// 404: genericError
// 500: genericError
func (h *Handler) RejectConsentRequest(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
challenge := r.URL.Query().Get("challenge")

var p RequestDeniedError
d := json.NewDecoder(r.Body)
d.DisallowUnknownFields()
Expand All @@ -557,15 +568,15 @@ func (h *Handler) RejectConsentRequest(w http.ResponseWriter, r *http.Request, p
return
}

hr, err := h.r.ConsentManager().GetConsentRequest(r.Context(), ps.ByName("challenge"))
hr, err := h.r.ConsentManager().GetConsentRequest(r.Context(), challenge)
if err != nil {
h.r.Writer().WriteError(w, r, errors.WithStack(err))
return
}

request, err := h.r.ConsentManager().HandleConsentRequest(r.Context(), ps.ByName("challenge"), &HandledConsentRequest{
request, err := h.r.ConsentManager().HandleConsentRequest(r.Context(), challenge, &HandledConsentRequest{
Error: &p,
Challenge: ps.ByName("challenge"),
Challenge: challenge,
RequestedAt: hr.RequestedAt,
})
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions consent/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func TestGetLoginRequest(t *testing.T) {
defer ts.Close()

c := &http.Client{}
resp, err := c.Get(ts.URL + LoginPath + "/" + challenge)
resp, err := c.Get(ts.URL + LoginPath + "?challenge=" + challenge)
require.NoError(t, err)
require.EqualValues(t, tc.status, resp.StatusCode)
})
Expand Down Expand Up @@ -169,7 +169,7 @@ func TestGetConsentRequest(t *testing.T) {
defer ts.Close()

c := &http.Client{}
resp, err := c.Get(ts.URL + ConsentPath + "/" + challenge)
resp, err := c.Get(ts.URL + ConsentPath + "?challenge=" + challenge)
require.NoError(t, err)
require.EqualValues(t, tc.status, resp.StatusCode)
})
Expand Down
2 changes: 1 addition & 1 deletion consent/strategy_default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,7 @@ func TestStrategy(t *testing.T) {

body := `{"grant_scope": ["scope-a"], "remember": true}`
require.NoError(t, err)
req, err := http.NewRequest("PUT", api.URL+"/oauth2/auth/requests/consent/"+r.URL.Query().Get("consent_challenge")+"/accept", strings.NewReader(body))
req, err := http.NewRequest("PUT", api.URL+"/oauth2/auth/requests/consent/accept?challenge="+r.URL.Query().Get("consent_challenge"), strings.NewReader(body))
req.Header.Add("Content-Type", "application/json")
require.NoError(t, err)

Expand Down
Loading