Repository: zitadel/oidc
Pull request #386 (2023-04-28 09:09:42)

docs: pull request template

codecov[bot]: ## [Codecov](https://codecov.io/gh/zitadel/oidc/pull/386?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zitadel) Report
> Merging [#386](https://codecov.io/gh/zitadel/oidc/pull/386?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zitadel) (c66d3f8) into [main](https://codecov.io/gh/zitadel/oidc/commit/7997994be483ad30c994de6a4bcc04f4dce682b7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zitadel) (7997994) will **not change** coverage.
> The diff coverage is `n/a`.

> :exclamation: Current head c66d3f8 differs from pull request most recent head 2bad4d1. Consider uploading reports for the commit 2bad4d1 to get more accurate results

```diff
@@           Coverage Diff           @@
##             main     #386   +/-   ##
=======================================
  Coverage   51.39%   51.39%           
=======================================
  Files          74       74           
  Lines        5676     5676           
=======================================
  Hits         2917     2917           
  Misses       2495     2495           
  Partials      264      264           
```





Patch:
diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md
new file mode 100644
index 0000000..811b195
--- /dev/null
+++ b/.github/pull_request_template.md
@@ -0,0 +1,15 @@
+### Definition of Ready
+
+- [ ] I am happy with the code
+- [ ] Short description of the feature/issue is added in the pr description
+- [ ] PR is linked to the corresponding user story
+- [ ] Acceptance criteria are met
+- [ ] All open todos and follow ups are defined in a new ticket and justified
+- [ ] Deviations from the acceptance criteria and design are agreed with the PO and documented.
+- [ ] No debug or dead code
+- [ ] Critical parts are tested automatically
+- [ ] Where possible E2E tests are implemented
+- [ ] Documentation/examples are up-to-date
+- [ ] All non-functional requirements are met
+- [ ] Functionality of the acceptance criteria is checked manually on the dev system.
+
diff --git a/SECURITY.md b/SECURITY.md
index 934426a..d682630 100644
--- a/SECURITY.md
+++ b/SECURITY.md
@@ -4,13 +4,21 @@ At ZITADEL we are extremely grateful for security aware people that disclose vul
 
 ## Supported Versions
 
-After the initial Release the following version support will apply
-
-| Version | Supported                               |
-| ------- | ------------------                      |
-| 0.x.x   | :x:                                     |
-| 1.x.x   | :white_check_mark:                      |
-| 2.x.x   | :white_check_mark: (not released)       |
+We currently support the following version of the OIDC framework:
+
+| Version  | Supported          | Branch      | Details                              |
+| -------- | ------------------ | ----------- | ------------------------------------ |
+| 0.x.x    | :x:                |             | not maintained                       |
+| <1.13    | :x:                |             | not maintained                       |
+| 1.13.x   | :lock: :warning:   | [1.13.x][1] | security only, [community effort][2] |
+| 2.x.x    | :heavy_check_mark: | [main][3]   | supported                            |
+| 3.0.0-xx | :white_check_mark: | [next][4]   | [developement branch][5]             |
+
+[1]: https://github.com/zitadel/oidc/tree/1.13.x
+[2]: https://github.com/zitadel/oidc/discussions/378
+[3]: https://github.com/zitadel/oidc/tree/main
+[4]: https://github.com/zitadel/oidc/tree/next
+[5]: https://github.com/zitadel/oidc/milestone/2
 
 ## Reporting a vulnerability
 
diff --git a/example/server/exampleop/op.go b/example/server/exampleop/op.go
index 1dc8bd1..7254585 100644
--- a/example/server/exampleop/op.go
+++ b/example/server/exampleop/op.go
@@ -34,7 +34,7 @@ type Storage interface {
 // SetupServer creates an OIDC server with Issuer=http://localhost:<port>
 //
 // Use one of the pre-made clients in storage/clients.go or register a new one.
-func SetupServer(issuer string, storage Storage) *mux.Router {
+func SetupServer(issuer string, storage Storage, extraOptions ...op.Option) *mux.Router {
 	// the OpenID Provider requires a 32-byte key for (token) encryption
 	// be sure to create a proper crypto random key and manage it securely!
 	key := sha256.Sum256([]byte("test"))
@@ -50,7 +50,7 @@ func SetupServer(issuer string, storage Storage) *mux.Router {
 	})
 
 	// creation of the OpenIDProvider with the just created in-memory Storage
-	provider, err := newOP(storage, issuer, key)
+	provider, err := newOP(storage, issuer, key, extraOptions...)
 	if err != nil {
 		log.Fatal(err)
 	}
@@ -79,7 +79,7 @@ func SetupServer(issuer string, storage Storage) *mux.Router {
 // newOP will create an OpenID Provider for localhost on a specified port with a given encryption key
 // and a predefined default logout uri
 // it will enable all options (see descriptions)
-func newOP(storage op.Storage, issuer string, key [32]byte) (op.OpenIDProvider, error) {
+func newOP(storage op.Storage, issuer string, key [32]byte, extraOptions ...op.Option) (op.OpenIDProvider, error) {
 	config := &op.Config{
 		CryptoKey: key,
 
@@ -112,10 +112,12 @@ func newOP(storage op.Storage, issuer string, key [32]byte) (op.OpenIDProvider,
 		},
 	}
 	handler, err := op.NewOpenIDProvider(issuer, config, storage,
-		//we must explicitly allow the use of the http issuer
-		op.WithAllowInsecure(),
-		// as an example on how to customize an endpoint this will change the authorization_endpoint from /authorize to /auth
-		op.WithCustomAuthEndpoint(op.NewEndpoint("auth")),
+		append([]op.Option{
+			// we must explicitly allow the use of the http issuer
+			op.WithAllowInsecure(),
+			// as an example on how to customize an endpoint this will change the authorization_endpoint from /authorize to /auth
+			op.WithCustomAuthEndpoint(op.NewEndpoint("auth")),
+		}, extraOptions...)...,
 	)
 	if err != nil {
 		return nil, err
diff --git a/example/server/storage/storage.go b/example/server/storage/storage.go
index a4c4f46..406300b 100644
--- a/example/server/storage/storage.go
+++ b/example/server/storage/storage.go
@@ -28,8 +28,10 @@ var serviceKey1 = &rsa.PublicKey{
 	E: 65537,
 }
 
-var _ op.Storage = &Storage{}
-var _ op.ClientCredentialsStorage = &Storage{}
+var (
+	_ op.Storage                  = &Storage{}
+	_ op.ClientCredentialsStorage = &Storage{}
+)
 
 // storage implements the op.Storage interface
 // typically you would implement this as a layer on top of your database
@@ -167,6 +169,12 @@ func (s *Storage) CreateAuthRequest(ctx context.Context, authReq *oidc.AuthReque
 	s.lock.Lock()
 	defer s.lock.Unlock()
 
+	if len(authReq.Prompt) == 1 && authReq.Prompt[0] == "none" {
+		// With prompt=none, there is no way for the user to log in
+		// so return error right away.
+		return nil, oidc.ErrLoginRequired()
+	}
+
 	// typically, you'll fill your storage / storage model with the information of the passed object
 	request := authRequestToInternal(authReq, userID)
 
diff --git a/go.mod b/go.mod
index 5f96c72..86c4f20 100644
--- a/go.mod
+++ b/go.mod
@@ -14,7 +14,7 @@ require (
 	github.com/rs/cors v1.9.0
 	github.com/sirupsen/logrus v1.9.0
 	github.com/stretchr/testify v1.8.2
-	golang.org/x/oauth2 v0.7.0
+	golang.org/x/oauth2 v0.8.0
 	golang.org/x/text v0.9.0
 	gopkg.in/square/go-jose.v2 v2.6.0
 )
@@ -26,8 +26,8 @@ require (
 	github.com/google/go-querystring v1.1.0 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	golang.org/x/crypto v0.7.0 // indirect
-	golang.org/x/net v0.9.0 // indirect
-	golang.org/x/sys v0.7.0 // indirect
+	golang.org/x/net v0.10.0 // indirect
+	golang.org/x/sys v0.8.0 // indirect
 	google.golang.org/appengine v1.6.7 // indirect
 	google.golang.org/protobuf v1.29.1 // indirect
 	gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
diff --git a/go.sum b/go.sum
index df4d60c..0e3906a 100644
--- a/go.sum
+++ b/go.sum
@@ -60,11 +60,11 @@ golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR
 golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20200202094626-16171245cfb2/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM=
-golang.org/x/net v0.9.0 h1:aWJ/m6xSmxWBx+V0XRHTlrYrPG56jKsLdTFmsSsCzOM=
-golang.org/x/net v0.9.0/go.mod h1:d48xBJpPfHeWQsugry2m+kC02ZBRGRgulfHnEXEuWns=
+golang.org/x/net v0.10.0 h1:X2//UzNDwYmtCLn7To6G58Wr6f5ahEAQgKNzv9Y951M=
+golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
-golang.org/x/oauth2 v0.7.0 h1:qe6s0zUXlPX80/dITx3440hWZ7GwMwgDDyrSGTPJG/g=
-golang.org/x/oauth2 v0.7.0/go.mod h1:hPLQkd9LyjfXTiRohC/41GhcFqxisoUQ99sCUOHO9x4=
+golang.org/x/oauth2 v0.8.0 h1:6dkIjl3j3LtZ/O3sTgZTMsLKSftL/B8Zgq4huOIIUu8=
+golang.org/x/oauth2 v0.8.0/go.mod h1:yr7u4HXZRm1R1kBWqr/xKNqewf0plRYoB7sla+BCIXE=
 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
@@ -73,8 +73,8 @@ golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.7.0 h1:3jlCCIQZPdOYu1h8BkNvLz8Kgwtae2cagcG/VamtZRU=
-golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.8.0 h1:EBmGv8NaZBZTWvrbjNoL6HVt+IVy3QDQpJs7VRIw3tU=
+golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=
diff --git a/pkg/client/rp/relying_party.go b/pkg/client/rp/relying_party.go
index 7127020..114599d 100644
--- a/pkg/client/rp/relying_party.go
+++ b/pkg/client/rp/relying_party.go
@@ -325,7 +325,7 @@ func Discover(issuer string, httpClient *http.Client) (Endpoints, error) {
 		return Endpoints{}, err
 	}
 	if discoveryConfig.Issuer != issuer {
-		return Endpoints{}, oidc.ErrIssuerInvalid
+		return Endpoints{}, fmt.Errorf("%w: Expected: %s, got: %s", oidc.ErrIssuerInvalid, discoveryConfig.Issuer, issuer)
 	}
 	return GetEndpoints(discoveryConfig), nil
 }
@@ -569,6 +569,11 @@ func WithPromptURLParam(prompt ...string) URLParamOpt {
 	return withPrompt(prompt...)
 }
 
+// WithResponseModeURLParam sets the `response_mode` parameter in a URL.
+func WithResponseModeURLParam(mode oidc.ResponseMode) URLParamOpt {
+	return withURLParam("response_mode", string(mode))
+}
+
 type AuthURLOpt func() []oauth2.AuthCodeOption
 
 // WithCodeChallenge sets the `code_challenge` params in the auth request
diff --git a/pkg/oidc/authorization.go b/pkg/oidc/authorization.go
index f620ecb..ace1de1 100644
--- a/pkg/oidc/authorization.go
+++ b/pkg/oidc/authorization.go
@@ -60,7 +60,7 @@ const (
 )
 
 // AuthRequest according to:
-//https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
+// https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
 type AuthRequest struct {
 	Scopes       SpaceDelimitedArray `json:"scope" schema:"scope"`
 	ResponseType ResponseType        `json:"response_type" schema:"response_type"`
@@ -100,3 +100,8 @@ func (a *AuthRequest) GetResponseType() ResponseType {
 func (a *AuthRequest) GetState() string {
 	return a.State
 }
+
+// GetResponseMode returns the optional ResponseMode
+func (a *AuthRequest) GetResponseMode() ResponseMode {
+	return a.ResponseMode
+}


Test patch:
diff --git a/pkg/client/integration_test.go b/pkg/client/integration_test.go
index 40e1bee..ea7225d 100644
--- a/pkg/client/integration_test.go
+++ b/pkg/client/integration_test.go
@@ -25,6 +25,7 @@ import (
 	"github.com/zitadel/oidc/v2/pkg/client/tokenexchange"
 	httphelper "github.com/zitadel/oidc/v2/pkg/http"
 	"github.com/zitadel/oidc/v2/pkg/oidc"
+	"github.com/zitadel/oidc/v2/pkg/op"
 )
 
 func TestRelyingPartySession(t *testing.T) {
@@ -280,6 +281,92 @@ func RunAuthorizationCodeFlow(t *testing.T, opServer *httptest.Server, clientID,
 	return provider, accessToken, refreshToken, idToken
 }
 
+func TestErrorFromPromptNone(t *testing.T) {
+	jar, err := cookiejar.New(nil)
+	require.NoError(t, err, "create cookie jar")
+	httpClient := &http.Client{
+		Timeout: time.Second * 5,
+		CheckRedirect: func(_ *http.Request, _ []*http.Request) error {
+			return http.ErrUseLastResponse
+		},
+		Jar: jar,
+	}
+
+	t.Log("------- start example OP ------")
+	targetURL := "http://local-site"
+	exampleStorage := storage.NewStorage(storage.NewUserStore(targetURL))
+	var dh deferredHandler
+	opServer := httptest.NewServer(&dh)
+	defer opServer.Close()
+	t.Logf("auth server at %s", opServer.URL)
+	dh.Handler = exampleop.SetupServer(opServer.URL, exampleStorage, op.WithHttpInterceptors(
+		func(next http.Handler) http.Handler {
+			return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				t.Logf("request to %s", r.URL)
+				next.ServeHTTP(w, r)
+			})
+		},
+	))
+	seed := rand.New(rand.NewSource(int64(os.Getpid()) + time.Now().UnixNano()))
+	clientID := t.Name() + "-" + strconv.FormatInt(seed.Int63(), 25)
+	clientSecret := "secret"
+	client := storage.WebClient(clientID, clientSecret, targetURL)
+	storage.RegisterClients(client)
+
+	t.Log("------- create RP ------")
+	key := []byte("test1234test1234")
+	cookieHandler := httphelper.NewCookieHandler(key, key, httphelper.WithUnsecure())
+	provider, err := rp.NewRelyingPartyOIDC(
+		opServer.URL,
+		clientID,
+		clientSecret,
+		targetURL,
+		[]string{"openid", "email", "profile", "offline_access"},
+		rp.WithPKCE(cookieHandler),
+		rp.WithVerifierOpts(
+			rp.WithIssuedAtOffset(5*time.Second),
+			rp.WithSupportedSigningAlgorithms("RS256", "RS384", "RS512", "ES256", "ES384", "ES512"),
+		),
+	)
+	require.NoError(t, err, "new rp")
+
+	t.Log("------- start auth flow with prompt=none ------- ")
+	state := "state-32892"
+	capturedW := httptest.NewRecorder()
+	localURL, err := url.Parse(targetURL + "/login")
+	require.NoError(t, err)
+
+	get := httptest.NewRequest("GET", localURL.String(), nil)
+	rp.AuthURLHandler(func() string { return state }, provider,
+		rp.WithPromptURLParam("none"),
+		rp.WithResponseModeURLParam(oidc.ResponseModeFragment),
+	)(capturedW, get)
+
+	defer func() {
+		if t.Failed() {
+			t.Log("response body (redirect from RP to OP)", capturedW.Body.String())
+		}
+	}()
+	require.GreaterOrEqual(t, capturedW.Code, 200, "captured response code")
+	require.Less(t, capturedW.Code, 400, "captured response code")
+
+	//nolint:bodyclose
+	resp := capturedW.Result()
+	jar.SetCookies(localURL, resp.Cookies())
+
+	startAuthURL, err := resp.Location()
+	require.NoError(t, err, "get redirect")
+	assert.NotEmpty(t, startAuthURL, "login url")
+	t.Log("Starting auth at", startAuthURL)
+
+	t.Log("------- get redirect from OP ------")
+	loginPageURL := getRedirect(t, "get redirect to login page", httpClient, startAuthURL)
+	t.Log("login page URL", loginPageURL)
+
+	require.Contains(t, loginPageURL.String(), `error=login_required`, "prompt=none should error")
+	require.Contains(t, loginPageURL.String(), `local-site#error=`, "response_mode=fragment means '#' instead of '?'")
+}
+
 type deferredHandler struct {
 	http.Handler
 }
diff --git a/pkg/client/rp/verifier_test.go b/pkg/client/rp/verifier_test.go
index 7588c1f..f4e0f9d 100644
--- a/pkg/client/rp/verifier_test.go
+++ b/pkg/client/rp/verifier_test.go
@@ -113,6 +113,18 @@ func TestVerifyIDToken(t *testing.T) {
 			clientID:    tu.ValidClientID,
 			tokenClaims: tu.ValidIDToken,
 		},
+		{
+			name:     "custom claims",
+			clientID: tu.ValidClientID,
+			tokenClaims: func() (string, *oidc.IDTokenClaims) {
+				return tu.NewIDTokenCustom(
+					tu.ValidIssuer, tu.ValidSubject, tu.ValidAudience,
+					tu.ValidExpiration, tu.ValidAuthTime, tu.ValidNonce,
+					tu.ValidACR, tu.ValidAMR, tu.ValidClientID, tu.ValidSkew, "",
+					map[string]any{"some": "thing"},
+				)
+			},
+		},
 		{
 			name:        "parse err",
 			clientID:    tu.ValidClientID,