Add OIDC middleware #2138

meher745 · 2025-08-06T14:05:06Z

This PR introduces comprehensive OpenID Connect (OIDC) support into the GoFr framework by adding:

OIDC Discovery Metadata Fetching & Caching (discovery.go)
Implements dynamic fetching and caching of OIDC provider metadata (including issuer, JWKS URI, and userinfo endpoint) from the standard .well-known/openid-configuration URL.

OIDC Userinfo Middleware (oidc.go)
Middleware to fetch user profile information from the OIDC userinfo endpoint after OAuth JWT token validation. This middleware injects the retrieved user info into the request context for downstream handlers to use.

Dedicated Tests (oidc_test.go, discovery_test.go)
Thorough test coverage for both userinfo middleware and discovery metadata fetching utilities, covering success scenarios, error handling, caching behavior, and edge cases.

Documentation (docs/advanced-guide/oidc.md)
Detailed usage guide explaining how to configure and use the new OIDC features alongside GoFr’s built-in OAuth middleware. Includes examples for discovery, middleware registration, and handler access to user info.

Implementation Highlights

discovery.go:

Fetches OIDC metadata from the provider’s discovery URL.
Caches metadata with configurable expiration to minimize network calls.

oidc.go:

Middleware extracts the validated Bearer token (via Authorization header).
Calls the provider’s userinfo endpoint with the token and parses JSON user details.
Injects user info into the Go standard context.Context, retrievable by the helper GetOIDCUserInfo.

Usage Flow:

Fetch discovery metadata at app startup.
Pass discovered JWKS URI and issuer to app.EnableOAuth.
Register the OIDC userinfo middleware after OAuth.
Access user profile info in route handlers via context helper.

Testing

All new tests pass:

oidc_test.go covers userinfo middleware success, token absence, userinfo endpoint failures, and JSON parsing errors.
discovery_test.go covers metadata fetching success, caching, HTTP errors, JSON errors, and network failures.

Existing package tests:

All existing and new OIDC-related tests pass locally.
There is one unrelated failing legacy test (TestGetJwtClaims) from the GoFr core package that does NOT concern this PR and does not affect the OIDC middleware functionality.

Thanks for considering this contribution!

…fo support - Implement OIDC userinfo middleware to fetch and inject user profile - Add discovery helper to fetch OIDC provider metadata dynamically - Include comprehensive tests for userinfo middleware and discovery - Add detailed docs on usage and integration

meher745 · 2025-08-06T14:06:05Z

Hi, @Umang01-hash @coolwednesday . Please review this PR. Thank you so much!

pkg/gofr/http/middleware/oidc.go

ccoVeille · 2025-08-07T12:09:46Z

pkg/gofr/http/middleware/discovery.go

+        return cachedMeta, nil
+    }
+
+    resp, err := http.Get(discoveryURL)


And http.Get without passing a contexf doesn't sound a good idea

pkg/gofr/http/middleware/discovery.go

- Use strings.CutPrefix for token extraction - DiscoveryCache struct with per-URL cache/mutex - HTTP requests now use context - Updated tests for middleware and discovery - Updated docs for new usage patterns

meher745 · 2025-08-10T11:44:44Z

Hi @ccoVeille @Umang01-hash @coolwednesday . I've updated the files according to the recommendations. Please review.

ccoVeille

Sound good to me, a minor remark

ccoVeille · 2025-08-12T10:53:58Z

pkg/gofr/version/version.go

 package version

-const Framework = "dev"
+const Framework = "v1.43.0"


Why this change?

Hi, this change was not intentional. It seems it got included in my PR because I accidentally staged all changes using git add . while preparing my commit. Please let me know if you’d like me to remove it. @ccoVeille

@meher745 Please revert back this change.

meher745 · 2025-08-20T16:38:54Z

@ccoVeille , kindly merge the pull request. Thanks!

ccoVeille · 2025-08-20T16:41:37Z

I'm an active code reviewer of gofr. But I'm not a maintainer. I cannot merge.

Let's wait for a maintainer feedback

@coolwednesday @Umang01-hash

akshat-kumar-singhal · 2025-08-22T05:16:36Z

@meher745 There's a lot of redundancy in your code right now - consider using the existing Auth Middleware which accepts AuthProvider.
cc @Umang01-hash @coolwednesday

Umang01-hash

@meher745 Here are a few review suggestions for your PR:

Instead of using dynamic errors (fmt.Errorf()) please use predefined errors.
For the documentation i see we can improve on how we are portraying this feature to user, the code included should be well formatted, include proper spaces and comments. Remove repeteted things and big technical jagrons let's try to keep it simple so that evry user can understand it. Also the documentation file is directly placed under docs/advanced-guide which is not correct. We need to follow the exact same way other documentations are following.
Please resolve all the linter and code quality errors in your code.

Umang01-hash · 2025-08-25T06:44:40Z

pkg/gofr/version/version.go

 package version

-const Framework = "dev"
+const Framework = "v1.43.0"


@meher745 Please revert back this change.

Umang01-hash · 2025-08-25T07:37:07Z

pkg/gofr/http/middleware/oidc.go

@@ -0,0 +1,76 @@
+// File: pkg/gofr/http/middleware/oidc.go
+
+package middleware


To integrate OIDC with the existing AuthProvider based AuthMiddleware, you can implement a new OIDCAuthProvider struct in oidc.go that satisfies the AuthProvider interface. This allows you to reuse the unified authentication flow for OIDC, just like Basic, API key, and OAuth. Simply extract (& validate if needed) the Bearer token in ExtractAuthHeader, and return user info or claims as needed. Then, pass your provider to AuthMiddleware for consistent middleware usage.

Umang01-hash · 2025-09-02T07:09:52Z

@meher745 Are you still working in this PR?

meher745 · 2025-09-02T07:23:48Z

@meher745 Are you still working in this PR?

Yes I am. I am almost done with the desired changes.

meher745 · 2025-09-05T04:22:49Z

Hi, @Umang01-hash @coolwednesday please let me know if the recent commits I made are fine. I'm kind of confused since I had to remove go.work.sum and go.sum, and build them again due to some import conflicts.

meher745 · 2025-09-19T08:51:27Z

@Umang01-hash @coolwednesday Please review, thanks.

…middleware

meher745 · 2025-09-20T10:08:55Z

@ccoVeille @Umang01-hash @coolwednesday . Kindly review. Thanking you in advance!

ccoVeille · 2025-09-20T13:32:01Z

go.mod

I don't get the version downgrade here

ccoVeille · 2025-09-20T13:33:50Z

pkg/gofr/http/middleware/discovery.go

+func NewDiscoveryCache(url string, cacheDuration time.Duration) *DiscoveryCache {
+	return &DiscoveryCache{
+		url:           url,
+		cacheDuration: cacheDuration,
+	}
+}


url variable name would shadow url package

ccoVeille · 2025-09-20T13:35:18Z

pkg/gofr/http/middleware/discovery.go

+
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, dc.url, http.NoBody)
+	if err != nil {
+		return nil, ErrFailedCreateDiscoveryRequest


Here and everywhere

I would use errors.Join or fmt.Errorf("%w: %w") to keep the err in the stack

ccoVeille · 2025-09-20T13:42:09Z

pkg/gofr/http/middleware/discovery.go

+	defer resp.Body.Close()
+
+	if resp.StatusCode != http.StatusOK {
+		return nil, ErrBadDiscoveryStatus


Returning the status code in the error is always interesting

Suggested change

return nil, ErrBadDiscoveryStatus

return nil, fmt.Errorf("%w: unexpected status code: %d", ErrBadDiscoveryStatus, resp.StatusCode)

ccoVeille · 2025-09-20T14:12:35Z

pkg/gofr/http/middleware/oidc.go

+	defer resp.Body.Close()
+
+	if resp.StatusCode != http.StatusOK {
+		return nil, ErrUserInfoBadStatus


Same here about returning the status code

akshat-kumar-singhal · 2025-09-21T06:27:08Z

pkg/gofr/http/middleware/oidc.go

+		return nil, ErrEmptyToken
+	}
+
+	req, err := http.NewRequestWithContext(r.Context(), http.MethodGet, p.UserInfoEndpoint, http.NoBody)


Consider injecting the HTTP client instead of creating a new one. Also, use existing constant for 'Authorization'.

akshat-kumar-singhal · 2025-09-21T06:28:09Z

pkg/gofr/http/middleware/oidc.go

+	authHeader := r.Header.Get("Authorization")
+
+	token, ok := strings.CutPrefix(authHeader, "Bearer ")
+	if !ok {
+		return nil, ErrMissingToken
+	}
+
+	token = strings.TrimSpace(token)
+	if token == "" {
+		return nil, ErrEmptyToken
+	}


this is present in "getAuthHeaderFromRequest()" auth.go

akshat-kumar-singhal · 2025-09-21T06:29:28Z

pkg/gofr/http/middleware/oidc.go

+	ErrMissingToken      = errors.New("missing bearer token")
+	ErrEmptyToken        = errors.New("empty bearer token")
+	ErrCreateRequest     = errors.New("failed to create userinfo request")
+	ErrUserInfoFetch     = errors.New("failed to fetch userinfo")
+	ErrUserInfoBadStatus = errors.New("userinfo endpoint returned error status")
+	ErrUserInfoJSON      = errors.New("failed to parse userinfo response")
+)


avoid duplicating errors - some of these are already present

Umang01-hash · 2025-10-06T05:24:54Z

Hey @meher745 !

There are a few review comments on your PR that still need to be addressed. Just checking in — are you currently working on this? If so, let’s aim to wrap it up soon, as it’s been pending for a while and we’re quite close to completing it.

If you need any help or clarification, feel free to reach out — happy to support however I can.

Looking forward to your update!

meher745 · 2025-10-07T21:41:07Z

Hi @Umang01-hash . I have gove through the suggestions and started working on them. I apologise for the delay since I had my semester exams going on. Will get back to you with updated pr soon! Thanks in advance.

Umang01-hash · 2025-10-08T05:03:59Z

Hi @meher745,

Thanks for the update — no worries at all! Looking forward to your updated PR.

ccoVeille reviewed Aug 7, 2025

View reviewed changes

refactor: update OIDC middleware & discovery per review feedback

6f16546

- Use strings.CutPrefix for token extraction - DiscoveryCache struct with per-URL cache/mutex - HTTP requests now use context - Updated tests for middleware and discovery - Updated docs for new usage patterns

Merge branch 'development' into add-oidc-middleware

5955516

ccoVeille approved these changes Aug 12, 2025

View reviewed changes

Merge branch 'development' into add-oidc-middleware

9f29991

coolwednesday added the gofr-soc label Aug 17, 2025

Umang01-hash requested changes Aug 25, 2025

View reviewed changes

Refactor OIDC code, fix linter issues, add documentation

19458af

meher745 force-pushed the add-oidc-middleware branch from 3e04bb5 to 19458af Compare September 5, 2025 03:27

Merge branch 'development' of github.com:meher745/gofr into add-oidc-…

1babe5c

…middleware

ccoVeille reviewed Sep 20, 2025

View reviewed changes

akshat-kumar-singhal suggested changes Sep 21, 2025

View reviewed changes

		@@ -0,0 +1,76 @@
		// File: pkg/gofr/http/middleware/oidc.go

		package middleware

	return nil, ErrBadDiscoveryStatus
	return nil, fmt.Errorf("%w: unexpected status code: %d", ErrBadDiscoveryStatus, resp.StatusCode)

Add OIDC middleware #2138

Are you sure you want to change the base?

Add OIDC middleware #2138

Uh oh!

Conversation

meher745 commented Aug 6, 2025

This PR introduces comprehensive OpenID Connect (OIDC) support into the GoFr framework by adding:

Implementation Highlights

Testing

Uh oh!

meher745 commented Aug 6, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

meher745 commented Aug 10, 2025

Uh oh!

ccoVeille left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

meher745 commented Aug 20, 2025

Uh oh!

ccoVeille commented Aug 20, 2025

Uh oh!

akshat-kumar-singhal commented Aug 22, 2025

Uh oh!

Umang01-hash left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Umang01-hash commented Sep 2, 2025

Uh oh!

meher745 commented Sep 2, 2025

Uh oh!

meher745 commented Sep 5, 2025

Uh oh!

meher745 commented Sep 19, 2025

Uh oh!

meher745 commented Sep 20, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Umang01-hash commented Oct 6, 2025

Uh oh!

meher745 commented Oct 7, 2025

Uh oh!

Umang01-hash commented Oct 8, 2025

Uh oh!

Uh oh!

Umang01-hash left a comment •

edited

Loading