Skip to content

Commit

Permalink
openid: Allow promp=none for https/localhost (#359)
Browse files Browse the repository at this point in the history
Signed-off-by: aeneasr <aeneas@ory.sh>
  • Loading branch information
aeneasr authored Apr 26, 2019
1 parent dce3111 commit 27bbe00
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 10 deletions.
8 changes: 3 additions & 5 deletions handler/openid/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,10 @@ func (v *OpenIDConnectRequestValidator) ValidatePrompt(ctx context.Context, req
// unless the identity of the client can be proven, the request SHOULD
// be processed as if no previous request had been approved.

// To make sure that we are not vulnerable to this type of attack, we will always require consent for public
// clients.

// If prompt is none - meaning that no consent should be requested, we must terminate with an error.
if stringslice.Has(prompt, "none") {
return errors.WithStack(fosite.ErrConsentRequired.WithHint("OAuth 2.0 Client is marked public and requires end-user consent, but \"prompt=none\" was requested."))
if !(req.GetRedirectURI().Scheme == "https" || (fosite.IsLocalhost(req.GetRedirectURI()) && req.GetRedirectURI().Scheme == "http")) {
return errors.WithStack(fosite.ErrConsentRequired.WithHint("OAuth 2.0 Client is marked public and redirect uri is not considered secure (https missing), but \"prompt=none\" was requested."))
}
}
}

Expand Down
48 changes: 43 additions & 5 deletions handler/openid/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,22 +47,54 @@ func TestValidatePrompt(t *testing.T) {
for k, tc := range []struct {
d string
prompt string
redirectURL string
isPublic bool
expectErr bool
idTokenHint string
s *DefaultSession
}{
{
d: "should fail because prompt=none should not work together with public clients",
prompt: "none",
isPublic: true,
expectErr: true,
d: "should fail because prompt=none should not work together with public clients and non-http-localhost",
prompt: "none",
isPublic: true,
expectErr: true,
redirectURL: "http://foo-bar/",
s: &DefaultSession{
Subject: "foo",
Claims: &jwt.IDTokenClaims{
Subject: "foo",
RequestedAt: time.Now().UTC(),
AuthTime: time.Now().UTC(),
AuthTime: time.Now().UTC().Add(-time.Minute),
},
},
},
{
d: "should fail because prompt=none should not work together with public clients",
prompt: "none",
isPublic: true,
expectErr: false,
redirectURL: "http://localhost/",
s: &DefaultSession{
Subject: "foo",
Claims: &jwt.IDTokenClaims{
Subject: "foo",
RequestedAt: time.Now().UTC(),
AuthTime: time.Now().UTC().Add(-time.Minute),
},
},
},
{
d: "should pass",
prompt: "none",
isPublic: true,
expectErr: false,
redirectURL: "https://foo-bar/",
s: &DefaultSession{
Subject: "foo",
Claims: &jwt.IDTokenClaims{
Subject: "foo",
RequestedAt: time.Now().UTC(),
AuthTime: time.Now().UTC().Add(-time.Minute),
},
},
},
Expand Down Expand Up @@ -230,6 +262,7 @@ func TestValidatePrompt(t *testing.T) {
Client: &fosite.DefaultClient{Public: tc.isPublic},
Session: tc.s,
},
RedirectURI: parse(tc.redirectURL),
})
if tc.expectErr {
assert.Error(t, err)
Expand All @@ -239,3 +272,8 @@ func TestValidatePrompt(t *testing.T) {
})
}
}

func parse(u string) *url.URL {
o, _ := url.Parse(u)
return o
}

0 comments on commit 27bbe00

Please sign in to comment.