Repository: zitadel/oidc (go)

Issue:
client/rs: error when issuer discovery has no introspection endpoint
**Describe the bug**
When an issuer does not provide an introspection endpoint during discovery, `newResourceServer()` fails with:
```
introspectURL and/or tokenURL is empty: please provide with either `WithStaticEndpoints` or a discovery url
```
Note that a token endpoint is always provided by the issuer.

According to the OpenID specs for [Discovery responses](https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationResponse), an introspection endpoint is not mandatory (not defined in the response object) while the token endpoint is required.

Three examples of issuers that do not provide any introspection endpoint are:
* Google: <https://accounts.google.com/.well-known/openid-configuration>
* Spotify: <https://accounts.spotify.com/.well-known/openid-configuration>
* PayPal: <https://www.paypal.com/.well-known/openid-configuration>

This is because of the following (incorrect) check after discovery that assumes that both endpoints must be present:
https://github.com/zitadel/oidc/blob/9e624986aaf6eb8854ffc4702d7d1cc755919ff5/pkg/client/rs/resource_server.go#L83-L85

In reality, the introspection endpoint is valid to be empty.

**To Reproduce**
Consider the following demonstration Go program:
```go
package main

import (
	"fmt"

	"github.com/zitadel/oidc/v2/pkg/client/rs"
)

const (
	Issuer = "https://accounts.google.com"
//	Issuer = "https://accounts.spotify.com"
//	Issuer = "https://www.paypal.com"
	ClientId = "client-id"
	ClientSecret = "client-secret"
)

func main() {
	rscc, err := rs.NewResourceServerClientCredentials(
		Issuer,
		ClientId,
		ClientSecret,
	)
	if err != nil {
		panic(err)
	}

	fmt.Printf("%+v\n", rscc)

	res, err := rs.Introspect(nil, rscc, "test-token")
	if err != nil {
		panic(err)
	}

	fmt.Printf("%+v\n", res)
}
```
When running the above program, an panic is emitted after the error from `NewResourceServerClientCredentials()`:
```
$ go run main.go
panic: introspectURL and/or tokenURL is empty: please provide with either `WithStaticEndpoints` or a discovery url

exit status 2
```

**Expected behavior**
Given that in the demonstration program above I'm using the discovery mechanism, I expect it to not fail when the issuer does not provide an introspection endpoint during discovery as it is an optional feature.

Instead, I expect the `rscc` object to be correctly instantiated and a future call to `rs.Introspect()` fails with an error related to the introspection endpoint not being set. This would allow to still use `rscc` (for the authorizer functionality) but without introspection support.

It can be argued that if an issuer is known to not provide an introspection endpoint, it can be given statically using `WithStaticEndpoints("", "https://some.host/introspection-endpoint")` (or a dummy value) but is highly unlikely that a provider has a manual introspection endpoint and does not provide it in discovery. This would just propagate a confusing error later during `rs.Introspect()`.

**Potential Solution**
I propose to instead accept that discovery may not provide an introspection endpoint, do not fail in this situation, and add a check for an empty introspection endpoint in `rs.Introspect()` to clearly indicate that the issuer does not support introspection.

In addition, discovery should only set `introspectURL` or `tokenURL` if they were not given in static endpoint arguments.

If the above makes sense, I already have this logic implemented here:
https://github.com/hhromic/oidc/compare/main...hhromic:oidc:fix-rs-discovery

As it can be seen, I also made a set of unit tests that cover this issue.
Let me know if you agree with the above proposal to open a PR with these changes.

Pull request:
fix(client/rs): do not error when issuer discovery has no introspection endpoint
This PR fixes `client/rs` to not error when an issuer discovery returns no introspection endpoint.

Summary of changes:
* `newResourceServer()` will only fill `rs.tokenURL` and `rs.introspectURL` if they were not given as static endpoints using `WithStaticEndpoints()`. This allows the discovery mechanism to always respect the user-provided static endpoints and only fill them when one or the other was not statically provided. Also, given that `config.introspectURL` is optional in the OIDC Discovery spec, it is no longer enforced to be present after discovery.
* `Introspect()` will now fail if the passed `ResourceServer` does not have an `IntrospectionURL()` set. This allows for graceful errors in case issuers do not provide an introspection endpoint.
* Added unit tests covering all situations using discovery and static endpoints.

Fixes #413 

### 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
- [ ] My code has no repetitions
- [ ] 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.



Patch:
diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md
deleted file mode 100644
index 49ccc49..0000000
--- a/.github/ISSUE_TEMPLATE/bug_report.md
+++ /dev/null
@@ -1,38 +0,0 @@
----
-name: 🐛 Bug report
-about: Create a report to help us improve
-title: ''
-labels: bug
-assignees: ''
-
----
-
-**Describe the bug**
-A clear and concise description of what the bug is.
-
-**To Reproduce**
-Steps to reproduce the behavior:
-1. Go to '...'
-2. Click on '....'
-3. Scroll down to '....'
-4. See error
-
-**Expected behavior**
-A clear and concise description of what you expected to happen.
-
-**Screenshots**
-If applicable, add screenshots to help explain your problem.
-
-**Desktop (please complete the following information):**
-- OS: [e.g. iOS]
-- Browser [e.g. chrome, safari]
-- Version [e.g. 22]
-
-**Smartphone (please complete the following information):**
-- Device: [e.g. iPhone6]
-- OS: [e.g. iOS8.1]
-- Browser [e.g. stock browser, safari]
-- Version [e.g. 22]
-
-**Additional context**
-Add any other context about the problem here.
\ No newline at end of file
diff --git a/.github/ISSUE_TEMPLATE/bug_report.yaml b/.github/ISSUE_TEMPLATE/bug_report.yaml
new file mode 100644
index 0000000..92465f9
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/bug_report.yaml
@@ -0,0 +1,56 @@
+name: Bug Report
+description: "Create a bug report to help us improve ZITADEL. Click [here](https://github.com/zitadel/zitadel/blob/main/CONTRIBUTING.md#product-management) to see how we process your issue."
+title: "[Bug]: "
+labels: ["bug"]
+body:
+  - type: markdown
+    attributes:
+      value: |
+        Thanks for taking the time to fill out this bug report!
+  - type: checkboxes
+    id: preflight
+    attributes:
+      label: Preflight Checklist
+      options:
+      - label:
+          I could not find a solution in the documentation, the existing issues or discussions
+        required: true
+      - label:
+          I have joined the [ZITADEL chat](https://zitadel.com/chat)
+  - type: input
+    id: version
+    attributes:
+      label: Version
+      description: Which version of the OIDC library are you using.
+  - type: textarea
+    id: impact
+    attributes:
+      label: Describe the problem caused by this bug
+      description: A clear and concise description of the problem you have and what the bug is.
+    validations:
+      required: true
+  - type: textarea
+    id: reproduce
+    attributes:
+      label: To reproduce
+      description: Steps to reproduce the behaviour
+      placeholder: |
+        Steps to reproduce the behavior:
+    validations:
+      required: true
+  - type: textarea
+    id: screenshots
+    attributes:
+      label: Screenshots
+      description: If applicable, add screenshots to help explain your problem.
+  - type: textarea
+    id: expected
+    attributes:
+      label: Expected behavior
+      description: A clear and concise description of what you expected to happen.
+      placeholder: As a [type of user], I want [some goal] so that [some reason].
+  - type: textarea
+    id: additional
+    attributes:
+      label: Additional Context
+      description: Please add any other infos that could be useful.
diff --git a/.github/ISSUE_TEMPLATE/docs.yaml b/.github/ISSUE_TEMPLATE/docs.yaml
new file mode 100644
index 0000000..04c1c0c
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/docs.yaml
@@ -0,0 +1,30 @@
+name: 📄 Documentation
+description: Create an issue for missing or wrong documentation.
+labels: ["docs"]
+body:
+  - type: markdown
+    attributes:
+      value: |
+        Thanks for taking the time to fill out this issue.
+  - type: checkboxes
+    id: preflight
+    attributes:
+      label: Preflight Checklist
+      options:
+      - label:
+          I could not find a solution in the existing issues, docs, nor discussions
+        required: true
+      - label:
+          I have joined the [ZITADEL chat](https://zitadel.com/chat)
+  - type: textarea
+    id: docs
+    attributes:
+      label: Describe the docs your are missing or that are wrong
+      placeholder: As a [type of user], I want [some goal] so that [some reason].
+    validations:
+      required: true
+  - type: textarea
+    id: additional
+    attributes:
+      label: Additional Context
+      description: Please add any other infos that could be useful.
diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md
deleted file mode 100644
index 118d30e..0000000
--- a/.github/ISSUE_TEMPLATE/feature_request.md
+++ /dev/null
@@ -1,20 +0,0 @@
----
-name: 🚀 Feature request
-about: Suggest an idea for this project
-title: ''
-labels: enhancement
-assignees: ''
-
----
-
-**Is your feature request related to a problem? Please describe.**
-A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]
-
-**Describe the solution you'd like**
-A clear and concise description of what you want to happen.
-
-**Describe alternatives you've considered**
-A clear and concise description of any alternative solutions or features you've considered.
-
-**Additional context**
-Add any other context or screenshots about the feature request here.
diff --git a/.github/ISSUE_TEMPLATE/improvement.yaml b/.github/ISSUE_TEMPLATE/improvement.yaml
new file mode 100644
index 0000000..2e2ddf4
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/improvement.yaml
@@ -0,0 +1,54 @@
+name: 🛠️ Improvement
+description: "Create an new issue for an improvment in ZITADEL"
+labels: ["improvement"]
+body:
+  - type: markdown
+    attributes:
+      value: |
+        Thanks for taking the time to fill out this improvement request
+  - type: checkboxes
+    id: preflight
+    attributes:
+      label: Preflight Checklist
+      options:
+      - label:
+          I could not find a solution in the existing issues, docs, nor discussions
+        required: true
+      - label:
+          I have joined the [ZITADEL chat](https://zitadel.com/chat)
+  - type: textarea
+    id: problem
+    attributes:
+      label: Describe your problem
+      description: Please describe your problem this improvement is supposed to solve.
+      placeholder: Describe the problem you have
+    validations:
+      required: true
+  - type: textarea
+    id: solution
+    attributes:
+      label: Describe your ideal solution
+      description: Which solution do you propose?
+      placeholder: As a [type of user], I want [some goal] so that [some reason].
+    validations:
+      required: true
+  - type: input
+    id: version
+    attributes:
+      label: Version
+      description: Which version of the OIDC Library are you using.
+  - type: dropdown
+    id: environment
+    attributes:
+      label: Environment
+      description: How do you use ZITADEL?
+      options:
+      - ZITADEL Cloud
+      - Self-hosted
+    validations:
+      required: true
+  - type: textarea
+    id: additional
+    attributes:
+      label: Additional Context
+      description: Please add any other infos that could be useful.
diff --git a/.github/ISSUE_TEMPLATE/proposal.yaml b/.github/ISSUE_TEMPLATE/proposal.yaml
new file mode 100644
index 0000000..af7acd5
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/proposal.yaml
@@ -0,0 +1,44 @@
+name: 💡 Proposal / Feature request
+description: "Create an issue for a feature request/proposal."
+labels: ["enhancement"]
+body:
+  - type: markdown
+    attributes:
+      value: |
+        Thanks for taking the time to fill out this proposal / feature reqeust
+  - type: checkboxes
+    id: preflight
+    attributes:
+      label: Preflight Checklist
+      options:
+      - label:
+          I could not find a solution in the existing issues, docs, nor discussions
+        required: true
+      - label:
+          I have joined the [ZITADEL chat](https://zitadel.com/chat)
+  - type: textarea
+    id: problem
+    attributes:
+      label: Describe your problem
+      description: Please describe your problem this proposal / feature is supposed to solve.
+      placeholder: Describe the problem you have.
+    validations:
+      required: true
+  - type: textarea
+    id: solution
+    attributes:
+      label: Describe your ideal solution
+      description: Which solution do you propose?
+      placeholder: As a [type of user], I want [some goal] so that [some reason].
+    validations:
+      required: true
+  - type: input
+    id: version
+    attributes:
+      label: Version
+      description: Which version of the OIDC Library are you using.
+  - type: textarea
+    id: additional
+    attributes:
+      label: Additional Context
+      description: Please add any other infos that could be useful.
diff --git a/pkg/client/rs/resource_server.go b/pkg/client/rs/resource_server.go
index 4e0353c..c641940 100644
--- a/pkg/client/rs/resource_server.go
+++ b/pkg/client/rs/resource_server.go
@@ -77,11 +77,15 @@ func newResourceServer(issuer string, authorizer func() (interface{}, error), op
 		if err != nil {
 			return nil, err
 		}
-		rs.tokenURL = config.TokenEndpoint
-		rs.introspectURL = config.IntrospectionEndpoint
+		if rs.tokenURL == "" {
+			rs.tokenURL = config.TokenEndpoint
+		}
+		if rs.introspectURL == "" {
+			rs.introspectURL = config.IntrospectionEndpoint
+		}
 	}
-	if rs.introspectURL == "" || rs.tokenURL == "" {
-		return nil, errors.New("introspectURL and/or tokenURL is empty: please provide with either `WithStaticEndpoints` or a discovery url")
+	if rs.tokenURL == "" {
+		return nil, errors.New("tokenURL is empty: please provide with either `WithStaticEndpoints` or a discovery url")
 	}
 	rs.authFn = authorizer
 	return rs, nil
@@ -113,6 +117,9 @@ func WithStaticEndpoints(tokenURL, introspectURL string) Option {
 }
 
 func Introspect(ctx context.Context, rp ResourceServer, token string) (*oidc.IntrospectionResponse, error) {
+	if rp.IntrospectionURL() == "" {
+		return nil, errors.New("resource server: introspection URL is empty")
+	}
 	authFn, err := rp.AuthFn()
 	if err != nil {
 		return nil, err


Test patch:
diff --git a/pkg/client/rs/resource_server_test.go b/pkg/client/rs/resource_server_test.go
new file mode 100644
index 0000000..b5fb496
--- /dev/null
+++ b/pkg/client/rs/resource_server_test.go
@@ -0,0 +1,219 @@
+package rs
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestNewResourceServer(t *testing.T) {
+	type args struct {
+		issuer     string
+		authorizer func() (interface{}, error)
+		options    []Option
+	}
+	type wantFields struct {
+		issuer        string
+		tokenURL      string
+		introspectURL string
+		authFn        func() (interface{}, error)
+	}
+	tests := []struct {
+		name       string
+		args       args
+		wantFields *wantFields
+		wantErr    bool
+	}{
+		{
+			name: "spotify-full-discovery",
+			args: args{
+				issuer:     "https://accounts.spotify.com",
+				authorizer: nil,
+				options:    []Option{},
+			},
+			wantFields: &wantFields{
+				issuer:        "https://accounts.spotify.com",
+				tokenURL:      "https://accounts.spotify.com/api/token",
+				introspectURL: "",
+				authFn:        nil,
+			},
+			wantErr: false,
+		},
+		{
+			name: "spotify-with-static-tokenurl",
+			args: args{
+				issuer:     "https://accounts.spotify.com",
+				authorizer: nil,
+				options: []Option{
+					WithStaticEndpoints(
+						"https://some.host/token-url",
+						"",
+					),
+				},
+			},
+			wantFields: &wantFields{
+				issuer:        "https://accounts.spotify.com",
+				tokenURL:      "https://some.host/token-url",
+				introspectURL: "",
+				authFn:        nil,
+			},
+			wantErr: false,
+		},
+		{
+			name: "spotify-with-static-introspecturl",
+			args: args{
+				issuer:     "https://accounts.spotify.com",
+				authorizer: nil,
+				options: []Option{
+					WithStaticEndpoints(
+						"",
+						"https://some.host/instrospect-url",
+					),
+				},
+			},
+			wantFields: &wantFields{
+				issuer:        "https://accounts.spotify.com",
+				tokenURL:      "https://accounts.spotify.com/api/token",
+				introspectURL: "https://some.host/instrospect-url",
+				authFn:        nil,
+			},
+			wantErr: false,
+		},
+		{
+			name: "spotify-with-all-static-endpoints",
+			args: args{
+				issuer:     "https://accounts.spotify.com",
+				authorizer: nil,
+				options: []Option{
+					WithStaticEndpoints(
+						"https://some.host/token-url",
+						"https://some.host/instrospect-url",
+					),
+				},
+			},
+			wantFields: &wantFields{
+				issuer:        "https://accounts.spotify.com",
+				tokenURL:      "https://some.host/token-url",
+				introspectURL: "https://some.host/instrospect-url",
+				authFn:        nil,
+			},
+			wantErr: false,
+		},
+		{
+			name: "bad-discovery",
+			args: args{
+				issuer:     "https://127.0.0.1:65535",
+				authorizer: nil,
+				options:    []Option{},
+			},
+			wantFields: nil,
+			wantErr:    true,
+		},
+		{
+			name: "bad-discovery-with-static-tokenurl",
+			args: args{
+				issuer:     "https://127.0.0.1:65535",
+				authorizer: nil,
+				options: []Option{
+					WithStaticEndpoints(
+						"https://some.host/token-url",
+						"",
+					),
+				},
+			},
+			wantFields: nil,
+			wantErr:    true,
+		},
+		{
+			name: "bad-discovery-with-static-introspecturl",
+			args: args{
+				issuer:     "https://127.0.0.1:65535",
+				authorizer: nil,
+				options: []Option{
+					WithStaticEndpoints(
+						"",
+						"https://some.host/instrospect-url",
+					),
+				},
+			},
+			wantFields: nil,
+			wantErr:    true,
+		},
+		{
+			name: "bad-discovery-with-all-static-endpoints",
+			args: args{
+				issuer:     "https://127.0.0.1:65535",
+				authorizer: nil,
+				options: []Option{
+					WithStaticEndpoints(
+						"https://some.host/token-url",
+						"https://some.host/instrospect-url",
+					),
+				},
+			},
+			wantFields: &wantFields{
+				issuer:        "https://127.0.0.1:65535",
+				tokenURL:      "https://some.host/token-url",
+				introspectURL: "https://some.host/instrospect-url",
+				authFn:        nil,
+			},
+			wantErr: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := newResourceServer(tt.args.issuer, tt.args.authorizer, tt.args.options...)
+			if tt.wantErr {
+				assert.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+			if tt.wantFields == nil {
+				return
+			}
+			assert.Equal(t, tt.wantFields.issuer, got.issuer)
+			assert.Equal(t, tt.wantFields.tokenURL, got.tokenURL)
+			assert.Equal(t, tt.wantFields.introspectURL, got.introspectURL)
+		})
+	}
+}
+
+func TestIntrospect(t *testing.T) {
+	type args struct {
+		ctx   context.Context
+		rp    ResourceServer
+		token string
+	}
+	rp, err := newResourceServer(
+		"https://accounts.spotify.com",
+		nil,
+	)
+	require.NoError(t, err)
+	tests := []struct {
+		name    string
+		args    args
+		wantErr bool
+	}{
+		{
+			name: "missing-introspect-url",
+			args: args{
+				ctx:   nil,
+				rp:    rp,
+				token: "my-token",
+			},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := Introspect(tt.args.ctx, tt.args.rp, tt.args.token)
+			if tt.wantErr {
+				assert.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+		})
+	}
+}