From 1934efa84c24c7da07ffbd4ddc5b3e1df9f1e807 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Fran=C3=A7ois=20Roy?= Date: Sun, 2 Aug 2026 08:05:39 -0700 Subject: [PATCH] feat: implement OAuth Client ID Metadata Document (#1525) (#1526) Co-authored-by: Elias Schneider --- .github/workflows/unit-tests.yml | 17 + .gitignore | 3 + backend/go.mod | 2 +- backend/internal/appconfig/model.go | 4 + backend/internal/appconfig/service.go | 32 + backend/internal/appconfig/service_test.go | 50 ++ .../internal/bootstrap/router_bootstrap.go | 4 +- .../internal/bootstrap/services_bootstrap.go | 7 +- backend/internal/common/errors.go | 9 + .../controller/app_config_controller.go | 1 - .../internal/controller/oidc_controller.go | 28 +- .../controller/well_known_controller.go | 33 +- .../controller/well_known_controller_test.go | 57 ++ backend/internal/dto/app_config_dto.go | 1 + backend/internal/dto/oidc_dto.go | 1 + .../internal/middleware/client_id_param.go | 56 ++ .../middleware/client_id_param_test.go | 69 ++ backend/internal/model/oidc.go | 35 +- backend/internal/oidc/cimd.go | 236 ++++++ backend/internal/oidc/cimd_test.go | 697 ++++++++++++++++++ backend/internal/oidc/client.go | 19 +- backend/internal/oidc/device_service.go | 1 + backend/internal/oidc/device_service_test.go | 2 +- .../internal/oidc/end_session_service_test.go | 5 +- .../oidc/introspection_handler_test.go | 4 +- backend/internal/oidc/module.go | 28 +- backend/internal/oidc/preview_test.go | 4 +- backend/internal/oidc/provider.go | 3 +- backend/internal/oidc/provider_test.go | 33 +- backend/internal/oidc/store.go | 187 ++++- backend/internal/oidc/token_handler_test.go | 12 +- .../internal/oidc/userinfo_handler_test.go | 2 +- backend/internal/service/e2etest_service.go | 18 +- backend/internal/service/oidc_service.go | 123 +++- backend/internal/service/oidc_service_test.go | 132 +++- backend/internal/utils/callback_url_util.go | 40 +- .../internal/utils/callback_url_util_test.go | 62 ++ backend/internal/utils/ip_util.go | 7 + backend/internal/utils/testing/actors.go | 54 +- ..._oidc_client_id_metadata_document.down.sql | 3 + ...00_oidc_client_id_metadata_document.up.sql | 6 + ..._oidc_client_id_metadata_document.down.sql | 9 + ...00_oidc_client_id_metadata_document.up.sql | 12 + frontend/messages/en.json | 13 + .../lib/components/confirm-dialog/index.ts | 2 +- .../lib/components/form/url-list-input.svelte | 55 ++ .../ui/card/card-description.svelte | 4 +- frontend/src/lib/services/apis-service.ts | 5 +- frontend/src/lib/services/oidc-service.ts | 40 +- .../types/application-configuration.type.ts | 2 + frontend/src/lib/types/oidc.type.ts | 5 +- frontend/src/lib/utils/cached-image-util.ts | 12 +- frontend/src/lib/utils/client-id-util.ts | 49 ++ frontend/src/routes/device/+page.svelte | 7 +- frontend/src/routes/interaction/+page.svelte | 9 +- .../src/routes/login/alternative/+page.svelte | 2 +- .../application-configuration/+page.svelte | 16 + .../app-config-dynamic-clients-form.svelte | 44 ++ .../settings/admin/oidc-clients/+page.svelte | 10 +- .../admin/oidc-clients/[id]/+page.svelte | 10 + .../settings/admin/oidc-clients/[id]/+page.ts | 6 +- .../federated-identities-input.svelte | 18 +- .../oidc-callback-url-input.svelte | 40 +- .../oidc-clients/oidc-client-form.svelte | 8 +- .../oidc-clients/oidc-client-list.svelte | 36 +- .../apps/authorized-oidc-client-card.svelte | 4 +- tests/resources/export/database.json | 23 +- 67 files changed, 2311 insertions(+), 217 deletions(-) create mode 100644 backend/internal/controller/well_known_controller_test.go create mode 100644 backend/internal/middleware/client_id_param.go create mode 100644 backend/internal/middleware/client_id_param_test.go create mode 100644 backend/internal/oidc/cimd.go create mode 100644 backend/internal/oidc/cimd_test.go create mode 100644 backend/resources/migrations/postgres/20260728120000_oidc_client_id_metadata_document.down.sql create mode 100644 backend/resources/migrations/postgres/20260728120000_oidc_client_id_metadata_document.up.sql create mode 100644 backend/resources/migrations/sqlite/20260728120000_oidc_client_id_metadata_document.down.sql create mode 100644 backend/resources/migrations/sqlite/20260728120000_oidc_client_id_metadata_document.up.sql create mode 100644 frontend/src/lib/components/form/url-list-input.svelte create mode 100644 frontend/src/lib/utils/client-id-util.ts create mode 100644 frontend/src/routes/settings/admin/application-configuration/forms/app-config-dynamic-clients-form.svelte diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 330ffe4f..f75f0903 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -37,3 +37,20 @@ jobs: - name: Run backend unit tests working-directory: backend run: go test "-tags=exclude_frontend,unit" -v ./... + + test-backend-race: + name: Backend (race detector) + runs-on: depot-ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-go@v6 + with: + go-version-file: "backend/go.mod" + cache-dependency-path: "backend/go.sum" + - name: Install dependencies + working-directory: backend + run: | + go get ./... + - name: Run backend unit tests with the race detector + working-directory: backend + run: go test -race "-tags=exclude_frontend,unit" ./... diff --git a/.gitignore b/.gitignore index 951266bd..33dd5e92 100644 --- a/.gitignore +++ b/.gitignore @@ -62,3 +62,6 @@ yarn-error.log* backend/cmd/__debug_* # Added by goreleaser init: dist/ + +/backend/go.work +/backend/go.work.sum \ No newline at end of file diff --git a/backend/go.mod b/backend/go.mod index 353160fb..13bdfee8 100644 --- a/backend/go.mod +++ b/backend/go.mod @@ -40,6 +40,7 @@ require ( github.com/ory/fosite v0.49.1-0.20250703093431-a5f0b09bf31c github.com/oschwald/maxminddb-golang/v2 v2.4.1 github.com/pires/go-proxyproto v0.15.0 + github.com/quic-go/quic-go v0.61.0 github.com/spf13/cobra v1.10.2 github.com/stretchr/testify v1.11.1 github.com/zitadel/exifremove v0.1.0 @@ -181,7 +182,6 @@ require ( github.com/prometheus/otlptranslator v1.0.0 // indirect github.com/prometheus/procfs v0.21.1 // indirect github.com/quic-go/qpack v0.6.0 // indirect - github.com/quic-go/quic-go v0.61.0 // indirect github.com/quic-go/webtransport-go v0.12.0 // indirect github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect github.com/robfig/cron/v3 v3.0.1 // indirect diff --git a/backend/internal/appconfig/model.go b/backend/internal/appconfig/model.go index 47436522..676069c4 100644 --- a/backend/internal/appconfig/model.go +++ b/backend/internal/appconfig/model.go @@ -61,6 +61,8 @@ type AppConfigModel struct { LdapAttributeGroupName AppConfigValue `json:"ldapAttributeGroupName"` LdapAdminGroupName AppConfigValue `json:"ldapAdminGroupName"` LdapSoftDeleteUsers AppConfigValue `json:"ldapSoftDeleteUsers" type:"bool"` + // OIDC + CIMDURLAllowlist AppConfigValue `json:"cimdUrlAllowlist"` // JSON-encoded array of strings } // Clone returns a deep copy of the AppConfigModel. @@ -145,6 +147,8 @@ func getDefaultConfig() *AppConfigModel { LdapAttributeGroupName: "", LdapAdminGroupName: "", LdapSoftDeleteUsers: "true", + // OIDC + CIMDURLAllowlist: "[]", } } diff --git a/backend/internal/appconfig/service.go b/backend/internal/appconfig/service.go index 3eb92f4a..d4c0ed55 100644 --- a/backend/internal/appconfig/service.go +++ b/backend/internal/appconfig/service.go @@ -2,6 +2,7 @@ package appconfig import ( "context" + "encoding/json" "errors" "fmt" "os" @@ -94,6 +95,24 @@ func (s *AppConfigService) GetConfig(parentCtx context.Context) (*AppConfigModel return &cfg, nil } +// GetCIMDURLAllowlist returns the configured CIMD metadata-document URL +// allowlist. Returns an empty slice if unset or malformed (which denies all). +func (s *AppConfigService) GetCIMDURLAllowlist() []string { + cfg, err := s.GetConfig(context.Background()) + if err != nil { + return nil + } + raw := string(cfg.CIMDURLAllowlist) + if raw == "" { + return nil + } + var patterns []string + if err := json.Unmarshal([]byte(raw), &patterns); err != nil { + return nil + } + return patterns +} + // UpdateAppConfig replaces the entire application configuration with the values from the input DTO. func (s *AppConfigService) UpdateAppConfig(ctx context.Context, input dto.AppConfigUpdateDto) ([]AppConfigVariable, error) { // If the UI config is disabled, we cannot continue @@ -101,6 +120,19 @@ func (s *AppConfigService) UpdateAppConfig(ctx context.Context, input dto.AppCon return nil, &common.UiConfigDisabledError{} } + // Validate the CIMD URL allowlist patterns, if provided + if input.CIMDURLAllowlist != "" { + var patterns []string + if err := json.Unmarshal([]byte(input.CIMDURLAllowlist), &patterns); err != nil { + return nil, &common.InvalidCIMDURLPatternError{Pattern: input.CIMDURLAllowlist} + } + for _, p := range patterns { + if err := utils.ValidateCallbackURLPattern(p); err != nil { + return nil, &common.InvalidCIMDURLPatternError{Pattern: p} + } + } + } + // Replace the entire config by invoking the actor cfg, err := s.invokeConfigActor(ctx, "replace", input) if err != nil { diff --git a/backend/internal/appconfig/service_test.go b/backend/internal/appconfig/service_test.go index 1e3b08ad..f06bf7d0 100644 --- a/backend/internal/appconfig/service_test.go +++ b/backend/internal/appconfig/service_test.go @@ -342,3 +342,53 @@ func TestService_ListAppConfig(t *testing.T) { assert.Equal(t, "XXXXXXXXXX", got) }) } + +func TestService_CIMDURLAllowlist(t *testing.T) { + t.Run("defaults to empty", func(t *testing.T) { + setUIConfigDisabled(t, false) + db := testutils.NewDatabaseForTest(t) + svc := newActorBackedService(t, db) + + assert.Empty(t, svc.GetCIMDURLAllowlist()) + }) + + t.Run("round-trips a valid allowlist", func(t *testing.T) { + setUIConfigDisabled(t, false) + db := testutils.NewDatabaseForTest(t) + svc := newActorBackedService(t, db) + + _, err := svc.UpdateAppConfig(t.Context(), dto.AppConfigUpdateDto{ + AppName: "App", + SessionDuration: "60", + CIMDURLAllowlist: `["https://app.example.com/**","https://*.trusted.com/oauth"]`, + }) + require.NoError(t, err) + + assert.Equal(t, + []string{"https://app.example.com/**", "https://*.trusted.com/oauth"}, + svc.GetCIMDURLAllowlist(), + ) + }) + + t.Run("rejects an invalid pattern", func(t *testing.T) { + setUIConfigDisabled(t, false) + db := testutils.NewDatabaseForTest(t) + svc := newActorBackedService(t, db) + + _, err := svc.UpdateAppConfig(t.Context(), dto.AppConfigUpdateDto{ + AppName: "App", + SessionDuration: "60", + CIMDURLAllowlist: `["javascript:alert(1)"]`, + }) + require.Error(t, err) + }) + + t.Run("returns empty on malformed value", func(t *testing.T) { + setUIConfigDisabled(t, false) + db := testutils.NewDatabaseForTest(t) + svc := newActorBackedService(t, db) + + require.NoError(t, svc.UpdateAppConfigValues(t.Context(), "cimdUrlAllowlist", "not-json")) + assert.Empty(t, svc.GetCIMDURLAllowlist()) + }) +} diff --git a/backend/internal/bootstrap/router_bootstrap.go b/backend/internal/bootstrap/router_bootstrap.go index c7d547a3..efe6e091 100644 --- a/backend/internal/bootstrap/router_bootstrap.go +++ b/backend/internal/bootstrap/router_bootstrap.go @@ -146,6 +146,8 @@ func registerRoutes(r *gin.Engine, db *gorm.DB, svc *services, rateLimitServices apiRateLimitMiddleware := rateLimitMiddleware.Add(middleware.RateLimitAPI) apiGroup := r.Group("/api", apiRateLimitMiddleware) + // Decode "~" client ID path params (used for CIMD URL client IDs). + apiGroup.Use(middleware.NewClientIDParamMiddleware().Add()) baseGroup := r.Group("/", apiRateLimitMiddleware) svc.apiKeyModule.RegisterRoutes(apiGroup, @@ -195,7 +197,7 @@ func registerRoutes(r *gin.Engine, db *gorm.DB, svc *services, rateLimitServices registerTestRoutes(apiGroup, db, svc) - controller.NewWellKnownController(baseGroup, svc.jwtService) + controller.NewWellKnownController(baseGroup, svc.jwtService, svc.appConfigService.GetCIMDURLAllowlist) // These are not rate-limited. controller.NewHealthzController(r) diff --git a/backend/internal/bootstrap/services_bootstrap.go b/backend/internal/bootstrap/services_bootstrap.go index 4b8db863..08cddcdc 100644 --- a/backend/internal/bootstrap/services_bootstrap.go +++ b/backend/internal/bootstrap/services_bootstrap.go @@ -116,8 +116,9 @@ func initServices( svc.apiModule = api.New(api.Dependencies{DB: db, Issuer: common.EnvConfig.AppURL}) svc.oidcModule, err = oidc.New(ctx, oidc.Dependencies{ - DB: db, - HTTPClient: httpClient, + DB: db, + HTTPClient: httpClient, + GetCIMDURLAllowlist: svc.appConfigService.GetCIMDURLAllowlist, Config: oidc.Config{ BaseURL: common.EnvConfig.AppURL, TokenBaseURL: common.EnvConfig.AppURL, @@ -134,7 +135,7 @@ func initServices( return nil, fmt.Errorf("failed to create OIDC module: %w", err) } - svc.oidcService, err = service.NewOidcService(db, svc.jwtService, svc.oidcModule.Preview, svc.scimService, httpClient, fileStorage) + svc.oidcService, err = service.NewOidcService(db, svc.jwtService, svc.oidcModule.Preview, svc.oidcModule, svc.scimService, httpClient, fileStorage) if err != nil { return nil, fmt.Errorf("failed to create OIDC service: %w", err) } diff --git a/backend/internal/common/errors.go b/backend/internal/common/errors.go index 9a026431..07f49561 100644 --- a/backend/internal/common/errors.go +++ b/backend/internal/common/errors.go @@ -69,6 +69,15 @@ func (e OidcInvalidCallbackURLError) Error() string { } func (e OidcInvalidCallbackURLError) HttpStatusCode() int { return http.StatusBadRequest } +type InvalidCIMDURLPatternError struct { + Pattern string +} + +func (e InvalidCIMDURLPatternError) Error() string { + return "invalid metadata document URL pattern: " + e.Pattern +} +func (e InvalidCIMDURLPatternError) HttpStatusCode() int { return http.StatusBadRequest } + type FileTypeNotSupportedError struct{} func (e FileTypeNotSupportedError) Error() string { return "file type not supported" } diff --git a/backend/internal/controller/app_config_controller.go b/backend/internal/controller/app_config_controller.go index e0b64f7d..cbb8eaa4 100644 --- a/backend/internal/controller/app_config_controller.go +++ b/backend/internal/controller/app_config_controller.go @@ -82,7 +82,6 @@ func (acc *AppConfigController) listAppConfigHandler(c *gin.Context) { configVariablesDto = append(configVariablesDto, dto.PublicAppConfigVariableDto{ Key: "tracingEnabled", Value: strconv.FormatBool(tracing.FrontendTracingEnabled()), - Type: "boolean", }) c.JSON(http.StatusOK, configVariablesDto) diff --git a/backend/internal/controller/oidc_controller.go b/backend/internal/controller/oidc_controller.go index 6285975f..afbaa99f 100644 --- a/backend/internal/controller/oidc_controller.go +++ b/backend/internal/controller/oidc_controller.go @@ -31,6 +31,7 @@ func NewOidcController(group *gin.RouterGroup, authMiddleware *middleware.AuthMi group.GET("/oidc/clients/:id", authMiddleware.Add(), oc.getClientHandler) group.GET("/oidc/clients/:id/meta", oc.getClientMetaDataHandler) group.PUT("/oidc/clients/:id", authMiddleware.Add(), oc.updateClientHandler) + group.POST("/oidc/clients/:id/refresh", authMiddleware.Add(), oc.refreshClientMetadataHandler) group.DELETE("/oidc/clients/:id", authMiddleware.Add(), oc.deleteClientHandler) group.PUT("/oidc/clients/:id/allowed-user-groups", authMiddleware.Add(), oc.updateAllowedUserGroupsHandler) @@ -76,7 +77,6 @@ func (oc *OidcController) getClientMetaDataHandler(c *gin.Context) { clientDto := dto.OidcClientMetaDataDto{} err = dto.MapStruct(client, &clientDto) if err == nil { - clientDto.HasDarkLogo = client.HasDarkLogo() c.JSON(http.StatusOK, clientDto) return } @@ -139,7 +139,7 @@ func (oc *OidcController) listClientsHandler(c *gin.Context) { _ = c.Error(err) return } - clientDto.HasDarkLogo = client.HasDarkLogo() + clientDto.AllowedUserGroupsCount, err = oc.oidcService.GetAllowedGroupsCountOfClient(c, client.ID) if err != nil { _ = c.Error(err) @@ -234,6 +234,30 @@ func (oc *OidcController) updateClientHandler(c *gin.Context) { c.JSON(http.StatusOK, clientDto) } +// refreshClientMetadataHandler godoc +// @Summary Refresh client metadata document +// @Description Force a re-fetch of the OAuth Client ID Metadata Document for a CIMD client +// @Tags OIDC +// @Produce json +// @Param id path string true "Client ID" +// @Success 200 {object} dto.OidcClientWithAllowedUserGroupsDto "Refreshed client" +// @Router /api/oidc/clients/{id}/refresh [post] +func (oc *OidcController) refreshClientMetadataHandler(c *gin.Context) { + client, err := oc.oidcService.RefreshClientMetadata(c.Request.Context(), c.Param("id")) + if err != nil { + _ = c.Error(err) + return + } + + var clientDto dto.OidcClientWithAllowedUserGroupsDto + if err := dto.MapStruct(client, &clientDto); err != nil { + _ = c.Error(err) + return + } + + c.JSON(http.StatusOK, clientDto) +} + // createClientSecretHandler godoc // @Summary Create client secret // @Description Set or generate a new secret for an OIDC client diff --git a/backend/internal/controller/well_known_controller.go b/backend/internal/controller/well_known_controller.go index cf8c1c84..f92ce4b4 100644 --- a/backend/internal/controller/well_known_controller.go +++ b/backend/internal/controller/well_known_controller.go @@ -3,9 +3,7 @@ package controller import ( "encoding/json" "fmt" - "log/slog" "net/http" - "os" "github.com/gin-gonic/gin" @@ -17,16 +15,10 @@ import ( // @Summary OIDC Discovery controller // @Description Initializes OIDC discovery and JWKS endpoints // @Tags Well Known -func NewWellKnownController(group *gin.RouterGroup, jwtService *service.JwtService) { - wkc := &WellKnownController{jwtService: jwtService} - - // Pre-compute the OIDC configuration document, which is static - var err error - wkc.oidcConfig, err = wkc.computeOIDCConfiguration() - if err != nil { - slog.Error("Failed to pre-compute OpenID Connect configuration document", slog.Any("error", err)) - os.Exit(1) - return +func NewWellKnownController(group *gin.RouterGroup, jwtService *service.JwtService, getCIMDURLAllowlist func() []string) { + wkc := &WellKnownController{ + jwtService: jwtService, + getCIMDURLAllowlist: getCIMDURLAllowlist, } group.GET("/.well-known/jwks.json", wkc.jwksHandler) @@ -34,8 +26,8 @@ func NewWellKnownController(group *gin.RouterGroup, jwtService *service.JwtServi } type WellKnownController struct { - jwtService *service.JwtService - oidcConfig []byte + jwtService *service.JwtService + getCIMDURLAllowlist func() []string } // jwksHandler godoc @@ -62,7 +54,12 @@ func (wkc *WellKnownController) jwksHandler(c *gin.Context) { // @Success 200 {object} object "OpenID Connect configuration" // @Router /.well-known/openid-configuration [get] func (wkc *WellKnownController) openIDConfigurationHandler(c *gin.Context) { - c.Data(http.StatusOK, "application/json; charset=utf-8", wkc.oidcConfig) + oidcConfig, err := wkc.computeOIDCConfiguration() + if err != nil { + _ = c.Error(err) + return + } + c.Data(http.StatusOK, "application/json; charset=utf-8", oidcConfig) } func (wkc *WellKnownController) computeOIDCConfiguration() ([]byte, error) { @@ -74,6 +71,11 @@ func (wkc *WellKnownController) computeOIDCConfiguration() ([]byte, error) { if err != nil { return nil, fmt.Errorf("failed to get key algorithm: %w", err) } + cimdSupported := false + if wkc.getCIMDURLAllowlist != nil { + cimdSupported = len(wkc.getCIMDURLAllowlist()) > 0 + } + config := map[string]any{ "issuer": appUrl, "authorization_endpoint": appUrl + "/authorize", @@ -98,6 +100,7 @@ func (wkc *WellKnownController) computeOIDCConfiguration() ([]byte, error) { "token_endpoint_auth_methods_supported": []string{"client_secret_basic", "client_secret_post", "none"}, "pushed_authorization_request_endpoint": internalAppUrl + "/api/oidc/par", "require_pushed_authorization_requests": false, + "client_id_metadata_document_supported": cimdSupported, } return json.Marshal(config) } diff --git a/backend/internal/controller/well_known_controller_test.go b/backend/internal/controller/well_known_controller_test.go new file mode 100644 index 00000000..fee1d095 --- /dev/null +++ b/backend/internal/controller/well_known_controller_test.go @@ -0,0 +1,57 @@ +package controller + +import ( + "encoding/json" + "testing" + + "github.com/lestrrat-go/jwx/v3/jwa" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/pocket-id/pocket-id/backend/internal/common" + "github.com/pocket-id/pocket-id/backend/internal/service" + jwkutils "github.com/pocket-id/pocket-id/backend/internal/utils/jwk" +) + +func newMinimalJwtService(t *testing.T) *service.JwtService { + t.Helper() + + key, err := jwkutils.GenerateKey(jwa.RS256().String(), "") + require.NoError(t, err, "failed to generate test JWK key") + + svc := &service.JwtService{} + require.NoError(t, svc.SetKey(key), "failed to set JWK key on JwtService") + return svc +} + +func TestClientIDMetadataDocumentDiscoveryFollowsAllowlist(t *testing.T) { + origURL := common.EnvConfig.AppURL + t.Cleanup(func() { + common.EnvConfig.AppURL = origURL + }) + + common.EnvConfig.AppURL = "https://test.example.com" + jwtSvc := newMinimalJwtService(t) + cimdURLAllowlist := []string(nil) + wkc := &WellKnownController{ + jwtService: jwtSvc, + getCIMDURLAllowlist: func() []string { + return cimdURLAllowlist + }, + } + + parse := func(t *testing.T) map[string]any { + t.Helper() + raw, err := wkc.computeOIDCConfiguration() + require.NoError(t, err) + var cfg map[string]any + require.NoError(t, json.Unmarshal(raw, &cfg)) + return cfg + } + + cimdURLAllowlist = []string{"https://client.example.com/**"} + assert.Equal(t, true, parse(t)["client_id_metadata_document_supported"]) + + cimdURLAllowlist = nil + assert.Equal(t, false, parse(t)["client_id_metadata_document_supported"]) +} diff --git a/backend/internal/dto/app_config_dto.go b/backend/internal/dto/app_config_dto.go index 7fbabb9a..a5835edc 100644 --- a/backend/internal/dto/app_config_dto.go +++ b/backend/internal/dto/app_config_dto.go @@ -55,4 +55,5 @@ type AppConfigUpdateDto struct { EmailLoginNotificationEnabled string `json:"emailLoginNotificationEnabled" binding:"required"` EmailApiKeyExpirationEnabled string `json:"emailApiKeyExpirationEnabled" binding:"required"` EmailVerificationEnabled string `json:"emailVerificationEnabled" binding:"required"` + CIMDURLAllowlist string `json:"cimdUrlAllowlist" binding:"omitempty,json"` } diff --git a/backend/internal/dto/oidc_dto.go b/backend/internal/dto/oidc_dto.go index 507364c9..aa0e1a17 100644 --- a/backend/internal/dto/oidc_dto.go +++ b/backend/internal/dto/oidc_dto.go @@ -10,6 +10,7 @@ type OidcClientMetaDataDto struct { HasDarkLogo bool `json:"hasDarkLogo"` LaunchURL *string `json:"launchURL"` RequiresReauthentication bool `json:"requiresReauthentication"` + ClientType string `json:"clientType"` } type OidcClientDto struct { diff --git a/backend/internal/middleware/client_id_param.go b/backend/internal/middleware/client_id_param.go new file mode 100644 index 00000000..db2461db --- /dev/null +++ b/backend/internal/middleware/client_id_param.go @@ -0,0 +1,56 @@ +package middleware + +import ( + "encoding/base64" + "strings" + + "github.com/gin-gonic/gin" +) + +// clientIDParamPrefix marks a path parameter whose value is a base64url-encoded +// client ID. CIMD client IDs are full https URLs, so they contain slashes and +// colons that cannot be carried in a single path segment. The frontend encodes +// such IDs as "~"; this middleware decodes them back before +// handlers read c.Param. +// +// The prefix "~" is unreserved in RFC 3986 (so proxies leave it intact) and never +// appears in raw pocket-id client IDs ([a-zA-Z0-9._-]+) or user UUIDs, making the +// encoding unambiguous and backward compatible: unprefixed params pass through +// untouched, so external API consumers using plain client IDs are unaffected. +const clientIDParamPrefix = "~" + +// decodedClientIDParamKeys lists the path parameter names that may carry an +// encoded client ID. +var decodedClientIDParamKeys = map[string]struct{}{ + "id": {}, + "clientId": {}, +} + +// ClientIDParamMiddleware decodes "~" client ID path parameters in +// place. Values without the prefix, or that fail to decode, are left unchanged. +type ClientIDParamMiddleware struct{} + +func NewClientIDParamMiddleware() *ClientIDParamMiddleware { + return &ClientIDParamMiddleware{} +} + +func (m *ClientIDParamMiddleware) Add() gin.HandlerFunc { + return func(c *gin.Context) { + for i, p := range c.Params { + if _, ok := decodedClientIDParamKeys[p.Key]; !ok { + continue + } + encoded, ok := strings.CutPrefix(p.Value, clientIDParamPrefix) + if !ok { + continue + } + decoded, err := base64.RawURLEncoding.DecodeString(encoded) + if err != nil { + continue + } + c.Params[i].Value = string(decoded) + } + + c.Next() + } +} diff --git a/backend/internal/middleware/client_id_param_test.go b/backend/internal/middleware/client_id_param_test.go new file mode 100644 index 00000000..e5670643 --- /dev/null +++ b/backend/internal/middleware/client_id_param_test.go @@ -0,0 +1,69 @@ +package middleware + +import ( + "encoding/base64" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/require" +) + +func TestClientIDParamMiddleware(t *testing.T) { + gin.SetMode(gin.TestMode) + + const cimdURL = "https://claude.ai/oauth/claude-code-client-metadata" + encoded := "~" + base64.RawURLEncoding.EncodeToString([]byte(cimdURL)) + + tests := []struct { + name string + param string + want string + }{ + {"plain client ID unchanged", "my-client_id.1", "my-client_id.1"}, + {"uuid unchanged", "550e8400-e29b-41d4-a716-446655440000", "550e8400-e29b-41d4-a716-446655440000"}, + {"encoded CIMD URL decoded", encoded, cimdURL}, + {"invalid base64 left as-is", "~!!!", "~!!!"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + router := gin.New() + router.Use(NewClientIDParamMiddleware().Add()) + + var got string + router.GET("/oidc/clients/:id/meta", func(c *gin.Context) { + got = c.Param("id") + c.Status(http.StatusOK) + }) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/oidc/clients/"+tt.param+"/meta", http.NoBody) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + require.Equal(t, http.StatusOK, w.Code) + require.Equal(t, tt.want, got) + }) + } +} + +func TestClientIDParamMiddlewareIgnoresNonClientParams(t *testing.T) { + gin.SetMode(gin.TestMode) + + router := gin.New() + router.Use(NewClientIDParamMiddleware().Add()) + + var got string + // "~"-prefixed value on a non-client param key must pass through untouched. + router.GET("/users/:userId", func(c *gin.Context) { + got = c.Param("userId") + c.Status(http.StatusOK) + }) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/users/~abc", http.NoBody) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + require.Equal(t, "~abc", got) +} diff --git a/backend/internal/model/oidc.go b/backend/internal/model/oidc.go index 5392091c..ae12c320 100644 --- a/backend/internal/model/oidc.go +++ b/backend/internal/model/oidc.go @@ -19,14 +19,22 @@ type UserAuthorizedOidcClient struct { Client OidcClient } +// OidcClientType identifies how an OIDC client was registered. +type OidcClientType string + +const ( + OidcClientTypeStandard OidcClientType = "standard" + OidcClientTypeCIMD OidcClientType = "cimd" +) + type OidcClient struct { Base Name string `sortable:"true"` Description string Secret string - CallbackURLs UrlList - LogoutCallbackURLs UrlList + CallbackURLs datatype.StringList + LogoutCallbackURLs datatype.StringList ImageType *string DarkImageType *string IsPublic bool @@ -36,8 +44,11 @@ type OidcClient struct { SkipConsent bool `sortable:"true" filterable:"true"` Credentials OidcClientCredentials LaunchURL *string - IsGroupRestricted bool `sortable:"true" filterable:"true"` - PkceSupported bool `sortable:"true" filterable:"true"` + IsGroupRestricted bool `sortable:"true" filterable:"true"` + PkceSupported bool `sortable:"true" filterable:"true"` + ClientType OidcClientType `gorm:"default:standard" sortable:"true" filterable:"true"` + MetadataExpiresAt *datatype.DateTime + MetadataGrantTypes datatype.StringList AllowedUserGroups []UserGroup `gorm:"many2many:oidc_clients_allowed_user_groups;"` CreatedByID *string @@ -53,6 +64,12 @@ func (c OidcClient) HasDarkLogo() bool { return c.DarkImageType != nil && *c.DarkImageType != "" } +// IsMetadataDocument reports whether the client was synthesized from an OAuth +// Client ID Metadata Document. Its ID is then the https URL of the document. +func (c OidcClient) IsMetadataDocument() bool { + return c.ClientType == OidcClientTypeCIMD +} + type OidcClientCredentials struct { //nolint:recvcheck FederatedIdentities []OidcClientFederatedIdentity `json:"federatedIdentities,omitempty"` } @@ -86,13 +103,3 @@ func (occ *OidcClientCredentials) Scan(value any) error { func (occ OidcClientCredentials) Value() (driver.Value, error) { return json.Marshal(occ) } - -type UrlList []string //nolint:recvcheck - -func (cu *UrlList) Scan(value any) error { - return utils.UnmarshalJSONFromDatabase(cu, value) -} - -func (cu UrlList) Value() (driver.Value, error) { - return json.Marshal(cu) -} diff --git a/backend/internal/oidc/cimd.go b/backend/internal/oidc/cimd.go new file mode 100644 index 00000000..a4560bff --- /dev/null +++ b/backend/internal/oidc/cimd.go @@ -0,0 +1,236 @@ +package oidc + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/url" + "strings" + + "github.com/ory/fosite" + + "github.com/pocket-id/pocket-id/backend/internal/model" + datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" + "github.com/pocket-id/pocket-id/backend/internal/utils" +) + +type cimdResolverConfig struct { + getURLAllowlist func() []string + transport http.RoundTripper + transportDecorator func(http.RoundTripper) http.RoundTripper +} + +type cimdClientResolver struct { + resolver *fosite.CIMDResolver + store *Store + policy cimdPolicy +} + +var _ fosite.ClientResolver = (*cimdClientResolver)(nil) +var _ fosite.CIMDClientPolicy = cimdPolicy{} + +func newCIMDClientResolver(store *Store, config cimdResolverConfig) *cimdClientResolver { + options := []fosite.CIMDFetcherOption{ + fosite.WithCIMDUserAgent("pocket-id/oidc-client-metadata-fetcher"), + fosite.WithCIMDExtraPrivateRanges(utils.LocalIPv6IPNets()), + } + if config.transport != nil { + options = append(options, fosite.WithCIMDTransport(config.transport)) + } + if config.transportDecorator != nil { + options = append(options, fosite.WithCIMDTransportDecorator(config.transportDecorator)) + } + + policy := cimdPolicy{getURLAllowlist: config.getURLAllowlist} + return &cimdClientResolver{ + resolver: &fosite.CIMDResolver{ + Fetcher: fosite.NewDefaultCIMDFetcher(options...), + Cache: store, + Materializer: store, + Policy: policy, + MaxConcurrentDiscoveries: 10, + }, + store: store, + policy: policy, + } +} + +func (r *cimdClientResolver) ResolveClient(ctx context.Context, clientID string, next fosite.ClientLookupFunc) (fosite.Client, error) { + if next == nil { + return nil, errors.New("registered client resolver is required") + } + + // Exclude persisted metadata clients so Fosite can apply its cache policy while still giving real registrations precedence + registeredOnly := func(ctx context.Context, clientID string) (fosite.Client, error) { + client, err := next(ctx, clientID) + if err != nil { + return nil, err + } + if pocketIDClient, ok := client.(Client); ok && pocketIDClient.IsMetadataDocument() { + return nil, fosite.ErrNotFound + } + return client, nil + } + return r.resolver.ResolveClient(ctx, clientID, registeredOnly) +} + +// RefreshMetadataClient forces a re-fetch of the metadata document for an already-cached CIMD client, bypassing the cache TTL +func (r *cimdClientResolver) RefreshMetadataClient(ctx context.Context, id string) (model.OidcClient, error) { + if !fosite.LooksLikeCIMDURL(id) { + return model.OidcClient{}, errors.New("client is not a client ID metadata document client") + } + if err := r.policy.AllowCIMDClient(ctx, id); err != nil { + return model.OidcClient{}, err + } + existing, err := r.store.firstClientByID(ctx, id) + if err != nil { + return model.OidcClient{}, err + } + if !existing.IsMetadataDocument() { + return model.OidcClient{}, errors.New("client is not a client ID metadata document client") + } + client, err := r.resolver.RefreshClient(ctx, id) + if err != nil { + return model.OidcClient{}, err + } + pocketIDClient, ok := client.(Client) + if !ok { + return model.OidcClient{}, errors.New("metadata resolver returned an incompatible client") + } + return pocketIDClient.OidcClient, nil +} + +type cimdPolicy struct { + getURLAllowlist func() []string +} + +func (p cimdPolicy) cimdURLAllowed(id string) bool { + if p.getURLAllowlist == nil { + return false + } + return utils.MatchesAnyURLPattern(p.getURLAllowlist(), id) +} + +// AllowCIMDClient applies Pocket ID's operator-managed dynamic-client allowlist +func (p cimdPolicy) AllowCIMDClient(_ context.Context, id string) error { + if !p.cimdURLAllowed(id) { + return errors.New("client ID is not in the metadata document allowlist") + } + return nil +} + +// ValidateCIMDClient restricts generic CIMD features to those supported by Pocket ID's client model +func (cimdPolicy) ValidateCIMDClient(_ context.Context, doc *fosite.ClientMetadataDocument) error { + // Require public-client authentication because Pocket ID does not persist CIMD key material + switch doc.TokenEndpointAuthMethod { + case "none": + default: + return fmt.Errorf("client metadata documents only support token_endpoint_auth_method %q, got %q", "none", doc.TokenEndpointAuthMethod) + } + + // Restrict metadata clients to grant types implemented by Pocket ID and require a flow that can initiate authorization + grantTypes := doc.GrantTypes + if len(grantTypes) == 0 { + grantTypes = []string{string(fosite.GrantTypeAuthorizationCode)} + } + hasInitiatingGrant := false + for _, grantType := range grantTypes { + switch grantType { + case string(fosite.GrantTypeAuthorizationCode), string(fosite.GrantTypeDeviceCode): + hasInitiatingGrant = true + case string(fosite.GrantTypeRefreshToken): + default: + return fmt.Errorf("client metadata document contains unsupported grant_type %q", grantType) + } + } + if !hasInitiatingGrant { + return errors.New("client metadata document must enable authorization_code or device_code") + } + + // Pocket ID only implements the code response type for metadata clients + responseTypes := doc.ResponseTypes + if len(responseTypes) == 0 { + responseTypes = []string{"code"} + } + for _, responseType := range responseTypes { + if responseType != "code" { + return fmt.Errorf("client metadata document contains unsupported response_type %q", responseType) + } + } + + return nil +} + +// validateMetadataRedirectURIs rejects self-asserted redirect URIs Pocket ID must not accept +func validateMetadataRedirectURIs(field string, uris []string) error { + for _, raw := range uris { + if strings.Contains(raw, "*") { + return fmt.Errorf("%s entry %q must not contain a wildcard", field, raw) + } + u, err := url.Parse(raw) + if err != nil { + return fmt.Errorf("%s entry %q is not a valid URL: %w", field, raw, err) + } + if !u.IsAbs() { + return fmt.Errorf("%s entry %q must be an absolute URL", field, raw) + } + // Mirrors the scheme restriction every administrator-registered callback URL passes + switch strings.ToLower(u.Scheme) { + case "javascript", "data": + return fmt.Errorf("%s entry %q uses a disallowed scheme", field, raw) + } + } + return nil +} + +// buildClientFromMetadata applies Pocket ID's persisted-client projection to validated generic metadata +func buildClientFromMetadata(doc *fosite.ClientMetadataDocument, rawURL string) (model.OidcClient, error) { + if err := validateMetadataRedirectURIs("redirect_uris", doc.RedirectURIs); err != nil { + return model.OidcClient{}, err + } + if err := validateMetadataRedirectURIs("post_logout_redirect_uris", doc.PostLogoutRedirectURIs); err != nil { + return model.OidcClient{}, err + } + + // Record what the document says the client restricts itself to, so it is not silently granted capabilities it never declared + // RFC 7591 section 2 defaults an omitted grant_types to authorization_code + grantTypes := doc.GrantTypes + if len(grantTypes) == 0 { + grantTypes = []string{"authorization_code"} + } + + client := model.OidcClient{ + Base: model.Base{ID: rawURL}, + Name: doc.ClientName, + CallbackURLs: datatype.StringList(doc.RedirectURIs), + LogoutCallbackURLs: datatype.StringList(doc.PostLogoutRedirectURIs), + ClientType: model.OidcClientTypeCIMD, + MetadataGrantTypes: datatype.StringList(grantTypes), + } + + switch doc.TokenEndpointAuthMethod { + case "none": + client.IsPublic = true + client.PkceEnabled = true + default: + return model.OidcClient{}, fmt.Errorf("client metadata documents only support token_endpoint_auth_method %q, got %q", "none", doc.TokenEndpointAuthMethod) + } + + if client.Name == "" { + if u, err := url.Parse(rawURL); err == nil { + client.Name = u.Host + } + } + + return client, nil +} + +// MaterializeCIMDClient converts validated generic metadata into Pocket ID's runtime client +func (s *Store) MaterializeCIMDClient(_ context.Context, doc *fosite.ClientMetadataDocument) (fosite.Client, error) { + client, err := buildClientFromMetadata(doc, doc.ClientID) + if err != nil { + return nil, err + } + return Client{OidcClient: client}, nil +} diff --git a/backend/internal/oidc/cimd_test.go b/backend/internal/oidc/cimd_test.go new file mode 100644 index 00000000..d6638ff5 --- /dev/null +++ b/backend/internal/oidc/cimd_test.go @@ -0,0 +1,697 @@ +package oidc + +import ( + "context" + "errors" + "net/http" + "strings" + "testing" + "time" + + "github.com/ory/fosite" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/gorm" + + "github.com/pocket-id/pocket-id/backend/internal/model" + datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" + testutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing" +) + +func TestBuildClientFromMetadata(t *testing.T) { + const id = "https://app.example.com/oauth/client" + + t.Run("public client maps to PKCE", func(t *testing.T) { + doc := &fosite.ClientMetadataDocument{ + ClientID: id, + ClientName: "Example App", + RedirectURIs: []string{"https://app.example.com/callback"}, + PostLogoutRedirectURIs: []string{"https://app.example.com/logout"}, + TokenEndpointAuthMethod: "none", + } + c, err := buildClientFromMetadata(doc, id) + require.NoError(t, err) + assert.Equal(t, id, c.ID) + assert.Equal(t, "Example App", c.Name) + assert.True(t, c.IsPublic) + assert.True(t, c.PkceEnabled) + assert.True(t, c.IsMetadataDocument()) + assert.Equal(t, []string{"https://app.example.com/callback"}, []string(c.CallbackURLs)) + assert.Equal(t, []string{"https://app.example.com/logout"}, []string(c.LogoutCallbackURLs)) + assert.Equal(t, []string{"authorization_code"}, []string(c.MetadataGrantTypes)) + assert.Empty(t, c.Credentials.FederatedIdentities) + }) + + t.Run("authenticated clients are rejected", func(t *testing.T) { + for _, m := range []string{"private_key_jwt", "client_secret_basic", "client_secret_post", "client_secret_jwt"} { //nolint:gosec // G101 false positive: authentication method names, not credentials + doc := &fosite.ClientMetadataDocument{ClientID: id, TokenEndpointAuthMethod: m} + _, err := buildClientFromMetadata(doc, id) + require.Errorf(t, err, "method %q", m) + } + }) + + t.Run("name falls back to the client ID host", func(t *testing.T) { + c, err := buildClientFromMetadata(&fosite.ClientMetadataDocument{ClientID: id, TokenEndpointAuthMethod: "none"}, id) + require.NoError(t, err) + assert.Equal(t, "app.example.com", c.Name) + }) +} + +func TestCIMDPolicyValidate(t *testing.T) { + policy := cimdPolicy{} + + for _, test := range []struct { + name string + grantTypes []string + responseTypes []string + wantError string + }{ + {name: "defaults are supported"}, + {name: "authorization code and refresh token are supported", grantTypes: []string{"authorization_code", "refresh_token"}}, + {name: "device code is supported", grantTypes: []string{string(fosite.GrantTypeDeviceCode)}}, + {name: "client credentials is rejected", grantTypes: []string{"client_credentials"}, wantError: "unsupported grant_type"}, + {name: "refresh token cannot initiate authorization", grantTypes: []string{"refresh_token"}, wantError: "must enable"}, + {name: "implicit response is rejected", grantTypes: []string{"authorization_code"}, responseTypes: []string{"token"}, wantError: "unsupported response_type"}, + } { + t.Run(test.name, func(t *testing.T) { + err := policy.ValidateCIMDClient(t.Context(), &fosite.ClientMetadataDocument{ + TokenEndpointAuthMethod: "none", + GrantTypes: test.grantTypes, + ResponseTypes: test.responseTypes, + }) + if test.wantError == "" { + require.NoError(t, err) + return + } + require.ErrorContains(t, err, test.wantError) + }) + } + + t.Run("omitted authentication method is rejected", func(t *testing.T) { + err := policy.ValidateCIMDClient(t.Context(), &fosite.ClientMetadataDocument{}) + require.ErrorContains(t, err, "token_endpoint_auth_method") + }) +} + +func TestMetadataClientChanges(t *testing.T) { + base := model.OidcClient{ + Name: "App", + CallbackURLs: datatype.StringList{"https://app/cb"}, + LogoutCallbackURLs: datatype.StringList{"https://app/lo"}, + IsPublic: true, + } + + t.Run("no changes", func(t *testing.T) { + assert.Empty(t, metadataClientChanges(base, base)) + }) + + t.Run("redirect_uris change", func(t *testing.T) { + next := base + next.CallbackURLs = datatype.StringList{"https://app/other"} + assert.Equal(t, []string{"redirect_uris"}, metadataClientChanges(base, next)) + }) + + t.Run("auth method change", func(t *testing.T) { + next := base + next.IsPublic = false + got := metadataClientChanges(base, next) + assert.Contains(t, got, "token_endpoint_auth_method") + }) + + t.Run("grant types change", func(t *testing.T) { + next := base + next.MetadataGrantTypes = datatype.StringList{"authorization_code", "refresh_token"} + assert.Contains(t, metadataClientChanges(base, next), "grant_types") + }) +} + +func TestRefreshMetadataClient(t *testing.T) { + const id = "https://8.8.8.8/oauth/client" + body := `{"client_id":"https://8.8.8.8/oauth/client","client_name":"App","redirect_uris":["https://app/cb"],"token_endpoint_auth_method":"none"}` + + t.Run("empty allowlist", func(t *testing.T) { + s := newMetadataStore(t, nil, func() []string { return nil }) + _, err := s.RefreshMetadataClient(t.Context(), id) + require.Error(t, err) + }) + + t.Run("non-URL id", func(t *testing.T) { + s := newMetadataStore(t, nil) + _, err := s.RefreshMetadataClient(t.Context(), "not-a-url") + require.Error(t, err) + }) + + t.Run("unknown client yields not found", func(t *testing.T) { + s := newMetadataStore(t, nil) + _, err := s.RefreshMetadataClient(t.Context(), id) + require.ErrorIs(t, err, gorm.ErrRecordNotFound) + }) + + t.Run("non-metadata client is rejected", func(t *testing.T) { + s := newMetadataStore(t, nil) + seed := model.OidcClient{Base: model.Base{ID: id}, Name: "Standard"} + require.NoError(t, s.db.Create(&seed).Error) + _, err := s.RefreshMetadataClient(t.Context(), id) + require.Error(t, err) + require.NotErrorIs(t, err, gorm.ErrRecordNotFound) + }) + + t.Run("forces re-fetch even when cache is fresh", func(t *testing.T) { + resp := testutils.NewMockResponse(http.StatusOK, body) //nolint:bodyclose // mock response, no real body + s := newMetadataStore(t, map[string]*http.Response{id: resp}) + + fresh := datatype.DateTime(time.Now().Add(time.Hour)) + seed := model.OidcClient{Base: model.Base{ID: id}, Name: "Old", IsPublic: true, PkceEnabled: true, ClientType: model.OidcClientTypeCIMD, MetadataExpiresAt: &fresh} + require.NoError(t, s.db.Create(&seed).Error) + + // A normal lookup still returns the cached value. + fc, err := s.GetClient(t.Context(), id) + require.NoError(t, err) + assert.Equal(t, "Old", fc.(Client).Name) + + // A forced refresh re-fetches and updates the cached client. + c, err := s.RefreshMetadataClient(t.Context(), id) + require.NoError(t, err) + assert.Equal(t, "App", c.Name) + assert.True(t, c.IsMetadataDocument()) + }) +} + +type metadataStore struct { + *Store + resolver *cimdClientResolver +} + +func (s *metadataStore) GetClient(ctx context.Context, id string) (fosite.Client, error) { + client, err := s.resolver.ResolveClient(ctx, id, s.Store.GetClient) + if err == nil { + return client, nil + } + if errors.Is(err, fosite.ErrNotFound) { + return nil, fosite.ErrNotFound + } + if fosite.LooksLikeCIMDURL(id) { + return nil, fosite.ErrInvalidClient.WithHint("The client metadata document could not be resolved.").WithWrap(err).WithDebug(err.Error()) + } + return nil, err +} + +func (s *metadataStore) RefreshMetadataClient(ctx context.Context, id string) (model.OidcClient, error) { + return s.resolver.RefreshMetadataClient(ctx, id) +} + +func newMetadataStore(t *testing.T, responses map[string]*http.Response, allowlists ...func() []string) *metadataStore { + t.Helper() + getAllowlist := func() []string { return []string{"*"} } + if len(allowlists) > 0 { + getAllowlist = allowlists[0] + } + store := NewStore(testutils.NewDatabaseForTest(t), nil) + return &metadataStore{ + Store: store, + resolver: newCIMDClientResolver(store, cimdResolverConfig{ + getURLAllowlist: getAllowlist, + transport: &testutils.MockRoundTripper{Responses: responses}, + }), + } +} + +func TestGetClient_CIMDURLAllowlist(t *testing.T) { + const id = "https://8.8.8.8/oauth/client" + body := `{"client_id":"https://8.8.8.8/oauth/client","client_name":"App","redirect_uris":["https://app/cb"],"token_endpoint_auth_method":"none"}` + + t.Run("empty allowlist denies without fetching", func(t *testing.T) { + resp := testutils.NewMockResponse(http.StatusOK, body) //nolint:bodyclose // mock response, no real body + s := newMetadataStore(t, map[string]*http.Response{id: resp}, + func() []string { return nil }, + ) + _, err := s.GetClient(t.Context(), id) + require.ErrorIs(t, err, fosite.ErrInvalidClient) + + var count int64 + require.NoError(t, s.db.Model(&model.OidcClient{}).Where("id = ?", id).Count(&count).Error) + assert.Equal(t, int64(0), count) + }) + + t.Run("non-matching allowlist denies", func(t *testing.T) { + resp := testutils.NewMockResponse(http.StatusOK, body) //nolint:bodyclose // mock response, no real body + s := newMetadataStore(t, map[string]*http.Response{id: resp}, + func() []string { return []string{"https://other.example.com/**"} }, + ) + _, err := s.GetClient(t.Context(), id) + require.ErrorIs(t, err, fosite.ErrInvalidClient) + }) + + t.Run("matching allowlist allows", func(t *testing.T) { + resp := testutils.NewMockResponse(http.StatusOK, body) //nolint:bodyclose // mock response, no real body + s := newMetadataStore(t, map[string]*http.Response{id: resp}, + func() []string { return []string{"https://8.8.8.8/**"} }, + ) + fc, err := s.GetClient(t.Context(), id) + require.NoError(t, err) + assert.Equal(t, id, fc.(Client).ID) + }) + + t.Run("refresh denied when not allowlisted", func(t *testing.T) { + resp := testutils.NewMockResponse(http.StatusOK, body) //nolint:bodyclose // mock response, no real body + s := newMetadataStore(t, map[string]*http.Response{id: resp}, + func() []string { return nil }, + ) + _, err := s.RefreshMetadataClient(t.Context(), id) + require.Error(t, err) + }) +} + +func TestGetClient_MetadataDocument(t *testing.T) { + const id = "https://8.8.8.8/oauth/client" + body := `{"client_id":"https://8.8.8.8/oauth/client","client_name":"App","redirect_uris":["https://app/cb"],"token_endpoint_auth_method":"none"}` + + t.Run("non-URL id falls through to the database", func(t *testing.T) { + s := newMetadataStore(t, nil) + _, err := s.GetClient(t.Context(), "does-not-exist") + require.ErrorIs(t, err, fosite.ErrNotFound) + }) + + t.Run("allowlist changes apply without rebuilding the store", func(t *testing.T) { + allowlist := []string(nil) + resp := testutils.NewMockResponse(http.StatusOK, body) //nolint:bodyclose // mock response, no real body + s := newMetadataStore(t, map[string]*http.Response{id: resp}, + func() []string { return allowlist }, + ) + + _, err := s.GetClient(t.Context(), id) + require.ErrorIs(t, err, fosite.ErrInvalidClient) + + allowlist = []string{"https://8.8.8.8/**"} + fc, err := s.GetClient(t.Context(), id) + require.NoError(t, err) + assert.Equal(t, id, fc.(Client).ID) + + allowlist = nil + _, err = s.GetClient(t.Context(), id) + require.ErrorIs(t, err, fosite.ErrInvalidClient) + }) + + t.Run("pre-registered URL client takes precedence", func(t *testing.T) { + resp := testutils.NewMockResponse(http.StatusOK, body) //nolint:bodyclose // mock response, no real body + s := newMetadataStore(t, map[string]*http.Response{id: resp}, + func() []string { return nil }, + ) + seed := model.OidcClient{Base: model.Base{ID: id}, Name: "Standard"} + require.NoError(t, s.db.Create(&seed).Error) + + resolved, err := s.GetClient(t.Context(), id) + require.NoError(t, err) + assert.Equal(t, "Standard", resolved.(Client).Name) + + stored, err := s.firstClientByID(t.Context(), id) + require.NoError(t, err) + assert.Equal(t, "Standard", stored.Name) + assert.False(t, stored.IsMetadataDocument()) + }) + + t.Run("no-store metadata is rejected and not persisted", func(t *testing.T) { + resp := testutils.NewMockResponse(http.StatusOK, body) //nolint:bodyclose // mock response, no real body + resp.Header.Set("Cache-Control", "no-store") + s := newMetadataStore(t, map[string]*http.Response{id: resp}) + + _, err := s.GetClient(t.Context(), id) + require.ErrorIs(t, err, fosite.ErrInvalidClient) + + var count int64 + require.NoError(t, s.db.Model(&model.OidcClient{}).Where("id = ?", id).Count(&count).Error) + assert.Zero(t, count) + }) + + t.Run("private_key_jwt metadata is rejected and not persisted", func(t *testing.T) { + privateKeyBody := `{"client_id":"https://8.8.8.8/oauth/client","redirect_uris":["https://app/cb"],"token_endpoint_auth_method":"private_key_jwt","jwks_uri":"https://8.8.4.4/jwks"}` //nolint:gosec // G101 false positive: authentication method name, not a credential + resp := testutils.NewMockResponse(http.StatusOK, privateKeyBody) //nolint:bodyclose // mock response, no real body + s := newMetadataStore(t, map[string]*http.Response{id: resp}) + + _, err := s.GetClient(t.Context(), id) + require.ErrorIs(t, err, fosite.ErrInvalidClient) + + var count int64 + require.NoError(t, s.db.Model(&model.OidcClient{}).Where("id = ?", id).Count(&count).Error) + assert.Zero(t, count) + }) + + t.Run("cached authenticated metadata client is rejected", func(t *testing.T) { + s := newMetadataStore(t, nil) + fresh := datatype.DateTime(time.Now().Add(time.Hour)) + seed := model.OidcClient{ + Base: model.Base{ID: id}, + ClientType: model.OidcClientTypeCIMD, + MetadataExpiresAt: &fresh, + Credentials: model.OidcClientCredentials{FederatedIdentities: []model.OidcClientFederatedIdentity{{ + Issuer: id, + JWKS: "https://8.8.4.4/jwks", + }}}, + } + require.NoError(t, s.db.Create(&seed).Error) + + _, err := s.GetClient(t.Context(), id) + require.ErrorIs(t, err, fosite.ErrInvalidClient) + }) + + t.Run("cached authenticated metadata client is replaced when the document becomes public", func(t *testing.T) { + resp := testutils.NewMockResponse(http.StatusOK, body) //nolint:bodyclose // mock response, no real body + s := newMetadataStore(t, map[string]*http.Response{id: resp}) + fresh := datatype.DateTime(time.Now().Add(time.Hour)) + seed := model.OidcClient{ + Base: model.Base{ID: id}, + ClientType: model.OidcClientTypeCIMD, + MetadataExpiresAt: &fresh, + Credentials: model.OidcClientCredentials{FederatedIdentities: []model.OidcClientFederatedIdentity{{ + Issuer: id, + JWKS: "https://8.8.4.4/jwks", + }}}, + } + require.NoError(t, s.db.Create(&seed).Error) + + resolved, err := s.GetClient(t.Context(), id) + require.NoError(t, err) + client := resolved.(Client) + assert.True(t, client.OidcClient.IsPublic) + assert.True(t, client.PkceEnabled) + assert.Empty(t, client.Credentials.FederatedIdentities) + }) + + t.Run("fetches, upserts, and reuses the cache", func(t *testing.T) { + resp := testutils.NewMockResponse(http.StatusOK, body) //nolint:bodyclose // mock response, no real body + resp.Header.Set("Cache-Control", "max-age=600") + s := newMetadataStore(t, map[string]*http.Response{id: resp}) + + fc, err := s.GetClient(t.Context(), id) + require.NoError(t, err) + c := fc.(Client).OidcClient + assert.Equal(t, id, c.ID) + assert.True(t, c.IsMetadataDocument()) + assert.True(t, c.IsPublic) + require.NotNil(t, c.MetadataExpiresAt) + + var count int64 + require.NoError(t, s.db.Model(&model.OidcClient{}).Where("id = ?", id).Count(&count).Error) + assert.Equal(t, int64(1), count) + + fc2, err := s.GetClient(t.Context(), id) + require.NoError(t, err) + assert.Equal(t, "App", fc2.(Client).Name) + }) + + // A display-only change must not cost the user their consent + t.Run("refetch when stale preserves consent", func(t *testing.T) { + resp := testutils.NewMockResponse(http.StatusOK, body) //nolint:bodyclose // mock response, no real body + s := newMetadataStore(t, map[string]*http.Response{id: resp}) + + stale := datatype.DateTime(time.Now().Add(-time.Hour)) + seed := model.OidcClient{ + Base: model.Base{ID: id}, Name: "Old", IsPublic: true, PkceEnabled: true, + ClientType: model.OidcClientTypeCIMD, + CallbackURLs: datatype.StringList{"https://app/cb"}, + MetadataExpiresAt: &stale, + } + require.NoError(t, s.db.Create(&seed).Error) + require.NoError(t, s.db.Exec( + "INSERT INTO user_authorized_oidc_clients (client_id, user_id, scope, last_used_at) VALUES (?, ?, ?, ?)", + id, "user-1", "openid", time.Now()).Error) + + fc, err := s.GetClient(t.Context(), id) + require.NoError(t, err) + assert.Equal(t, "App", fc.(Client).Name) + + var consent int64 + require.NoError(t, s.db.Table("user_authorized_oidc_clients"). + Where("client_id = ?", id).Count(&consent).Error) + assert.Equal(t, int64(1), consent) + }) + + // Whoever controls the document could otherwise repoint an already-consented user's authorization code at a URL of their choosing, with no prompt + t.Run("changed redirect_uris revoke consent", func(t *testing.T) { + resp := testutils.NewMockResponse(http.StatusOK, body) //nolint:bodyclose // mock response, no real body + s := newMetadataStore(t, map[string]*http.Response{id: resp}) + + stale := datatype.DateTime(time.Now().Add(-time.Hour)) + seed := model.OidcClient{ + Base: model.Base{ID: id}, Name: "App", IsPublic: true, PkceEnabled: true, + ClientType: model.OidcClientTypeCIMD, + CallbackURLs: datatype.StringList{"https://app/previous-cb"}, + MetadataExpiresAt: &stale, + } + require.NoError(t, s.db.Create(&seed).Error) + require.NoError(t, s.db.Exec( + "INSERT INTO user_authorized_oidc_clients (client_id, user_id, scope, last_used_at) VALUES (?, ?, ?, ?)", + id, "user-1", "openid", time.Now()).Error) + + _, err := s.GetClient(t.Context(), id) + require.NoError(t, err) + + var consent int64 + require.NoError(t, s.db.Table("user_authorized_oidc_clients"). + Where("client_id = ?", id).Count(&consent).Error) + assert.Zero(t, consent, "consent must not survive a redirect_uris change") + }) + + t.Run("failed consent revocation rolls back refreshed metadata", func(t *testing.T) { + resp := testutils.NewMockResponse(http.StatusOK, body) //nolint:bodyclose // mock response, no real body + s := newMetadataStore(t, map[string]*http.Response{id: resp}) + + stale := datatype.DateTime(time.Now().Add(-time.Hour)) + seed := model.OidcClient{ + Base: model.Base{ID: id}, Name: "App", IsPublic: true, PkceEnabled: true, + ClientType: model.OidcClientTypeCIMD, + CallbackURLs: datatype.StringList{"https://app/previous-cb"}, + MetadataExpiresAt: &stale, + } + require.NoError(t, s.db.Create(&seed).Error) + require.NoError(t, s.db.Exec( + "INSERT INTO user_authorized_oidc_clients (client_id, user_id, scope, last_used_at) VALUES (?, ?, ?, ?)", + id, "user-1", "openid", time.Now()).Error) + require.NoError(t, s.db.Exec(` + CREATE TRIGGER reject_cimd_consent_delete + BEFORE DELETE ON user_authorized_oidc_clients + BEGIN + SELECT RAISE(ABORT, 'consent deletion blocked'); + END; + `).Error) + + _, err := s.GetClient(t.Context(), id) + require.Error(t, err) + + stored, err := s.firstClientByID(t.Context(), id) + require.NoError(t, err) + assert.Equal(t, []string{"https://app/previous-cb"}, []string(stored.CallbackURLs)) + var consent int64 + require.NoError(t, s.db.Table("user_authorized_oidc_clients").Where("client_id = ?", id).Count(&consent).Error) + assert.Equal(t, int64(1), consent) + }) +} + +// Pocket ID matches administrator-registered callback URLs as wildcard patterns, so a self-asserted "*" would otherwise match every redirect URI in existence +func TestBuildClientFromMetadata_RejectsPatternRedirectURIs(t *testing.T) { + const id = "https://app.example.com/oauth/client" + + unsupportedByPocketID := []string{ + "*", + "https://*.example.com/cb", + "javascript:alert(1)", + } + for _, uri := range unsupportedByPocketID { + t.Run("redirect_uris "+uri, func(t *testing.T) { + doc := &fosite.ClientMetadataDocument{ + ClientID: id, + RedirectURIs: []string{uri}, + TokenEndpointAuthMethod: "none", + } + _, err := buildClientFromMetadata(doc, id) + require.Error(t, err) + }) + } + + // post_logout_redirect_uris go through the same validation + doc := &fosite.ClientMetadataDocument{ + ClientID: id, + RedirectURIs: []string{"https://app.example.com/cb"}, + PostLogoutRedirectURIs: []string{"*"}, + TokenEndpointAuthMethod: "none", + } + _, err := buildClientFromMetadata(doc, id) + require.Error(t, err) +} + +func TestMatchRedirectURI_MetadataClientExactMatchSucceeds(t *testing.T) { + metadataClient := Client{OidcClient: model.OidcClient{ + Base: model.Base{ID: "https://app.example.com/oauth/client"}, + ClientType: model.OidcClientTypeCIMD, + CallbackURLs: datatype.StringList{"https://app.example.com/callback"}, + }} + + matched, err := matchRedirectURI("https://app.example.com/callback", metadataClient) + require.NoError(t, err) + require.NotNil(t, matched) + assert.Equal(t, "https://app.example.com/callback", matched.String()) +} + +// A document declaring only authorization_code must not silently receive refresh_token and device_code +func TestClient_DeclaredCapabilitiesAreEnforced(t *testing.T) { + t.Run("grant types are restricted to the declaration", func(t *testing.T) { + client := Client{OidcClient: model.OidcClient{ + ClientType: model.OidcClientTypeCIMD, + IsPublic: true, + MetadataGrantTypes: datatype.StringList{"authorization_code"}, + }} + assert.Equal(t, fosite.Arguments{"authorization_code"}, client.GetGrantTypes()) + }) + + t.Run("declared refresh_token is honoured", func(t *testing.T) { + client := Client{OidcClient: model.OidcClient{ + ClientType: model.OidcClientTypeCIMD, + IsPublic: true, + MetadataGrantTypes: datatype.StringList{"authorization_code", "refresh_token"}, + }} + assert.Equal(t, fosite.Arguments{"authorization_code", "refresh_token"}, client.GetGrantTypes()) + }) + + t.Run("an empty declaration uses the RFC default", func(t *testing.T) { + client := Client{OidcClient: model.OidcClient{ClientType: model.OidcClientTypeCIMD, IsPublic: true}} + assert.Equal(t, fosite.Arguments{"authorization_code"}, client.GetGrantTypes()) + }) + + t.Run("registered clients are unaffected", func(t *testing.T) { + client := Client{OidcClient: model.OidcClient{ + ClientType: model.OidcClientTypeStandard, + IsPublic: true, + MetadataGrantTypes: datatype.StringList{"authorization_code"}, + }} + assert.Contains(t, client.GetGrantTypes(), "refresh_token") + }) +} + +func TestBuildClientFromMetadata_RecordsDeclaredCapabilities(t *testing.T) { + const id = "https://app.example.com/oauth/client" + + t.Run("declared values are recorded", func(t *testing.T) { + doc := &fosite.ClientMetadataDocument{ + ClientID: id, + RedirectURIs: []string{"https://app.example.com/cb"}, + TokenEndpointAuthMethod: "none", + GrantTypes: []string{"authorization_code", "refresh_token"}, + } + client, err := buildClientFromMetadata(doc, id) + require.NoError(t, err) + assert.Equal(t, datatype.StringList{"authorization_code", "refresh_token"}, client.MetadataGrantTypes) + }) + + t.Run("omitted grant_types defaults to authorization_code", func(t *testing.T) { + doc := &fosite.ClientMetadataDocument{ + ClientID: id, + RedirectURIs: []string{"https://app.example.com/cb"}, + TokenEndpointAuthMethod: "none", + } + client, err := buildClientFromMetadata(doc, id) + require.NoError(t, err) + assert.Equal(t, datatype.StringList{"authorization_code"}, client.MetadataGrantTypes) + }) +} + +// The allowlist is the operator's only gate on which URLs may become clients, and it is matched with the same wildcard syntax as callback URLs +func TestCIMDURLAllowlist_HostilePatterns(t *testing.T) { + const id = "https://8.8.8.8/oauth/client" + body := `{"client_id":"https://8.8.8.8/oauth/client","client_name":"App","redirect_uris":["https://app/cb"],"token_endpoint_auth_method":"none"}` + + denied := []struct { + name string + allowlist []string + }{ + {"empty list denies", nil}, + {"different host denies", []string{"https://other.example.com/**"}}, + {"different scheme denies", []string{"http://8.8.8.8/**"}}, + {"host as a path segment denies", []string{"https://evil.example/8.8.8.8/**"}}, + {"prefix of the host denies", []string{"https://8.8.8.8.evil.example/**"}}, + } + for _, tc := range denied { + t.Run(tc.name, func(t *testing.T) { + resp := testutils.NewMockResponse(http.StatusOK, body) //nolint:bodyclose // mock response, no real body + s := newMetadataStore(t, map[string]*http.Response{id: resp}, func() []string { return tc.allowlist }) + _, err := s.GetClient(t.Context(), id) + require.ErrorIs(t, err, fosite.ErrInvalidClient) + + var count int64 + require.NoError(t, s.db.Model(&model.OidcClient{}).Where("id = ?", id).Count(&count).Error) + assert.Zero(t, count, "a denied client must never be persisted") + }) + } + + // "*" matches everything, which is the fully open configuration and the operator's choice rather than a bug + t.Run("bare wildcard allows everything", func(t *testing.T) { + resp := testutils.NewMockResponse(http.StatusOK, body) //nolint:bodyclose // mock response, no real body + s := newMetadataStore(t, map[string]*http.Response{id: resp}, func() []string { return []string{"*"} }) + _, err := s.GetClient(t.Context(), id) + require.NoError(t, err) + }) +} + +// Section 3 constrains the Client Identifier URL, and none of these vectors may reach a fetch +func TestGetClient_HostileClientIDURLs(t *testing.T) { + hostile := []string{ + "http://8.8.8.8/oauth/client", // not https + "https://8.8.8.8", // no path component + "https://user:pass@8.8.8.8/oauth/client", // userinfo + "https://8.8.8.8/oauth/client#frag", // fragment + "https://8.8.8.8/oauth/client?x=1", // query component + "https://8.8.8.8/oauth/../client", // dot segments + "https://127.0.0.1/oauth/client", // loopback + "https://169.254.169.254/latest/meta-data", // cloud metadata + } + + for _, id := range hostile { + t.Run(id, func(t *testing.T) { + s := newMetadataStore(t, nil) + _, err := s.GetClient(t.Context(), id) + require.Error(t, err, "must not be accepted as a Client Identifier URL") + + var count int64 + require.NoError(t, s.db.Model(&model.OidcClient{}).Where("id = ?", id).Count(&count).Error) + assert.Zero(t, count) + }) + } +} + +// Section 5.2 forbids caching error responses or invalid documents, and section 5 requires every non-200 status to be treated as an error +func TestGetClient_MetadataFailuresAreNotPersisted(t *testing.T) { + const id = "https://8.8.8.8/oauth/client" + + cases := []struct { + name string + response *http.Response + }{ + {"404", testutils.NewMockResponse(http.StatusNotFound, `{}`)}, //nolint:bodyclose // mock + {"500", testutils.NewMockResponse(http.StatusInternalServerError, `{}`)}, //nolint:bodyclose // mock + {"204", testutils.NewMockResponse(http.StatusNoContent, ``)}, //nolint:bodyclose // mock + {"invalid JSON", testutils.NewMockResponse(http.StatusOK, `{not json`)}, //nolint:bodyclose // mock + {"truncated JSON", testutils.NewMockResponse(http.StatusOK, `{"client_id":`)}, //nolint:bodyclose // mock + {"empty body", testutils.NewMockResponse(http.StatusOK, ``)}, //nolint:bodyclose // mock + {"null body", testutils.NewMockResponse(http.StatusOK, `null`)}, //nolint:bodyclose // mock + {"client_id mismatch", testutils.NewMockResponse(http.StatusOK, `{"client_id":"https://evil/x"}`)}, //nolint:bodyclose // mock + {"wrong client_id type", testutils.NewMockResponse(http.StatusOK, `{"client_id":123}`)}, //nolint:bodyclose // mock + {"oversize document", testutils.NewMockResponse(http.StatusOK, `{"client_id":"https://8.8.8.8/oauth/client","padding":"`+strings.Repeat("a", 6*1024)+`"}`)}, //nolint:bodyclose // mock + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + s := newMetadataStore(t, map[string]*http.Response{id: tc.response}) + _, err := s.GetClient(t.Context(), id) + require.Error(t, err) + + var count int64 + require.NoError(t, s.db.Model(&model.OidcClient{}).Where("id = ?", id).Count(&count).Error) + assert.Zero(t, count, "a failed or invalid document must never be cached") + }) + } + + t.Run("no response at all", func(t *testing.T) { + s := newMetadataStore(t, nil) + _, err := s.GetClient(t.Context(), id) + require.Error(t, err) + }) +} diff --git a/backend/internal/oidc/client.go b/backend/internal/oidc/client.go index 69627785..181734e5 100644 --- a/backend/internal/oidc/client.go +++ b/backend/internal/oidc/client.go @@ -1,6 +1,8 @@ package oidc import ( + "slices" + "github.com/ory/fosite" "github.com/pocket-id/pocket-id/backend/internal/model" ) @@ -33,7 +35,22 @@ func (c Client) GetGrantTypes() fosite.Arguments { if !c.IsPublic() { grantTypes = append(grantTypes, string(fosite.GrantTypeClientCredentials)) } - return grantTypes + + if !c.IsMetadataDocument() { + return grantTypes + } + if len(c.MetadataGrantTypes) == 0 { + return fosite.Arguments{string(fosite.GrantTypeAuthorizationCode)} + } + + // If the client is a CIMD client, we need to filter the grant types based on the metadata document. + allowed := make(fosite.Arguments, 0, len(c.MetadataGrantTypes)) + for _, value := range c.MetadataGrantTypes { + if slices.Contains([]string(grantTypes), value) { + allowed = append(allowed, value) + } + } + return allowed } func (c Client) GetResponseTypes() fosite.Arguments { diff --git a/backend/internal/oidc/device_service.go b/backend/internal/oidc/device_service.go index b530e17d..43888fef 100644 --- a/backend/internal/oidc/device_service.go +++ b/backend/internal/oidc/device_service.go @@ -212,6 +212,7 @@ func (s *deviceService) getDeviceCodeInfo(ctx context.Context, userCode, userID HasDarkLogo: client.HasDarkLogo(), LaunchURL: client.LaunchURL, RequiresReauthentication: client.RequiresReauthentication, + ClientType: string(client.ClientType), }, Scope: scope, ScopeInfo: scopeInfo, diff --git a/backend/internal/oidc/device_service_test.go b/backend/internal/oidc/device_service_test.go index d329d5a5..f93de187 100644 --- a/backend/internal/oidc/device_service_test.go +++ b/backend/internal/oidc/device_service_test.go @@ -157,7 +157,7 @@ func newTestDeviceService(t *testing.T, clientID, userID string, requiresReauthe BaseURL: "https://issuer.example.com", TokenBaseURL: "https://issuer.example.com", Secret: []byte("test-secret"), - }) + }, nil) require.NoError(t, err) claimsService := newClaimsService(db, nil, "", nil) diff --git a/backend/internal/oidc/end_session_service_test.go b/backend/internal/oidc/end_session_service_test.go index 7c05c954..cdb8d664 100644 --- a/backend/internal/oidc/end_session_service_test.go +++ b/backend/internal/oidc/end_session_service_test.go @@ -16,6 +16,7 @@ import ( "github.com/pocket-id/pocket-id/backend/internal/common" "github.com/pocket-id/pocket-id/backend/internal/dto" "github.com/pocket-id/pocket-id/backend/internal/model" + datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" testutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing" ) @@ -25,7 +26,7 @@ import ( // would be an open redirect. func TestLogoutCallbackURL(t *testing.T) { noURLs := &model.OidcClient{Base: model.Base{ID: "c"}} - withURLs := &model.OidcClient{Base: model.Base{ID: "c"}, LogoutCallbackURLs: model.UrlList{ + withURLs := &model.OidcClient{Base: model.Base{ID: "c"}, LogoutCallbackURLs: datatype.StringList{ "https://app.example/logout", "https://app.example/logout2", "https://*.example/logout", @@ -130,7 +131,7 @@ func TestEndSessionService(t *testing.T) { require.NoError(t, db.Create(&model.OidcClient{ Base: model.Base{ID: clientID}, Name: "Test Client", - LogoutCallbackURLs: model.UrlList{"https://app.example/logout"}, + LogoutCallbackURLs: datatype.StringList{"https://app.example/logout"}, }).Error) require.NoError(t, db.Create(&model.User{Base: model.Base{ID: userID}, Username: "tim"}).Error) require.NoError(t, db.Create(&model.UserAuthorizedOidcClient{UserID: userID, ClientID: clientID}).Error) diff --git a/backend/internal/oidc/introspection_handler_test.go b/backend/internal/oidc/introspection_handler_test.go index 585bce12..abc8ac3c 100644 --- a/backend/internal/oidc/introspection_handler_test.go +++ b/backend/internal/oidc/introspection_handler_test.go @@ -42,7 +42,7 @@ func TestIntrospectionHandlerBindsTokenToCallerClient(t *testing.T) { BaseURL: "https://issuer.example.com", TokenBaseURL: "https://issuer.example.com", Secret: []byte("test-secret"), - }) + }, nil) require.NoError(t, err) issueAccessToken := func(t *testing.T, requestID, clientID, subject string) string { @@ -146,7 +146,7 @@ func TestIntrospectionHandlerAllowsReusedFederatedClientAssertion(t *testing.T) BaseURL: baseURL, TokenBaseURL: baseURL, Secret: []byte("test-secret"), - }) + }, nil) require.NoError(t, err) session := NewEmptySession() diff --git a/backend/internal/oidc/module.go b/backend/internal/oidc/module.go index 9a4a5b52..8703a837 100644 --- a/backend/internal/oidc/module.go +++ b/backend/internal/oidc/module.go @@ -9,6 +9,7 @@ import ( "github.com/gin-gonic/gin" "github.com/lestrrat-go/jwx/v3/jwa" "github.com/pocket-id/pocket-id/backend/internal/model" + "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "gorm.io/gorm" ) @@ -42,6 +43,8 @@ type Dependencies struct { Config Config HTTPClient *http.Client + GetCIMDURLAllowlist func() []string + Signer TokenSigner CustomClaims CustomClaimSource Reauth ReauthenticationTokenConsumer @@ -52,8 +55,9 @@ type Dependencies struct { type Module struct { Preview *ClientPreviewBuilder - config Config - store *Store + config Config + store *Store + cimdResolver *cimdClientResolver authorizationHandler *authorizationHandler tokenHandler *tokenHandler @@ -66,11 +70,19 @@ type Module struct { func New(ctx context.Context, deps Dependencies) (*Module, error) { store := NewStore(deps.DB, deps.APIAccess).WithIssuer(deps.Config.BaseURL) + cimdResolver := newCIMDClientResolver(store, cimdResolverConfig{ + getURLAllowlist: deps.GetCIMDURLAllowlist, + transportDecorator: func(transport http.RoundTripper) http.RoundTripper { + return otelhttp.NewTransport(transport) + }, + }) + store.clientResolver = cimdResolver + authenticator, err := newFederatedClientAuthenticator(ctx, store, deps.HTTPClient, deps.Config.BaseURL) if err != nil { return nil, fmt.Errorf("failed to create federated client authenticator: %w", err) } - provider, err := newProvider(store, authenticator, deps.Signer, deps.Config) + provider, err := newProvider(store, authenticator, deps.Signer, deps.Config, cimdResolver) if err != nil { return nil, fmt.Errorf("failed to create OAuth2 provider: %w", err) } @@ -85,8 +97,9 @@ func New(ctx context.Context, deps Dependencies) (*Module, error) { return &Module{ Preview: previewBuilder, - config: deps.Config, - store: store, + config: deps.Config, + store: store, + cimdResolver: cimdResolver, authorizationHandler: newAuthorizationHandler(provider, authorizationService, deps.Config.BaseURL), tokenHandler: newTokenHandler(provider, claimsService, deps.APIAccess), @@ -98,6 +111,11 @@ func New(ctx context.Context, deps Dependencies) (*Module, error) { }, nil } +// RefreshClientMetadata forces a re-fetch of the OAuth Client ID Metadata Document. +func (m *Module) RefreshClientMetadata(ctx context.Context, clientID string) (model.OidcClient, error) { + return m.cimdResolver.RefreshMetadataClient(ctx, clientID) +} + func (m *Module) RegisterRoutes(rootGroup *gin.RouterGroup, apiGroup *gin.RouterGroup, optionalBrowserAuth gin.HandlerFunc, browserAuth gin.HandlerFunc) { rootGroup.GET("/authorize", optionalBrowserAuth, m.authorizationHandler.authorize) rootGroup.POST("/authorize", optionalBrowserAuth, m.authorizationHandler.authorize) diff --git a/backend/internal/oidc/preview_test.go b/backend/internal/oidc/preview_test.go index 95059346..c33b6823 100644 --- a/backend/internal/oidc/preview_test.go +++ b/backend/internal/oidc/preview_test.go @@ -21,7 +21,7 @@ func TestClientPreviewBuilderUsesFositeTokenStrategies(t *testing.T) { BaseURL: "https://issuer.example.com", TokenBaseURL: "https://issuer.example.com", Secret: []byte("test-secret"), - }) + }, nil) require.NoError(t, err) builder := newClientPreviewBuilder(newClaimsService(db, nil, "https://issuer.example.com", nil), provider.tokenStrategies) @@ -70,7 +70,7 @@ func TestClientPreviewBuilderIgnoresUnknownScopes(t *testing.T) { BaseURL: "https://issuer.example.com", TokenBaseURL: "https://issuer.example.com", Secret: []byte("test-secret"), - }) + }, nil) require.NoError(t, err) require.NoError(t, db.Create(&model.User{ diff --git a/backend/internal/oidc/provider.go b/backend/internal/oidc/provider.go index 6b0ce792..4a83f95e 100644 --- a/backend/internal/oidc/provider.go +++ b/backend/internal/oidc/provider.go @@ -29,7 +29,7 @@ type tokenStrategies struct { config *fosite.Config } -func newProvider(store *Store, authenticator *federatedClientAuthenticator, signer TokenSigner, config Config) (*oidcProvider, error) { +func newProvider(store *Store, authenticator *federatedClientAuthenticator, signer TokenSigner, config Config, clientResolver fosite.ClientResolver) (*oidcProvider, error) { secret, err := DeriveGlobalSecret(config.Secret) if err != nil { return nil, err @@ -56,6 +56,7 @@ func newProvider(store *Store, authenticator *federatedClientAuthenticator, sign RefreshTokenScopes: []string{}, GlobalSecret: secret, JWTScopeClaimKey: jwt.JWTScopeFieldBoth, + ClientResolver: clientResolver, } keyGetter := func(context.Context) (interface{}, error) { diff --git a/backend/internal/oidc/provider_test.go b/backend/internal/oidc/provider_test.go index 261468d1..03f57996 100644 --- a/backend/internal/oidc/provider_test.go +++ b/backend/internal/oidc/provider_test.go @@ -20,6 +20,7 @@ import ( "github.com/ory/fosite" fositeoauth2 "github.com/ory/fosite/handler/oauth2" "github.com/pocket-id/pocket-id/backend/internal/model" + datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" testutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing" "github.com/stretchr/testify/require" ) @@ -61,7 +62,7 @@ func TestProviderIssuesJWTAccessTokens(t *testing.T) { BaseURL: "https://issuer.example.com", TokenBaseURL: "https://issuer.example.com", Secret: []byte("test-secret"), - }) + }, nil) require.NoError(t, err) session := NewEmptySession() @@ -135,7 +136,7 @@ func TestProviderInsecureCallbackURLCompatibility(t *testing.T) { require.NoError(t, db.Create(&model.OidcClient{ Base: model.Base{ID: "test-client"}, Name: "Test Client", - CallbackURLs: model.UrlList{"http://client.example.com/callback"}, + CallbackURLs: datatype.StringList{"http://client.example.com/callback"}, }).Error) provider, err := newProvider(NewStore(db, nil), nil, testTokenSigner{key: signerKey}, Config{ //nolint:gosec // static test-only provider secret @@ -143,7 +144,7 @@ func TestProviderInsecureCallbackURLCompatibility(t *testing.T) { TokenBaseURL: "https://issuer.example.com", Secret: []byte("test-secret"), AllowInsecureCallbackURLs: tt.allowInsecureCallbackURLs, - }) + }, nil) require.NoError(t, err) req := httptest.NewRequestWithContext( @@ -172,14 +173,14 @@ func TestProviderAcceptsWildcardRedirectURI(t *testing.T) { require.NoError(t, db.Create(&model.OidcClient{ Base: model.Base{ID: "test-client"}, Name: "Test Client", - CallbackURLs: model.UrlList{"https://*.example.com/callback"}, + CallbackURLs: datatype.StringList{"https://*.example.com/callback"}, }).Error) provider, err := newProvider(NewStore(db, nil), nil, testTokenSigner{key: signerKey}, Config{ //nolint:gosec // static test-only provider secret BaseURL: "https://issuer.example.com", TokenBaseURL: "https://issuer.example.com", Secret: []byte("test-secret"), - }) + }, nil) require.NoError(t, err) const requestedRedirectURI = "https://tenant.example.com/callback" @@ -204,7 +205,7 @@ func TestProviderAcceptsPushedAuthorizationWildcardRedirectURI(t *testing.T) { require.NoError(t, db.Create(&model.OidcClient{ Base: model.Base{ID: "test-client"}, Name: "Test Client", - CallbackURLs: model.UrlList{"https://*.example.com/callback"}, + CallbackURLs: datatype.StringList{"https://*.example.com/callback"}, IsPublic: true, }).Error) @@ -212,7 +213,7 @@ func TestProviderAcceptsPushedAuthorizationWildcardRedirectURI(t *testing.T) { BaseURL: "https://issuer.example.com", TokenBaseURL: "https://issuer.example.com", Secret: []byte("test-secret"), - }) + }, nil) require.NoError(t, err) const requestedRedirectURI = "https://tenant.example.com/callback" @@ -237,14 +238,14 @@ func TestProviderRejectsUnmatchedWildcardRedirectURI(t *testing.T) { require.NoError(t, db.Create(&model.OidcClient{ Base: model.Base{ID: "test-client"}, Name: "Test Client", - CallbackURLs: model.UrlList{"https://*.example.com/callback"}, + CallbackURLs: datatype.StringList{"https://*.example.com/callback"}, }).Error) provider, err := newProvider(NewStore(db, nil), nil, testTokenSigner{key: signerKey}, Config{ //nolint:gosec // static test-only provider secret BaseURL: "https://issuer.example.com", TokenBaseURL: "https://issuer.example.com", Secret: []byte("test-secret"), - }) + }, nil) require.NoError(t, err) const requestedRedirectURI = "https://evil.example.net/callback" @@ -277,14 +278,14 @@ func TestProviderAcceptsUnsignedRequestObject(t *testing.T) { require.NoError(t, db.Create(&model.OidcClient{ Base: model.Base{ID: "test-client"}, Name: "Test Client", - CallbackURLs: model.UrlList{"https://client.example.com/callback"}, + CallbackURLs: datatype.StringList{"https://client.example.com/callback"}, }).Error) provider, err := newProvider(NewStore(db, nil), nil, testTokenSigner{key: signerKey}, Config{ //nolint:gosec // static test-only provider secret BaseURL: "https://issuer.example.com", TokenBaseURL: "https://issuer.example.com", Secret: []byte("test-secret"), - }) + }, nil) require.NoError(t, err) requestObject := encodeRequestObject(t, @@ -318,14 +319,14 @@ func TestProviderRejectsSignedRequestObject(t *testing.T) { require.NoError(t, db.Create(&model.OidcClient{ Base: model.Base{ID: "test-client"}, Name: "Test Client", - CallbackURLs: model.UrlList{"https://client.example.com/callback"}, + CallbackURLs: datatype.StringList{"https://client.example.com/callback"}, }).Error) provider, err := newProvider(NewStore(db, nil), nil, testTokenSigner{key: signerKey}, Config{ //nolint:gosec // static test-only provider secret BaseURL: "https://issuer.example.com", TokenBaseURL: "https://issuer.example.com", Secret: []byte("test-secret"), - }) + }, nil) require.NoError(t, err) // The signature is never verified: the request object must already be rejected because only @@ -401,7 +402,7 @@ func TestProviderIssuesAndValidatesTokensForSupportedAlgorithms(t *testing.T) { BaseURL: "https://issuer.example.com", TokenBaseURL: "https://issuer.example.com", Secret: []byte("test-secret"), - }) + }, nil) require.NoError(t, err) session := NewEmptySession() @@ -464,14 +465,14 @@ func TestProviderIgnoresUnknownScopes(t *testing.T) { require.NoError(t, db.Create(&model.OidcClient{ Base: model.Base{ID: "test-client"}, Name: "Test Client", - CallbackURLs: model.UrlList{"https://app.example.com/callback"}, + CallbackURLs: datatype.StringList{"https://app.example.com/callback"}, }).Error) provider, err := newProvider(NewStore(db, nil), nil, testTokenSigner{key: signerKey}, Config{ //nolint:gosec // static test-only provider secret BaseURL: "https://issuer.example.com", TokenBaseURL: "https://issuer.example.com", Secret: []byte("test-secret"), - }) + }, nil) require.NoError(t, err) // Clients such as MCP clients blindly request scopes Pocket ID does not support, like diff --git a/backend/internal/oidc/store.go b/backend/internal/oidc/store.go index 0f9118e5..c3aff4ae 100644 --- a/backend/internal/oidc/store.go +++ b/backend/internal/oidc/store.go @@ -4,7 +4,10 @@ import ( "context" "encoding/json" "errors" + "fmt" + "log/slog" "net/url" + "slices" "time" "github.com/ory/fosite" @@ -48,9 +51,10 @@ func NewStore(db *gorm.DB, apiAccess APIAccessProvider) *Store { } type Store struct { - db *gorm.DB - apiAccess APIAccessProvider - issuer string + db *gorm.DB + apiAccess APIAccessProvider + issuer string + clientResolver fosite.ClientResolver } // WithIssuer sets the issuer that is added as an extra audience to access tokens carrying an identity scope, so they can be presented to Pocket ID's own endpoints such as /userinfo @@ -115,6 +119,169 @@ func (s *Store) GetClient(ctx context.Context, id string) (fosite.Client, error) return client, nil } +// resolvePersistedClient restores a client from storage and falls back to the configured generic resolver for uncached clients +func (s *Store) resolvePersistedClient(ctx context.Context, id string) (fosite.Client, error) { + if s.clientResolver != nil { + return s.clientResolver.ResolveClient(ctx, id, s.GetClient) + } + return s.GetClient(ctx, id) +} + +// clientFromModel populates the provider-specific runtime fields on a stored client +func (s *Store) clientFromModel(ctx context.Context, tx *gorm.DB, clientModel model.OidcClient) (Client, error) { + client := Client{OidcClient: clientModel} + + // Populate the custom-API scopes and audiences the client may request only when the API feature is wired + if s.apiAccess != nil { + apiScopes, apiAudiences, err := s.apiAccess.ClientAPIScopes(ctx, tx, clientModel.ID) + if err != nil { + return Client{}, err + } + + client.apiScopes = apiScopes + client.apiAudiences = apiAudiences + } + + return client, nil +} + +// LoadCIMDClient loads only clients that Pocket ID previously associated with a metadata document +func (s *Store) LoadCIMDClient(ctx context.Context, id string) (fosite.CIMDCachedClient, bool, error) { + clientModel, err := s.firstClientByID(ctx, id) + if errors.Is(err, gorm.ErrRecordNotFound) { + return fosite.CIMDCachedClient{}, false, nil + } + if err != nil { + return fosite.CIMDCachedClient{}, false, err + } + if !clientModel.IsMetadataDocument() { + return fosite.CIMDCachedClient{}, false, nil + } + + client, err := s.clientFromModel(ctx, s.dbFor(ctx), clientModel) + if err != nil { + return fosite.CIMDCachedClient{}, false, err + } + var expiresAt time.Time + if clientModel.MetadataExpiresAt != nil { + expiresAt = time.Time(*clientModel.MetadataExpiresAt) + } + // Force incompatible cached entries through discovery so current policy applies before they can be used + if !clientModel.IsPublic || !clientModel.PkceEnabled || len(clientModel.Credentials.FederatedIdentities) > 0 { + expiresAt = time.Time{} + } + return fosite.CIMDCachedClient{Client: client, ExpiresAt: expiresAt}, true, nil +} + +// StoreCIMDClient persists metadata-derived fields while preserving local consent and policy state +func (s *Store) StoreCIMDClient(ctx context.Context, resolved fosite.Client, _ *fosite.ClientMetadataDocument, expiresAt time.Time) (fosite.Client, error) { + client, ok := resolved.(Client) + if !ok { + return nil, errors.New("metadata resolver returned an incompatible client") + } + expiry := datatype.DateTime(expiresAt) + client.MetadataExpiresAt = &expiry + + var changes []string + var revokeConsent bool + var stored fosite.Client + err := withTx(ctx, s.db, func(ctx context.Context) error { + existing, err := s.firstClientByID(ctx, client.ID) + found := err == nil + if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) { + return err + } + if found && !existing.IsMetadataDocument() { + return errors.New("client is already registered without a metadata document") + } + if found { + changes = metadataClientChanges(existing, client.OidcClient) + // Every detected change except the display name invalidates what the user agreed to + revokeConsent = slices.ContainsFunc(changes, func(field string) bool { return field != "client_name" }) + } + + // Persist the refreshed metadata and consent invalidation together so a failed deletion cannot suppress the next revocation attempt + if err := s.upsertMetadataClient(ctx, s.dbFor(ctx), &client.OidcClient, found); err != nil { + return err + } + + // A security-relevant document change invalidates consent because it changes what the user previously approved + if revokeConsent { + err := s.dbFor(ctx). + Where("client_id = ?", client.ID). + Delete(&model.UserAuthorizedOidcClient{}). + Error + if err != nil { + return fmt.Errorf("failed to revoke consent after metadata change: %w", err) + } + } + + // Reload inside the transaction so DB-managed columns and preloads are populated consistently + stored, err = s.GetClient(ctx, client.ID) + return err + }) + if err != nil { + return nil, err + } + + if len(changes) > 0 { + slog.InfoContext(ctx, "Client metadata changed", + slog.String("client_id", client.ID), + slog.Any("changed_fields", changes), + ) + } + if revokeConsent { + slog.WarnContext(ctx, "Revoked existing user consent after a security-relevant client metadata change", + slog.String("client_id", client.ID), + ) + } + + return stored, nil +} + +// metadataClientChanges returns the names of security-relevant metadata fields that differ between the stored client and a freshly fetched one +func metadataClientChanges(old, next model.OidcClient) []string { + var changed []string + if !slices.Equal([]string(old.CallbackURLs), next.CallbackURLs) { + changed = append(changed, "redirect_uris") + } + if !slices.Equal([]string(old.LogoutCallbackURLs), next.LogoutCallbackURLs) { + changed = append(changed, "post_logout_redirect_uris") + } + if old.IsPublic != next.IsPublic { + changed = append(changed, "token_endpoint_auth_method") + } + if old.Name != next.Name { + changed = append(changed, "client_name") + } + if !slices.Equal(effectiveMetadataGrantTypes(old.MetadataGrantTypes), effectiveMetadataGrantTypes(next.MetadataGrantTypes)) { + changed = append(changed, "grant_types") + } + return changed +} + +func effectiveMetadataGrantTypes(grantTypes datatype.StringList) []string { + if len(grantTypes) == 0 { + return []string{string(fosite.GrantTypeAuthorizationCode)} + } + return grantTypes +} + +// upsertMetadataClient inserts a new managed client or updates the metadata-derived columns of an existing one, leaving consent, grants, and group links untouched +func (s *Store) upsertMetadataClient(ctx context.Context, tx *gorm.DB, client *model.OidcClient, update bool) error { + if !update { + return tx.WithContext(ctx). + Clauses(clause.OnConflict{Columns: []clause.Column{{Name: "id"}}, DoNothing: true}). + Create(client).Error + } + return tx.WithContext(ctx). + Model(&model.OidcClient{Base: model.Base{ID: client.ID}}). + Select("Name", "CallbackURLs", "LogoutCallbackURLs", "Credentials", + "IsPublic", "PkceEnabled", "ClientType", "MetadataExpiresAt", + "MetadataGrantTypes"). + Updates(client).Error +} + func (s *Store) ClientAssertionJWTValid(ctx context.Context, jti string) error { var count int64 err := s.dbFor(ctx). @@ -142,6 +309,18 @@ func (s *Store) SetClientAssertionJWT(ctx context.Context, jti string, exp time. return err } +func (s *Store) firstClientByID(ctx context.Context, id string) (model.OidcClient, error) { + var client model.OidcClient + err := s.dbFor(ctx). + Preload("AllowedUserGroups"). + First(&client, "id = ?", id). + Error + if err != nil { + return model.OidcClient{}, err + } + return client, nil +} + // Satisfies fositeoauth2.CoreStorage func (s *Store) CreateAuthorizeCodeSession(ctx context.Context, code string, request fosite.Requester) error { @@ -742,7 +921,7 @@ func (s *Store) decodeDeviceRequester(ctx context.Context, data string) (fosite. } func (s *Store) requesterFromStored(ctx context.Context, stored storedRequester) (fosite.Requester, error) { - client, err := s.GetClient(ctx, stored.ClientID) + client, err := s.resolvePersistedClient(ctx, stored.ClientID) if err != nil { return nil, err } diff --git a/backend/internal/oidc/token_handler_test.go b/backend/internal/oidc/token_handler_test.go index abaacd33..d272af09 100644 --- a/backend/internal/oidc/token_handler_test.go +++ b/backend/internal/oidc/token_handler_test.go @@ -62,7 +62,7 @@ func TestTokenHandlerClientCredentialsGrant(t *testing.T) { BaseURL: baseURL, TokenBaseURL: baseURL, Secret: []byte(secret), - }) + }, nil) require.NoError(t, err) handler := newTokenHandler(provider, newClaimsService(db, nil, baseURL, nil), nil) @@ -121,7 +121,7 @@ func TestTokenHandlerClientCredentialsDropsIdentityScopes(t *testing.T) { BaseURL: baseURL, TokenBaseURL: baseURL, Secret: []byte(secret), - }) + }, nil) require.NoError(t, err) handler := newTokenHandler(provider, newClaimsService(db, nil, baseURL, nil), nil) @@ -183,7 +183,7 @@ func TestTokenHandlerClientCredentialsUsesClientSubjectGrants(t *testing.T) { BaseURL: baseURL, TokenBaseURL: baseURL, Secret: []byte(secret), - }) + }, nil) require.NoError(t, err) handler := newTokenHandler(provider, newClaimsService(db, nil, baseURL, nil), apiAccess) @@ -256,7 +256,7 @@ func TestTokenHandlerClientCredentialsDefaultsResourceScopes(t *testing.T) { BaseURL: baseURL, TokenBaseURL: baseURL, Secret: []byte(secret), - }) + }, nil) require.NoError(t, err) handler := newTokenHandler(provider, newClaimsService(db, nil, baseURL, nil), apiAccess) @@ -411,7 +411,7 @@ func TestTokenHandlerRefreshGrantRevalidatesUser(t *testing.T) { BaseURL: baseURL, TokenBaseURL: baseURL, Secret: []byte(secret), - }) + }, nil) require.NoError(t, err) handler := newTokenHandler(provider, newClaimsService(db, nil, baseURL, nil), nil) @@ -555,7 +555,7 @@ func TestTokenHandlerRefreshGrantPreservesAudienceAndScope(t *testing.T) { BaseURL: baseURL, TokenBaseURL: baseURL, Secret: []byte(secret), - }) + }, nil) require.NoError(t, err) handler := newTokenHandler(provider, newClaimsService(db, nil, baseURL, nil), apiAccess) diff --git a/backend/internal/oidc/userinfo_handler_test.go b/backend/internal/oidc/userinfo_handler_test.go index 20083252..d541d20f 100644 --- a/backend/internal/oidc/userinfo_handler_test.go +++ b/backend/internal/oidc/userinfo_handler_test.go @@ -51,7 +51,7 @@ func TestUserInfoHandler(t *testing.T) { BaseURL: baseURL, TokenBaseURL: baseURL, Secret: []byte("test-secret"), - }) + }, nil) require.NoError(t, err) handler := newUserInfoHandler(provider, newClaimsService(db, nil, baseURL, nil), baseURL) diff --git a/backend/internal/service/e2etest_service.go b/backend/internal/service/e2etest_service.go index 05bc4071..58b4f87e 100644 --- a/backend/internal/service/e2etest_service.go +++ b/backend/internal/service/e2etest_service.go @@ -176,8 +176,8 @@ func (s *TestService) SeedDatabase(baseURL string) error { Description: "This is an example description for Nextcloud", LaunchURL: new("https://nextcloud.local"), Secret: "$2a$10$9dypwot8nGuCjT6wQWWpJOckZfRprhe2EkwpKizxS/fpVHrOLEJHC", // w2mUeZISmEvIDMEDvpY0PnxQIpj1m3zY - CallbackURLs: model.UrlList{"http://nextcloud.localhost/auth/callback"}, - LogoutCallbackURLs: model.UrlList{"http://nextcloud.localhost/auth/logout/callback"}, + CallbackURLs: datatype.StringList{"http://nextcloud.localhost/auth/callback"}, + LogoutCallbackURLs: datatype.StringList{"http://nextcloud.localhost/auth/logout/callback"}, ImageType: new("png"), CreatedByID: new(users[0].ID), }, @@ -187,7 +187,7 @@ func (s *TestService) SeedDatabase(baseURL string) error { }, Name: "Immich", Secret: "$2a$10$Ak.FP8riD1ssy2AGGbG.gOpnp/rBpymd74j0nxNMtW0GG1Lb4gzxe", // PYjrE9u4v9GVqXKi52eur0eb2Ci4kc0x - CallbackURLs: model.UrlList{"http://immich.localhost/auth/callback"}, + CallbackURLs: datatype.StringList{"http://immich.localhost/auth/callback"}, CreatedByID: new(users[1].ID), IsGroupRestricted: true, AllowedUserGroups: []model.UserGroup{ @@ -200,8 +200,8 @@ func (s *TestService) SeedDatabase(baseURL string) error { }, Name: "Tailscale", Secret: "$2a$10$xcRReBsvkI1XI6FG8xu/pOgzeF00bH5Wy4d/NThwcdi3ZBpVq/B9a", // n4VfQeXlTzA6yKpWbR9uJcMdSx2qH0Lo - CallbackURLs: model.UrlList{"http://tailscale.localhost/auth/callback"}, - LogoutCallbackURLs: model.UrlList{"http://tailscale.localhost/auth/logout/callback"}, + CallbackURLs: datatype.StringList{"http://tailscale.localhost/auth/callback"}, + LogoutCallbackURLs: datatype.StringList{"http://tailscale.localhost/auth/logout/callback"}, IsGroupRestricted: true, CreatedByID: new(users[0].ID), AllowedUserGroups: []model.UserGroup{ @@ -214,7 +214,7 @@ func (s *TestService) SeedDatabase(baseURL string) error { }, Name: "Federated", Secret: "$2a$10$Ak.FP8riD1ssy2AGGbG.gOpnp/rBpymd74j0nxNMtW0GG1Lb4gzxe", // PYjrE9u4v9GVqXKi52eur0eb2Ci4kc0x - CallbackURLs: model.UrlList{"http://federated.localhost/auth/callback"}, + CallbackURLs: datatype.StringList{"http://federated.localhost/auth/callback"}, CreatedByID: new(users[1].ID), AllowedUserGroups: []model.UserGroup{}, Credentials: model.OidcClientCredentials{ @@ -234,7 +234,7 @@ func (s *TestService) SeedDatabase(baseURL string) error { }, Name: "SCIM Client", Secret: "$2a$10$h4wfa8gI7zavDAxwzSq1sOwYU4e8DwK1XZ8ZweNnY5KzlJ3Iz.qdK", // nQbiuMRG7FpdK2EnDd5MBivWQeKFXohn - CallbackURLs: model.UrlList{"http://scimclient.localhost/auth/callback"}, + CallbackURLs: datatype.StringList{"http://scimclient.localhost/auth/callback"}, CreatedByID: new(users[0].ID), IsGroupRestricted: true, AllowedUserGroups: []model.UserGroup{ @@ -248,7 +248,7 @@ func (s *TestService) SeedDatabase(baseURL string) error { }, Name: "PAR Test Client", Secret: "$2a$10$9dypwot8nGuCjT6wQWWpJOckZfRprhe2EkwpKizxS/fpVHrOLEJHC", // w2mUeZISmEvIDMEDvpY0PnxQIpj1m3zY - CallbackURLs: model.UrlList{"http://par-client.localhost/auth/callback"}, + CallbackURLs: datatype.StringList{"http://par-client.localhost/auth/callback"}, CreatedByID: new(users[0].ID), }, { @@ -257,7 +257,7 @@ func (s *TestService) SeedDatabase(baseURL string) error { }, Name: "Skip Consent Client", Secret: "$2a$10$9dypwot8nGuCjT6wQWWpJOckZfRprhe2EkwpKizxS/fpVHrOLEJHC", // w2mUeZISmEvIDMEDvpY0PnxQIpj1m3zY - CallbackURLs: model.UrlList{"http://skip-consent.localhost/auth/callback"}, + CallbackURLs: datatype.StringList{"http://skip-consent.localhost/auth/callback"}, CreatedByID: new(users[0].ID), // Trusted client that bypasses the consent screen by default SkipConsent: true, diff --git a/backend/internal/service/oidc_service.go b/backend/internal/service/oidc_service.go index c9cdef80..f5979a0f 100644 --- a/backend/internal/service/oidc_service.go +++ b/backend/internal/service/oidc_service.go @@ -37,10 +37,11 @@ const ( ) type OidcService struct { - db *gorm.DB - jwtService *JwtService - previewBuilder oidcClientPreviewBuilder - scimService *ScimService + db *gorm.DB + jwtService *JwtService + previewBuilder oidcClientPreviewBuilder + metadataRefresher metadataRefresher + scimService *ScimService httpClient *http.Client fileStorage storage.FileStorage @@ -50,21 +51,27 @@ type oidcClientPreviewBuilder interface { BuildClientPreview(ctx context.Context, client model.OidcClient, userID string, scopes []string, authenticationMethod string) (*oidc.ClientPreview, error) } +type metadataRefresher interface { + RefreshClientMetadata(ctx context.Context, clientID string) (model.OidcClient, error) +} + func NewOidcService( db *gorm.DB, jwtService *JwtService, previewBuilder oidcClientPreviewBuilder, + metadataRefresher metadataRefresher, scimService *ScimService, httpClient *http.Client, fileStorage storage.FileStorage, ) (s *OidcService, err error) { s = &OidcService{ - db: db, - jwtService: jwtService, - previewBuilder: previewBuilder, - scimService: scimService, - httpClient: httpClient, - fileStorage: fileStorage, + db: db, + jwtService: jwtService, + previewBuilder: previewBuilder, + metadataRefresher: metadataRefresher, + scimService: scimService, + httpClient: httpClient, + fileStorage: fileStorage, } return s, nil @@ -74,6 +81,22 @@ func (s *OidcService) GetClient(ctx context.Context, clientID string) (model.Oid return s.getClientInternal(ctx, clientID, s.db, false) } +// RefreshClientMetadata forces a re-fetch of the OAuth Client ID Metadata Document +// for a CIMD client, bypassing the cache TTL, and returns the refreshed client. +func (s *OidcService) RefreshClientMetadata(ctx context.Context, clientID string) (model.OidcClient, error) { + if s.metadataRefresher == nil { + return model.OidcClient{}, &common.ValidationError{Message: "client ID metadata documents are not enabled"} + } + client, err := s.metadataRefresher.RefreshClientMetadata(ctx, clientID) + if err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return model.OidcClient{}, err + } + return model.OidcClient{}, &common.ValidationError{Message: err.Error()} + } + return client, nil +} + func (s *OidcService) getClientInternal(ctx context.Context, clientID string, tx *gorm.DB, forUpdate bool) (model.OidcClient, error) { var client model.OidcClient q := tx. @@ -161,10 +184,7 @@ func (s *OidcService) UpdateClient(ctx context.Context, clientID string, input d tx.Rollback() }() - var client model.OidcClient - err := tx.WithContext(ctx). - Preload("CreatedBy"). - First(&client, "id = ?", clientID).Error + client, err := s.getClientInternal(ctx, clientID, tx, true) if err != nil { return model.OidcClient{}, err } @@ -179,7 +199,22 @@ func (s *OidcService) UpdateClient(ctx context.Context, clientID string, input d } } - err = tx.WithContext(ctx).Save(&client).Error + // Metadata refresh owns all other CIMD columns, so an admin update must never write back a stale metadata snapshot + if client.IsMetadataDocument() { + err = tx.WithContext(ctx). + Model(&client). + Select( + "Description", + "RequiresReauthentication", + "RequiresPushedAuthorizationRequests", + "SkipConsent", + "LaunchURL", + "IsGroupRestricted", + ). + Updates(&client).Error + } else { + err = tx.WithContext(ctx).Save(&client).Error + } if err != nil { return model.OidcClient{}, err } @@ -208,25 +243,32 @@ func (s *OidcService) UpdateClient(ctx context.Context, clientID string, input d } func updateOIDCClientModelFromDto(client *model.OidcClient, input *dto.OidcClientUpdateDto) { - // Base fields - client.Name = input.Name + // Update fields that remain locally managed for every client type client.Description = input.Description - client.CallbackURLs = input.CallbackURLs - client.LogoutCallbackURLs = input.LogoutCallbackURLs - client.IsPublic = input.IsPublic - // PKCE is required for public clients - client.PkceEnabled = input.IsPublic || input.PkceEnabled - // Reset any pkce support prompt if previously flagged - if !input.PkceEnabled { - client.PkceSupported = false - } client.RequiresReauthentication = input.RequiresReauthentication client.RequiresPushedAuthorizationRequests = input.RequiresPushedAuthorizationRequests client.SkipConsent = input.SkipConsent client.LaunchURL = input.LaunchURL client.IsGroupRestricted = input.IsGroupRestricted - // Credentials + // Preserve fields that are sourced from the client metadata document + if client.IsMetadataDocument() { + return + } + + // Update registration fields for manually configured clients + client.Name = input.Name + client.CallbackURLs = input.CallbackURLs + client.LogoutCallbackURLs = input.LogoutCallbackURLs + client.IsPublic = input.IsPublic + // PKCE is required for public clients + client.PkceEnabled = input.IsPublic || input.PkceEnabled + // Reset any PKCE support prompt if previously flagged + if !input.PkceEnabled { + client.PkceSupported = false + } + + // Replace the federated credentials with the submitted configuration client.Credentials.FederatedIdentities = make([]model.OidcClientFederatedIdentity, len(input.Credentials.FederatedIdentities)) for i, fi := range input.Credentials.FederatedIdentities { client.Credentials.FederatedIdentities[i] = model.OidcClientFederatedIdentity{ @@ -255,11 +297,11 @@ func (s *OidcService) DeleteClient(ctx context.Context, clientID string) error { // Delete images if present // Note that storage operations must be done outside of a transaction if client.ImageType != nil && *client.ImageType != "" { - old := path.Join("oidc-client-images", client.ID+"."+*client.ImageType) + old := oidcClientImagePath(client.ID, "", *client.ImageType) _ = s.fileStorage.Delete(ctx, old) } if client.DarkImageType != nil && *client.DarkImageType != "" { - old := path.Join("oidc-client-images", client.ID+"-dark."+*client.DarkImageType) + old := oidcClientImagePath(client.ID, "-dark", *client.DarkImageType) _ = s.fileStorage.Delete(ctx, old) } @@ -281,6 +323,10 @@ func (s *OidcService) CreateClientSecret(ctx context.Context, clientID string, i return "", err } + if client.IsPublic { + return "", &common.ValidationError{Message: "cannot create a secret for a public client"} + } + clientSecret := input.Secret if clientSecret == "" { clientSecret, err = utils.GenerateRandomAlphanumericString(32) @@ -339,7 +385,7 @@ func (s *OidcService) GetClientLogo(ctx context.Context, clientID string, light if mimeType == "" { return nil, 0, "", fmt.Errorf("unsupported image type '%s'", ext) } - key := path.Join("oidc-client-images", client.ID+suffix+"."+ext) + key := oidcClientImagePath(client.ID, suffix, ext) reader, size, err := s.fileStorage.Open(ctx, key) if err != nil { return nil, 0, "", err @@ -359,7 +405,7 @@ func (s *OidcService) UpdateClientLogo(ctx context.Context, clientID string, fil darkSuffix = "-dark" } - imagePath := path.Join("oidc-client-images", clientID+darkSuffix+"."+fileType) + imagePath := oidcClientImagePath(clientID, darkSuffix, fileType) reader, err := file.Open() if err != nil { return err @@ -441,7 +487,7 @@ func (s *OidcService) deleteClientLogoInternal(ctx context.Context, clientID str } // All storage operations must be performed outside of a database transaction - imagePath := path.Join("oidc-client-images", client.ID+imagePathSuffix+"."+oldImageType) + imagePath := oidcClientImagePath(client.ID, imagePathSuffix, oldImageType) err = s.fileStorage.Delete(ctx, imagePath) if err != nil { return err @@ -625,6 +671,7 @@ func (s *OidcService) ListAccessibleOidcClients(ctx context.Context, userID stri LaunchURL: client.LaunchURL, HasLogo: client.HasLogo(), HasDarkLogo: client.HasDarkLogo(), + ClientType: string(client.ClientType), }, LastUsedAt: lastUsedAt, } @@ -761,7 +808,7 @@ func (s *OidcService) downloadAndSaveLogoFromURL(parentCtx context.Context, clie return err } - imagePath := path.Join("oidc-client-images", clientID+darkSuffix+"."+ext) + imagePath := oidcClientImagePath(clientID, darkSuffix, ext) err = s.fileStorage.Save(ctx, imagePath, strippedReader) if errors.Is(err, utils.ErrSizeExceeded) { return errLogoTooLarge @@ -823,13 +870,21 @@ func (s *OidcService) updateClientLogoType(ctx context.Context, clientID string, // Storage operations must be executed outside of a transaction if currentType != nil && *currentType != ext { - old := path.Join("oidc-client-images", client.ID+darkSuffix+"."+*currentType) + old := oidcClientImagePath(client.ID, darkSuffix, *currentType) _ = s.fileStorage.Delete(ctx, old) } return nil } +func oidcClientImagePath(clientID string, suffix string, extension string) string { + storageID := clientID + if !dto.ValidateClientID(clientID) { + storageID = "cimd-" + utils.CreateSha256Hash(clientID) + } + return path.Join("oidc-client-images", storageID+suffix+"."+extension) +} + func (s *OidcService) GetClientScimServiceProvider(ctx context.Context, clientID string) (model.ScimServiceProvider, error) { var provider model.ScimServiceProvider err := s.db. diff --git a/backend/internal/service/oidc_service_test.go b/backend/internal/service/oidc_service_test.go index 5806970e..4ff209bb 100644 --- a/backend/internal/service/oidc_service_test.go +++ b/backend/internal/service/oidc_service_test.go @@ -15,6 +15,7 @@ import ( "github.com/pocket-id/pocket-id/backend/internal/common" "github.com/pocket-id/pocket-id/backend/internal/dto" "github.com/pocket-id/pocket-id/backend/internal/model" + datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" "github.com/pocket-id/pocket-id/backend/internal/storage" "github.com/pocket-id/pocket-id/backend/internal/utils" testutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing" @@ -37,7 +38,7 @@ func TestOidcService_updateClientLogoType(t *testing.T) { // Create a test client client := model.OidcClient{ Name: "Test Client", - CallbackURLs: model.UrlList{"https://example.com/callback"}, + CallbackURLs: datatype.StringList{"https://example.com/callback"}, } err = db.Create(&client).Error require.NoError(t, err) @@ -152,6 +153,18 @@ func TestOidcService_updateClientLogoType(t *testing.T) { }) } +func TestOidcClientImagePath(t *testing.T) { + const metadataClientID = "https://app.example.com/oauth/client" + + assert.Equal(t, "oidc-client-images/client-id.png", oidcClientImagePath("client-id", "", "png")) + assert.Equal( + t, + "oidc-client-images/cimd-"+utils.CreateSha256Hash(metadataClientID)+"-dark.webp", + oidcClientImagePath(metadataClientID, "-dark", "webp"), + ) + assert.NotContains(t, oidcClientImagePath(metadataClientID, "", "png"), "app.example.com") +} + func TestOidcService_downloadAndSaveLogoFromURL(t *testing.T) { const publicLogoHost = "https://8.8.8.8" @@ -165,7 +178,7 @@ func TestOidcService_downloadAndSaveLogoFromURL(t *testing.T) { // Create a test client client := model.OidcClient{ Name: "Test Client", - CallbackURLs: model.UrlList{"https://example.com/callback"}, + CallbackURLs: datatype.StringList{"https://example.com/callback"}, } err = db.Create(&client).Error require.NoError(t, err) @@ -457,7 +470,7 @@ func TestOidcService_downloadAndSaveLogoFromURL(t *testing.T) { func TestOidcService_CreateClient_withDescription(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) require.NoError(t, err) description := "A test client description" @@ -482,7 +495,7 @@ func TestOidcService_CreateClient_withDescription(t *testing.T) { func TestOidcService_CreateClient_withoutDescription(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) require.NoError(t, err) input := dto.OidcClientCreateDto{ @@ -504,7 +517,7 @@ func TestOidcService_CreateClient_withoutDescription(t *testing.T) { func TestOidcService_CreateClientSecret_withCustomSecret(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) require.NoError(t, err) client := model.OidcClient{Name: "Test Client"} @@ -527,13 +540,13 @@ func TestOidcService_CreateClientSecret_withCustomSecret(t *testing.T) { func TestOidcService_UpdateClient_description(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) require.NoError(t, err) // Create a client without a description client := model.OidcClient{ Name: "Test Client", - CallbackURLs: model.UrlList{"https://example.com/callback"}, + CallbackURLs: datatype.StringList{"https://example.com/callback"}, } err = db.Create(&client).Error require.NoError(t, err) @@ -565,9 +578,104 @@ func TestOidcService_UpdateClient_description(t *testing.T) { assert.Empty(t, fetched.Description) } +func TestOidcService_UpdateClient_CIMDPreservesMetadataFields(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) + require.NoError(t, err) + + client := model.OidcClient{ + Name: "Metadata Client", + CallbackURLs: datatype.StringList{"https://metadata.example.com/callback"}, + LogoutCallbackURLs: datatype.StringList{"https://metadata.example.com/logout"}, + IsPublic: true, + PkceEnabled: true, + Credentials: model.OidcClientCredentials{ + FederatedIdentities: []model.OidcClientFederatedIdentity{{ + Issuer: "https://metadata.example.com/client.json", + Subject: "https://metadata.example.com/client.json", + JWKS: "https://metadata.example.com/jwks.json", + }}, + }, + ClientType: model.OidcClientTypeCIMD, + } + require.NoError(t, db.Create(&client).Error) + + launchURL := "https://app.example.com" + input := dto.OidcClientUpdateDto{ + Name: "Overridden Client", + Description: "Locally managed description", + CallbackURLs: []string{"https://override.example.com/callback"}, + LogoutCallbackURLs: []string{"https://override.example.com/logout"}, + IsPublic: false, + PkceEnabled: false, + RequiresReauthentication: true, + RequiresPushedAuthorizationRequests: true, + SkipConsent: true, + LaunchURL: &launchURL, + IsGroupRestricted: true, + Credentials: dto.OidcClientCredentialsDto{ + FederatedIdentities: []dto.OidcClientFederatedIdentityDto{{ + Issuer: "https://override.example.com", + JWKS: "https://override.example.com/jwks.json", + }}, + }, + } + + _, err = s.UpdateClient(t.Context(), client.ID, input) + require.NoError(t, err) + + var fetched model.OidcClient + require.NoError(t, db.First(&fetched, "id = ?", client.ID).Error) + assert.Equal(t, client.Name, fetched.Name) + assert.Equal(t, client.CallbackURLs, fetched.CallbackURLs) + assert.Equal(t, client.LogoutCallbackURLs, fetched.LogoutCallbackURLs) + assert.Equal(t, client.IsPublic, fetched.IsPublic) + assert.Equal(t, client.PkceEnabled, fetched.PkceEnabled) + assert.Equal(t, client.Credentials, fetched.Credentials) + assert.Equal(t, input.Description, fetched.Description) + assert.Equal(t, input.RequiresReauthentication, fetched.RequiresReauthentication) + assert.Equal(t, input.RequiresPushedAuthorizationRequests, fetched.RequiresPushedAuthorizationRequests) + assert.Equal(t, input.SkipConsent, fetched.SkipConsent) + assert.Equal(t, input.LaunchURL, fetched.LaunchURL) + assert.Equal(t, input.IsGroupRestricted, fetched.IsGroupRestricted) +} + +func TestOidcService_UpdateClient_CIMDDoesNotOverwriteConcurrentMetadataRefresh(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) + require.NoError(t, err) + + client := model.OidcClient{ + Name: "Original metadata name", + CallbackURLs: datatype.StringList{"https://metadata.example.com/callback"}, + ClientType: model.OidcClientTypeCIMD, + } + require.NoError(t, db.Create(&client).Error) + + // Simulate metadata refresh changing a document-owned column after the admin request read its snapshot + require.NoError(t, db.Exec(` + CREATE TRIGGER refresh_metadata_before_admin_update + BEFORE UPDATE OF description ON oidc_clients + BEGIN + UPDATE oidc_clients SET name = 'Refreshed metadata name' WHERE id = OLD.id; + END; + `).Error) + + input := dto.OidcClientUpdateDto{Description: "Locally managed description"} + _, err = s.UpdateClient(t.Context(), client.ID, input) + require.NoError(t, err) + + var fetched model.OidcClient + require.NoError(t, db.First(&fetched, "id = ?", client.ID).Error) + assert.Equal(t, "Refreshed metadata name", fetched.Name) + assert.Equal(t, input.Description, fetched.Description) +} + func TestOidcService_ListAccessibleOidcClients_requiresExplicitGroupPermission(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) require.NoError(t, err) allowedGroup := model.UserGroup{Name: "allowed", FriendlyName: "Allowed"} @@ -581,10 +689,10 @@ func TestOidcService_ListAccessibleOidcClients_requiresExplicitGroupPermission(t require.NoError(t, db.Create(&userWithoutGroup).Error) clients := []model.OidcClient{ - {Name: "Unrestricted", CallbackURLs: model.UrlList{"https://unrestricted.example.com/callback"}}, - {Name: "Restricted without groups", CallbackURLs: model.UrlList{"https://empty.example.com/callback"}, IsGroupRestricted: true}, - {Name: "Restricted to user group", CallbackURLs: model.UrlList{"https://allowed.example.com/callback"}, IsGroupRestricted: true, AllowedUserGroups: []model.UserGroup{allowedGroup}}, - {Name: "Restricted to other group", CallbackURLs: model.UrlList{"https://other.example.com/callback"}, IsGroupRestricted: true, AllowedUserGroups: []model.UserGroup{otherGroup}}, + {Name: "Unrestricted", CallbackURLs: datatype.StringList{"https://unrestricted.example.com/callback"}}, + {Name: "Restricted without groups", CallbackURLs: datatype.StringList{"https://empty.example.com/callback"}, IsGroupRestricted: true}, + {Name: "Restricted to user group", CallbackURLs: datatype.StringList{"https://allowed.example.com/callback"}, IsGroupRestricted: true, AllowedUserGroups: []model.UserGroup{allowedGroup}}, + {Name: "Restricted to other group", CallbackURLs: datatype.StringList{"https://other.example.com/callback"}, IsGroupRestricted: true, AllowedUserGroups: []model.UserGroup{otherGroup}}, } for i := range clients { require.NoError(t, db.Create(&clients[i]).Error) diff --git a/backend/internal/utils/callback_url_util.go b/backend/internal/utils/callback_url_util.go index d92af89d..6f8ae07d 100644 --- a/backend/internal/utils/callback_url_util.go +++ b/backend/internal/utils/callback_url_util.go @@ -123,6 +123,18 @@ func GetCallbackURLFromList(urls []string, inputCallbackURL string) (callbackURL return "", nil } +// MatchesAnyURLPattern reports whether input matches any pattern in the list, +// using the same wildcard rules as callback URLs. An empty list never matches. +func MatchesAnyURLPattern(patterns []string, input string) bool { + for _, pattern := range patterns { + matches, err := matchCallbackURL(pattern, input) + if err == nil && matches { + return true + } + } + return false +} + func loopbackURLWithWildcardPort(input string) string { u, _ := url.Parse(input) @@ -201,8 +213,14 @@ func normalizeToURLPatternStandard(pattern string) string { var result strings.Builder result.Grow(len(pattern) + 5) // Add 5 for some extra capacity, hoping to avoid many re-allocations - // First, process the base + writeNormalizedBase(&result, patternBase) + writeNormalizedPath(&result, patternPath) + return result.String() +} + +// writeNormalizedBase escapes the colons in the scheme and authority that urlpattern would otherwise read as wildcards +func writeNormalizedBase(result *strings.Builder, patternBase string) { // 0 = scheme // 1 = hostname (optionally with username/password) - before IPv6 start (no `[` found) // 2 = is matching IPv6 (until `]`) @@ -223,6 +241,12 @@ func normalizeToURLPatternStandard(pattern string) string { case '[': // Start of IPv6 match step = 2 + case ':': + // urlpattern reads ":name" as a single-segment wildcard, but the only wildcards this package supports are * and ** + // A colon that introduces a port is followed by a digit, so it stays structural and everything else is escaped to a literal + if !isPortSeparator(patternBase, i) { + result.WriteByte('\\') + } } case 2: if patternBase[i] == '/' || patternBase[i] == ']' || patternBase[i] == '[' { @@ -243,8 +267,10 @@ func normalizeToURLPatternStandard(pattern string) string { // Write the byte result.WriteByte(patternBase[i]) } +} - // Next, process the path +// writeNormalizedPath converts * and ** into the wildcards urlpattern understands, leaving every other character literal +func writeNormalizedPath(result *strings.Builder, patternPath string) { for i := 0; i < len(patternPath); i++ { if patternPath[i] == '*' { // Replace globstar with a single asterisk @@ -257,11 +283,19 @@ func normalizeToURLPatternStandard(pattern string) string { result.WriteString(strconv.Itoa(i)) } } else { + // A literal colon in the path would otherwise be read as a ":name" wildcard + if patternPath[i] == ':' { + result.WriteByte('\\') + } // Add the byte result.WriteByte(patternPath[i]) } } - return result.String() +} + +// isPortSeparator reports whether the colon at index i separates the host from a port +func isPortSeparator(s string, i int) bool { + return i+1 < len(s) && s[i+1] >= '0' && s[i+1] <= '9' } func extractPath(url string) (base string, path string) { diff --git a/backend/internal/utils/callback_url_util_test.go b/backend/internal/utils/callback_url_util_test.go index 251d3ae2..7bac62ad 100644 --- a/backend/internal/utils/callback_url_util_test.go +++ b/backend/internal/utils/callback_url_util_test.go @@ -699,6 +699,29 @@ func TestGetCallbackURLFromList_LoopbackSpecialHandling(t *testing.T) { } } +func TestMatchesAnyURLPattern(t *testing.T) { + tests := []struct { + name string + patterns []string + input string + want bool + }{ + {"empty list denies", nil, "https://app.example.com/oauth/client", false}, + {"empty slice denies", []string{}, "https://app.example.com/oauth/client", false}, + {"exact match", []string{"https://app.example.com/oauth/client"}, "https://app.example.com/oauth/client", true}, + {"wildcard path", []string{"https://app.example.com/**"}, "https://app.example.com/oauth/client", true}, + {"wildcard host segment", []string{"https://*.example.com/oauth/client"}, "https://app.example.com/oauth/client", true}, + {"star matches all", []string{"*"}, "https://anything.example.com/x", true}, + {"no match", []string{"https://other.example.com/**"}, "https://app.example.com/oauth/client", false}, + {"second pattern matches", []string{"https://a.example.com/**", "https://app.example.com/**"}, "https://app.example.com/oauth/client", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, MatchesAnyURLPattern(tt.patterns, tt.input)) + }) + } +} + func TestLoopbackURLWithWildcardPort(t *testing.T) { tests := []struct { name string @@ -838,3 +861,42 @@ func TestGetCallbackURLFromList_MultiplePatterns(t *testing.T) { }) } } + +// The only wildcards this package supports are * and ** +// urlpattern additionally reads ":name" as a single-segment wildcard, so a literal colon in a +// pattern must never widen what it matches +func TestMatchCallbackURL_ColonIsNotAWildcard(t *testing.T) { + tests := []struct { + name string + pattern string + input string + want bool + }{ + {"host label is literal", "https://:host.example.com/cb", "https://evil.example.com/cb", false}, + {"host label matches itself", "https://:host.example.com/cb", "https://:host.example.com/cb", true}, + {"path segment is literal", "https://app.example.com/a:b", "https://app.example.com/a:other", false}, + {"path segment matches itself", "https://app.example.com/a:b", "https://app.example.com/a:b", true}, + {"userinfo is literal", "https://user:pass@app.example.com/cb", "https://user:other@app.example.com/cb", false}, + {"userinfo matches itself", "https://user:pass@app.example.com/cb", "https://user:pass@app.example.com/cb", true}, + + // Structural colons must keep working + {"port is matched exactly", "https://app.example.com:8080/cb", "https://app.example.com:8080/cb", true}, + {"port mismatch is rejected", "https://app.example.com:8080/cb", "https://app.example.com:9090/cb", false}, + {"ipv6 host", "https://[::1]/cb", "https://[::1]/cb", true}, + {"ipv6 host with port", "https://[::1]:8080/cb", "https://[::1]:8080/cb", true}, + + // The supported wildcards are unaffected + {"single asterisk spans one segment", "https://app.example.com/*/cb", "https://app.example.com/x/cb", true}, + {"single asterisk does not span two", "https://app.example.com/*/cb", "https://app.example.com/x/y/cb", false}, + {"globstar spans many segments", "https://app.example.com/**", "https://app.example.com/a/b/c", true}, + {"asterisk in host", "https://*.example.com/cb", "https://sub.example.com/cb", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := matchCallbackURL(tt.pattern, tt.input) + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/backend/internal/utils/ip_util.go b/backend/internal/utils/ip_util.go index d91850ad..3683df93 100644 --- a/backend/internal/utils/ip_util.go +++ b/backend/internal/utils/ip_util.go @@ -28,6 +28,13 @@ var tailscaleIPNets = []*net.IPNet{ {IP: net.IPv4(100, 64, 0, 0), Mask: net.CIDRMask(10, 32)}, // 100.64.0.0/10 } +// LocalIPv6IPNets returns the extra IPv6 ranges configured via LOCAL_IPV6_RANGES +// that are treated as local/private. It is used to extend SSRF protection in +// components that classify IPs independently (e.g. the fosite CIMD fetcher). +func LocalIPv6IPNets() []*net.IPNet { + return localIPv6Ranges +} + func IsLocalIPv6(ip net.IP) bool { if ip.To4() != nil { return false diff --git a/backend/internal/utils/testing/actors.go b/backend/internal/utils/testing/actors.go index 34d8c544..cb0aba95 100644 --- a/backend/internal/utils/testing/actors.go +++ b/backend/internal/utils/testing/actors.go @@ -6,12 +6,16 @@ package testing import ( "context" + "crypto/tls" + "errors" "net" "testing" "time" "github.com/italypaleale/francis/components/standalone" "github.com/italypaleale/francis/host/local" + "github.com/quic-go/quic-go" + "github.com/quic-go/quic-go/http3" "github.com/stretchr/testify/require" ) @@ -26,8 +30,9 @@ const testActorHostPSK = "pocket-id-test-actor-host-psk-32bytes" func NewActorHostForTest(t *testing.T, register func(t *testing.T, h *local.Host)) *local.Host { t.Helper() + address := freeLoopbackUDPAddr(t) hostOpts := []local.HostOption{ - local.WithAddress(freeLoopbackAddr(t)), + local.WithAddress(address), local.WithRuntimePSKs([]byte(testActorHostPSK)), local.WithStandaloneMemoryProvider(standalone.StandaloneMemoryOptions{}), local.WithShutdownGracePeriod(time.Second), @@ -62,19 +67,58 @@ func NewActorHostForTest(t *testing.T, register func(t *testing.T, h *local.Host t.Fatal("timed out waiting for the actor host to become ready") } + // Francis signals host readiness before starting the peer server, so wait for a remote TLS response before a fast test can trigger cleanup + waitForActorHostPeerServer(t, address, errCh) + return h } -// freeLoopbackAddr reserves a free loopback port and returns its address +// waitForActorHostPeerServer waits until the WebTransport listener has passed the startup point that races with shutdown +func waitForActorHostPeerServer(t *testing.T, address string, errCh <-chan error) { + t.Helper() + + // The probe intentionally omits the Francis client certificate because a remote TLS rejection is enough to prove the peer server is accepting connections + //nolint:gosec + tlsConfig := &tls.Config{ + InsecureSkipVerify: true, + NextProtos: []string{http3.NextProtoH3}, + } + deadline := time.Now().Add(10 * time.Second) + + for time.Now().Before(deadline) { + probeCtx, probeCancel := context.WithTimeout(t.Context(), 200*time.Millisecond) + conn, err := quic.DialAddr(probeCtx, address, tlsConfig, &quic.Config{}) + probeCancel() + if conn != nil { + _ = conn.CloseWithError(0, "readiness probe complete") + return + } + + var transportErr *quic.TransportError + if errors.As(err, &transportErr) && transportErr.Remote { + return + } + + select { + case runErr := <-errCh: + t.Fatalf("actor host stopped before its peer server became ready: %v", runErr) + case <-time.After(10 * time.Millisecond): + } + } + + t.Fatalf("timed out waiting for actor host peer server %s", address) +} + +// freeLoopbackUDPAddr reserves a free loopback UDP port and returns its address // The port is released before returning, so the actor host can bind it -func freeLoopbackAddr(t *testing.T) string { +func freeLoopbackUDPAddr(t *testing.T) string { t.Helper() var lc net.ListenConfig - lis, err := lc.Listen(t.Context(), "tcp", "127.0.0.1:0") + lis, err := lc.ListenPacket(t.Context(), "udp", "127.0.0.1:0") require.NoError(t, err) - addr := lis.Addr().String() + addr := lis.LocalAddr().String() err = lis.Close() require.NoError(t, err) diff --git a/backend/resources/migrations/postgres/20260728120000_oidc_client_id_metadata_document.down.sql b/backend/resources/migrations/postgres/20260728120000_oidc_client_id_metadata_document.down.sql new file mode 100644 index 00000000..46e54885 --- /dev/null +++ b/backend/resources/migrations/postgres/20260728120000_oidc_client_id_metadata_document.down.sql @@ -0,0 +1,3 @@ +ALTER TABLE oidc_clients DROP COLUMN client_type; +ALTER TABLE oidc_clients DROP COLUMN metadata_expires_at; +ALTER TABLE oidc_clients DROP COLUMN metadata_grant_types; diff --git a/backend/resources/migrations/postgres/20260728120000_oidc_client_id_metadata_document.up.sql b/backend/resources/migrations/postgres/20260728120000_oidc_client_id_metadata_document.up.sql new file mode 100644 index 00000000..8b79f81d --- /dev/null +++ b/backend/resources/migrations/postgres/20260728120000_oidc_client_id_metadata_document.up.sql @@ -0,0 +1,6 @@ +ALTER TABLE oidc_clients + ADD COLUMN client_type TEXT NOT NULL DEFAULT 'standard'; +ALTER TABLE oidc_clients + ADD COLUMN metadata_expires_at TIMESTAMPTZ; +ALTER TABLE oidc_clients + ADD COLUMN metadata_grant_types JSONB; diff --git a/backend/resources/migrations/sqlite/20260728120000_oidc_client_id_metadata_document.down.sql b/backend/resources/migrations/sqlite/20260728120000_oidc_client_id_metadata_document.down.sql new file mode 100644 index 00000000..4e3f8317 --- /dev/null +++ b/backend/resources/migrations/sqlite/20260728120000_oidc_client_id_metadata_document.down.sql @@ -0,0 +1,9 @@ +PRAGMA foreign_keys= OFF; +BEGIN; + +ALTER TABLE oidc_clients DROP COLUMN client_type; +ALTER TABLE oidc_clients DROP COLUMN metadata_expires_at; +ALTER TABLE oidc_clients DROP COLUMN metadata_grant_types; + +COMMIT; +PRAGMA foreign_keys= ON; diff --git a/backend/resources/migrations/sqlite/20260728120000_oidc_client_id_metadata_document.up.sql b/backend/resources/migrations/sqlite/20260728120000_oidc_client_id_metadata_document.up.sql new file mode 100644 index 00000000..5e988bc4 --- /dev/null +++ b/backend/resources/migrations/sqlite/20260728120000_oidc_client_id_metadata_document.up.sql @@ -0,0 +1,12 @@ +PRAGMA foreign_keys= OFF; +BEGIN; + +ALTER TABLE oidc_clients + ADD COLUMN client_type TEXT NOT NULL DEFAULT 'standard'; +ALTER TABLE oidc_clients + ADD COLUMN metadata_expires_at DATETIME; +ALTER TABLE oidc_clients + ADD COLUMN metadata_grant_types BLOB; + +COMMIT; +PRAGMA foreign_keys= ON; diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 1030fc41..c6d535e7 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -7,6 +7,7 @@ "key": "Key", "value": "Value", "remove_custom_claim": "Remove custom claim", + "remove_url": "Remove URL {identifier}", "add_custom_claim": "Add custom claim", "add_another": "Add another", "select_a_date": "Select a date", @@ -296,6 +297,17 @@ "enabled": "Enabled", "disabled": "Disabled", "oidc_client_updated_successfully": "OIDC client updated successfully", + "client_type": "Type", + "client_type_standard": "Standard", + "client_type_metadata_document": "Metadata Document", + "cimd_client_managed_fields_title": "Some properties are read-only", + "cimd_client_managed_fields_description": "This client is managed through a Client ID Metadata Document (CIMD), so properties supplied by the document can't be edited here.", + "client_id_metadata_documents": "Client ID Metadata Documents", + "client_id_metadata_documents_description": "Client ID Metadata Documents (CIMD) let OAuth clients identify themselves using a URL. No preregistration necessary.", + "cimd_url_allowlist": "Allowed metadata document URLs", + "cimd_url_allowlist_description": "Restrict which client ID metadata document URLs are accepted. Wildcards are supported. An empty list blocks all URLs.", + "refresh": "Refresh", + "oidc_client_metadata_refreshed_successfully": "Client metadata document refreshed successfully", "create_new_client_secret": "Create new client secret", "are_you_sure_you_want_to_create_a_new_client_secret": "Are you sure you want to create a new client secret? The old one will be invalidated.", "generate": "Generate", @@ -563,5 +575,6 @@ "approve": "Approve", "or": "or", "visit_and_enter": "Visit {url} and enter:", + "oidc": "OIDC", "device_login_request_expired": "Your QR-Code has expired. Please start the sign-in process again." } diff --git a/frontend/src/lib/components/confirm-dialog/index.ts b/frontend/src/lib/components/confirm-dialog/index.ts index f6385d38..e59f7e14 100644 --- a/frontend/src/lib/components/confirm-dialog/index.ts +++ b/frontend/src/lib/components/confirm-dialog/index.ts @@ -6,7 +6,7 @@ import ConfirmDialog from './confirm-dialog.svelte'; interface ConfirmDialogState { open: boolean; title: string; - message: string | AnyFormattedMessage; + message: string | AnyFormattedMessage; confirm: { label: string; destructive: boolean; diff --git a/frontend/src/lib/components/form/url-list-input.svelte b/frontend/src/lib/components/form/url-list-input.svelte new file mode 100644 index 00000000..081e0e01 --- /dev/null +++ b/frontend/src/lib/components/form/url-list-input.svelte @@ -0,0 +1,55 @@ + + +
+
+ {#each urls as url, i (i)} +
+ + +
+ {/each} +
+ +
diff --git a/frontend/src/lib/components/ui/card/card-description.svelte b/frontend/src/lib/components/ui/card/card-description.svelte index 293d8250..08333f97 100644 --- a/frontend/src/lib/components/ui/card/card-description.svelte +++ b/frontend/src/lib/components/ui/card/card-description.svelte @@ -1,6 +1,6 @@ + +
+
+ + + + +
+ +
+
+
diff --git a/frontend/src/routes/settings/admin/oidc-clients/+page.svelte b/frontend/src/routes/settings/admin/oidc-clients/+page.svelte index d926701f..e75c0ecd 100644 --- a/frontend/src/routes/settings/admin/oidc-clients/+page.svelte +++ b/frontend/src/routes/settings/admin/oidc-clients/+page.svelte @@ -7,6 +7,7 @@ import appConfigStore from '$lib/stores/application-configuration-store'; import clientSecretStore from '$lib/stores/client-secret-store'; import type { OidcClientCreateWithLogo } from '$lib/types/oidc.type'; + import { encodeClientIdParam } from '$lib/utils/client-id-util'; import { axiosErrorToast } from '$lib/utils/error-util'; import { LucideMinus, ShieldCheck, ShieldPlus } from '@lucide/svelte'; import { toast } from 'svelte-sonner'; @@ -20,6 +21,7 @@ async function createOIDCClient(client: OidcClientCreateWithLogo) { try { + clientSecretStore.clear(); const createdClient = await oidcService.createClient(client); const logoPromise = client.logo @@ -30,9 +32,11 @@ : Promise.resolve(); await Promise.all([logoPromise, darkLogoPromise]); - const clientSecret = await oidcService.createClientSecret(createdClient.id); - clientSecretStore.set(clientSecret); - goto(`/settings/admin/oidc-clients/${createdClient.id}`); + if (!createdClient.isPublic) { + const clientSecret = await oidcService.createClientSecret(createdClient.id); + clientSecretStore.set(clientSecret); + } + goto(`/settings/admin/oidc-clients/${encodeClientIdParam(createdClient.id)}`); toast.success(m.oidc_client_created_successfully()); return true; } catch (e) { diff --git a/frontend/src/routes/settings/admin/oidc-clients/[id]/+page.svelte b/frontend/src/routes/settings/admin/oidc-clients/[id]/+page.svelte index b777e74b..df276cd8 100644 --- a/frontend/src/routes/settings/admin/oidc-clients/[id]/+page.svelte +++ b/frontend/src/routes/settings/admin/oidc-clients/[id]/+page.svelte @@ -232,6 +232,16 @@ {/if} +{#if client.clientType === 'cimd'} + + + {m.cimd_client_managed_fields_title()} + + {m.cimd_client_managed_fields_description()} + + +{/if} +
{ const oidcService = new OidcService(); + const id = decodeClientIdParam(params.id); - const clientPromise = oidcService.getClient(params.id); + const clientPromise = oidcService.getClient(id); const scimServiceProviderPromise = oidcService - .getScimResourceProvider(params.id) + .getScimResourceProvider(id) .then((p) => p) .catch(() => undefined); const oidcConfigurationPromise = fetch('/.well-known/openid-configuration').then( diff --git a/frontend/src/routes/settings/admin/oidc-clients/federated-identities-input.svelte b/frontend/src/routes/settings/admin/oidc-clients/federated-identities-input.svelte index 4ba845d3..30f48fd5 100644 --- a/frontend/src/routes/settings/admin/oidc-clients/federated-identities-input.svelte +++ b/frontend/src/routes/settings/admin/oidc-clients/federated-identities-input.svelte @@ -14,10 +14,12 @@ let { federatedIdentities = $bindable([]), errors, + disabled = false, ...restProps }: HTMLAttributes & { federatedIdentities: OidcClientFederatedIdentity[]; errors?: z.core.$ZodIssue[]; + disabled?: boolean; children?: Snippet; } = $props(); @@ -62,6 +64,7 @@ label={m.federated_client_credentials()} description={m.federated_client_credentials_description()} docsLink="https://pocket-id.org/docs/guides/oidc-client-authentication" + {disabled} >
{#each federatedIdentities as identity, i (identity)} @@ -74,6 +77,7 @@ size="sm" onclick={() => removeFederatedIdentity(i)} aria-label="Remove federated identity" + {disabled} > @@ -89,6 +93,7 @@ value={identity.issuer} oninput={(e) => updateFederatedIdentity(i, 'issuer', e.currentTarget.value)} aria-invalid={!!getFieldError(i, 'issuer')} + {disabled} /> {#if getFieldError(i, 'issuer')} {getFieldError(i, 'issuer')} @@ -103,6 +108,7 @@ value={identity.subject || ''} oninput={(e) => updateFederatedIdentity(i, 'subject', e.currentTarget.value)} aria-invalid={!!getFieldError(i, 'subject')} + {disabled} /> {#if getFieldError(i, 'subject')} {getFieldError(i, 'subject')} @@ -117,6 +123,7 @@ value={identity.audience || ''} oninput={(e) => updateFederatedIdentity(i, 'audience', e.currentTarget.value)} aria-invalid={!!getFieldError(i, 'audience')} + {disabled} /> {#if getFieldError(i, 'audience')} {getFieldError(i, 'audience')} @@ -131,6 +138,7 @@ value={identity.jwks || ''} oninput={(e) => updateFederatedIdentity(i, 'jwks', e.currentTarget.value)} aria-invalid={!!getFieldError(i, 'jwks')} + {disabled} /> {#if getFieldError(i, 'jwks')} {getFieldError(i, 'jwks')} @@ -142,6 +150,7 @@ description={m.replay_protection_description()} checked={identity.replayProtection} onCheckedChange={(checked) => updateFederatedIdentity(i, 'replayProtection', checked)} + {disabled} />
@@ -149,7 +158,14 @@ - - - {/each} - + + {#if error} {error} {/if} - diff --git a/frontend/src/routes/settings/admin/oidc-clients/oidc-client-form.svelte b/frontend/src/routes/settings/admin/oidc-clients/oidc-client-form.svelte index 5fdd59ea..ee52128b 100644 --- a/frontend/src/routes/settings/admin/oidc-clients/oidc-client-form.svelte +++ b/frontend/src/routes/settings/admin/oidc-clients/oidc-client-form.svelte @@ -41,6 +41,7 @@ let darkLogoDataURL: string | null = $state( existingClient?.hasDarkLogo ? cachedOidcClientLogo.getUrl(existingClient!.id, false) : null ); + const isCIMDClient = $derived(existingClient?.clientType === 'cimd'); const client = { id: '', @@ -202,6 +203,7 @@ class="w-full" description={m.client_name_description()} bind:input={$inputs.name} + disabled={isCIMDClient} />
@@ -334,6 +339,7 @@ {/if} diff --git a/frontend/src/routes/settings/admin/oidc-clients/oidc-client-list.svelte b/frontend/src/routes/settings/admin/oidc-clients/oidc-client-list.svelte index 33cae62e..b5c34cca 100644 --- a/frontend/src/routes/settings/admin/oidc-clients/oidc-client-list.svelte +++ b/frontend/src/routes/settings/admin/oidc-clients/oidc-client-list.svelte @@ -11,8 +11,9 @@ } from '$lib/types/advanced-table.type'; import type { OidcClient, OidcClientWithAllowedUserGroupsCount } from '$lib/types/oidc.type'; import { cachedOidcClientLogo } from '$lib/utils/cached-image-util'; + import { encodeClientIdParam } from '$lib/utils/client-id-util'; import { axiosErrorToast } from '$lib/utils/error-util'; - import { LucidePencil, LucideTrash } from '@lucide/svelte'; + import { LucidePencil, LucideRefreshCcw, LucideTrash } from '@lucide/svelte'; import { mode } from 'mode-watcher'; import { toast } from 'svelte-sonner'; @@ -30,6 +31,11 @@ { label: m.no(), value: false } ]; + const clientTypeFilterValues = [ + { label: m.client_type_standard(), value: 'standard' }, + { label: m.client_type_metadata_document(), value: 'cimd' } + ]; + const columns: AdvancedTableColumn[] = [ { label: 'ID', column: 'id', hidden: true }, { label: m.logo(), key: 'logo', cell: LogoCell }, @@ -46,6 +52,14 @@ sortable: true, filterableValues: booleanFilterValues }, + { + label: m.client_type(), + column: 'clientType', + sortable: true, + filterableValues: clientTypeFilterValues, + value: (item) => + item.clientType === 'cimd' ? m.client_type_metadata_document() : m.client_type_standard() + }, { label: m.pkce(), column: 'pkceEnabled', @@ -79,12 +93,18 @@ } ]; - const actions: CreateAdvancedTableActions = () => [ + const actions: CreateAdvancedTableActions = (client) => [ { label: m.edit(), primary: true, icon: LucidePencil, - onClick: (client) => goto(`/settings/admin/oidc-clients/${client.id}`) + onClick: (client) => goto(`/settings/admin/oidc-clients/${encodeClientIdParam(client.id)}`) + }, + { + label: m.refresh(), + icon: LucideRefreshCcw, + hidden: client.clientType !== 'cimd', + onClick: (client) => refreshClient(client) }, { label: m.delete(), @@ -94,6 +114,16 @@ } ]; + async function refreshClient(client: OidcClient) { + try { + await oidcService.refreshClient(client.id); + await refresh(); + toast.success(m.oidc_client_metadata_refreshed_successfully()); + } catch (e) { + axiosErrorToast(e); + } + } + async function deleteClient(client: OidcClient) { openConfirmDialog({ title: m.delete_name({ name: client.name }), diff --git a/frontend/src/routes/settings/apps/authorized-oidc-client-card.svelte b/frontend/src/routes/settings/apps/authorized-oidc-client-card.svelte index dd44c07d..62936fcb 100644 --- a/frontend/src/routes/settings/apps/authorized-oidc-client-card.svelte +++ b/frontend/src/routes/settings/apps/authorized-oidc-client-card.svelte @@ -9,6 +9,7 @@ import userStore from '$lib/stores/user-store'; import type { AccessibleOidcClient, OidcClientMetaData } from '$lib/types/oidc.type'; import { cachedApplicationLogo, cachedOidcClientLogo } from '$lib/utils/cached-image-util'; + import { encodeClientIdParam } from '$lib/utils/client-id-util'; import { LucideBan, LucideEllipsisVertical, @@ -79,7 +80,8 @@ {#if $userStore?.isAdmin} goto(`/settings/admin/oidc-clients/${client.id}`)} + onclick={() => + goto(`/settings/admin/oidc-clients/${encodeClientIdParam(client.id)}`)} > {m.edit()} {/if} diff --git a/tests/resources/export/database.json b/tests/resources/export/database.json index 8c699ee3..eaab7596 100644 --- a/tests/resources/export/database.json +++ b/tests/resources/export/database.json @@ -1,6 +1,6 @@ { "provider": "sqlite", - "version": 20260727120000, + "version": 20260728120000, "tableOrder": [ "users", "user_groups", @@ -87,6 +87,7 @@ "oidc_clients": [ { "callback_urls": "WyJodHRwOi8vbmV4dGNsb3VkLmxvY2FsaG9zdC9hdXRoL2NhbGxiYWNrIl0=", + "client_type": "standard", "created_at": "2025-11-25T12:39:02Z", "created_by_id": "f4b89dc2-62fb-46bf-9f5f-c34f4eafe93e", "credentials": "e30=", @@ -97,6 +98,8 @@ "is_public": false, "launch_url": "https://nextcloud.local", "logout_callback_urls": "WyJodHRwOi8vbmV4dGNsb3VkLmxvY2FsaG9zdC9hdXRoL2xvZ291dC9jYWxsYmFjayJd", + "metadata_expires_at": null, + "metadata_grant_types": "bnVsbA==", "name": "Nextcloud", "description": "This is an example description for Nextcloud", "pkce_enabled": false, @@ -108,6 +111,7 @@ }, { "callback_urls": "WyJodHRwOi8vaW1taWNoLmxvY2FsaG9zdC9hdXRoL2NhbGxiYWNrIl0=", + "client_type": "standard", "created_at": "2025-11-25T12:39:02Z", "created_by_id": "1cd19686-f9a6-43f4-a41f-14a0bf5b4036", "credentials": "e30=", @@ -118,6 +122,8 @@ "is_public": false, "launch_url": null, "logout_callback_urls": "bnVsbA==", + "metadata_expires_at": null, + "metadata_grant_types": "bnVsbA==", "name": "Immich", "description": "", "pkce_enabled": false, @@ -129,6 +135,7 @@ }, { "callback_urls": "WyJodHRwOi8vdGFpbHNjYWxlLmxvY2FsaG9zdC9hdXRoL2NhbGxiYWNrIl0=", + "client_type": "standard", "created_at": "2025-11-25T12:39:02Z", "created_by_id": "f4b89dc2-62fb-46bf-9f5f-c34f4eafe93e", "credentials": "e30=", @@ -139,6 +146,8 @@ "is_public": false, "launch_url": null, "logout_callback_urls": "WyJodHRwOi8vdGFpbHNjYWxlLmxvY2FsaG9zdC9hdXRoL2xvZ291dC9jYWxsYmFjayJd", + "metadata_expires_at": null, + "metadata_grant_types": "bnVsbA==", "name": "Tailscale", "description": "", "pkce_enabled": false, @@ -150,6 +159,7 @@ }, { "callback_urls": "WyJodHRwOi8vZmVkZXJhdGVkLmxvY2FsaG9zdC9hdXRoL2NhbGxiYWNrIl0=", + "client_type": "standard", "created_at": "2025-11-25T12:39:02Z", "created_by_id": "1cd19686-f9a6-43f4-a41f-14a0bf5b4036", "credentials": "eyJmZWRlcmF0ZWRJZGVudGl0aWVzIjpbeyJpc3N1ZXIiOiJodHRwczovL2V4dGVybmFsLWlkcC5sb2NhbCIsInN1YmplY3QiOiJjNDgyMzJmZi1mZjY1LTQ1ZWQtYWU5Ni03YWZhOGE5YjQ0M2IiLCJhdWRpZW5jZSI6ImFwaTovL1BvY2tldElEIiwiandrcyI6Imh0dHA6Ly9sb2NhbGhvc3Q6MTQxMS9hcGkvZXh0ZXJuYWxpZHAvandrcy5qc29uIn1dfQ==", @@ -160,6 +170,8 @@ "is_public": false, "launch_url": null, "logout_callback_urls": "bnVsbA==", + "metadata_expires_at": null, + "metadata_grant_types": "bnVsbA==", "name": "Federated", "description": "", "pkce_enabled": false, @@ -171,6 +183,7 @@ }, { "callback_urls": "WyJodHRwOi8vc2NpbWNsaWVudC5sb2NhbGhvc3QvYXV0aC9jYWxsYmFjayJd", + "client_type": "standard", "created_at": "2025-11-25T12:39:02Z", "created_by_id": "f4b89dc2-62fb-46bf-9f5f-c34f4eafe93e", "dark_image_type": null, @@ -180,6 +193,8 @@ "is_public": false, "launch_url": null, "logout_callback_urls": "bnVsbA==", + "metadata_expires_at": null, + "metadata_grant_types": "bnVsbA==", "name": "SCIM Client", "description": "", "pkce_enabled": false, @@ -191,6 +206,7 @@ }, { "callback_urls": "WyJodHRwOi8vcGFyLWNsaWVudC5sb2NhbGhvc3QvYXV0aC9jYWxsYmFjayJd", + "client_type": "standard", "created_at": "2025-11-25T12:39:02Z", "created_by_id": "f4b89dc2-62fb-46bf-9f5f-c34f4eafe93e", "credentials": "e30=", @@ -201,6 +217,8 @@ "is_public": false, "launch_url": null, "logout_callback_urls": "bnVsbA==", + "metadata_expires_at": null, + "metadata_grant_types": "bnVsbA==", "name": "PAR Test Client", "description": "", "pkce_enabled": false, @@ -212,6 +230,7 @@ }, { "callback_urls": "WyJodHRwOi8vc2tpcC1jb25zZW50LmxvY2FsaG9zdC9hdXRoL2NhbGxiYWNrIl0=", + "client_type": "standard", "created_at": "2025-11-25T12:39:02Z", "created_by_id": "f4b89dc2-62fb-46bf-9f5f-c34f4eafe93e", "credentials": "e30=", @@ -222,6 +241,8 @@ "is_public": false, "launch_url": null, "logout_callback_urls": "bnVsbA==", + "metadata_expires_at": null, + "metadata_grant_types": "bnVsbA==", "name": "Skip Consent Client", "description": "", "pkce_enabled": false,