Repository: zitadel/oidc
Pull request #598 (2024-04-30 11:28:18)

feat(op): authorize callback handler as argument in legacy server registration
This change requires an additional argument to the op.RegisterLegacyServer constructor which passes the Authorize Callback Handler.
This allows implementations to use their own handler instead of the one provided by the package.
The current handler is exported for legacy behavior.

This change is not considered breaking, as RegisterLegacyServer is flagged experimental.

Related to https://github.com/zitadel/zitadel/issues/6882

### Definition of Ready

- [x] I am happy with the code
- [x] Short description of the feature/issue is added in the pr description
- [x] PR is linked to the corresponding user story
- [x] Acceptance criteria are met
- [x] All open todos and follow ups are defined in a new ticket and justified
- [x] Deviations from the acceptance criteria and design are agreed with the PO and documented.
- [x] No debug or dead code
- [x] My code has no repetitions
- [x] Critical parts are tested automatically
- [x] Where possible E2E tests are implemented
- [x] Documentation/examples are up-to-date
- [x] All non-functional requirements are met
- [x] Functionality of the acceptance criteria is checked manually on the dev system.




Patch:
diff --git a/example/server/exampleop/op.go b/example/server/exampleop/op.go
index e502536..e8ef892 100644
--- a/example/server/exampleop/op.go
+++ b/example/server/exampleop/op.go
@@ -80,7 +80,7 @@ func SetupServer(issuer string, storage Storage, logger *slog.Logger, wrapServer
 
 	handler := http.Handler(provider)
 	if wrapServer {
-		handler = op.RegisterLegacyServer(op.NewLegacyServer(provider, *op.DefaultEndpoints))
+		handler = op.RegisterLegacyServer(op.NewLegacyServer(provider, *op.DefaultEndpoints), op.AuthorizeCallbackHandler(provider))
 	}
 
 	// we register the http handler of the OP on the root, so that the discovery endpoint (/.well-known/openid-configuration)
diff --git a/pkg/op/auth_request.go b/pkg/op/auth_request.go
index 4b5837a..923b9a7 100644
--- a/pkg/op/auth_request.go
+++ b/pkg/op/auth_request.go
@@ -61,7 +61,7 @@ func authorizeHandler(authorizer Authorizer) func(http.ResponseWriter, *http.Req
 	}
 }
 
-func authorizeCallbackHandler(authorizer Authorizer) func(http.ResponseWriter, *http.Request) {
+func AuthorizeCallbackHandler(authorizer Authorizer) func(http.ResponseWriter, *http.Request) {
 	return func(w http.ResponseWriter, r *http.Request) {
 		AuthorizeCallback(w, r, authorizer)
 	}
diff --git a/pkg/op/op.go b/pkg/op/op.go
index 9fd6b30..61c2449 100644
--- a/pkg/op/op.go
+++ b/pkg/op/op.go
@@ -135,7 +135,7 @@ func CreateRouter(o OpenIDProvider, interceptors ...HttpInterceptor) chi.Router
 	router.HandleFunc(readinessEndpoint, readyHandler(o.Probes()))
 	router.HandleFunc(oidc.DiscoveryEndpoint, discoveryHandler(o, o.Storage()))
 	router.HandleFunc(o.AuthorizationEndpoint().Relative(), authorizeHandler(o))
-	router.HandleFunc(authCallbackPath(o), authorizeCallbackHandler(o))
+	router.HandleFunc(authCallbackPath(o), AuthorizeCallbackHandler(o))
 	router.HandleFunc(o.TokenEndpoint().Relative(), tokenHandler(o))
 	router.HandleFunc(o.IntrospectionEndpoint().Relative(), introspectionHandler(o))
 	router.HandleFunc(o.UserinfoEndpoint().Relative(), userinfoHandler(o))
diff --git a/pkg/op/server_legacy.go b/pkg/op/server_legacy.go
index 6b6d4b3..126fde1 100644
--- a/pkg/op/server_legacy.go
+++ b/pkg/op/server_legacy.go
@@ -22,17 +22,16 @@ type ExtendedLegacyServer interface {
 }
 
 // RegisterLegacyServer registers a [LegacyServer] or an extension thereof.
-// It takes care of registering the IssuerFromRequest middleware
-// and Authorization Callback Routes.
+// It takes care of registering the IssuerFromRequest middleware.
+// The authorizeCallbackHandler is registered on `/callback` under the authorization endpoint.
 // Neither are part of the bare [Server] interface.
 //
 // EXPERIMENTAL: may change until v4
-func RegisterLegacyServer(s ExtendedLegacyServer, options ...ServerOption) http.Handler {
-	provider := s.Provider()
+func RegisterLegacyServer(s ExtendedLegacyServer, authorizeCallbackHandler http.HandlerFunc, options ...ServerOption) http.Handler {
 	options = append(options,
-		WithHTTPMiddleware(intercept(provider.IssuerFromRequest)),
+		WithHTTPMiddleware(intercept(s.Provider().IssuerFromRequest)),
 		WithSetRouter(func(r chi.Router) {
-			r.HandleFunc(s.Endpoints().Authorization.Relative()+authCallbackPathSuffix, authorizeCallbackHandler(provider))
+			r.HandleFunc(s.Endpoints().Authorization.Relative()+authCallbackPathSuffix, authorizeCallbackHandler)
 		}),
 	)
 	return RegisterServer(s, s.Endpoints(), options...)


Test patch:
diff --git a/pkg/op/server_http_routes_test.go b/pkg/op/server_http_routes_test.go
index 8b3fa02..2c83ad3 100644
--- a/pkg/op/server_http_routes_test.go
+++ b/pkg/op/server_http_routes_test.go
@@ -32,7 +32,7 @@ func jwtProfile() (string, error) {
 }
 
 func TestServerRoutes(t *testing.T) {
-	server := op.RegisterLegacyServer(op.NewLegacyServer(testProvider, *op.DefaultEndpoints))
+	server := op.RegisterLegacyServer(op.NewLegacyServer(testProvider, *op.DefaultEndpoints), op.AuthorizeCallbackHandler(testProvider))
 
 	storage := testProvider.Storage().(routesTestStorage)
 	ctx := op.ContextWithIssuer(context.Background(), testIssuer)