Skip to content

Commit

Permalink
consent: Add check for empty subject in AcceptLoginRequest (ory#1308)
Browse files Browse the repository at this point in the history
Closes ory#1254

Signed-off-by: Kevin Minehart <[email protected]>
  • Loading branch information
kminehart authored and aeneasr committed Mar 17, 2019
1 parent a073966 commit 1d963c2
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 0 deletions.
3 changes: 3 additions & 0 deletions consent/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,9 @@ func (h *Handler) AcceptLoginRequest(w http.ResponseWriter, r *http.Request, ps
h.H.WriteErrorCode(w, r, http.StatusBadRequest, errors.WithStack(err))
return
}
if p.Subject == "" {
h.H.WriteErrorCode(w, r, http.StatusBadRequest, errors.New("Subject from payload can not be empty"))
}

p.Challenge = ps.ByName("challenge")
ar, err := h.M.GetAuthenticationRequest(r.Context(), ps.ByName("challenge"))
Expand Down
1 change: 1 addition & 0 deletions consent/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ type HandledAuthenticationRequest struct {
ACR string `json:"acr"`

// Subject is the user ID of the end-user that authenticated.
// required: true
Subject string `json:"subject"`

// ForceSubjectIdentifier forces the "pairwise" user ID of the end-user that authenticated. The "pairwise" user ID refers to the
Expand Down
36 changes: 36 additions & 0 deletions oauth2/oauth2_auth_code_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,42 @@ func TestAuthCodeWithDefaultStrategy(t *testing.T) {
_, err := oauthConfig.TokenSource(oauth2.NoContext, token).Token()
require.Error(t, err)
},
}, {
d: "Checks if request fails when subject is empty",
authURL: oauthConfig.AuthCodeURL("some-hardcoded-state", oauth2.SetAuthURLParam("audience", "https://not-ory-api/")),
cj: newCookieJar(),
lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
challenge, res, err := apiClient.GetLoginRequest(r.URL.Query().Get("login_challenge"))
require.NoError(t, err)
require.EqualValues(t, http.StatusOK, res.StatusCode)
assert.False(t, challenge.Skip)
v, res, err := apiClient.AcceptLoginRequest(r.URL.Query().Get("login_challenge"), swagger.AcceptLoginRequest{
Subject: "", Remember: false, RememberFor: 0, Acr: "1",
})
require.Error(t, err)
require.EqualValues(t, http.StatusBadRequest, res.StatusCode)
require.NotEmpty(t, v.RedirectTo)
http.Redirect(w, r, v.RedirectTo, http.StatusFound)
}
},
cph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) {
return func(w http.ResponseWriter, r *http.Request) { t.Fatal("This should not have been called") }
},
cb: func(t *testing.T) httprouter.Handle {
return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
code = r.URL.Query().Get("code")
require.Empty(t, code)
w.WriteHeader(http.StatusOK)
}
},
expectOAuthAuthError: true,
expectOAuthTokenError: false,
expectIDToken: false,
expectRefreshToken: false,
assertAccessToken: func(t *testing.T, token string) {
require.Empty(t, token)
},
},
{
d: "Perform OAuth2 flow with refreshing which works just fine",
Expand Down

0 comments on commit 1d963c2

Please sign in to comment.