diff --git a/handler/openid/validator.go b/handler/openid/validator.go index cf4416b6e..837999df0 100644 --- a/handler/openid/validator.go +++ b/handler/openid/validator.go @@ -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.")) + } } } diff --git a/handler/openid/validator_test.go b/handler/openid/validator_test.go index 66158dc80..db7b8dbb0 100644 --- a/handler/openid/validator_test.go +++ b/handler/openid/validator_test.go @@ -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), }, }, }, @@ -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) @@ -239,3 +272,8 @@ func TestValidatePrompt(t *testing.T) { }) } } + +func parse(u string) *url.URL { + o, _ := url.Parse(u) + return o +}