diff --git a/backend/go.mod b/backend/go.mod index 1d8267a2..afe0c4bc 100644 --- a/backend/go.mod +++ b/backend/go.mod @@ -248,4 +248,4 @@ require ( modernc.org/memory v1.11.0 // indirect ) -replace github.com/ory/fosite => github.com/pocket-id/fosite v0.0.0-20260702114848-b55fb7985fde +replace github.com/ory/fosite => github.com/pocket-id/fosite v0.0.0-20260706141717-ee376ba4821b diff --git a/backend/go.sum b/backend/go.sum index 284e950a..b0e02011 100644 --- a/backend/go.sum +++ b/backend/go.sum @@ -426,8 +426,8 @@ github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/pocket-id/fosite v0.0.0-20260702114848-b55fb7985fde h1:cJZogAqQhjgDL0hj4hhJ2l5ppLvK8Vx38bJusOrHPyU= -github.com/pocket-id/fosite v0.0.0-20260702114848-b55fb7985fde/go.mod h1:KeQ7tTIBm3DyeBnKcKLnPbSdrd6ttM6w3TD3yy9x8rM= +github.com/pocket-id/fosite v0.0.0-20260706141717-ee376ba4821b h1:/y/hIU6OzNiFWFd2eXSAR4wgbgU592TP5hXuszoPPjY= +github.com/pocket-id/fosite v0.0.0-20260706141717-ee376ba4821b/go.mod h1:KeQ7tTIBm3DyeBnKcKLnPbSdrd6ttM6w3TD3yy9x8rM= github.com/prometheus/client_golang v1.23.2 h1:Je96obch5RDVy3FDMndoUsjAhG5Edi49h0RJWRi/o0o= github.com/prometheus/client_golang v1.23.2/go.mod h1:Tb1a6LWHB3/SPIzCoaDXI4I8UHKeFTEQ1YCr+0Gyqmg= github.com/prometheus/client_model v0.6.2 h1:oBsgwpGs7iVziMvrGhE53c/GrLUsZdHnqNwqPLxwZyk= diff --git a/backend/internal/api/dto.go b/backend/internal/api/dto.go new file mode 100644 index 00000000..98163bba --- /dev/null +++ b/backend/internal/api/dto.go @@ -0,0 +1,66 @@ +package api + +import ( + datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" +) + +// apiResponseDto is the full representation of an API including its permissions +type apiResponseDto struct { + ID string `json:"id"` + Name string `json:"name"` + Resource string `json:"resource"` + CreatedAt datatype.DateTime `json:"createdAt"` + Permissions []apiPermissionResponseDto `json:"permissions"` +} + +type apiPermissionResponseDto struct { + ID string `json:"id"` + Key string `json:"key"` + Name string `json:"name"` + Description *string `json:"description,omitempty"` +} + +// apiListItemDto is the lightweight representation used in list responses +type apiListItemDto struct { + ID string `json:"id"` + Name string `json:"name"` + Resource string `json:"resource"` + CreatedAt datatype.DateTime `json:"createdAt"` + PermissionCount int `json:"permissionCount"` +} + +// apiCreateDto is the payload for creating an API +// The resource identifier is only accepted here because changing it later would invalidate every token already minted for the API +type apiCreateDto struct { + Name string `json:"name" binding:"required,min=1,max=50" unorm:"nfc"` + Resource string `json:"resource" binding:"required,resource_uri,max=350" unorm:"nfc"` +} + +// apiUpdateDto is the payload for updating an API +// The resource identifier is intentionally not updatable +type apiUpdateDto struct { + Name string `json:"name" binding:"required,min=1,max=50" unorm:"nfc"` +} + +type apiPermissionInputDto struct { + Key string `json:"key" binding:"required,min=1,max=128" unorm:"nfc"` + Name string `json:"name" binding:"required,min=1,max=50" unorm:"nfc"` + Description *string `json:"description" binding:"omitempty,max=200"` +} + +// apiPermissionsUpdateDto replaces the full permission set of an API +type apiPermissionsUpdateDto struct { + Permissions []apiPermissionInputDto `json:"permissions" binding:"omitempty,dive"` +} + +// clientApiAccessDto is the set of API permissions a client is allowed to request, split by subject type +// User-delegated permissions may be requested on behalf of a signed-in user, client permissions may be obtained by the client itself through the client credentials grant +type clientApiAccessDto struct { + UserDelegatedPermissionIDs []string `json:"userDelegatedPermissionIds"` + ClientPermissionIDs []string `json:"clientPermissionIds"` +} + +type clientApiAccessUpdateDto struct { + UserDelegatedPermissionIDs []string `json:"userDelegatedPermissionIds" binding:"omitempty,dive,required"` + ClientPermissionIDs []string `json:"clientPermissionIds" binding:"omitempty,dive,required"` +} diff --git a/backend/internal/api/handler.go b/backend/internal/api/handler.go new file mode 100644 index 00000000..1da33ccd --- /dev/null +++ b/backend/internal/api/handler.go @@ -0,0 +1,236 @@ +package api + +import ( + "net/http" + + "github.com/gin-gonic/gin" + + "github.com/pocket-id/pocket-id/backend/internal/dto" + "github.com/pocket-id/pocket-id/backend/internal/utils" +) + +type handler struct { + service *Service +} + +func newHandler(service *Service) *handler { + return &handler{service: service} +} + +// list godoc +// @Summary List APIs +// @Description Get a paginated list of APIs with optional search and sorting +// @Tags APIs +// @Produce json +// @Param search query string false "Search term to filter APIs by name or resource" +// @Param pagination[page] query int false "Page number for pagination" default(1) +// @Param pagination[limit] query int false "Number of items per page" default(20) +// @Param sort[column] query string false "Column to sort by" +// @Param sort[direction] query string false "Sort direction (asc or desc)" default("asc") +// @Success 200 {object} dto.Paginated[apiListItemDto] +// @Router /api/apis [get] +func (h *handler) list(c *gin.Context) { + search := c.Query("search") + listRequestOptions := utils.ParseListRequestOptions(c) + + apis, pagination, err := h.service.List(c.Request.Context(), search, listRequestOptions) + if err != nil { + _ = c.Error(err) + return + } + + items := make([]apiListItemDto, len(apis)) + for i, api := range apis { + var item apiListItemDto + if err := dto.MapStruct(api, &item); err != nil { + _ = c.Error(err) + return + } + item.Resource = api.Audience + item.PermissionCount = len(api.Permissions) + items[i] = item + } + + c.JSON(http.StatusOK, dto.Paginated[apiListItemDto]{ + Data: items, + Pagination: pagination, + }) +} + +// get godoc +// @Summary Get API by ID +// @Description Retrieve a single API including its permissions +// @Tags APIs +// @Produce json +// @Param id path string true "API ID" +// @Success 200 {object} apiResponseDto +// @Router /api/apis/{id} [get] +func (h *handler) get(c *gin.Context) { + api, err := h.service.Get(c.Request.Context(), nil, c.Param("id")) + if err != nil { + _ = c.Error(err) + return + } + + h.respond(c, http.StatusOK, api) +} + +// create godoc +// @Summary Create API +// @Description Create a new API resource server +// @Tags APIs +// @Accept json +// @Produce json +// @Param api body apiCreateDto true "API information" +// @Success 201 {object} apiResponseDto "Created API" +// @Router /api/apis [post] +func (h *handler) create(c *gin.Context) { + var input apiCreateDto + if err := dto.ShouldBindWithNormalizedJSON(c, &input); err != nil { + _ = c.Error(err) + return + } + + api, err := h.service.Create(c.Request.Context(), input) + if err != nil { + _ = c.Error(err) + return + } + + h.respond(c, http.StatusCreated, api) +} + +// update godoc +// @Summary Update API +// @Description Update an existing API by ID +// @Tags APIs +// @Accept json +// @Produce json +// @Param id path string true "API ID" +// @Param api body apiUpdateDto true "API information" +// @Success 200 {object} apiResponseDto "Updated API" +// @Router /api/apis/{id} [put] +func (h *handler) update(c *gin.Context) { + var input apiUpdateDto + if err := dto.ShouldBindWithNormalizedJSON(c, &input); err != nil { + _ = c.Error(err) + return + } + + api, err := h.service.Update(c.Request.Context(), c.Param("id"), input) + if err != nil { + _ = c.Error(err) + return + } + + h.respond(c, http.StatusOK, api) +} + +// delete godoc +// @Summary Delete API +// @Description Delete an API by ID +// @Tags APIs +// @Param id path string true "API ID" +// @Success 204 "No Content" +// @Router /api/apis/{id} [delete] +func (h *handler) delete(c *gin.Context) { + if err := h.service.Delete(c.Request.Context(), c.Param("id")); err != nil { + _ = c.Error(err) + return + } + + c.Status(http.StatusNoContent) +} + +// updatePermissions godoc +// @Summary Update API permissions +// @Description Replace the full set of permissions for an API +// @Tags APIs +// @Accept json +// @Produce json +// @Param id path string true "API ID" +// @Param permissions body apiPermissionsUpdateDto true "Permissions to set" +// @Success 200 {object} apiResponseDto "Updated API" +// @Router /api/apis/{id}/permissions [put] +func (h *handler) updatePermissions(c *gin.Context) { + var input apiPermissionsUpdateDto + if err := dto.ShouldBindWithNormalizedJSON(c, &input); err != nil { + _ = c.Error(err) + return + } + + api, err := h.service.UpdatePermissions(c.Request.Context(), c.Param("id"), input) + if err != nil { + _ = c.Error(err) + return + } + + h.respond(c, http.StatusOK, api) +} + +// getClientAccess godoc +// @Summary Get client API access +// @Description Get the API permissions an OIDC client is allowed to request, split into user-delegated and client (machine-to-machine) access +// @Tags APIs +// @Produce json +// @Param clientId path string true "OIDC Client ID" +// @Success 200 {object} clientApiAccessDto +// @Router /api/api-access/{clientId} [get] +func (h *handler) getClientAccess(c *gin.Context) { + access, err := h.service.GetClientAPIAccess(c.Request.Context(), c.Param("clientId")) + if err != nil { + _ = c.Error(err) + return + } + + c.JSON(http.StatusOK, newClientApiAccessDto(access)) +} + +// updateClientAccess godoc +// @Summary Update client API access +// @Description Replace the API permissions an OIDC client is allowed to request, split into user-delegated and client (machine-to-machine) access +// @Tags APIs +// @Accept json +// @Produce json +// @Param clientId path string true "OIDC Client ID" +// @Param access body clientApiAccessUpdateDto true "Allowed permission IDs per subject type" +// @Success 200 {object} clientApiAccessDto +// @Router /api/api-access/{clientId} [put] +func (h *handler) updateClientAccess(c *gin.Context) { + var input clientApiAccessUpdateDto + err := c.ShouldBindJSON(&input) + if err != nil { + _ = c.Error(err) + return + } + + applied, err := h.service.SetClientAPIAccess(c.Request.Context(), c.Param("clientId"), ClientAPIAccess(input)) + if err != nil { + _ = c.Error(err) + return + } + + c.JSON(http.StatusOK, newClientApiAccessDto(applied)) +} + +// newClientApiAccessDto always serializes both permission lists as arrays rather than null +func newClientApiAccessDto(access ClientAPIAccess) clientApiAccessDto { + dto := clientApiAccessDto(access) + if dto.UserDelegatedPermissionIDs == nil { + dto.UserDelegatedPermissionIDs = []string{} + } + if dto.ClientPermissionIDs == nil { + dto.ClientPermissionIDs = []string{} + } + return dto +} + +func (h *handler) respond(c *gin.Context, status int, api API) { + var responseDto apiResponseDto + if err := dto.MapStruct(api, &responseDto); err != nil { + _ = c.Error(err) + return + } + responseDto.Resource = api.Audience + c.JSON(status, responseDto) +} diff --git a/backend/internal/api/models.go b/backend/internal/api/models.go new file mode 100644 index 00000000..9235a5cc --- /dev/null +++ b/backend/internal/api/models.go @@ -0,0 +1,38 @@ +package api + +import ( + "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/oidc" +) + +type API struct { + model.Base + + Name string `sortable:"true"` + Audience string `sortable:"true"` + UpdatedAt *datatype.DateTime + + Permissions []Permission `gorm:"foreignKey:APIID;references:ID;constraint:OnDelete:CASCADE"` +} + +type Permission struct { + model.Base + + APIID string `gorm:"column:api_id"` + Key string `sortable:"true"` + Name string + Description *string +} + +func (Permission) TableName() string { return "api_permissions" } + +type OidcClientAllowedAPIPermission struct { + OidcClientID string + APIPermissionID string + SubjectType oidc.SubjectType +} + +func (OidcClientAllowedAPIPermission) TableName() string { + return "oidc_clients_allowed_api_permissions" +} diff --git a/backend/internal/api/module.go b/backend/internal/api/module.go new file mode 100644 index 00000000..82e3b177 --- /dev/null +++ b/backend/internal/api/module.go @@ -0,0 +1,77 @@ +package api + +import ( + "context" + + "github.com/gin-gonic/gin" + "gorm.io/gorm" + + "github.com/pocket-id/pocket-id/backend/internal/oidc" +) + +type Dependencies struct { + DB *gorm.DB + // Issuer is the OpenID Provider issuer URL, reserved so a custom API cannot claim it as its audience + Issuer string +} + +type Module struct { + service *Service + handler *handler +} + +func New(deps Dependencies) *Module { + service := newService(deps.DB, deps.Issuer) + return &Module{ + service: service, + handler: newHandler(service), + } +} + +// ClientAPIScopes implements the OIDC module's APIAccessProvider interface +func (m *Module) ClientAPIScopes(ctx context.Context, tx *gorm.DB, clientID string) (scopes []string, audiences []string, err error) { + return m.service.ClientAPIScopesAndAudiences(ctx, tx, clientID) +} + +// AllowedScopesForAudience implements the OIDC module's APIAccessProvider interface +func (m *Module) AllowedScopesForAudience(ctx context.Context, clientID, audience string, subjectType oidc.SubjectType) (scopes []string, apiExists bool, err error) { + return m.service.AllowedScopesForAudience(ctx, clientID, audience, subjectType) +} + +// DescribePermissions implements the OIDC module's APIAccessProvider interface +func (m *Module) DescribePermissions(ctx context.Context, audience string, keys []string) ([]oidc.PermissionInfo, error) { + permissions, err := m.service.DescribePermissions(ctx, audience, keys) + if err != nil { + return nil, err + } + + infos := make([]oidc.PermissionInfo, len(permissions)) + for i, permission := range permissions { + description := "" + if permission.Description != nil { + description = *permission.Description + } + infos[i] = oidc.PermissionInfo{Key: permission.Key, Name: permission.Name, Description: description} + } + + return infos, nil +} + +// RegisterRoutes mounts the admin CRUD endpoints +// adminAuth is passed in as a gin handler so the module does not import internal/middleware +func (m *Module) RegisterRoutes(apiGroup *gin.RouterGroup, adminAuth gin.HandlerFunc) { + apis := apiGroup.Group("/apis") + apis.Use(adminAuth) + apis.GET("", m.handler.list) + apis.POST("", m.handler.create) + apis.GET("/:id", m.handler.get) + apis.PUT("/:id", m.handler.update) + apis.DELETE("/:id", m.handler.delete) + apis.PUT("/:id/permissions", m.handler.updatePermissions) + + // The per-client API-access allow-list lives on a separate path so it does not collide with the /apis/:id wildcard + access := apiGroup.Group("/api-access") + access.Use(adminAuth) + access.GET("/:clientId", m.handler.getClientAccess) + access.PUT("/:clientId", m.handler.updateClientAccess) +} diff --git a/backend/internal/api/service.go b/backend/internal/api/service.go new file mode 100644 index 00000000..c1015173 --- /dev/null +++ b/backend/internal/api/service.go @@ -0,0 +1,480 @@ +package api + +import ( + "context" + "errors" + "fmt" + "strings" + "time" + + "github.com/ory/fosite" + "github.com/pocket-id/pocket-id/backend/internal/common" + "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/oidc" + "github.com/pocket-id/pocket-id/backend/internal/utils" + "gorm.io/gorm" + "gorm.io/gorm/clause" +) + +// isPermissionKeyReserved reports whether the key is a scope or claim name owned by Pocket ID's built-in identity layer +// A custom API permission must not reuse one, otherwise its scope string would collide with a standard OIDC scope or claim +func isPermissionKeyReserved(key string) bool { + switch strings.ToLower(key) { + case "openid", "profile", "email", "email_verified", "groups", "offline_access": + return true + default: + return false + } +} + +// isValidPermissionKey reports whether the key consists only of RFC 6749 scope-token characters, which are printable ASCII without space, double-quote or backslash +// This keeps a key safe as a space-delimited value in the token scope claim and free of the control character used to qualify consent records +func isValidPermissionKey(key string) bool { + return fosite.IsValidScopeToken(key) +} + +// Service holds the business logic for managing APIs and their permissions +type Service struct { + db *gorm.DB + issuer string +} + +func newService(db *gorm.DB, issuer string) *Service { + return &Service{db: db, issuer: issuer} +} + +// isIssuerAudience reports whether the audience refers to Pocket ID itself (the issuer) +// A custom API must not claim the issuer as its audience, otherwise its tokens would be indistinguishable from Pocket ID's own identity tokens +func isIssuerAudience(audience, issuer string) bool { + return issuer != "" && strings.ToLower(strings.TrimRight(audience, "/")) == issuer +} + +func (s *Service) List(ctx context.Context, search string, listRequestOptions utils.ListRequestOptions) (apis []API, response utils.PaginationResponse, err error) { + query := s.db. + WithContext(ctx). + Preload("Permissions"). + Model(&API{}) + + if listRequestOptions.Sort.Column == "resource" { + listRequestOptions.Sort.Column = "audience" + } + + if search != "" { + like := "%" + search + "%" + query = query.Where("name LIKE ? OR audience LIKE ?", like, like) + } + + response, err = utils.PaginateFilterAndSort(listRequestOptions, query, &apis) + return apis, response, err +} + +// Get loads an API and its permissions +func (s *Service) Get(ctx context.Context, tx *gorm.DB, id string) (api API, err error) { + query := s.db.WithContext(ctx) + if tx != nil { + query = tx.WithContext(ctx).Clauses(clause.Locking{Strength: "UPDATE"}) + } + + err = query. + Preload("Permissions"). + Where("id = ?", id). + First(&api). + Error + return api, err +} + +func (s *Service) Create(ctx context.Context, input apiCreateDto) (api API, err error) { + // Reject the issuer as an audience so a custom API cannot impersonate Pocket ID's own identity tokens + if isIssuerAudience(input.Resource, s.issuer) { + return API{}, &common.ValidationError{Message: "the resource is reserved by Pocket ID and cannot be used for a custom API"} + } + + api = API{ + Name: input.Name, + Audience: input.Resource, + } + + err = s.db.WithContext(ctx).Create(&api).Error + if err != nil { + if errors.Is(err, gorm.ErrDuplicatedKey) { + return API{}, &common.AlreadyInUseError{Property: "resource"} + } + return API{}, err + } + + return api, nil +} + +func (s *Service) Update(ctx context.Context, id string, input apiUpdateDto) (api API, err error) { + tx := s.db.Begin() + defer func() { + tx.Rollback() + }() + + api, err = s.Get(ctx, tx, id) + if err != nil { + return API{}, err + } + + api.Name = input.Name + api.UpdatedAt = new(datatype.DateTime(time.Now())) + + err = tx.WithContext(ctx).Save(&api).Error + if err != nil { + return API{}, err + } + + if err = tx.Commit().Error; err != nil { + return API{}, err + } + + return api, nil +} + +func (s *Service) Delete(ctx context.Context, id string) error { + tx := s.db.Begin() + defer func() { + tx.Rollback() + }() + + api, err := s.Get(ctx, tx, id) + if err != nil { + return err + } + + if err = s.deletePermissions(ctx, tx, collectIDs(api.Permissions)); err != nil { + return err + } + + if err = tx.WithContext(ctx).Delete(&API{}, "id = ?", id).Error; err != nil { + return err + } + + return tx.Commit().Error +} + +// UpdatePermissions replaces the full permission set of an API, matching existing permissions by key +// Unchanged keys keep their grants, removed keys and their client grants are deleted, and new keys are inserted +func (s *Service) UpdatePermissions(ctx context.Context, id string, input apiPermissionsUpdateDto) (api API, err error) { + tx := s.db.Begin() + defer func() { + tx.Rollback() + }() + + api, err = s.Get(ctx, tx, id) + if err != nil { + return API{}, err + } + + // Reject keys with invalid characters, that collide with Pocket ID's reserved scopes and claims, or that repeat within the request before persisting anything + // A duplicate key would otherwise be silently coalesced last-wins into the map below, dropping a row behind a 200 + seen := make(map[string]struct{}, len(input.Permissions)) + for _, permission := range input.Permissions { + if !isValidPermissionKey(permission.Key) { + return API{}, &common.ValidationError{Message: fmt.Sprintf("the permission key %q contains invalid characters", permission.Key)} + } + if isPermissionKeyReserved(permission.Key) { + return API{}, &common.ValidationError{Message: fmt.Sprintf("the permission key %q is reserved by Pocket ID", permission.Key)} + } + _, ok := seen[permission.Key] + if ok { + return API{}, &common.ValidationError{Message: fmt.Sprintf("the permission key %q is listed more than once", permission.Key)} + } + seen[permission.Key] = struct{}{} + } + + existing := make(map[string]Permission, len(api.Permissions)) + for _, p := range api.Permissions { + existing[p.Key] = p + } + + wanted := make(map[string]apiPermissionInputDto, len(input.Permissions)) + var removedIDs []string + for _, in := range input.Permissions { + wanted[in.Key] = in + } + + // Delete permissions whose key is no longer wanted + for key, p := range existing { + if _, ok := wanted[key]; !ok { + removedIDs = append(removedIDs, p.ID) + } + } + if err = s.deletePermissions(ctx, tx, removedIDs); err != nil { + return API{}, err + } + + // Insert new keys and update the display fields of existing ones + for key, in := range wanted { + if cur, ok := existing[key]; ok { + err = tx.WithContext(ctx). + Model(&Permission{}). + Where("id = ?", cur.ID). + Updates(map[string]any{"name": in.Name, "description": in.Description}). + Error + if err != nil { + return API{}, err + } + continue + } + + newPermission := Permission{ + APIID: api.ID, + Key: in.Key, + Name: in.Name, + Description: in.Description, + } + if err = tx.WithContext(ctx).Create(&newPermission).Error; err != nil { + return API{}, err + } + } + + err = tx.WithContext(ctx). + Model(&API{}). + Where("id = ?", api.ID). + Update("updated_at", new(datatype.DateTime(time.Now()))). + Error + if err != nil { + return API{}, err + } + + api, err = s.Get(ctx, tx, id) + if err != nil { + return API{}, err + } + + if err = tx.Commit().Error; err != nil { + return API{}, err + } + + return api, nil +} + +// ClientAPIAccess is the set of API permissions granted to a client, split by the subject the resulting tokens act for +// User-delegated permissions may be requested on behalf of a signed-in user, client permissions may be obtained by the client itself through the client credentials grant +type ClientAPIAccess struct { + UserDelegatedPermissionIDs []string + ClientPermissionIDs []string +} + +// GetClientAPIAccess returns the API permissions a client is allowed to request, split by subject type +// Only custom-API permissions are tracked here because the identity scopes are freely requestable by every client +func (s *Service) GetClientAPIAccess(ctx context.Context, clientID string) (access ClientAPIAccess, err error) { + var rows []OidcClientAllowedAPIPermission + err = s.db.WithContext(ctx). + Where("oidc_client_id = ?", clientID). + Find(&rows). + Error + if err != nil { + return ClientAPIAccess{}, err + } + + for _, row := range rows { + switch row.SubjectType { + case oidc.SubjectTypeClient: + access.ClientPermissionIDs = append(access.ClientPermissionIDs, row.APIPermissionID) + case oidc.SubjectTypeUser: + access.UserDelegatedPermissionIDs = append(access.UserDelegatedPermissionIDs, row.APIPermissionID) + default: + // Nop - ignore + } + } + + return access, nil +} + +// SetClientAPIAccess replaces the client's API-access grants for both subject types with the given permission IDs +// Unknown permission IDs are ignored +// It returns the access that was actually applied +func (s *Service) SetClientAPIAccess(ctx context.Context, clientID string, access ClientAPIAccess) (applied ClientAPIAccess, err error) { + tx := s.db.Begin() + defer func() { + tx.Rollback() + }() + + // Ensure the client exists so callers get a 404 for an unknown client + var client model.OidcClient + if err = tx.WithContext(ctx).Select("id").Where("id = ?", clientID).First(&client).Error; err != nil { + return ClientAPIAccess{}, err + } + + applied.UserDelegatedPermissionIDs, err = s.filterAssignablePermissionIDs(ctx, tx, access.UserDelegatedPermissionIDs) + if err != nil { + return ClientAPIAccess{}, err + } + applied.ClientPermissionIDs, err = s.filterAssignablePermissionIDs(ctx, tx, access.ClientPermissionIDs) + if err != nil { + return ClientAPIAccess{}, err + } + + // Replace the grants for this client + err = tx.WithContext(ctx). + Where("oidc_client_id = ?", clientID). + Delete(&OidcClientAllowedAPIPermission{}). + Error + if err != nil { + return ClientAPIAccess{}, err + } + + rows := make([]OidcClientAllowedAPIPermission, 0, len(applied.UserDelegatedPermissionIDs)+len(applied.ClientPermissionIDs)) + for _, permissionID := range applied.UserDelegatedPermissionIDs { + rows = append(rows, OidcClientAllowedAPIPermission{OidcClientID: clientID, APIPermissionID: permissionID, SubjectType: oidc.SubjectTypeUser}) + } + for _, permissionID := range applied.ClientPermissionIDs { + rows = append(rows, OidcClientAllowedAPIPermission{OidcClientID: clientID, APIPermissionID: permissionID, SubjectType: oidc.SubjectTypeClient}) + } + if len(rows) > 0 { + if err = tx.WithContext(ctx).Create(&rows).Error; err != nil { + return ClientAPIAccess{}, err + } + } + + if err = tx.Commit().Error; err != nil { + return ClientAPIAccess{}, err + } + + return applied, nil +} + +// ClientAPIScopesAndAudiences returns the permission keys a client may request and the distinct audiences of the custom APIs those permissions belong to, across both subject types +// The OIDC module uses this to widen fosite's scope and audience validation for the client; the per-flow subject-type enforcement happens when the resource is resolved +func (s *Service) ClientAPIScopesAndAudiences(ctx context.Context, tx *gorm.DB, clientID string) (scopes []string, audiences []string, err error) { + if tx == nil { + tx = s.db + } + + var rows []struct { + Key string + Audience string + } + err = tx.WithContext(ctx). + Table("oidc_clients_allowed_api_permissions AS g"). + Select("api_permissions.key AS key, apis.audience AS audience"). + Joins("JOIN api_permissions ON api_permissions.id = g.api_permission_id"). + Joins("JOIN apis ON apis.id = api_permissions.api_id"). + Where("g.oidc_client_id = ?", clientID). + Scan(&rows). + Error + if err != nil { + return nil, nil, err + } + + scopeSeen := make(map[string]struct{}, len(rows)) + audienceSeen := make(map[string]struct{}, len(rows)) + scopes = make([]string, 0, len(rows)) + audiences = make([]string, 0, len(rows)) + for _, row := range rows { + if _, ok := scopeSeen[row.Key]; !ok { + scopeSeen[row.Key] = struct{}{} + scopes = append(scopes, row.Key) + } + if _, ok := audienceSeen[row.Audience]; !ok { + audienceSeen[row.Audience] = struct{}{} + audiences = append(audiences, row.Audience) + } + } + + return scopes, audiences, nil +} + +// AllowedScopesForAudience returns the permission keys the client is allowed for the API identified by the given audience and subject type, plus whether such an API exists +func (s *Service) AllowedScopesForAudience(ctx context.Context, clientID, audience string, subjectType oidc.SubjectType) (scopes []string, apiExists bool, err error) { + var api API + err = s.db.WithContext(ctx). + Select("id"). + Where("audience = ?", audience). + First(&api). + Error + if errors.Is(err, gorm.ErrRecordNotFound) { + return nil, false, nil + } + if err != nil { + return nil, false, err + } + + err = s.db.WithContext(ctx). + Table("api_permissions"). + Select("api_permissions.key"). + Joins("JOIN oidc_clients_allowed_api_permissions g ON g.api_permission_id = api_permissions.id AND g.oidc_client_id = ? AND g.subject_type = ?", clientID, subjectType). + Where("api_permissions.api_id = ?", api.ID). + Pluck("api_permissions.key", &scopes). + Error + if err != nil { + return nil, true, err + } + + return scopes, true, nil +} + +// DescribePermissions returns the permission rows of the API identified by the given audience whose key is in keys +// The consent screen uses these to show friendly names instead of raw scope keys +func (s *Service) DescribePermissions(ctx context.Context, audience string, keys []string) ([]Permission, error) { + if len(keys) == 0 { + return nil, nil + } + + var permissions []Permission + err := s.db.WithContext(ctx). + Model(&Permission{}). + Joins("JOIN apis ON apis.id = api_permissions.api_id"). + Where("apis.audience = ? AND api_permissions.key IN ?", audience, keys). + Find(&permissions). + Error + if err != nil { + return nil, err + } + + return permissions, nil +} + +// filterAssignablePermissionIDs returns the subset of the given permission IDs that exist +func (s *Service) filterAssignablePermissionIDs(ctx context.Context, tx *gorm.DB, permissionIDs []string) ([]string, error) { + if len(permissionIDs) == 0 { + return nil, nil + } + + var valid []string + err := tx.WithContext(ctx). + Model(&Permission{}). + Where("id IN ?", permissionIDs). + Pluck("id", &valid). + Error + if err != nil { + return nil, err + } + + return valid, nil +} + +// deletePermissions removes permissions by ID along with any client allow-list grants that reference them +// The explicit grant delete keeps this correct even when the database does not enforce ON DELETE CASCADE at runtime +func (s *Service) deletePermissions(ctx context.Context, tx *gorm.DB, permissionIDs []string) error { + if tx == nil { + tx = s.db + } + if len(permissionIDs) == 0 { + return nil + } + + err := tx.WithContext(ctx). + Where("api_permission_id IN ?", permissionIDs). + Delete(&OidcClientAllowedAPIPermission{}). + Error + if err != nil { + return err + } + + return tx.WithContext(ctx). + Where("id IN ?", permissionIDs). + Delete(&Permission{}). + Error +} + +func collectIDs(permissions []Permission) []string { + ids := make([]string, len(permissions)) + for i, p := range permissions { + ids[i] = p.ID + } + return ids +} diff --git a/backend/internal/api/service_test.go b/backend/internal/api/service_test.go new file mode 100644 index 00000000..cec48c88 --- /dev/null +++ b/backend/internal/api/service_test.go @@ -0,0 +1,283 @@ +package api + +import ( + "testing" + + "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/model" + "github.com/pocket-id/pocket-id/backend/internal/oidc" + testutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing" +) + +func TestAPICrudAndPermissionDiff(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + svc := New(Dependencies{DB: db}).service + + created, err := svc.Create(t.Context(), apiCreateDto{Name: "Orders API", Resource: "https://api.orders.example.com"}) + require.NoError(t, err) + assert.NotEmpty(t, created.ID) + + // The resource is unique. + _, err = svc.Create(t.Context(), apiCreateDto{Name: "Dup", Resource: "https://api.orders.example.com"}) + require.ErrorIs(t, err, &common.AlreadyInUseError{}) + + desc := "Read orders" + updated, err := svc.UpdatePermissions(t.Context(), created.ID, apiPermissionsUpdateDto{Permissions: []apiPermissionInputDto{ + {Key: "read:orders", Name: "Read orders", Description: &desc}, + {Key: "write:orders", Name: "Write orders"}, + }}) + require.NoError(t, err) + assert.Len(t, updated.Permissions, 2) + + // Grant a client the read:orders permission for both subject types, then remove that permission + // and confirm the grants are cleaned up while write:orders (and its key) survives. + readPerm := findPermission(updated, "read:orders") + require.NotNil(t, readPerm) + require.NoError(t, db.Create(&model.OidcClient{Base: model.Base{ID: "client-1"}, Name: "Client 1"}).Error) + require.NoError(t, db.Create(&OidcClientAllowedAPIPermission{OidcClientID: "client-1", APIPermissionID: readPerm.ID, SubjectType: oidc.SubjectTypeUser}).Error) + require.NoError(t, db.Create(&OidcClientAllowedAPIPermission{OidcClientID: "client-1", APIPermissionID: readPerm.ID, SubjectType: oidc.SubjectTypeClient}).Error) + + updated, err = svc.UpdatePermissions(t.Context(), created.ID, apiPermissionsUpdateDto{Permissions: []apiPermissionInputDto{ + {Key: "write:orders", Name: "Write orders (renamed)"}, + }}) + require.NoError(t, err) + require.Len(t, updated.Permissions, 1) + assert.Equal(t, "write:orders", updated.Permissions[0].Key) + assert.Equal(t, "Write orders (renamed)", updated.Permissions[0].Name) + + var grantCount int64 + require.NoError(t, db.Model(&OidcClientAllowedAPIPermission{}).Where("api_permission_id = ?", readPerm.ID).Count(&grantCount).Error) + assert.Equal(t, int64(0), grantCount) + + renamed, err := svc.Update(t.Context(), created.ID, apiUpdateDto{Name: "Orders"}) + require.NoError(t, err) + assert.Equal(t, "Orders", renamed.Name) + require.NotNil(t, renamed.UpdatedAt) + + require.NoError(t, svc.Delete(t.Context(), created.ID)) + _, err = svc.Get(t.Context(), nil, created.ID) + require.Error(t, err) +} + +func TestClientApiAccessAllowList(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + svc := New(Dependencies{DB: db}).service + + require.NoError(t, db.Create(&model.OidcClient{Base: model.Base{ID: "client-1"}, Name: "Client 1"}).Error) + + orders, err := svc.Create(t.Context(), apiCreateDto{Name: "Orders", Resource: "https://api.orders.example.com"}) + require.NoError(t, err) + orders, err = svc.UpdatePermissions(t.Context(), orders.ID, apiPermissionsUpdateDto{Permissions: []apiPermissionInputDto{ + {Key: "read:orders", Name: "Read"}, + {Key: "write:orders", Name: "Write"}, + }}) + require.NoError(t, err) + readID := findPermission(orders, "read:orders").ID + writeID := findPermission(orders, "write:orders").ID + + // Unknown IDs are filtered out, and the subject types are stored independently. + applied, err := svc.SetClientAPIAccess(t.Context(), "client-1", ClientAPIAccess{ + UserDelegatedPermissionIDs: []string{readID, "does-not-exist"}, + ClientPermissionIDs: []string{writeID, "does-not-exist"}, + }) + require.NoError(t, err) + assert.ElementsMatch(t, []string{readID}, applied.UserDelegatedPermissionIDs) + assert.ElementsMatch(t, []string{writeID}, applied.ClientPermissionIDs) + + got, err := svc.GetClientAPIAccess(t.Context(), "client-1") + require.NoError(t, err) + assert.ElementsMatch(t, []string{readID}, got.UserDelegatedPermissionIDs) + assert.ElementsMatch(t, []string{writeID}, got.ClientPermissionIDs) + + // The same permission can be granted for both subject types, and both sets are fully replaced on each call. + _, err = svc.SetClientAPIAccess(t.Context(), "client-1", ClientAPIAccess{ + UserDelegatedPermissionIDs: []string{readID, writeID}, + ClientPermissionIDs: []string{readID}, + }) + require.NoError(t, err) + got, err = svc.GetClientAPIAccess(t.Context(), "client-1") + require.NoError(t, err) + assert.ElementsMatch(t, []string{readID, writeID}, got.UserDelegatedPermissionIDs) + assert.ElementsMatch(t, []string{readID}, got.ClientPermissionIDs) + + // Clearing one subject type leaves the other untouched. + _, err = svc.SetClientAPIAccess(t.Context(), "client-1", ClientAPIAccess{ClientPermissionIDs: []string{readID}}) + require.NoError(t, err) + got, err = svc.GetClientAPIAccess(t.Context(), "client-1") + require.NoError(t, err) + assert.Empty(t, got.UserDelegatedPermissionIDs) + assert.ElementsMatch(t, []string{readID}, got.ClientPermissionIDs) + + // Clearing everything. + _, err = svc.SetClientAPIAccess(t.Context(), "client-1", ClientAPIAccess{}) + require.NoError(t, err) + got, err = svc.GetClientAPIAccess(t.Context(), "client-1") + require.NoError(t, err) + assert.Empty(t, got.UserDelegatedPermissionIDs) + assert.Empty(t, got.ClientPermissionIDs) + + // An unknown client is rejected (surfaces as 404 at the HTTP layer). + _, err = svc.SetClientAPIAccess(t.Context(), "nope", ClientAPIAccess{UserDelegatedPermissionIDs: []string{readID}}) + require.Error(t, err) +} + +// TestAllowedScopesForAudienceFiltersBySubjectType guards that the scopes resolved for a flow +// only come from the grants of that flow's subject type. +func TestAllowedScopesForAudienceFiltersBySubjectType(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + svc := New(Dependencies{DB: db}).service + + require.NoError(t, db.Create(&model.OidcClient{Base: model.Base{ID: "client-1"}, Name: "Client 1"}).Error) + + orders, err := svc.Create(t.Context(), apiCreateDto{Name: "Orders", Resource: "https://api.orders.example.com"}) + require.NoError(t, err) + orders, err = svc.UpdatePermissions(t.Context(), orders.ID, apiPermissionsUpdateDto{Permissions: []apiPermissionInputDto{ + {Key: "read:orders", Name: "Read"}, + {Key: "write:orders", Name: "Write"}, + }}) + require.NoError(t, err) + readID := findPermission(orders, "read:orders").ID + writeID := findPermission(orders, "write:orders").ID + + _, err = svc.SetClientAPIAccess(t.Context(), "client-1", ClientAPIAccess{ + UserDelegatedPermissionIDs: []string{readID}, + ClientPermissionIDs: []string{writeID}, + }) + require.NoError(t, err) + + userScopes, exists, err := svc.AllowedScopesForAudience(t.Context(), "client-1", "https://api.orders.example.com", oidc.SubjectTypeUser) + require.NoError(t, err) + require.True(t, exists) + assert.ElementsMatch(t, []string{"read:orders"}, userScopes) + + clientScopes, exists, err := svc.AllowedScopesForAudience(t.Context(), "client-1", "https://api.orders.example.com", oidc.SubjectTypeClient) + require.NoError(t, err) + require.True(t, exists) + assert.ElementsMatch(t, []string{"write:orders"}, clientScopes) + + // The fosite widening still sees the union of both subject types. + scopes, audiences, err := svc.ClientAPIScopesAndAudiences(t.Context(), nil, "client-1") + require.NoError(t, err) + assert.ElementsMatch(t, []string{"read:orders", "write:orders"}, scopes) + assert.ElementsMatch(t, []string{"https://api.orders.example.com"}, audiences) +} + +func TestUpdatePermissionsRejectsReservedKeys(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + svc := New(Dependencies{DB: db}).service + + orders, err := svc.Create(t.Context(), apiCreateDto{Name: "Orders", Resource: "https://api.orders.example.com"}) + require.NoError(t, err) + + for _, key := range []string{"openid", "profile", "email", "email_verified", "groups", "offline_access", "Email"} { + _, err := svc.UpdatePermissions(t.Context(), orders.ID, apiPermissionsUpdateDto{Permissions: []apiPermissionInputDto{ + {Key: key, Name: "Reserved"}, + }}) + require.Error(t, err, "key %q must be rejected", key) + var validationErr *common.ValidationError + require.ErrorAs(t, err, &validationErr) + } +} + +func TestUpdatePermissionsRejectsDuplicateKeys(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + svc := New(Dependencies{DB: db}).service + + orders, err := svc.Create(t.Context(), apiCreateDto{Name: "Orders", Resource: "https://api.orders.example.com"}) + require.NoError(t, err) + + // Two rows with the same key must be rejected rather than silently coalesced last-wins + _, err = svc.UpdatePermissions(t.Context(), orders.ID, apiPermissionsUpdateDto{Permissions: []apiPermissionInputDto{ + {Key: "read:orders", Name: "Read"}, + {Key: "read:orders", Name: "Read again"}, + }}) + require.Error(t, err) + var validationErr *common.ValidationError + require.ErrorAs(t, err, &validationErr) +} + +func TestUpdatePermissionsRejectsInvalidKeyCharacters(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + svc := New(Dependencies{DB: db}).service + + orders, err := svc.Create(t.Context(), apiCreateDto{Name: "Orders", Resource: "https://api.orders.example.com"}) + require.NoError(t, err) + + // A space corrupts the space-delimited scope claim, and the unit separator is the consent delimiter + for _, key := range []string{"read orders", "read\x1forders", "read\"orders", "bad\\key", "tab\tkey"} { + _, err := svc.UpdatePermissions(t.Context(), orders.ID, apiPermissionsUpdateDto{Permissions: []apiPermissionInputDto{ + {Key: key, Name: "Invalid"}, + }}) + require.Error(t, err, "key %q must be rejected", key) + var validationErr *common.ValidationError + require.ErrorAs(t, err, &validationErr) + } + + // A valid scope-token key is accepted + _, err = svc.UpdatePermissions(t.Context(), orders.ID, apiPermissionsUpdateDto{Permissions: []apiPermissionInputDto{ + {Key: "read:orders", Name: "Read"}, + }}) + require.NoError(t, err) +} + +func TestCreateRejectsIssuerResource(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + const issuer = "https://id.example.com" + svc := New(Dependencies{DB: db, Issuer: issuer}).service + + // The issuer itself, a trailing-slash variant, and a different-cased variant are all reserved + for _, resource := range []string{issuer, issuer + "/", "https://ID.example.com"} { + _, err := svc.Create(t.Context(), apiCreateDto{Name: "Reserved", Resource: resource}) + require.Error(t, err, "resource %q must be rejected", resource) + var validationErr *common.ValidationError + require.ErrorAs(t, err, &validationErr) + } + + // A normal resource is accepted + _, err := svc.Create(t.Context(), apiCreateDto{Name: "Orders", Resource: "https://api.orders.example.com"}) + require.NoError(t, err) +} + +func TestCreateAcceptsAbsoluteResourceURIs(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + svc := New(Dependencies{DB: db}).service + + for _, resource := range []string{"https://api.orders.example.com", "api://PocketID", "urn:my-app"} { + _, err := svc.Create(t.Context(), apiCreateDto{Name: "Orders", Resource: resource}) + require.NoError(t, err, "resource %q must be accepted", resource) + } +} + +func TestDescribePermissions(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + svc := New(Dependencies{DB: db}).service + + orders, err := svc.Create(t.Context(), apiCreateDto{Name: "Orders", Resource: "https://api.orders.example.com"}) + require.NoError(t, err) + desc := "Read orders" + _, err = svc.UpdatePermissions(t.Context(), orders.ID, apiPermissionsUpdateDto{Permissions: []apiPermissionInputDto{ + {Key: "read:orders", Name: "Read orders", Description: &desc}, + {Key: "write:orders", Name: "Write orders"}, + }}) + require.NoError(t, err) + + infos, err := svc.DescribePermissions(t.Context(), "https://api.orders.example.com", []string{"read:orders", "unknown"}) + require.NoError(t, err) + require.Len(t, infos, 1) + assert.Equal(t, "read:orders", infos[0].Key) + assert.Equal(t, "Read orders", infos[0].Name) + require.NotNil(t, infos[0].Description) + assert.Equal(t, "Read orders", *infos[0].Description) +} + +func findPermission(api API, key string) *Permission { + for i := range api.Permissions { + if api.Permissions[i].Key == key { + return &api.Permissions[i] + } + } + return nil +} diff --git a/backend/internal/bootstrap/router_bootstrap.go b/backend/internal/bootstrap/router_bootstrap.go index 32b2f9c5..c8cb5c81 100644 --- a/backend/internal/bootstrap/router_bootstrap.go +++ b/backend/internal/bootstrap/router_bootstrap.go @@ -157,6 +157,7 @@ func registerRoutes(r *gin.Engine, db *gorm.DB, svc *services, rateLimitServices controller.NewAppImagesController(apiGroup, authMiddleware, svc.appImagesService) controller.NewAuditLogController(apiGroup, svc.auditLogService, authMiddleware) controller.NewUserGroupController(apiGroup, authMiddleware, svc.userGroupService) + svc.apiModule.RegisterRoutes(apiGroup, authMiddleware.Add()) controller.NewCustomClaimController(apiGroup, authMiddleware, svc.customClaimService) controller.NewVersionController(apiGroup, authMiddleware, svc.versionService) controller.NewScimController(apiGroup, authMiddleware, svc.scimService) diff --git a/backend/internal/bootstrap/services_bootstrap.go b/backend/internal/bootstrap/services_bootstrap.go index dce5ab38..3a670b9a 100644 --- a/backend/internal/bootstrap/services_bootstrap.go +++ b/backend/internal/bootstrap/services_bootstrap.go @@ -9,6 +9,7 @@ import ( "github.com/pocket-id/pocket-id/backend/internal/job" "gorm.io/gorm" + "github.com/pocket-id/pocket-id/backend/internal/api" "github.com/pocket-id/pocket-id/backend/internal/common" "github.com/pocket-id/pocket-id/backend/internal/oidc" "github.com/pocket-id/pocket-id/backend/internal/service" @@ -39,6 +40,7 @@ type services struct { oidcModule *oidc.Module webauthnModule *webauthn.Module userSignUpModule *usersignup.Module + apiModule *api.Module } // Initializes all services @@ -80,6 +82,8 @@ func initServices(ctx context.Context, db *gorm.DB, httpClient *http.Client, ima svc.scimService = service.NewScimService(db, scheduler, httpClient) + svc.apiModule = api.New(api.Dependencies{DB: db, Issuer: common.EnvConfig.AppURL}) + svc.oidcModule, err = oidc.New(ctx, oidc.Dependencies{ DB: db, HTTPClient: httpClient, @@ -92,6 +96,7 @@ func initServices(ctx context.Context, db *gorm.DB, httpClient *http.Client, ima CustomClaims: svc.customClaimService, Reauth: svc.webauthnModule, AuditLog: svc.auditLogService, + APIAccess: svc.apiModule, }) if err != nil { return nil, fmt.Errorf("failed to create OIDC module: %w", err) diff --git a/backend/internal/dto/oidc_dto.go b/backend/internal/dto/oidc_dto.go index 427c0dca..34c4c963 100644 --- a/backend/internal/dto/oidc_dto.go +++ b/backend/internal/dto/oidc_dto.go @@ -89,8 +89,15 @@ type OidcDeviceAuthorizationResponseDto struct { Interval int `json:"interval"` } +type ScopeInfoDto struct { + Key string `json:"key"` + Name string `json:"name"` + Description string `json:"description,omitempty"` +} + type DeviceCodeInfoDto struct { Scope []string `json:"scope"` + ScopeInfo []ScopeInfoDto `json:"scopeInfo"` AuthorizationRequired bool `json:"authorizationRequired"` ReauthenticationRequired bool `json:"reauthenticationRequired"` Client OidcClientMetaDataDto `json:"client"` diff --git a/backend/internal/dto/validations.go b/backend/internal/dto/validations.go index 9d99d6cf..0f4ab9f6 100644 --- a/backend/internal/dto/validations.go +++ b/backend/internal/dto/validations.go @@ -6,6 +6,7 @@ import ( "strings" "time" + "github.com/ory/fosite" "github.com/pocket-id/pocket-id/backend/internal/utils" "github.com/gin-gonic/gin/binding" @@ -47,6 +48,9 @@ func init() { "callback_url_pattern": func(fl validator.FieldLevel) bool { return ValidateCallbackURLPattern(fl.Field().String()) }, + "resource_uri": func(fl validator.FieldLevel) bool { + return ValidateResourceURI(fl.Field().String()) + }, } for k, v := range validators { err := engine.RegisterValidation(k, v) @@ -66,6 +70,27 @@ func ValidateClientID(clientID string) bool { return validateClientIDRegex.MatchString(clientID) } +// isActiveContentScheme reports whether the URL scheme can carry executable content, so it must never be accepted where a URL might later be rendered as a link +func isActiveContentScheme(scheme string) bool { + switch strings.ToLower(scheme) { + case "javascript", "data": + return true + default: + return false + } +} + +// ValidateResourceURI validates RFC 8707 resource identifiers +func ValidateResourceURI(str string) bool { + if !fosite.IsValidResourceIndicatorURI(str) { + return false + } + + // Reject active-content schemes so a resource identifier can never carry executable content if it is ever surfaced as a link + u, _ := url.Parse(str) + return !isActiveContentScheme(u.Scheme) +} + // ValidateCallbackURL validates the input callback URL func ValidateCallbackURL(str string) bool { // Ensure the URL is a valid one and that the protocol is not "javascript:" or "data:" @@ -74,12 +99,7 @@ func ValidateCallbackURL(str string) bool { return false } - switch strings.ToLower(u.Scheme) { - case "javascript", "data": - return false - default: - return true - } + return !isActiveContentScheme(u.Scheme) } // ValidateCallbackURLPattern validates callback URL patterns, with support for wildcards. diff --git a/backend/internal/dto/validations_test.go b/backend/internal/dto/validations_test.go index 4d557ad1..c752743e 100644 --- a/backend/internal/dto/validations_test.go +++ b/backend/internal/dto/validations_test.go @@ -58,6 +58,35 @@ func TestValidateClientID(t *testing.T) { } } +func TestValidateResourceURI(t *testing.T) { + tests := []struct { + name string + input string + expected bool + }{ + {"valid https URI", "https://api.example.com", true}, + {"valid custom scheme URI", "api://PocketID", true}, + {"valid URN", "urn:my-app", true}, + {"valid query component", "https://api.example.com?tenant=1", true}, + {"invalid relative path", "/foo", false}, + {"invalid plain string", "foo", false}, + {"invalid fragment", "https://api.example.com#tokens", false}, + {"invalid empty fragment", "https://api.example.com#", false}, + {"invalid unescaped path space", "https://api.example.com/a b", false}, + {"invalid unescaped opaque space", "urn:my app", false}, + {"invalid malformed URI", "http://[::1", false}, + {"invalid javascript scheme", "javascript:alert(1)", false}, + {"invalid data scheme", "data:text/html,", false}, + {"empty", "", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, ValidateResourceURI(tt.input)) + }) + } +} + func TestValidateCallbackURL(t *testing.T) { tests := []struct { name string diff --git a/backend/internal/middleware/error_handler.go b/backend/internal/middleware/error_handler.go index 7df172dc..9c34b6af 100644 --- a/backend/internal/middleware/error_handler.go +++ b/backend/internal/middleware/error_handler.go @@ -105,6 +105,8 @@ func handleValidationError(validationErrors validator.ValidationErrors) string { errorMessage = fmt.Sprintf("%s must only contain letters, numbers, underscores, dots, hyphens, and '@' symbols and not start or end with a special character", fieldName) case "url": errorMessage = fmt.Sprintf("%s must be a valid URL", fieldName) + case "resource_uri": + errorMessage = fmt.Sprintf("%s must be an absolute URI without whitespace or a fragment", fieldName) case "min": errorMessage = fmt.Sprintf("%s must be at least %s characters long", fieldName, ve.Param()) case "max": diff --git a/backend/internal/oidc/access_token_scope.go b/backend/internal/oidc/access_token_scope.go new file mode 100644 index 00000000..54212624 --- /dev/null +++ b/backend/internal/oidc/access_token_scope.go @@ -0,0 +1,59 @@ +package oidc + +import ( + "context" + "slices" + + "github.com/ory/fosite" + fositeoauth2 "github.com/ory/fosite/handler/oauth2" +) + +// isIdentityScope reports whether the scope is an OIDC identity scope whose presence lets a token be presented to Pocket ID's own identity endpoints such as /userinfo +// offline_access is deliberately excluded: it only requests a refresh token and is not tied to any resource server +func isIdentityScope(scope string) bool { + return scope != "offline_access" && isStandardScope(scope) +} + +// hasIdentityScope reports whether any of the granted scopes is an OIDC identity scope +func hasIdentityScope(scopes fosite.Arguments) bool { + return slices.ContainsFunc(scopes, isIdentityScope) +} + +// withIdentityAudience returns a view of the requester whose granted audience additionally includes the issuer when the token carries an identity scope +// This lets an access token that was granted an identity scope be presented to Pocket ID's own identity endpoints such as /userinfo, even when it was also audienced to a custom API +// The issuer is added only to the materialized access token and never to the underlying grant, so a refresh does not have to re-whitelist the issuer against the client and a machine token whose identity scopes were stripped never receives it +func withIdentityAudience(requester fosite.Requester, issuer string) fosite.Requester { + if issuer == "" || !hasIdentityScope(requester.GetGrantedScopes()) { + return requester + } + + granted := requester.GetGrantedAudience() + if granted.Has(issuer) { + return requester + } + + audience := make(fosite.Arguments, 0, len(granted)+1) + audience = append(audience, granted...) + audience = append(audience, issuer) + return identityAudienceRequester{Requester: requester, grantedAudience: audience} +} + +// identityAudienceRequester overrides only the granted audience of the wrapped requester +type identityAudienceRequester struct { + fosite.Requester + grantedAudience fosite.Arguments +} + +func (r identityAudienceRequester) GetGrantedAudience() fosite.Arguments { + return r.grantedAudience +} + +// identityAudienceAccessTokenStrategy wraps the access token strategy so an access token granted an identity scope also lists the issuer in its audience, keeping the self-contained JWT consistent with what is persisted for introspection and userinfo +type identityAudienceAccessTokenStrategy struct { + fositeoauth2.CoreStrategy + issuer string +} + +func (s identityAudienceAccessTokenStrategy) GenerateAccessToken(ctx context.Context, requester fosite.Requester) (string, string, error) { + return s.CoreStrategy.GenerateAccessToken(ctx, withIdentityAudience(requester, s.issuer)) +} diff --git a/backend/internal/oidc/access_token_scope_test.go b/backend/internal/oidc/access_token_scope_test.go new file mode 100644 index 00000000..cd2d716a --- /dev/null +++ b/backend/internal/oidc/access_token_scope_test.go @@ -0,0 +1,67 @@ +package oidc + +import ( + "testing" + + "github.com/ory/fosite" + "github.com/stretchr/testify/assert" + + "github.com/pocket-id/pocket-id/backend/internal/model" +) + +func newScopeTestRequester(clientID string, granted fosite.Arguments, audience fosite.Arguments) *fosite.Request { + request := fosite.NewRequest() + request.Client = Client{ + OidcClient: model.OidcClient{ + Base: model.Base{ID: clientID}, + }, + } + request.GrantedScope = granted + request.GrantedAudience = audience + return request +} + +func TestWithIdentityAudienceAddsIssuerForIdentityScopes(t *testing.T) { + const issuer = "https://issuer.example.com" + + // A token granted an identity scope alongside a custom API also lists the issuer so it can be presented to /userinfo + apiRequest := newScopeTestRequester("client-1", + fosite.Arguments{"openid", "read:orders"}, + fosite.Arguments{"https://api.orders.example.com"}, + ) + assert.ElementsMatch(t, + fosite.Arguments{"https://api.orders.example.com", issuer}, + withIdentityAudience(apiRequest, issuer).GetGrantedAudience(), + ) + + // A plain login token gains the issuer audience alongside the client it was bound to + loginRequest := newScopeTestRequester("client-1", + fosite.Arguments{"openid", "profile", "email"}, + fosite.Arguments{"client-1"}, + ) + assert.ElementsMatch(t, + fosite.Arguments{"client-1", issuer}, + withIdentityAudience(loginRequest, issuer).GetGrantedAudience(), + ) +} + +func TestWithIdentityAudienceLeavesNonIdentityTokensUntouched(t *testing.T) { + const issuer = "https://issuer.example.com" + + // A token audienced only to a custom API with no identity scope never gains the issuer, so it cannot reach /userinfo + apiOnly := newScopeTestRequester("client-1", + fosite.Arguments{"read:orders"}, + fosite.Arguments{"https://api.orders.example.com"}, + ) + assert.Equal(t, fosite.Arguments{"https://api.orders.example.com"}, withIdentityAudience(apiOnly, issuer).GetGrantedAudience()) + + // offline_access alone is not an identity scope, so it does not add the issuer either + offlineOnly := newScopeTestRequester("client-1", + fosite.Arguments{"offline_access", "read:orders"}, + fosite.Arguments{"https://api.orders.example.com"}, + ) + assert.Equal(t, fosite.Arguments{"https://api.orders.example.com"}, withIdentityAudience(offlineOnly, issuer).GetGrantedAudience()) + + // An empty issuer disables the behavior entirely, leaving the audience untouched + assert.Equal(t, fosite.Arguments{"client-1"}, withIdentityAudience(newScopeTestRequester("client-1", fosite.Arguments{"openid"}, fosite.Arguments{"client-1"}), "").GetGrantedAudience()) +} diff --git a/backend/internal/oidc/api_resource.go b/backend/internal/oidc/api_resource.go new file mode 100644 index 00000000..3efbd561 --- /dev/null +++ b/backend/internal/oidc/api_resource.go @@ -0,0 +1,89 @@ +package oidc + +import ( + "context" + "slices" + + "github.com/ory/fosite" + "gorm.io/gorm" +) + +type SubjectType = fosite.ResourceIndicatorSubjectType + +const ( + // SubjectTypeUser covers user-delegated access: every flow whose access token acts on behalf of an end user + SubjectTypeUser = fosite.ResourceOwnerSubject + // SubjectTypeClient covers client access: the client credentials grant, where the client acts as itself without a user + SubjectTypeClient = fosite.ClientSubject +) + +var standardScopes = fosite.Arguments{"openid", "profile", "email", "groups", "offline_access"} + +func isStandardScope(scope string) bool { + return slices.Contains(standardScopes, scope) +} + +// PermissionInfo is the display information for an API permission used to show friendly names and descriptions on the consent screen +type PermissionInfo struct { + Key string + Name string + Description string +} + +// APIAccessProvider is implemented by the api feature module +// It lets the OIDC module widen per-client scope and audience validation and resolve RFC 8707 resources to the permission keys a client may be granted +type APIAccessProvider interface { + // ClientAPIScopes returns the custom-API permission keys and the distinct API audiences a client is allowed to request across all subject types + ClientAPIScopes(ctx context.Context, tx *gorm.DB, clientID string) (scopes []string, audiences []string, err error) + // AllowedScopesForAudience returns the permission keys the client is allowed for the API identified by the given audience and subject type, and whether such an API exists + AllowedScopesForAudience(ctx context.Context, clientID, audience string, subjectType SubjectType) (scopes []string, apiExists bool, err error) + // DescribePermissions returns the display information for the given permission keys of the API identified by audience + // Unknown keys are omitted + DescribePermissions(ctx context.Context, audience string, keys []string) ([]PermissionInfo, error) +} + +type resourceAccessProvider struct { + provider APIAccessProvider +} + +func fositeResourceAccess(provider APIAccessProvider) fosite.ResourceIndicatorAccessProvider { + if provider == nil { + return nil + } + return resourceAccessProvider{provider: provider} +} + +func (p resourceAccessProvider) AllowedScopesForResource(ctx context.Context, client fosite.Client, resource string, subjectType fosite.ResourceIndicatorSubjectType) (fosite.Arguments, bool, error) { + scopes, exists, err := p.provider.AllowedScopesForAudience(ctx, client.GetID(), resource, subjectType) + return scopes, exists, err +} + +// resolveResource maps an RFC 8707 resource, which may be empty, to the audience to stamp on the issued token and the subset of requestedScopes that may be granted +// An empty resource is a plain login token bound to the requesting client and yields only identity scopes +// The subject type selects which of the client's grants apply: user-delegated flows only see user grants, the client credentials grant only sees client grants +func resolveResource(ctx context.Context, provider APIAccessProvider, clientID, resource string, requestedScopes []string, subjectType SubjectType) (audience string, grantedScopes []string, err error) { + grant, err := fosite.ResolveResourceIndicatorGrant(ctx, fositeResourceAccess(provider), &fosite.DefaultClient{ID: clientID}, resource, requestedScopes, standardScopes, subjectType) + if err != nil { + return "", nil, err + } + + return grant.Audience, grant.Scopes, nil +} + +// consentScopeKey qualifies a custom-API scope by its audience so the same permission key on two different APIs is consented to separately +// The unit-separator delimiter is collision-free because audiences are validated as URIs and permission keys are restricted to RFC 6749 scope-token characters, so neither can contain it +// Standard identity scopes stay bare for backward compatibility with existing consents +func consentScopeKey(audience, scope string) string { + if isStandardScope(scope) { + return scope + } + return audience + "\x1f" + scope +} + +func consentScopeKeys(audience string, scopes []string) []string { + keys := make([]string, len(scopes)) + for i, scope := range scopes { + keys[i] = consentScopeKey(audience, scope) + } + return keys +} diff --git a/backend/internal/oidc/api_resource_test.go b/backend/internal/oidc/api_resource_test.go new file mode 100644 index 00000000..a28b6014 --- /dev/null +++ b/backend/internal/oidc/api_resource_test.go @@ -0,0 +1,174 @@ +package oidc + +import ( + "context" + "slices" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/gorm" +) + +// fakeAPIAccess implements APIAccessProvider from an audience -> subject type -> allowed-scopes map. +// An audience present in the map exists as an API even when a subject type has no grants. +type fakeAPIAccess struct { + allowed map[string]map[SubjectType][]string +} + +// userAccess builds a fakeAPIAccess with only user-delegated grants, for tests that don't care about the split. +func userAccess(allowed map[string][]string) fakeAPIAccess { + f := fakeAPIAccess{allowed: map[string]map[SubjectType][]string{}} + for audience, scopes := range allowed { + f.allowed[audience] = map[SubjectType][]string{SubjectTypeUser: scopes} + } + return f +} + +func (f fakeAPIAccess) ClientAPIScopes(_ context.Context, _ *gorm.DB, _ string) ([]string, []string, error) { + seen := map[string]struct{}{} + var scopes, audiences []string + for audience, bySubject := range f.allowed { + audiences = append(audiences, audience) + for _, scopeKeys := range bySubject { + for _, key := range scopeKeys { + if _, ok := seen[key]; !ok { + seen[key] = struct{}{} + scopes = append(scopes, key) + } + } + } + } + return scopes, audiences, nil +} + +func (f fakeAPIAccess) AllowedScopesForAudience(_ context.Context, _ string, audience string, subjectType SubjectType) ([]string, bool, error) { + bySubject, exists := f.allowed[audience] + if !exists { + return nil, false, nil + } + return bySubject[subjectType], true, nil +} + +func (f fakeAPIAccess) DescribePermissions(_ context.Context, audience string, keys []string) ([]PermissionInfo, error) { + bySubject, ok := f.allowed[audience] + if !ok { + return nil, nil + } + var infos []PermissionInfo + for _, key := range keys { + for _, allowed := range bySubject { + if slices.Contains(allowed, key) { + infos = append(infos, PermissionInfo{Key: key, Name: key}) + break + } + } + } + return infos, nil +} + +func TestResolveResourceDefaultIsLoginToken(t *testing.T) { + // With no resource the token is a plain login token audienced to the requesting client + audience, granted, err := resolveResource(t.Context(), nil, "client-1", "", []string{"openid", "profile"}, SubjectTypeUser) + require.NoError(t, err) + assert.Equal(t, "client-1", audience) + assert.Equal(t, []string{"openid", "profile"}, granted) +} + +func TestResolveResourceRejectsCustomScopeWithoutResource(t *testing.T) { + // Requesting a custom scope without targeting its API must be rejected, not dropped. + _, _, err := resolveResource(t.Context(), nil, "client-1", "", []string{"openid", "read:orders"}, SubjectTypeUser) + require.Error(t, err) +} + +func TestResolveResourceCustomAPIGrantsValidScopes(t *testing.T) { + provider := userAccess(map[string][]string{ + "https://api.orders.example.com": {"read:orders", "write:orders"}, + }) + + audience, granted, err := resolveResource(t.Context(), provider, "client-1", "https://api.orders.example.com", []string{"openid", "read:orders"}, SubjectTypeUser) + require.NoError(t, err) + assert.Equal(t, "https://api.orders.example.com", audience) + // openid stays (identity, for the ID token); read:orders is allowed for this API. + assert.ElementsMatch(t, []string{"openid", "read:orders"}, granted) +} + +func TestResolveResourceRejectsScopeFromAnotherAPI(t *testing.T) { + provider := userAccess(map[string][]string{ + "https://api.orders.example.com": {"read:orders", "write:orders"}, + }) + // write:billing belongs to a different API than the one targeted -> rejected. + _, _, err := resolveResource(t.Context(), provider, "client-1", "https://api.orders.example.com", []string{"openid", "write:billing"}, SubjectTypeUser) + require.Error(t, err) +} + +func TestResolveResourceUnknownIsRejected(t *testing.T) { + provider := userAccess(map[string][]string{}) + _, _, err := resolveResource(t.Context(), provider, "client-1", "https://api.unknown.example.com", []string{"read"}, SubjectTypeUser) + require.Error(t, err) +} + +func TestResolveResourceUnauthorizedClientIsRejected(t *testing.T) { + // The API exists but the client has no allowed permissions for it. + provider := userAccess(map[string][]string{ + "https://api.orders.example.com": {}, + }) + _, _, err := resolveResource(t.Context(), provider, "client-1", "https://api.orders.example.com", []string{"read:orders"}, SubjectTypeUser) + require.Error(t, err) +} + +// TestResolveResourceSubjectTypesAreIndependent guards the core of the user/client separation: +// a permission granted for one subject type must not leak into the other, so a scope users may +// delegate cannot be minted machine-to-machine and a machine scope cannot ride along on a login. +func TestResolveResourceSubjectTypesAreIndependent(t *testing.T) { + provider := fakeAPIAccess{allowed: map[string]map[SubjectType][]string{ + "https://api.orders.example.com": { + SubjectTypeUser: {"read:orders"}, + SubjectTypeClient: {"write:orders"}, + }, + }} + + // The user-delegated grant works for user flows... + audience, granted, err := resolveResource(t.Context(), provider, "client-1", "https://api.orders.example.com", []string{"read:orders"}, SubjectTypeUser) + require.NoError(t, err) + assert.Equal(t, "https://api.orders.example.com", audience) + assert.ElementsMatch(t, []string{"read:orders"}, granted) + + // ...but not for the client itself. + _, _, err = resolveResource(t.Context(), provider, "client-1", "https://api.orders.example.com", []string{"read:orders"}, SubjectTypeClient) + require.Error(t, err) + + // The client grant works machine-to-machine... + audience, granted, err = resolveResource(t.Context(), provider, "client-1", "https://api.orders.example.com", []string{"write:orders"}, SubjectTypeClient) + require.NoError(t, err) + assert.Equal(t, "https://api.orders.example.com", audience) + assert.ElementsMatch(t, []string{"write:orders"}, granted) + + // ...but users cannot be asked to delegate it. + _, _, err = resolveResource(t.Context(), provider, "client-1", "https://api.orders.example.com", []string{"write:orders"}, SubjectTypeUser) + require.Error(t, err) +} + +// TestResolveResourceClientWithoutClientGrantsIsDenied models a client that only has user-delegated +// grants: the API must stay unreachable through the client credentials grant entirely. +func TestResolveResourceClientWithoutClientGrantsIsDenied(t *testing.T) { + provider := fakeAPIAccess{allowed: map[string]map[SubjectType][]string{ + "https://api.orders.example.com": { + SubjectTypeUser: {"read:orders"}, + }, + }} + + _, _, err := resolveResource(t.Context(), provider, "client-1", "https://api.orders.example.com", nil, SubjectTypeClient) + require.Error(t, err) +} + +func TestConsentScopeKeyQualifiesCustomScopesOnly(t *testing.T) { + // Identity scopes stay bare for backward compatibility with existing consents. + assert.Equal(t, "profile", consentScopeKey("https://api.orders.example.com", "profile")) + // Custom scopes are qualified by audience so the same key on two APIs is distinct. + assert.Equal(t, "https://api.orders.example.com\x1fread", consentScopeKey("https://api.orders.example.com", "read")) + assert.NotEqual(t, + consentScopeKey("https://api.orders.example.com", "read"), + consentScopeKey("https://api.billing.example.com", "read"), + ) +} diff --git a/backend/internal/oidc/authorization_service.go b/backend/internal/oidc/authorization_service.go index 4222da08..c41b7f95 100644 --- a/backend/internal/oidc/authorization_service.go +++ b/backend/internal/oidc/authorization_service.go @@ -3,6 +3,7 @@ package oidc import ( "context" "errors" + "log/slog" "net/url" "slices" "strconv" @@ -18,13 +19,14 @@ import ( "gorm.io/gorm/clause" ) -func newAuthorizationService(db *gorm.DB, interactionSessionService *interactionSessionService, claimsService *ClaimsService, reauth ReauthenticationTokenConsumer, auditLog AuditLogger) *authorizationService { +func newAuthorizationService(db *gorm.DB, interactionSessionService *interactionSessionService, claimsService *ClaimsService, reauth ReauthenticationTokenConsumer, auditLog AuditLogger, apiAccess APIAccessProvider) *authorizationService { return &authorizationService{ db: db, interactionSessionService: interactionSessionService, claimsService: claimsService, reauth: reauth, auditLog: auditLog, + apiAccess: apiAccess, } } @@ -34,6 +36,17 @@ type authorizationService struct { claimsService *ClaimsService reauth ReauthenticationTokenConsumer auditLog AuditLogger + apiAccess APIAccessProvider +} + +// resolveGrant resolves the RFC 8707 resource of a request into the token audience, the scopes that may actually be granted, and the audience-qualified keys used to record and check consent +// It always resolves against the client's user-delegated grants because every flow that passes through here acts on behalf of a user +func (s *authorizationService) resolveGrant(ctx context.Context, clientID, resource string, requestedScopes []string) (audience string, grantedScopes []string, consentKeys []string, err error) { + audience, grantedScopes, err = resolveResource(ctx, s.apiAccess, clientID, resource, requestedScopes, SubjectTypeUser) + if err != nil { + return "", nil, nil, err + } + return audience, grantedScopes, consentScopeKeys(audience, grantedScopes), nil } type requestMeta struct { @@ -93,7 +106,8 @@ func (s *authorizationService) authorize(ctx context.Context, input authorizeInp client := input.requester.GetClient().(Client) prompt := newPromptValues(input.requester.GetRequestForm().Get("prompt")) - if err := validateClientPKCERequirement(client, input.requester); err != nil { + err := validateClientPKCERequirement(client, input.requester) + if err != nil { return authorizationResult{}, err } @@ -102,11 +116,32 @@ func (s *authorizationService) authorize(ctx context.Context, input authorizeInp return authorizationResult{}, err } - // Reject authorization requests that require PAR when the request is not a resumed interaction and doesn't have a valid PAR. + // Reject authorization requests that require PAR when the request is not a resumed interaction and doesn't have a valid PAR if client.RequiresPushedAuthorizationRequests && !input.hasPushedAuthorizationRequest && interactionSession == nil { return authorizationResult{}, &common.OidcPARRequiredError{} } + resource, err := input.requester.GetResource() + if err != nil { + return authorizationResult{}, err + } + + // Validate the requested scopes against the targeted API up front, before the user authenticates or reaches the consent screen + // This rejects a custom permission requested without, or with the wrong, resource at the authorize endpoint itself + _, _, _, err = s.resolveGrant(ctx, client.GetID(), resource, input.requester.GetRequestedScopes()) + if err != nil { + // resolveGrant distinguishes an unknown API, an API this client is not granted, and a scope not allowed for the API + // This validation runs before authentication, so returning those distinct errors would let anyone holding a public client_id diff the responses to enumerate which API audiences exist and which ones the client may request + // Collapse every resource-targeted failure into one generic invalid_request so the pre-auth response reveals no backend state, while keeping the underlying reason in the server log for operators + // A request that names no resource cannot leak API topology, so its scope error is returned unchanged to help legitimate integrations + if resource != "" { + slog.DebugContext(ctx, "Rejected authorize request with an invalid or unauthorized resource or scope", "client_id", client.GetID(), "error", err.Error()) + return authorizationResult{}, fosite.ErrInvalidRequest.WithHint("The 'resource' or 'scope' parameter is invalid.") + } + + return authorizationResult{}, err + } + if input.userID == "" { if prompt.has("none") { return authorizationResult{}, fosite.ErrLoginRequired @@ -137,24 +172,19 @@ func (s *authorizationService) authorize(ctx context.Context, input authorizeInp var result authorizationResult err = withTx(ctx, s.db, func(ctx context.Context) error { - var err error - result, err = s.authorizeAuthenticated(ctx, req) - if err != nil { - return err + var txErr error + result, txErr = s.authorizeAuthenticated(ctx, req) + if txErr != nil { + return txErr } - if codeChallenge != "" && - !client.PkceEnabled && - !client.PkceSupported { - + if codeChallenge != "" && !client.PkceEnabled && !client.PkceSupported { tx := dbFromContext(ctx, s.db) - _ = flagPkceSupportedClient(ctx, client.GetID(), tx) } return nil }) - if err != nil { return authorizationResult{}, err } @@ -163,7 +193,8 @@ func (s *authorizationService) authorize(ctx context.Context, input authorizeInp return result, nil } - if err := s.claimsService.applyIDTokenClaims(ctx, result.Session, input.requester.GetGrantedScopes()); err != nil { + err = s.claimsService.applyIDTokenClaims(ctx, result.Session, input.requester.GetGrantedScopes()) + if err != nil { return authorizationResult{}, err } @@ -223,16 +254,23 @@ func (s *authorizationService) authorizeAuthenticated(ctx context.Context, req a } } - hasAlreadyAuthorizedClient, err := s.consent(ctx, req.userID, req.client.GetID(), req.requester.GetRequestedScopes()) + resource, err := req.requester.GetResource() + if err != nil { + return authorizationResult{}, err + } + audience, grantedScopes, consentKeys, err := s.resolveGrant(ctx, req.client.GetID(), resource, req.requester.GetRequestedScopes()) + if err != nil { + return authorizationResult{}, err + } + + hasAlreadyAuthorizedClient, err := s.consent(ctx, req.userID, req.client.GetID(), consentKeys) if err != nil { return authorizationResult{}, err } session := s.buildAuthorizedSession(req, interactionSession, authenticationTime) - for _, scope := range req.requester.GetRequestedScopes() { - req.requester.GrantScope(scope) - } + req.requester.GrantResourceIndicator(audience, grantedScopes) authorizationEvent := model.AuditLogEventClientAuthorization if !hasAlreadyAuthorizedClient { @@ -264,7 +302,15 @@ func flagPkceSupportedClient(ctx context.Context, clientID string, tx *gorm.DB) func (s *authorizationService) resolveRequirements(ctx context.Context, req authorizeRequest, interactionSession *InteractionSession) (interactionRequirements, time.Time, error) { authenticationTime := req.authenticationTime - hasAlreadyAuthorizedClient, err := s.hasAuthorizedClient(ctx, req.client.GetID(), req.userID, req.requester.GetRequestedScopes()) + resource, err := req.requester.GetResource() + if err != nil { + return interactionRequirements{}, authenticationTime, err + } + _, _, consentKeys, err := s.resolveGrant(ctx, req.client.GetID(), resource, req.requester.GetRequestedScopes()) + if err != nil { + return interactionRequirements{}, authenticationTime, err + } + hasAlreadyAuthorizedClient, err := s.hasAuthorizedClient(ctx, req.client.GetID(), req.userID, consentKeys) if err != nil { return interactionRequirements{}, authenticationTime, err } @@ -464,7 +510,67 @@ func (s *authorizationService) getInteractionSession(ctx context.Context, intera return interactionSessionForUser{}, err } - return newInteractionSessionForUser(interactionSession) + return s.buildInteractionForUser(ctx, interactionSession) +} + +// buildInteractionForUser builds the consent-screen DTO and enriches it with display information for the requested custom-API permissions +func (s *authorizationService) buildInteractionForUser(ctx context.Context, interactionSession InteractionSession) (interactionSessionForUser, error) { + result, err := newInteractionSessionForUser(interactionSession) + if err != nil { + return interactionSessionForUser{}, err + } + + scopeInfo, err := s.resolveScopeInfo(ctx, interactionSession) + if err != nil { + return interactionSessionForUser{}, err + } + // Always serialize a possibly empty array rather than null + if scopeInfo == nil { + scopeInfo = []scopeInfoDto{} + } + result.ScopeInfo = scopeInfo + + return result, nil +} + +// resolveScopeInfo resolves display names and descriptions for the requested non-standard scopes, looked up against the API targeted by the request's RFC 8707 resource +// Standard identity scopes are rendered by the client +func (s *authorizationService) resolveScopeInfo(ctx context.Context, interactionSession InteractionSession) ([]scopeInfoDto, error) { + return s.resolveScopeInfoForRequest(ctx, interactionSession.Parameters["resource"], interactionSession.Scopes) +} + +// resolveScopeInfoForRequest resolves display names and descriptions for the requested non-standard scopes against the API identified by resource +// The browser and device consent flows share it so both show friendly permission names instead of raw scope keys +func (s *authorizationService) resolveScopeInfoForRequest(ctx context.Context, resource string, scopes []string) ([]scopeInfoDto, error) { + if s.apiAccess == nil { + return nil, nil + } + + if resource == "" { + return nil, nil + } + + customKeys := make([]string, 0, len(scopes)) + for _, scope := range scopes { + if !isStandardScope(scope) { + customKeys = append(customKeys, scope) + } + } + if len(customKeys) == 0 { + return nil, nil + } + + infos, err := s.apiAccess.DescribePermissions(ctx, resource, customKeys) + if err != nil { + return nil, err + } + + scopeInfo := make([]scopeInfoDto, len(infos)) + for i, info := range infos { + scopeInfo[i] = scopeInfoDto(info) + } + + return scopeInfo, nil } func (s *authorizationService) completeInteractionStep(ctx context.Context, interactionSessionID, userID string, step interactionStep, reauthenticationToken string, authenticationTime time.Time, meta requestMeta) (completeInteractionResponse, error) { @@ -514,7 +620,7 @@ func (s *authorizationService) completeInteractionStep(ctx context.Context, inte return completeInteractionResponse{RedirectURL: authorizeRedirectURL(interactionSession.ID)}, nil } - interaction, err := newInteractionSessionForUser(interactionSession) + interaction, err := s.buildInteractionForUser(ctx, interactionSession) if err != nil { return completeInteractionResponse{}, err } @@ -566,7 +672,12 @@ func (s *authorizationService) completeConsentStep(ctx context.Context, interact if err := bindInteractionSessionUser(interactionSession, userID); err != nil { return err } - hasAlreadyAuthorizedClient, err := s.consent(ctx, userID, interactionSession.ClientID, interactionSession.Scopes) + resource := interactionSession.Parameters["resource"] + _, _, consentKeys, err := s.resolveGrant(ctx, interactionSession.ClientID, resource, interactionSession.Scopes) + if err != nil { + return err + } + hasAlreadyAuthorizedClient, err := s.consent(ctx, userID, interactionSession.ClientID, consentKeys) if err != nil { return err } @@ -596,7 +707,12 @@ func (s *authorizationService) populatePostAuthenticationRequirements(ctx contex func (s *authorizationService) interactionRequirementsForUser(ctx context.Context, userID string, interactionSession *InteractionSession, authenticationTime time.Time) (interactionRequirements, error) { prompt := newPromptValues(interactionSession.Parameters["prompt"]) - hasAlreadyAuthorizedClient, err := s.hasAuthorizedClient(ctx, interactionSession.ClientID, userID, interactionSession.Scopes) + resource := interactionSession.Parameters["resource"] + _, _, consentKeys, err := s.resolveGrant(ctx, interactionSession.ClientID, resource, interactionSession.Scopes) + if err != nil { + return interactionRequirements{}, err + } + hasAlreadyAuthorizedClient, err := s.hasAuthorizedClient(ctx, interactionSession.ClientID, userID, consentKeys) if err != nil { return interactionRequirements{}, err } diff --git a/backend/internal/oidc/authorization_service_test.go b/backend/internal/oidc/authorization_service_test.go index 03b5a8a0..22b184c3 100644 --- a/backend/internal/oidc/authorization_service_test.go +++ b/backend/internal/oidc/authorization_service_test.go @@ -30,7 +30,7 @@ func (f *fakeAuditLogger) Create(_ context.Context, event model.AuditLogEvent, _ func TestAuthorizationServiceAuthorizeLogsClientAuthorization(t *testing.T) { db := testutils.NewDatabaseForTest(t) auditLogger := &fakeAuditLogger{} - service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, auditLogger) + service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, auditLogger, nil) const ( userID = "test-user" @@ -63,10 +63,83 @@ func TestAuthorizationServiceAuthorizeLogsClientAuthorization(t *testing.T) { require.Equal(t, model.AuditLogData{"clientName": "Test Client"}, auditLogger.data[0]) } +func TestAuthorizationServiceRejectsCustomScopeWithoutResource(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil, nil) + + const ( + userID = "test-user" + clientID = "test-client" + ) + require.NoError(t, db.Create(&model.User{Base: model.Base{ID: userID}}).Error) + require.NoError(t, db.Create(&model.OidcClient{Base: model.Base{ID: clientID}, Name: "Test Client"}).Error) + + // A custom permission requested with no resource must be rejected at the + // authorize endpoint, not displayed on the consent screen. + requester := newTestAuthorizeRequesterWithForm("bad-scope-request", clientID, url.Values{}) + requester.(*fosite.AuthorizeRequest).RequestedScope = fosite.Arguments{"openid", "users:read"} + + _, err := service.authorize(t.Context(), authorizeInput{ + userID: userID, + authenticationTime: time.Now().UTC(), + requester: requester, + meta: requestMeta{}, + }) + require.Error(t, err) + + var rfcErr *fosite.RFC6749Error + require.ErrorAs(t, err, &rfcErr) + require.Equal(t, "invalid_scope", rfcErr.ErrorField) +} + +// TestAuthorizationServiceCollapsesResourceErrorsBeforeAuthentication guards the pre-authentication resource validation against API enumeration. +// An unknown API and an API the client is simply not granted are different backend states, but the authorize endpoint runs before the user logs in, so both must surface the exact same generic error; otherwise anyone holding a public client_id could diff the responses to map which API audiences exist and which ones the client may request. +func TestAuthorizationServiceCollapsesResourceErrorsBeforeAuthentication(t *testing.T) { + const ( + clientID = "test-client" + knownAPI = "https://api.orders.example.com" + unknownAPI = "https://api.unknown.example.com" + ) + + authorizeWithResource := func(t *testing.T, apiAccess APIAccessProvider, resource string) error { + t.Helper() + db := testutils.NewDatabaseForTest(t) + require.NoError(t, db.Create(&model.OidcClient{Base: model.Base{ID: clientID}, Name: "Test Client"}).Error) + service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil, apiAccess) + + requester := newTestAuthorizeRequesterWithForm("resource-probe", clientID, url.Values{"resource": {resource}}) + _, err := service.authorize(t.Context(), authorizeInput{ + userID: "test-user", + authenticationTime: time.Now().UTC(), + requester: requester, + }) + return err + } + + // The targeted API does not exist at all + unknownErr := authorizeWithResource(t, userAccess(map[string][]string{}), unknownAPI) + // The targeted API exists but this client has been granted none of its permissions + ungrantedErr := authorizeWithResource(t, userAccess(map[string][]string{knownAPI: {}}), knownAPI) + + require.Error(t, unknownErr) + require.Error(t, ungrantedErr) + + var unknownRFC, ungrantedRFC *fosite.RFC6749Error + require.ErrorAs(t, unknownErr, &unknownRFC) + require.ErrorAs(t, ungrantedErr, &ungrantedRFC) + + // Both states collapse to the identical generic error, so an anonymous caller cannot tell an unknown API from an ungranted one + require.Equal(t, "invalid_request", unknownRFC.ErrorField) + require.Equal(t, unknownRFC.ErrorField, ungrantedRFC.ErrorField) + require.Equal(t, unknownRFC.GetDescription(), ungrantedRFC.GetDescription()) + // The ungranted case must not leak fosite's access_denied, which would reveal that the API exists + require.NotEqual(t, "access_denied", ungrantedRFC.ErrorField) +} + func TestAuthorizationServiceConsentStepLogsNewClientAuthorization(t *testing.T) { db := testutils.NewDatabaseForTest(t) auditLogger := &fakeAuditLogger{} - service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, auditLogger) + service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, auditLogger, nil) const ( userID = "test-user" @@ -100,7 +173,7 @@ func TestAuthorizationServiceConsentStepLogsNewClientAuthorization(t *testing.T) func TestAuthorizationServiceAuthorizeConsumesInteractionSession(t *testing.T) { db := testutils.NewDatabaseForTest(t) - service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil) + service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil, nil) const ( userID = "test-user" @@ -184,7 +257,7 @@ func TestInteractionSessionServiceGetRejectsExpiredSession(t *testing.T) { func TestAuthorizationServiceAuthorizeBindsScopesToInteractionSession(t *testing.T) { db := testutils.NewDatabaseForTest(t) - service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil) + service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil, nil) const ( userID = "test-user" @@ -239,7 +312,7 @@ func TestAuthorizationServiceAuthorizeBindsScopesToInteractionSession(t *testing func TestAuthorizationServiceAuthorizeRejectsInteractionSessionOfOtherClient(t *testing.T) { db := testutils.NewDatabaseForTest(t) - service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil) + service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil, nil) const ( userID = "test-user" @@ -278,7 +351,7 @@ func TestAuthorizationServiceAuthorizeRejectsInteractionSessionOfOtherClient(t * func TestAuthorizationServiceAuthorizeSwitchesUserAndResetsRequirements(t *testing.T) { db := testutils.NewDatabaseForTest(t) - service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil) + service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil, nil) const ( userID = "test-user" @@ -338,7 +411,7 @@ func TestAuthorizationServiceAuthorizeSwitchesUserAndResetsRequirements(t *testi func TestAuthorizationServiceAuthorizeRequiresLoginForUserBoundInteraction(t *testing.T) { db := testutils.NewDatabaseForTest(t) - service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil) + service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil, nil) const ( userID = "test-user" @@ -371,7 +444,7 @@ func TestAuthorizationServiceAuthorizeRequiresLoginForUserBoundInteraction(t *te func TestAuthorizationServiceCompleteInteractionBindsUserToSession(t *testing.T) { db := testutils.NewDatabaseForTest(t) - service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil) + service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil, nil) const ( userID = "test-user" @@ -407,7 +480,7 @@ func TestAuthorizationServiceCompleteInteractionBindsUserToSession(t *testing.T) func TestAuthorizationServiceCompleteInteractionSwitchesUserAndResetsRequirements(t *testing.T) { db := testutils.NewDatabaseForTest(t) - service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil) + service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil, nil) const ( userID = "test-user" @@ -460,7 +533,7 @@ func TestAuthorizationServiceCompleteInteractionSwitchesUserAndResetsRequirement // still be required for that user rather than being inherited from the initiator. func TestAuthorizationServiceSelectAccountRecomputesConsentForSelectedUser(t *testing.T) { db := testutils.NewDatabaseForTest(t) - service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil) + service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil, nil) const ( initiatorID = "initiator-user" @@ -522,7 +595,7 @@ func TestValidateClientPKCERequirement(t *testing.T) { // flag is honored for confidential clients (fosite only enforces PKCE for public clients). func TestAuthorizationServiceAuthorizeEnforcesPerClientPKCE(t *testing.T) { db := testutils.NewDatabaseForTest(t) - service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil) + service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil, nil) const ( userID = "test-user" @@ -558,7 +631,7 @@ func TestAuthorizationServiceAuthorizeEnforcesPerClientPKCE(t *testing.T) { func TestAuthorizationServiceAuthorizePARRequiredClient(t *testing.T) { db := testutils.NewDatabaseForTest(t) - service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil) + service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil, nil) const ( userID = "test-user" @@ -624,7 +697,7 @@ func TestAuthorizationServiceAuthorizePARRequiredClient(t *testing.T) { func TestAuthorizationServiceInteractionRequestQuery(t *testing.T) { db := testutils.NewDatabaseForTest(t) - service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil) + service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil, nil) const ( clientID = "test-client" @@ -664,7 +737,7 @@ func TestAuthorizationServiceInteractionRequestQuery(t *testing.T) { func TestAuthorizationServiceAuthorizeUsesLoginAuthenticationTime(t *testing.T) { db := testutils.NewDatabaseForTest(t) - service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil) + service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil, nil) const ( userID = "test-user" @@ -706,7 +779,7 @@ func TestAuthorizationServiceAuthorizeUsesLoginAuthenticationTime(t *testing.T) func TestAuthorizationServiceAuthorizeRequiresReauthenticationWhenMaxAgeExceeded(t *testing.T) { db := testutils.NewDatabaseForTest(t) - service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil) + service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil, nil) const ( userID = "test-user" @@ -743,7 +816,7 @@ func TestAuthorizationServiceAuthorizeRequiresReauthenticationWhenMaxAgeExceeded func TestAuthorizationServiceAuthorizeUsesCompletedReauthenticationTime(t *testing.T) { db := testutils.NewDatabaseForTest(t) - service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil) + service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil, nil) const ( userID = "test-user" @@ -790,7 +863,7 @@ func TestAuthorizationServiceAuthorizeUsesCompletedReauthenticationTime(t *testi func TestAuthorizationServiceAuthorizeUsesOriginalInteractionRequestTime(t *testing.T) { db := testutils.NewDatabaseForTest(t) - service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil) + service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil, nil) const ( userID = "test-user" @@ -941,7 +1014,7 @@ func TestConsentRequired(t *testing.T) { func TestAuthorizationServiceSkipConsentGrantsWithoutInteraction(t *testing.T) { db := testutils.NewDatabaseForTest(t) auditLogger := &fakeAuditLogger{} - service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, auditLogger) + service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, auditLogger, nil) const ( userID = "test-user" @@ -974,7 +1047,7 @@ func TestAuthorizationServiceSkipConsentGrantsWithoutInteraction(t *testing.T) { // A client with SkipConsent must still show the consent screen when the request explicitly asks for it with prompt=consent func TestAuthorizationServiceSkipConsentHonorsPromptConsent(t *testing.T) { db := testutils.NewDatabaseForTest(t) - service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil) + service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil), nil, nil, nil) const ( userID = "test-user" diff --git a/backend/internal/oidc/client.go b/backend/internal/oidc/client.go index 67522bca..69627785 100644 --- a/backend/internal/oidc/client.go +++ b/backend/internal/oidc/client.go @@ -5,11 +5,11 @@ import ( "github.com/pocket-id/pocket-id/backend/internal/model" ) -var _ fosite.Client = (*Client)(nil) -var _ fosite.ResponseModeClient = (*Client)(nil) - type Client struct { model.OidcClient + + apiScopes []string + apiAudiences []string } func (c Client) GetID() string { @@ -41,7 +41,14 @@ func (c Client) GetResponseTypes() fosite.Arguments { } func (c Client) GetScopes() fosite.Arguments { - return fosite.Arguments{"openid", "profile", "email", "groups", "offline_access"} + scopes := make(fosite.Arguments, 5, 5+len(c.apiScopes)) + scopes[0] = "openid" + scopes[1] = "profile" + scopes[2] = "email" + scopes[3] = "groups" + scopes[4] = "offline_access" + scopes = append(scopes, c.apiScopes...) + return scopes } func (c Client) IsPublic() bool { @@ -49,7 +56,10 @@ func (c Client) IsPublic() bool { } func (c Client) GetAudience() fosite.Arguments { - return fosite.Arguments{c.ID} + audience := make(fosite.Arguments, 0, len(c.apiAudiences)+1) + audience = append(audience, c.ID) + audience = append(audience, c.apiAudiences...) + return audience } func (c Client) GetResponseModes() []fosite.ResponseModeType { diff --git a/backend/internal/oidc/client_test.go b/backend/internal/oidc/client_test.go new file mode 100644 index 00000000..1674f817 --- /dev/null +++ b/backend/internal/oidc/client_test.go @@ -0,0 +1,11 @@ +package oidc + +import ( + "github.com/ory/fosite" +) + +// Interface assertions +var ( + _ fosite.Client = (*Client)(nil) + _ fosite.ResponseModeClient = (*Client)(nil) +) diff --git a/backend/internal/oidc/device_service.go b/backend/internal/oidc/device_service.go index 882395a3..2e4ec219 100644 --- a/backend/internal/oidc/device_service.go +++ b/backend/internal/oidc/device_service.go @@ -88,12 +88,15 @@ func (s *deviceService) acceptDeviceCode(ctx context.Context, userCode, userID, return fosite.ErrAccessDenied.WithHint("You are not allowed to access this service.") } - for _, scope := range request.GetRequestedScopes() { - request.GrantScope(scope) + resource, err := request.GetResource() + if err != nil { + return err } - for _, audience := range request.GetRequestedAudience() { - request.GrantAudience(audience) + audience, grantedScopes, consentKeys, err := s.authorizationService.resolveGrant(ctx, client.GetID(), resource, request.GetRequestedScopes()) + if err != nil { + return err } + request.GrantResourceIndicator(audience, grantedScopes) return withTx(ctx, s.db, func(ctx context.Context) error { if client.RequiresReauthentication { @@ -118,7 +121,7 @@ func (s *deviceService) acceptDeviceCode(ctx context.Context, userCode, userID, } request.SetSession(session) - hasAlreadyAuthorizedClient, err := s.authorizationService.consent(ctx, userID, client.GetID(), request.GetRequestedScopes()) + hasAlreadyAuthorizedClient, err := s.authorizationService.consent(ctx, userID, client.GetID(), consentKeys) if err != nil { return err } @@ -151,9 +154,17 @@ func (s *deviceService) getDeviceCodeInfo(ctx context.Context, userCode, userID } client := request.GetClient().(Client) + resource, err := request.GetResource() + if err != nil { + return nil, err + } authorizationRequired := true if userID != "" { - hasAuthorizedClient, err := s.authorizationService.hasAuthorizedClient(ctx, client.GetID(), userID, request.GetRequestedScopes()) + _, _, consentKeys, err := s.authorizationService.resolveGrant(ctx, client.GetID(), resource, request.GetRequestedScopes()) + if err != nil { + return nil, err + } + hasAuthorizedClient, err := s.authorizationService.hasAuthorizedClient(ctx, client.GetID(), userID, consentKeys) if err != nil { return nil, err } @@ -161,6 +172,21 @@ func (s *deviceService) getDeviceCodeInfo(ctx context.Context, userCode, userID authorizationRequired = consentRequired(hasAuthorizedClient, client.SkipConsent, nil) } + // Resolve friendly names for the requested custom-API permissions so the device consent screen matches the browser flow + scopeInfo, err := s.authorizationService.resolveScopeInfoForRequest(ctx, resource, request.GetRequestedScopes()) + if err != nil { + return nil, err + } + // Always serialize a possibly empty array rather than null + scopeInfoDtos := make([]dto.ScopeInfoDto, len(scopeInfo)) + for i, info := range scopeInfo { + scopeInfoDtos[i] = dto.ScopeInfoDto{ + Key: info.Key, + Name: info.Name, + Description: info.Description, + } + } + return &dto.DeviceCodeInfoDto{ Client: dto.OidcClientMetaDataDto{ ID: client.ID, @@ -171,6 +197,7 @@ func (s *deviceService) getDeviceCodeInfo(ctx context.Context, userCode, userID RequiresReauthentication: client.RequiresReauthentication, }, Scope: request.GetRequestedScopes(), + ScopeInfo: scopeInfoDtos, AuthorizationRequired: authorizationRequired, ReauthenticationRequired: client.RequiresReauthentication, }, nil diff --git a/backend/internal/oidc/device_service_test.go b/backend/internal/oidc/device_service_test.go index 8a451bab..6fb392fb 100644 --- a/backend/internal/oidc/device_service_test.go +++ b/backend/internal/oidc/device_service_test.go @@ -2,8 +2,9 @@ package oidc import ( "context" + "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" - "crypto/rsa" "net/http" "net/http/httptest" "net/url" @@ -95,8 +96,8 @@ func newTestDeviceServiceWithCode(t *testing.T, clientID, userID string, require RequiresReauthentication: requiresReauthentication, }).Error) - store := NewStore(db) - signerKey, err := rsa.GenerateKey(rand.Reader, 2048) + store := NewStore(db, nil) + signerKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.NoError(t, err) provider, err := newProvider(store, nil, testTokenSigner{key: signerKey}, Config{ //nolint:gosec // static test-only provider secret BaseURL: "https://issuer.example.com", @@ -106,7 +107,7 @@ func newTestDeviceServiceWithCode(t *testing.T, clientID, userID string, require require.NoError(t, err) claimsService := newClaimsService(db, nil, "", nil) - authorizationService := newAuthorizationService(db, newInteractionSessionService(db), claimsService, reauth, &fakeAuditLogger{}) + authorizationService := newAuthorizationService(db, newInteractionSessionService(db), claimsService, reauth, &fakeAuditLogger{}, nil) service := newDeviceService(provider, store, provider.deviceStrategy, authorizationService, claimsService, &fakeAuditLogger{}, db) form := url.Values{ diff --git a/backend/internal/oidc/end_session_service_test.go b/backend/internal/oidc/end_session_service_test.go index 3342fe8b..7c05c954 100644 --- a/backend/internal/oidc/end_session_service_test.go +++ b/backend/internal/oidc/end_session_service_test.go @@ -1,6 +1,8 @@ package oidc import ( + "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" "crypto/rsa" "net/url" @@ -83,7 +85,7 @@ func TestEndSessionService(t *testing.T) { jti = "id-token-jti" ) - key, err := rsa.GenerateKey(rand.Reader, 2048) + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.NoError(t, err) signer := testTokenSigner{key: key} @@ -117,7 +119,7 @@ func TestEndSessionService(t *testing.T) { } token, err := builder.Build() require.NoError(t, err) - signed, err := jwt.Sign(token, jwt.WithKey(jwa.RS256(), key)) + signed, err := jwt.Sign(token, jwt.WithKey(jwa.ES256(), key)) require.NoError(t, err) return string(signed) } @@ -132,7 +134,7 @@ func TestEndSessionService(t *testing.T) { }).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) - store := NewStore(db) + store := NewStore(db, nil) return newEndSessionService(db, store, signer, baseURL), store } diff --git a/backend/internal/oidc/interaction_session_dto.go b/backend/internal/oidc/interaction_session_dto.go index 0d685cf9..8ccb102a 100644 --- a/backend/internal/oidc/interaction_session_dto.go +++ b/backend/internal/oidc/interaction_session_dto.go @@ -16,11 +16,18 @@ const ( type interactionSessionForUser struct { ID string `json:"id"` Scopes []string `json:"scopes"` + ScopeInfo []scopeInfoDto `json:"scopeInfo"` Client dto.OidcClientMetaDataDto `json:"client"` CurrentStep interactionStep `json:"currentStep,omitempty"` RequiredSteps []interactionStep `json:"requiredSteps"` } +type scopeInfoDto struct { + Key string `json:"key"` + Name string `json:"name"` + Description string `json:"description,omitempty"` +} + type completeInteractionRequest struct { Step interactionStep `json:"step"` } diff --git a/backend/internal/oidc/introspection_handler_test.go b/backend/internal/oidc/introspection_handler_test.go index 53d5365c..585bce12 100644 --- a/backend/internal/oidc/introspection_handler_test.go +++ b/backend/internal/oidc/introspection_handler_test.go @@ -1,8 +1,9 @@ package oidc import ( + "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" - "crypto/rsa" "encoding/json" "net/http" "net/http/httptest" @@ -34,10 +35,10 @@ func TestIntrospectionHandlerBindsTokenToCallerClient(t *testing.T) { require.NoError(t, db.Create(&model.OidcClient{Base: model.Base{ID: "client-a"}, Name: "Client A"}).Error) require.NoError(t, db.Create(&model.OidcClient{Base: model.Base{ID: "client-b"}, Name: "Client B"}).Error) - signerKey, err := rsa.GenerateKey(rand.Reader, 2048) + signerKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.NoError(t, err) - provider, err := newProvider(NewStore(db), nil, testTokenSigner{key: signerKey}, Config{ //nolint:gosec // static test-only provider secret + 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"), @@ -135,11 +136,11 @@ func TestIntrospectionHandlerAllowsReusedFederatedClientAssertion(t *testing.T) }, }).Error) - store := NewStore(db) + store := NewStore(db, nil) authenticator, err := newFederatedClientAuthenticator(t.Context(), store, newJWKSetHTTPClient(t, jwks), baseURL) require.NoError(t, err) - signerKey, err := rsa.GenerateKey(rand.Reader, 2048) + signerKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.NoError(t, err) provider, err := newProvider(store, authenticator, testTokenSigner{key: signerKey}, Config{ //nolint:gosec // static test-only provider secret BaseURL: baseURL, diff --git a/backend/internal/oidc/module.go b/backend/internal/oidc/module.go index b0952ab6..7ba3688c 100644 --- a/backend/internal/oidc/module.go +++ b/backend/internal/oidc/module.go @@ -45,6 +45,7 @@ type Dependencies struct { CustomClaims CustomClaimSource Reauth ReauthenticationTokenConsumer AuditLog AuditLogger + APIAccess APIAccessProvider } type Module struct { @@ -63,7 +64,7 @@ type Module struct { } func New(ctx context.Context, deps Dependencies) (*Module, error) { - store := NewStore(deps.DB) + store := NewStore(deps.DB, deps.APIAccess).WithIssuer(deps.Config.BaseURL) 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) @@ -76,7 +77,7 @@ func New(ctx context.Context, deps Dependencies) (*Module, error) { claimsService := newClaimsService(deps.DB, deps.CustomClaims, deps.Config.BaseURL, deps.Signer) previewBuilder := newClientPreviewBuilder(claimsService, provider.tokenStrategies) interactionSessionService := newInteractionSessionService(deps.DB) - authorizationService := newAuthorizationService(deps.DB, interactionSessionService, claimsService, deps.Reauth, deps.AuditLog) + authorizationService := newAuthorizationService(deps.DB, interactionSessionService, claimsService, deps.Reauth, deps.AuditLog, deps.APIAccess) deviceService := newDeviceService(provider, store, provider.deviceStrategy, authorizationService, claimsService, deps.AuditLog, deps.DB) endSessionService := newEndSessionService(deps.DB, store, deps.Signer, deps.Config.BaseURL) @@ -87,8 +88,8 @@ func New(ctx context.Context, deps Dependencies) (*Module, error) { store: store, authorizationHandler: newAuthorizationHandler(provider, authorizationService, deps.Config.BaseURL), - tokenHandler: newTokenHandler(provider, claimsService), - userInfoHandler: newUserInfoHandler(provider, claimsService), + tokenHandler: newTokenHandler(provider, claimsService, deps.APIAccess), + userInfoHandler: newUserInfoHandler(provider, claimsService, deps.Config.BaseURL), parHandler: newPARHandler(provider), introspectionHandler: newIntrospectionHandler(provider, authenticator, deps.Config.BaseURL), endSessionHandler: newEndSessionHandler(endSessionService, deps.Config.BaseURL), diff --git a/backend/internal/oidc/preview_test.go b/backend/internal/oidc/preview_test.go index 87787273..45b797e2 100644 --- a/backend/internal/oidc/preview_test.go +++ b/backend/internal/oidc/preview_test.go @@ -1,8 +1,9 @@ package oidc import ( + "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" - "crypto/rsa" "testing" "github.com/stretchr/testify/require" @@ -13,10 +14,10 @@ import ( func TestClientPreviewBuilderUsesFositeTokenStrategies(t *testing.T) { db := testutils.NewDatabaseForTest(t) - signerKey, err := rsa.GenerateKey(rand.Reader, 2048) + signerKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.NoError(t, err) - provider, err := newProvider(NewStore(db), nil, testTokenSigner{key: signerKey}, Config{ //nolint:gosec // static test-only provider secret + 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"), @@ -45,7 +46,8 @@ func TestClientPreviewBuilderUsesFositeTokenStrategies(t *testing.T) { require.Equal(t, "https://issuer.example.com", preview.AccessToken["iss"]) require.ElementsMatch(t, []string{"openid", "email"}, stringSliceClaim(t, preview.AccessToken["scp"])) - require.ElementsMatch(t, []string{clientID}, stringSliceClaim(t, preview.AccessToken["aud"])) + // The identity scopes add the issuer to the audience so the previewed token would also work at /userinfo + require.ElementsMatch(t, []string{clientID, "https://issuer.example.com"}, stringSliceClaim(t, preview.AccessToken["aud"])) require.NotContains(t, preview.AccessToken, "type") require.Equal(t, userID, preview.IDToken["sub"]) @@ -60,10 +62,10 @@ func TestClientPreviewBuilderUsesFositeTokenStrategies(t *testing.T) { func TestClientPreviewBuilderRejectsInvalidScope(t *testing.T) { db := testutils.NewDatabaseForTest(t) - signerKey, err := rsa.GenerateKey(rand.Reader, 2048) + signerKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.NoError(t, err) - provider, err := newProvider(NewStore(db), nil, testTokenSigner{key: signerKey}, Config{ //nolint:gosec // static test-only provider secret + 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"), diff --git a/backend/internal/oidc/provider.go b/backend/internal/oidc/provider.go index 5dff50b6..249e33bd 100644 --- a/backend/internal/oidc/provider.go +++ b/backend/internal/oidc/provider.go @@ -13,6 +13,7 @@ import ( fositeoauth2 "github.com/ory/fosite/handler/oauth2" "github.com/ory/fosite/handler/openid" "github.com/ory/fosite/handler/rfc8628" + "github.com/ory/fosite/token/jwt" "github.com/pocket-id/pocket-id/backend/internal/utils" "golang.org/x/crypto/hkdf" ) @@ -52,6 +53,7 @@ func newProvider(store *Store, authenticator *federatedClientAuthenticator, sign FormPostHTMLTemplate: formPostTemplate, RefreshTokenScopes: []string{}, GlobalSecret: secret, + JWTScopeClaimKey: jwt.JWTScopeFieldBoth, } keyGetter := func(context.Context) (interface{}, error) { @@ -65,6 +67,10 @@ func newProvider(store *Store, authenticator *federatedClientAuthenticator, sign HMACSHAStrategy: coreStrategy, Config: fositeConfig, } + + // Wrap the access token strategy so an access token granted an identity scope also lists the issuer in its audience + // This lets it be presented to Pocket ID's own identity endpoints such as /userinfo, while a token audienced only to a custom API is not accepted there + apiAccessTokenStrategy := identityAudienceAccessTokenStrategy{CoreStrategy: accessTokenStrategy, issuer: config.BaseURL} idTokenStrategy := &openid.DefaultStrategy{ Signer: sig, Config: fositeConfig, @@ -73,7 +79,7 @@ func newProvider(store *Store, authenticator *federatedClientAuthenticator, sign fositeConfig, store, &compose.CommonStrategy{ - CoreStrategy: accessTokenStrategy, + CoreStrategy: apiAccessTokenStrategy, RFC8628CodeStrategy: deviceStrategy, OpenIDConnectTokenStrategy: idTokenStrategy, Signer: sig, @@ -96,7 +102,7 @@ func newProvider(store *Store, authenticator *federatedClientAuthenticator, sign OAuth2Provider: provider, deviceStrategy: deviceStrategy, tokenStrategies: tokenStrategies{ - accessToken: accessTokenStrategy, + accessToken: apiAccessTokenStrategy, idToken: idTokenStrategy, config: fositeConfig, }, diff --git a/backend/internal/oidc/provider_test.go b/backend/internal/oidc/provider_test.go index 16e92d8c..33f63e1a 100644 --- a/backend/internal/oidc/provider_test.go +++ b/backend/internal/oidc/provider_test.go @@ -23,7 +23,7 @@ import ( ) type testTokenSigner struct { - key *rsa.PrivateKey + key *ecdsa.PrivateKey } func (s testTokenSigner) GetPrivateKey() any { @@ -31,7 +31,7 @@ func (s testTokenSigner) GetPrivateKey() any { } func (s testTokenSigner) GetKeyAlg() (jwa.KeyAlgorithm, error) { - return jwa.RS256(), nil + return jwa.ES256(), nil } func (s testTokenSigner) GetKeyID() (string, bool) { @@ -52,10 +52,10 @@ func TestDeriveGlobalSecretUsesStableValue(t *testing.T) { func TestProviderIssuesJWTAccessTokens(t *testing.T) { db := testutils.NewDatabaseForTest(t) - signerKey, err := rsa.GenerateKey(rand.Reader, 2048) + signerKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.NoError(t, err) - provider, err := newProvider(NewStore(db), nil, testTokenSigner{key: signerKey}, Config{ //nolint:gosec // static test-only provider secret + 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"), @@ -87,7 +87,7 @@ func TestProviderIssuesJWTAccessTokens(t *testing.T) { func TestProviderAcceptsWildcardRedirectURI(t *testing.T) { db := testutils.NewDatabaseForTest(t) - signerKey, err := rsa.GenerateKey(rand.Reader, 2048) + signerKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.NoError(t, err) require.NoError(t, db.Create(&model.OidcClient{ Base: model.Base{ID: "test-client"}, @@ -95,7 +95,7 @@ func TestProviderAcceptsWildcardRedirectURI(t *testing.T) { CallbackURLs: model.UrlList{"https://*.example.com/callback"}, }).Error) - provider, err := newProvider(NewStore(db), nil, testTokenSigner{key: signerKey}, Config{ //nolint:gosec // static test-only provider secret + 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"), @@ -119,7 +119,7 @@ func TestProviderAcceptsWildcardRedirectURI(t *testing.T) { func TestProviderAcceptsPushedAuthorizationWildcardRedirectURI(t *testing.T) { db := testutils.NewDatabaseForTest(t) - signerKey, err := rsa.GenerateKey(rand.Reader, 2048) + signerKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.NoError(t, err) require.NoError(t, db.Create(&model.OidcClient{ Base: model.Base{ID: "test-client"}, @@ -128,7 +128,7 @@ func TestProviderAcceptsPushedAuthorizationWildcardRedirectURI(t *testing.T) { IsPublic: true, }).Error) - provider, err := newProvider(NewStore(db), nil, testTokenSigner{key: signerKey}, Config{ //nolint:gosec // static test-only provider secret + 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"), @@ -152,7 +152,7 @@ func TestProviderAcceptsPushedAuthorizationWildcardRedirectURI(t *testing.T) { func TestProviderRejectsUnmatchedWildcardRedirectURI(t *testing.T) { db := testutils.NewDatabaseForTest(t) - signerKey, err := rsa.GenerateKey(rand.Reader, 2048) + signerKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.NoError(t, err) require.NoError(t, db.Create(&model.OidcClient{ Base: model.Base{ID: "test-client"}, @@ -160,7 +160,7 @@ func TestProviderRejectsUnmatchedWildcardRedirectURI(t *testing.T) { CallbackURLs: model.UrlList{"https://*.example.com/callback"}, }).Error) - provider, err := newProvider(NewStore(db), nil, testTokenSigner{key: signerKey}, Config{ //nolint:gosec // static test-only provider secret + 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"), @@ -227,7 +227,7 @@ func TestProviderIssuesAndValidatesTokensForSupportedAlgorithms(t *testing.T) { db := testutils.NewDatabaseForTest(t) require.NoError(t, db.Create(&model.OidcClient{Base: model.Base{ID: "test-client"}, Name: "Test Client"}).Error) - provider, err := newProvider(NewStore(db), nil, algTestSigner{key: tc.gen(t), alg: tc.alg}, Config{ //nolint:gosec // static test-only provider secret + provider, err := newProvider(NewStore(db, nil), nil, algTestSigner{key: tc.gen(t), alg: tc.alg}, Config{ //nolint:gosec // static test-only provider secret BaseURL: "https://issuer.example.com", TokenBaseURL: "https://issuer.example.com", Secret: []byte("test-secret"), diff --git a/backend/internal/oidc/store.go b/backend/internal/oidc/store.go index b6991043..c155eed7 100644 --- a/backend/internal/oidc/store.go +++ b/backend/internal/oidc/store.go @@ -43,12 +43,21 @@ var ( // NewStore creates the fosite storage. Exported for packages that need to seed or // revoke sessions (e.g. the e2e test service). -func NewStore(db *gorm.DB) *Store { - return &Store{db: db} +func NewStore(db *gorm.DB, apiAccess APIAccessProvider) *Store { + return &Store{db: db, apiAccess: apiAccess} } type Store struct { - db *gorm.DB + db *gorm.DB + apiAccess APIAccessProvider + issuer string +} + +// 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 +// It returns the store to allow chaining at construction +func (s *Store) WithIssuer(issuer string) *Store { + s.issuer = issuer + return s } type storedRequester struct { @@ -77,19 +86,33 @@ type storedRequester struct { // Satisfies fosite.Storage func (s *Store) GetClient(ctx context.Context, id string) (fosite.Client, error) { - var client model.OidcClient - err := s.dbFor(ctx). + tx := s.dbFor(ctx) + + var clientModel model.OidcClient + err := tx. Preload("AllowedUserGroups"). - First(&client, "id = ?", id). + First(&clientModel, "id = ?", id). Error if errors.Is(err, gorm.ErrRecordNotFound) { return nil, fosite.ErrNotFound - } - if err != nil { + } else if err != nil { return nil, err } - return Client{OidcClient: client}, nil + 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, id) + if err != nil { + return nil, err + } + + client.apiScopes = apiScopes + client.apiAudiences = apiAudiences + } + + return client, nil } func (s *Store) ClientAssertionJWTValid(ctx context.Context, jti string) error { @@ -141,7 +164,9 @@ func (s *Store) InvalidateAuthorizeCodeSession(ctx context.Context, code string) } func (s *Store) CreateAccessTokenSession(ctx context.Context, signature string, request fosite.Requester) error { - return s.upsertSession(ctx, sessionKindAccessToken, signature, request, "", true, fosite.AccessToken) + // userinfo and introspection read the granted audience from the persisted access token session, so an access token granted an identity scope is stored with the issuer added to its audience, letting it be presented to Pocket ID's own identity endpoints + // A token audienced only to a custom API carries no identity scope here and so never gains the issuer audience + return s.upsertSession(ctx, sessionKindAccessToken, signature, withIdentityAudience(request, s.issuer), "", true, fosite.AccessToken) } func (s *Store) GetAccessTokenSession(ctx context.Context, signature string, _ fosite.Session) (fosite.Requester, error) { @@ -206,7 +231,7 @@ func (s *Store) RevokeSessionsByIDTokenHint(ctx context.Context, userID, clientI } func RevokeUserClientSessions(ctx context.Context, db *gorm.DB, userID, clientID string) error { - s := NewStore(db) + s := NewStore(db, nil) requestIDs, _, err := s.findUserClientRequestIDs(ctx, userID, clientID, "") if err != nil { return err diff --git a/backend/internal/oidc/store_test.go b/backend/internal/oidc/store_test.go index 143db987..45a95a05 100644 --- a/backend/internal/oidc/store_test.go +++ b/backend/internal/oidc/store_test.go @@ -20,7 +20,7 @@ import ( // enforces this with a conditional UPDATE, so the second GetPARSession returns ErrNotFound. func TestStoreGetPARSessionIsSingleUse(t *testing.T) { db := testutils.NewDatabaseForTest(t) - store := NewStore(db) + store := NewStore(db, nil) const ( clientID = "par-client" @@ -65,7 +65,7 @@ func TestStoreGetPARSessionIsSingleUse(t *testing.T) { // active fails closed (ErrNotFound) instead of minting a second token set from one code. func TestStoreInvalidateAuthorizeCodeSessionIsAtomic(t *testing.T) { db := testutils.NewDatabaseForTest(t) - store := NewStore(db) + store := NewStore(db, nil) const ( clientID = "code-client" @@ -99,7 +99,7 @@ func TestStoreInvalidateAuthorizeCodeSessionIsAtomic(t *testing.T) { func TestStoreRevokeSessionsByIDTokenHintRevokesMatchingFositeSessions(t *testing.T) { db := testutils.NewDatabaseForTest(t) - store := NewStore(db) + store := NewStore(db, nil) const ( userID = "test-user-123" @@ -151,7 +151,7 @@ func TestStoreRevokeSessionsByIDTokenHintRevokesMatchingFositeSessions(t *testin func TestStoreRevokeSessionsByIDTokenHintSkipsSessionsWithoutMatchingJTI(t *testing.T) { db := testutils.NewDatabaseForTest(t) - store := NewStore(db) + store := NewStore(db, nil) const ( userID = "test-user-123" diff --git a/backend/internal/oidc/token_handler.go b/backend/internal/oidc/token_handler.go index 9fddd82e..44c83ea2 100644 --- a/backend/internal/oidc/token_handler.go +++ b/backend/internal/oidc/token_handler.go @@ -2,6 +2,7 @@ package oidc import ( "log/slog" + "slices" "github.com/gin-gonic/gin" "github.com/ory/fosite" @@ -10,12 +11,14 @@ import ( type tokenHandler struct { provider fosite.OAuth2Provider claimsService *ClaimsService + apiAccess APIAccessProvider } -func newTokenHandler(provider fosite.OAuth2Provider, claimsService *ClaimsService) *tokenHandler { +func newTokenHandler(provider fosite.OAuth2Provider, claimsService *ClaimsService, apiAccess APIAccessProvider) *tokenHandler { return &tokenHandler{ provider: provider, claimsService: claimsService, + apiAccess: apiAccess, } } @@ -42,17 +45,41 @@ func (h *tokenHandler) token(c *gin.Context) { if client, ok := accessRequest.GetClient().(Client); ok { // Re-validate the resource owner on every user-bound grant. - if err := h.claimsService.ValidateUserAccess(ctx, requestSession.Subject, client); err != nil { + err := h.claimsService.ValidateUserAccess(ctx, requestSession.Subject, client) + if err != nil { slog.WarnContext(ctx, "Rejected token request: user no longer allowed to access client", "error", err.Error()) h.provider.WriteAccessError(ctx, c.Writer, accessRequest, err) return } - // Bind every issued JWT access token to the requesting client so it always carries an aud claim. - accessRequest.GrantAudience(client.GetID()) + // The client credentials grant has no authorize step so the RFC 8707 resource is resolved here to stamp the API audience and limit the granted scope to what the client is allowed for that API + // It resolves against the client-subject grants: a permission delegated by users does not let the client act as itself + // The other grants had their audience and scope resolved at authorize or device time and restored from storage, so they must be left untouched + if accessRequest.GetGrantTypes().Has(string(fosite.GrantTypeClientCredentials)) { + resource, err := accessRequest.GetResource() + if err != nil { + h.provider.WriteAccessError(ctx, c.Writer, accessRequest, err) + return + } + audience, grantedScopes, err := resolveResource(ctx, h.apiAccess, client.GetID(), resource, accessRequest.GetRequestedScopes(), SubjectTypeClient) + if err != nil { + h.provider.WriteAccessError(ctx, c.Writer, accessRequest, err) + return + } + // A client credentials token has no resource owner, so it must never carry identity scopes such as openid or profile + // Dropping them keeps machine tokens out of the userinfo endpoint, which is gated on the openid scope + grantedScopes = slices.DeleteFunc(grantedScopes, isStandardScope) + accessReq, ok := accessRequest.(*fosite.AccessRequest) + if ok { + accessReq.GrantedScope = grantedScopes + accessReq.GrantedAudience = nil + } + accessRequest.GrantResourceIndicator(audience, grantedScopes) + } } - if err := h.claimsService.applyIDTokenClaims(ctx, requestSession, accessRequest.GetGrantedScopes()); err != nil { + err = h.claimsService.applyIDTokenClaims(ctx, requestSession, accessRequest.GetGrantedScopes()) + if err != nil { slog.ErrorContext(ctx, "Failed to apply ID token claims", "error", err) h.provider.WriteAccessError(ctx, c.Writer, accessRequest, err) return @@ -61,7 +88,8 @@ func (h *tokenHandler) token(c *gin.Context) { // The client credentials grant has no resource owner, so no subject is ever set. Assign a // stable synthetic subject so the issued JWT access token still carries a subclaim. if requestSession.Subject == "" { - if client, ok := accessRequest.GetClient().(Client); ok && accessRequest.GetGrantTypes().Has(string(fosite.GrantTypeClientCredentials)) { + client, ok := accessRequest.GetClient().(Client) + if ok && accessRequest.GetGrantTypes().Has(string(fosite.GrantTypeClientCredentials)) { requestSession.Subject = "client-" + client.GetID() } } diff --git a/backend/internal/oidc/token_handler_test.go b/backend/internal/oidc/token_handler_test.go index 0adee2fa..54ca1c39 100644 --- a/backend/internal/oidc/token_handler_test.go +++ b/backend/internal/oidc/token_handler_test.go @@ -1,12 +1,15 @@ package oidc import ( + "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" - "crypto/rsa" "encoding/json" + "maps" "net/http" "net/http/httptest" "net/url" + "sort" "strings" "testing" "time" @@ -42,7 +45,7 @@ func TestTokenHandlerClientCredentialsGrant(t *testing.T) { ) db := testutils.NewDatabaseForTest(t) - key, err := rsa.GenerateKey(rand.Reader, 2048) + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.NoError(t, err) hashed, err := bcrypt.GenerateFromPassword([]byte(clientPlain), bcrypt.DefaultCost) @@ -54,13 +57,13 @@ func TestTokenHandlerClientCredentialsGrant(t *testing.T) { IsPublic: false, }).Error) - provider, err := newProvider(NewStore(db), nil, testTokenSigner{key: key}, Config{ + provider, err := newProvider(NewStore(db, nil), nil, testTokenSigner{key: key}, Config{ BaseURL: baseURL, TokenBaseURL: baseURL, Secret: []byte(secret), }) require.NoError(t, err) - handler := newTokenHandler(provider, newClaimsService(db, nil, baseURL, nil)) + handler := newTokenHandler(provider, newClaimsService(db, nil, baseURL, nil), nil) form := url.Values{"grant_type": {"client_credentials"}} req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/api/oidc/token", strings.NewReader(form.Encode())) @@ -77,9 +80,220 @@ func TestTokenHandlerClientCredentialsGrant(t *testing.T) { require.NotEmpty(t, body["access_token"], "client_credentials must issue a token, got error: %v", body["error"]) claims := decodeJWTPart(t, body["access_token"].(string), 1) + // With no resource requested, the client_credentials token is a plain token bound to the requesting client require.Contains(t, jwtAudience(claims), clientID, "access token must be audience-bound to the client") } +// TestTokenHandlerClientCredentialsDropsIdentityScopes guards that a machine token never +// carries identity scopes. A client_credentials grant has a synthetic subject and no real +// user, so if it could keep the openid scope it would slip past the userinfo endpoint +// (which gates on openid) and probe for user PII. The handler must strip identity scopes. +func TestTokenHandlerClientCredentialsDropsIdentityScopes(t *testing.T) { + gin.SetMode(gin.TestMode) + + const ( + baseURL = "https://issuer.example.com" + secret = "test-secret" + clientID = "cc-client" + clientPlain = "cc-secret-value" + ) + + db := testutils.NewDatabaseForTest(t) + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + + hashed, err := bcrypt.GenerateFromPassword([]byte(clientPlain), bcrypt.DefaultCost) + require.NoError(t, err) + require.NoError(t, db.Create(&model.OidcClient{ + Base: model.Base{ID: clientID}, + Name: "Client Credentials Client", + Secret: string(hashed), + IsPublic: false, + }).Error) + + provider, err := newProvider(NewStore(db, nil), nil, testTokenSigner{key: key}, Config{ + BaseURL: baseURL, + TokenBaseURL: baseURL, + Secret: []byte(secret), + }) + require.NoError(t, err) + handler := newTokenHandler(provider, newClaimsService(db, nil, baseURL, nil), nil) + + form := url.Values{"grant_type": {"client_credentials"}, "scope": {"openid"}} + req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/api/oidc/token", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.SetBasicAuth(clientID, clientPlain) + + rec := httptest.NewRecorder() + c, _ := gin.CreateTestContext(rec) + c.Request = req + handler.token(c) + + var body map[string]any + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &body)) + require.NotEmpty(t, body["access_token"], "client_credentials must issue a token, got error: %v", body["error"]) + + // Introspect the issued token the same way the userinfo endpoint does: openid must be gone. + _, accessRequest, err := provider.IntrospectToken(t.Context(), body["access_token"].(string), fosite.AccessToken, NewEmptySession()) + require.NoError(t, err) + require.False(t, accessRequest.GetGrantedScopes().Has("openid"), "client_credentials token must not carry the openid scope") +} + +// TestTokenHandlerClientCredentialsUsesClientSubjectGrants guards the user/client grant +// separation on the machine-to-machine path: a permission granted only for user-delegated +// access must not be mintable via client_credentials, while a client-subject grant must be. +func TestTokenHandlerClientCredentialsUsesClientSubjectGrants(t *testing.T) { + gin.SetMode(gin.TestMode) + + const ( + baseURL = "https://issuer.example.com" + secret = "test-secret" + clientID = "cc-client" + clientPlain = "cc-secret-value" + apiAudience = "https://api.orders.example.com" + ) + + db := testutils.NewDatabaseForTest(t) + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + + hashed, err := bcrypt.GenerateFromPassword([]byte(clientPlain), bcrypt.DefaultCost) + require.NoError(t, err) + require.NoError(t, db.Create(&model.OidcClient{ + Base: model.Base{ID: clientID}, + Name: "Client Credentials Client", + Secret: string(hashed), + IsPublic: false, + }).Error) + + apiAccess := fakeAPIAccess{allowed: map[string]map[SubjectType][]string{ + apiAudience: { + SubjectTypeUser: {"read:orders"}, + SubjectTypeClient: {"write:orders"}, + }, + }} + + provider, err := newProvider(NewStore(db, apiAccess), nil, testTokenSigner{key: key}, Config{ + BaseURL: baseURL, + TokenBaseURL: baseURL, + Secret: []byte(secret), + }) + require.NoError(t, err) + handler := newTokenHandler(provider, newClaimsService(db, nil, baseURL, nil), apiAccess) + + requestToken := func(t *testing.T, scope string) map[string]any { + t.Helper() + form := url.Values{ + "grant_type": {"client_credentials"}, + "scope": {scope}, + "resource": {apiAudience}, + } + req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/api/oidc/token", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.SetBasicAuth(clientID, clientPlain) + + rec := httptest.NewRecorder() + c, _ := gin.CreateTestContext(rec) + c.Request = req + handler.token(c) + + var body map[string]any + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &body)) + return body + } + + // The client-subject grant is issued and audienced to the API + body := requestToken(t, "openid write:orders") + require.NotEmpty(t, body["access_token"], "client-granted scope must be issued, got error: %v (%v)", body["error"], body["error_description"]) + claims := decodeJWTPart(t, body["access_token"].(string), 1) + require.Equal(t, []string{apiAudience}, jwtAudience(claims), "access token must be audience-bound to the API") + require.Equal(t, []string{"write:orders"}, jwtScopes(claims), "identity scopes must be stripped from machine tokens") + + // The permission users may delegate is not available to the client itself + body = requestToken(t, "read:orders") + require.Empty(t, body["access_token"], "user-delegated permission must not be mintable machine-to-machine") + require.Equal(t, "invalid_scope", body["error"]) +} + +func TestTokenHandlerClientCredentialsDefaultsResourceScopes(t *testing.T) { + gin.SetMode(gin.TestMode) + + const ( + baseURL = "https://issuer.example.com" + secret = "test-secret" + clientID = "cc-client" + clientPlain = "cc-secret-value" + apiAudience = "https://api.orders.example.com" + ) + + db := testutils.NewDatabaseForTest(t) + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + + hashed, err := bcrypt.GenerateFromPassword([]byte(clientPlain), bcrypt.DefaultCost) + require.NoError(t, err) + require.NoError(t, db.Create(&model.OidcClient{ + Base: model.Base{ID: clientID}, + Name: "Client Credentials Client", + Secret: string(hashed), + IsPublic: false, + }).Error) + + apiAccess := fakeAPIAccess{allowed: map[string]map[SubjectType][]string{ + apiAudience: { + SubjectTypeUser: {"read:profile"}, + SubjectTypeClient: {"read:orders", "write:orders"}, + }, + }} + + provider, err := newProvider(NewStore(db, apiAccess), nil, testTokenSigner{key: key}, Config{ + BaseURL: baseURL, + TokenBaseURL: baseURL, + Secret: []byte(secret), + }) + require.NoError(t, err) + handler := newTokenHandler(provider, newClaimsService(db, nil, baseURL, nil), apiAccess) + + requestToken := func(t *testing.T, target string, form url.Values) map[string]any { + t.Helper() + req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, target, strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.SetBasicAuth(clientID, clientPlain) + + rec := httptest.NewRecorder() + c, _ := gin.CreateTestContext(rec) + c.Request = req + handler.token(c) + + var body map[string]any + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &body)) + require.NotEmpty(t, body["access_token"], "client credentials request must issue a token, got error: %v (%v)", body["error"], body["error_description"]) + return body + } + + for _, tc := range []struct { + name string + target string + form url.Values + }{ + { + name: "resource parameter", + target: "/api/oidc/token", + form: url.Values{ + "grant_type": {"client_credentials"}, + "resource": {apiAudience}, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + body := requestToken(t, tc.target, tc.form) + claims := decodeJWTPart(t, body["access_token"].(string), 1) + require.Equal(t, []string{apiAudience}, jwtAudience(claims), "access token must be audience-bound to the API") + require.Equal(t, []string{"read:orders", "write:orders"}, jwtScopes(claims), "all client-subject API scopes must be granted by default") + }) + } +} + // jwtAudience normalizes the `aud` claim (string or []string) into a slice. func jwtAudience(claims map[string]any) []string { switch aud := claims["aud"].(type) { @@ -98,6 +312,32 @@ func jwtAudience(claims map[string]any) []string { } } +// jwtScopes extracts the scope claim from a decoded access token JWT as a sorted slice +// It reads the RFC 9068 `scp` list claim and falls back to the space-delimited `scope` string +func jwtScopes(claims map[string]any) []string { + var out []string + scp, ok := claims["scp"].([]any) + if ok { + for _, s := range scp { + str, ok := s.(string) + if ok { + out = append(out, str) + } + } + } + + if out == nil { + scope, ok := claims["scope"].(string) + if ok && scope != "" { + out = strings.Fields(scope) + } + } + + sort.Strings(out) + + return out +} + // TestTokenHandlerRefreshGrantRevalidatesUser is the regression guard for the most // security-sensitive part of the fosite migration: fosite's refresh-token grant replays // the stored session without reloading the user, so the token handler must re-check the @@ -114,7 +354,7 @@ func TestTokenHandlerRefreshGrantRevalidatesUser(t *testing.T) { secret = "test-secret" ) - key, err := rsa.GenerateKey(rand.Reader, 2048) + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.NoError(t, err) signer := testTokenSigner{key: key} @@ -155,19 +395,19 @@ func TestTokenHandlerRefreshGrantRevalidatesUser(t *testing.T) { request.GrantedAudience = fosite.Arguments{clientID} request.Session = session - require.NoError(t, NewStore(db).CreateRefreshTokenSession(t.Context(), signature, "", request)) + require.NoError(t, NewStore(db, nil).CreateRefreshTokenSession(t.Context(), signature, "", request)) return token } doRefresh := func(t *testing.T, db *gorm.DB, clientID, refreshToken string) map[string]any { t.Helper() - provider, err := newProvider(NewStore(db), nil, signer, Config{ + provider, err := newProvider(NewStore(db, nil), nil, signer, Config{ BaseURL: baseURL, TokenBaseURL: baseURL, Secret: []byte(secret), }) require.NoError(t, err) - handler := newTokenHandler(provider, newClaimsService(db, nil, baseURL, nil)) + handler := newTokenHandler(provider, newClaimsService(db, nil, baseURL, nil), nil) form := url.Values{ "grant_type": {"refresh_token"}, @@ -241,3 +481,160 @@ func TestTokenHandlerRefreshGrantRevalidatesUser(t *testing.T) { require.Equal(t, "access_denied", body["error"]) }) } + +func TestTokenHandlerRefreshGrantPreservesAudienceAndScope(t *testing.T) { + gin.SetMode(gin.TestMode) + + const ( + baseURL = "https://issuer.example.com" + secret = "test-secret" + apiResource = "https://api.orders.example.com" + ) + + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + signer := testTokenSigner{key: key} + + // grantedAPI is a client that has been granted read:orders on the Orders API + grantedAPI := userAccess(map[string][]string{apiResource: {"read:orders"}}) + // revokedAPI stands in for the same client after its API grant was removed: no APIs, no scopes + revokedAPI := userAccess(map[string][]string{}) + + // mintRefreshToken stores an active refresh-token session with the given granted scope and audience, standing in for a token issued by an earlier authorize, and returns the opaque token + mintRefreshToken := func(t *testing.T, db *gorm.DB, clientID, userID string, grantedScope, grantedAudience fosite.Arguments) string { + t.Helper() + globalSecret, err := DeriveGlobalSecret([]byte(secret)) + require.NoError(t, err) + strategy := compose.NewOAuth2HMACStrategy(&fosite.Config{ + GlobalSecret: globalSecret, + RefreshTokenLifespan: 30 * 24 * time.Hour, + }) + token, signature, err := strategy.GenerateRefreshToken(t.Context(), nil) + require.NoError(t, err) + + now := time.Now().UTC() + session := NewEmptySession() + session.Subject = userID + session.Claims = &fositejwt.IDTokenClaims{ + Subject: userID, + RequestedAt: now, + AuthTime: now, + Extra: map[string]any{}, + } + session.SetExpiresAt(fosite.RefreshToken, now.Add(30*24*time.Hour)) + session.SetExpiresAt(fosite.AccessToken, now.Add(time.Hour)) + + request := fosite.NewRequest() + request.ID = "refresh-req-" + userID + request.RequestedAt = now + request.Client = Client{OidcClient: model.OidcClient{Base: model.Base{ID: clientID}, IsPublic: true}} + request.RequestedScope = grantedScope + request.GrantedScope = grantedScope + request.RequestedAudience = grantedAudience + request.GrantedAudience = grantedAudience + request.Session = session + + err = NewStore(db, nil).CreateRefreshTokenSession(t.Context(), signature, "", request) + require.NoError(t, err) + + return token + } + + // doRefresh runs the refresh grant through the real HTTP handler; apiAccess widens the client's + // allowed scopes and audiences the same way the api module does in production, and extra merges + // additional form parameters (such as a widened scope) over the base refresh request + doRefresh := func(t *testing.T, db *gorm.DB, apiAccess APIAccessProvider, clientID, refreshToken string, extra url.Values) map[string]any { + t.Helper() + provider, err := newProvider(NewStore(db, apiAccess), nil, signer, Config{ + BaseURL: baseURL, + TokenBaseURL: baseURL, + Secret: []byte(secret), + }) + require.NoError(t, err) + handler := newTokenHandler(provider, newClaimsService(db, nil, baseURL, nil), nil) + + form := url.Values{ + "grant_type": {"refresh_token"}, + "refresh_token": {refreshToken}, + "client_id": {clientID}, + } + maps.Copy(form, extra) + req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/api/oidc/token", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + rec := httptest.NewRecorder() + c, _ := gin.CreateTestContext(rec) + c.Request = req + handler.token(c) + + var body map[string]any + err = json.Unmarshal(rec.Body.Bytes(), &body) + require.NoError(t, err) + + return body + } + + seedUserAndClient := func(t *testing.T, db *gorm.DB, clientID, userID string) { + t.Helper() + rErr := db.Create(&model.OidcClient{Base: model.Base{ID: clientID}, Name: "Client", IsPublic: true}).Error + require.NoError(t, rErr) + rErr = db.Create(&model.User{Base: model.Base{ID: userID}, Username: "tim"}).Error + require.NoError(t, rErr) + } + + t.Run("refresh keeps the API audience and adds the issuer so the token still reaches userinfo", func(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + const clientID, userID = "client-api", "user-api" + seedUserAndClient(t, db, clientID, userID) + + token := mintRefreshToken(t, db, clientID, userID, + fosite.Arguments{"openid", "read:orders"}, + fosite.Arguments{apiResource}, + ) + body := doRefresh(t, db, grantedAPI, clientID, token, nil) + require.NotEmpty(t, body["access_token"], "expected a new access token, got error: %v", body["error"]) + // The grant still carries openid, so the identity side keeps issuing an ID token on refresh + require.NotEmpty(t, body["id_token"]) + + claims := decodeJWTPart(t, body["access_token"].(string), 1) + // The refreshed access token stays bound to the original API audience and re-adds the issuer so it keeps working at userinfo, never widening to any other API + require.ElementsMatch(t, []string{apiResource, baseURL}, jwtAudience(claims)) + // A token that requested openid alongside the API keeps the identity scope on the access token, matching what it was granted + require.Equal(t, []string{"openid", "read:orders"}, jwtScopes(claims)) + }) + + t.Run("refresh cannot upscope beyond the original grant via the scope parameter", func(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + const clientID, userID = "client-upscope", "user-upscope" + seedUserAndClient(t, db, clientID, userID) + + token := mintRefreshToken(t, db, clientID, userID, + fosite.Arguments{"openid", "read:orders"}, + fosite.Arguments{apiResource}, + ) + // The refresh handler ignores the scope parameter and replays the stored grant, so asking for write:orders is a no-op rather than an escalation + body := doRefresh(t, db, grantedAPI, clientID, token, url.Values{"scope": {"openid read:orders write:orders"}}) + require.NotEmpty(t, body["access_token"], "expected a new access token, got error: %v", body["error"]) + + claims := decodeJWTPart(t, body["access_token"].(string), 1) + require.ElementsMatch(t, []string{apiResource, baseURL}, jwtAudience(claims)) + // write:orders never appears on the refreshed token even though it was requested + require.Equal(t, []string{"openid", "read:orders"}, jwtScopes(claims)) + }) + + t.Run("revoking the client API grant makes the next refresh fail", func(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + const clientID, userID = "client-revoked", "user-revoked" + seedUserAndClient(t, db, clientID, userID) + + token := mintRefreshToken(t, db, clientID, userID, + fosite.Arguments{"openid", "read:orders"}, + fosite.Arguments{apiResource}, + ) + // With the grant revoked the client no longer advertises read:orders, so the refresh handler rejects replaying that stored scope + // Revocation is therefore enforced on the very next refresh rather than lingering until the refresh token expires + body := doRefresh(t, db, revokedAPI, clientID, token, nil) + require.Empty(t, body["access_token"]) + require.Equal(t, "invalid_scope", body["error"]) + }) +} diff --git a/backend/internal/oidc/tx.go b/backend/internal/oidc/tx.go index 92aa5c70..13b18c3b 100644 --- a/backend/internal/oidc/tx.go +++ b/backend/internal/oidc/tx.go @@ -3,6 +3,7 @@ package oidc import ( "context" "errors" + "fmt" "log/slog" "gorm.io/gorm" @@ -19,32 +20,48 @@ func contextWithTx(ctx context.Context, tx *gorm.DB) context.Context { } func dbFromContext(ctx context.Context, fallback *gorm.DB) *gorm.DB { - if tx, ok := ctx.Value(txContextKey{}).(*gorm.DB); ok { + tx, ok := ctx.Value(txContextKey{}).(*gorm.DB) + if ok { return tx.WithContext(ctx) } + return fallback.WithContext(ctx) } // withTx runs fn inside a transaction, committing on nil error. Nested calls join the // outer transaction. func withTx(ctx context.Context, db *gorm.DB, fn func(ctx context.Context) error) error { - if _, ok := ctx.Value(txContextKey{}).(*gorm.DB); ok { + _, ok := ctx.Value(txContextKey{}).(*gorm.DB) + if ok { return fn(ctx) } tx := db.WithContext(ctx).Begin() if tx.Error != nil { - return tx.Error + return fmt.Errorf("error starting transaction: %w", tx.Error) } + + var committed bool defer func() { - err := tx.Rollback().Error - if err != nil && !errors.Is(err, gorm.ErrInvalidTransaction) { - slog.ErrorContext(ctx, "Failed to rollback transaction", "error", err) + if committed { + return + } + rErr := tx.Rollback().Error + if rErr != nil && !errors.Is(rErr, gorm.ErrInvalidTransaction) { + slog.ErrorContext(ctx, "Failed to rollback transaction", "error", rErr) } }() - if err := fn(contextWithTx(ctx, tx)); err != nil { + err := fn(contextWithTx(ctx, tx)) + if err != nil { return err } - return tx.Commit().Error + + err = tx.Commit().Error + if err != nil { + return fmt.Errorf("error committing transaction: %w", err) + } + committed = true + + return nil } diff --git a/backend/internal/oidc/userinfo_handler.go b/backend/internal/oidc/userinfo_handler.go index a490be9c..23b80b08 100644 --- a/backend/internal/oidc/userinfo_handler.go +++ b/backend/internal/oidc/userinfo_handler.go @@ -1,22 +1,26 @@ package oidc import ( + "errors" "fmt" "net/http" "github.com/gin-gonic/gin" "github.com/ory/fosite" + "gorm.io/gorm" ) type userInfoHandler struct { provider fosite.OAuth2Provider claimsService *ClaimsService + issuer string } -func newUserInfoHandler(provider fosite.OAuth2Provider, claimsService *ClaimsService) *userInfoHandler { +func newUserInfoHandler(provider fosite.OAuth2Provider, claimsService *ClaimsService, issuer string) *userInfoHandler { return &userInfoHandler{ provider: provider, claimsService: claimsService, + issuer: issuer, } } @@ -47,8 +51,27 @@ func (h *userInfoHandler) userInfo(c *gin.Context) { return } + // userinfo is one of Pocket ID's own identity endpoints, so the presented token must be audienced to Pocket ID itself (the issuer) + // A token granted an identity scope carries the issuer audience and is accepted here even when it also targets a custom API, while a token audienced only to a custom API belongs to that third-party resource server and cannot be replayed here to read the user's profile + if !accessRequest.GetGrantedAudience().Has(h.issuer) { + writeUserInfoError(c, fosite.ErrAccessDenied.WithDescription("The access token is not audienced to this server and cannot be used to access user information.")) + return + } + + // userinfo serves OIDC identity tokens, which are exactly the ones granted the openid scope + // An access token issued purely for a custom API never carries openid and is rejected here regardless of its audience + if !accessRequest.GetGrantedScopes().Has("openid") { + writeUserInfoError(c, fosite.ErrAccessDenied.WithDescription("The access token is missing the openid scope.")) + return + } + claims, err := h.claimsService.GetUserClaims(ctx, session.GetSubject(), accessRequest.GetGrantedScopes()) if err != nil { + // A token whose subject no longer resolves to a user is an authentication failure, not a missing resource + if errors.Is(err, gorm.ErrRecordNotFound) { + writeUserInfoError(c, fosite.ErrRequestUnauthorized.WithDescription("The access token is invalid")) + return + } _ = c.Error(err) return } diff --git a/backend/internal/oidc/userinfo_handler_test.go b/backend/internal/oidc/userinfo_handler_test.go index a72db276..b65701d8 100644 --- a/backend/internal/oidc/userinfo_handler_test.go +++ b/backend/internal/oidc/userinfo_handler_test.go @@ -1,8 +1,9 @@ package oidc import ( + "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" - "crypto/rsa" "encoding/json" "net/http" "net/http/httptest" @@ -43,17 +44,17 @@ func TestUserInfoHandler(t *testing.T) { EmailVerified: true, }).Error) - key, err := rsa.GenerateKey(rand.Reader, 2048) + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.NoError(t, err) - provider, err := newProvider(NewStore(db), nil, testTokenSigner{key: key}, Config{ + provider, err := newProvider(NewStore(db, nil).WithIssuer(baseURL), nil, testTokenSigner{key: key}, Config{ BaseURL: baseURL, TokenBaseURL: baseURL, Secret: []byte("test-secret"), }) require.NoError(t, err) - handler := newUserInfoHandler(provider, newClaimsService(db, nil, baseURL, nil)) + handler := newUserInfoHandler(provider, newClaimsService(db, nil, baseURL, nil), baseURL) issueAccessToken := func(t *testing.T, requestID, subject string, scopes ...string) string { t.Helper() @@ -67,6 +68,8 @@ func TestUserInfoHandler(t *testing.T) { request.GrantTypes = fosite.Arguments{string(fosite.GrantTypeClientCredentials)} request.RequestedScope = fosite.Arguments(scopes) request.GrantedScope = fosite.Arguments(scopes) + // The grant is bound to the requesting client + // The issuer store adds the issuer audience when the token carries an identity scope, which is what userinfo gates on request.RequestedAudience = fosite.Arguments{clientID} request.GrantedAudience = fosite.Arguments{clientID} @@ -103,6 +106,90 @@ func TestUserInfoHandler(t *testing.T) { require.NotContains(t, claims, "given_name") }) + t.Run("token without the openid scope is rejected", func(t *testing.T) { + // A token issued purely for a custom API carries no openid scope and must not read profile claims + session := NewEmptySession() + session.Subject = userID + session.SetExpiresAt(fosite.AccessToken, time.Now().UTC().Add(time.Hour)) + + request := fosite.NewAccessRequest(session) + request.ID = "req-no-openid" + request.Client = Client{OidcClient: model.OidcClient{Base: model.Base{ID: clientID}}} + request.GrantTypes = fosite.Arguments{string(fosite.GrantTypeClientCredentials)} + request.RequestedScope = fosite.Arguments{"read:orders"} + request.GrantedScope = fosite.Arguments{"read:orders"} + request.RequestedAudience = fosite.Arguments{"https://api.example.com"} + request.GrantedAudience = fosite.Arguments{"https://api.example.com"} + + response, err := provider.NewAccessResponse(t.Context(), request) + require.NoError(t, err) + + rec, c := call(t, response.GetAccessToken()) + require.Empty(t, c.Errors) + require.Equal(t, http.StatusForbidden, rec.Code) + }) + + t.Run("token audienced to a custom API can read userinfo when openid was granted", func(t *testing.T) { + // A token that requested identity scopes alongside a custom API resource is materialized with the issuer added to its audience, so it may read userinfo by the client's explicit opt-in + session := NewEmptySession() + session.Subject = userID + session.SetExpiresAt(fosite.AccessToken, time.Now().UTC().Add(time.Hour)) + + request := fosite.NewAccessRequest(session) + request.ID = "req-api-audience" + request.Client = Client{OidcClient: model.OidcClient{Base: model.Base{ID: clientID}}} + request.GrantTypes = fosite.Arguments{string(fosite.GrantTypeClientCredentials)} + request.RequestedScope = fosite.Arguments{"openid", "email", "read:orders"} + request.GrantedScope = fosite.Arguments{"openid", "email", "read:orders"} + request.RequestedAudience = fosite.Arguments{"https://api.orders.example.com"} + request.GrantedAudience = fosite.Arguments{"https://api.orders.example.com"} + + response, err := provider.NewAccessResponse(t.Context(), request) + require.NoError(t, err) + + rec, c := call(t, response.GetAccessToken()) + require.Empty(t, c.Errors) + require.Equal(t, http.StatusOK, rec.Code) + + var claims map[string]any + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &claims)) + require.Equal(t, userID, claims["sub"]) + require.Equal(t, "tim@example.com", claims["email"]) + }) + + t.Run("token audienced only to a custom API is rejected", func(t *testing.T) { + // A token that carries no identity scope never gains the issuer audience, so it cannot be replayed at userinfo even though it has a valid resource owner + session := NewEmptySession() + session.Subject = userID + session.SetExpiresAt(fosite.AccessToken, time.Now().UTC().Add(time.Hour)) + + request := fosite.NewAccessRequest(session) + request.ID = "req-api-only" + request.Client = Client{OidcClient: model.OidcClient{Base: model.Base{ID: clientID}}} + request.GrantTypes = fosite.Arguments{string(fosite.GrantTypeClientCredentials)} + request.RequestedScope = fosite.Arguments{"read:orders"} + request.GrantedScope = fosite.Arguments{"read:orders"} + request.RequestedAudience = fosite.Arguments{"https://api.orders.example.com"} + request.GrantedAudience = fosite.Arguments{"https://api.orders.example.com"} + + response, err := provider.NewAccessResponse(t.Context(), request) + require.NoError(t, err) + + rec, c := call(t, response.GetAccessToken()) + require.Empty(t, c.Errors) + require.Equal(t, http.StatusForbidden, rec.Code) + require.Contains(t, rec.Body.String(), "not audienced to this server") + }) + + t.Run("token whose user no longer exists is rejected with 401", func(t *testing.T) { + // A valid token whose subject was deleted is an auth failure, not a 404 + token := issueAccessToken(t, "req-ghost", "ghost-user", "openid") + rec, c := call(t, token) + require.Empty(t, c.Errors) + require.Equal(t, http.StatusUnauthorized, rec.Code) + require.Contains(t, rec.Header().Get("WWW-Authenticate"), `Bearer error="request_unauthorized"`) + }) + t.Run("missing access token is rejected", func(t *testing.T) { rec, c := call(t, "") require.Empty(t, c.Errors) diff --git a/backend/internal/service/e2etest_service.go b/backend/internal/service/e2etest_service.go index 0a824c88..ae8401f4 100644 --- a/backend/internal/service/e2etest_service.go +++ b/backend/internal/service/e2etest_service.go @@ -24,6 +24,7 @@ import ( "github.com/pocket-id/pocket-id/backend/internal/apikey" "gorm.io/gorm" + "github.com/pocket-id/pocket-id/backend/internal/api" "github.com/pocket-id/pocket-id/backend/internal/common" "github.com/pocket-id/pocket-id/backend/internal/model" datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" @@ -319,6 +320,62 @@ func (s *TestService) SeedDatabase(baseURL string) error { } } + ordersAPI := api.API{ + Base: model.Base{ + ID: "f6a8b3c1-2d4e-4a6b-8c9d-0e1f2a3b4c5d", + }, + Name: "Orders API", + Audience: "https://api.orders.test", + } + if err := tx.Create(&ordersAPI).Error; err != nil { + return err + } + + apiPermissions := []api.Permission{ + { + Base: model.Base{ + ID: "1a2b3c4d-5e6f-4a7b-8c9d-0e1f2a3b4c5d", + }, + APIID: ordersAPI.ID, + Key: "read:orders", + Name: "Read orders", + Description: new("Read order data"), + }, + { + Base: model.Base{ + ID: "2b3c4d5e-6f7a-4b8c-9d0e-1f2a3b4c5d6e", + }, + APIID: ordersAPI.ID, + Key: "write:orders", + Name: "Write orders", + Description: new("Create and modify orders"), + }, + } + for _, permission := range apiPermissions { + if err := tx.Create(&permission).Error; err != nil { + return err + } + } + + // Immich is allowed to request read:orders on behalf of users and to obtain write:orders for itself via the client credentials grant + allowedAPIPermissions := []api.OidcClientAllowedAPIPermission{ + { + OidcClientID: oidcClients[1].ID, + APIPermissionID: apiPermissions[0].ID, + SubjectType: oidc.SubjectTypeUser, + }, + { + OidcClientID: oidcClients[1].ID, + APIPermissionID: apiPermissions[1].ID, + SubjectType: oidc.SubjectTypeClient, + }, + } + for _, allowed := range allowedAPIPermissions { + if err := tx.Create(&allowed).Error; err != nil { + return err + } + } + // To generate a new key pair, run the following command: // openssl genpkey -algorithm EC -pkeyopt ec_paramgen_curve:P-256 | \ // openssl pkcs8 -topk8 -nocrypt | tee >(openssl pkey -pubout) @@ -776,7 +833,9 @@ type fositeTokenSession struct { func (s *TestService) seedFositeTokenSession(ctx context.Context, session fositeTokenSession) error { request := s.newFositeTokenRequest(session) - store := oidc.NewStore(s.db) + store := oidc. + NewStore(s.db, nil). + WithIssuer(common.EnvConfig.AppURL) switch session.Kind { case "access_token": return store.CreateAccessTokenSession(ctx, session.Signature, request) diff --git a/backend/internal/service/export_service.go b/backend/internal/service/export_service.go index 616017eb..578f3daf 100644 --- a/backend/internal/service/export_service.go +++ b/backend/internal/service/export_service.go @@ -57,7 +57,7 @@ func (s *ExportService) extractDatabase() (DatabaseExport, error) { Tables: map[string][]map[string]any{}, // These tables need to be inserted in a specific order because of foreign key constraints // Not all tables are listed here, because not all tables are order-dependent - TableOrder: []string{"users", "user_groups", "oidc_clients", "signup_tokens"}, + TableOrder: []string{"users", "user_groups", "oidc_clients", "signup_tokens", "apis", "api_permissions", "oidc_clients_allowed_api_permissions"}, } for table := range schema { diff --git a/backend/resources/migrations/postgres/20260624120000_oauth_apis.down.sql b/backend/resources/migrations/postgres/20260624120000_oauth_apis.down.sql new file mode 100644 index 00000000..7b57f2c7 --- /dev/null +++ b/backend/resources/migrations/postgres/20260624120000_oauth_apis.down.sql @@ -0,0 +1,3 @@ +DROP TABLE IF EXISTS oidc_clients_allowed_api_permissions; +DROP TABLE IF EXISTS api_permissions; +DROP TABLE IF EXISTS apis; diff --git a/backend/resources/migrations/postgres/20260624120000_oauth_apis.up.sql b/backend/resources/migrations/postgres/20260624120000_oauth_apis.up.sql new file mode 100644 index 00000000..a3f40fea --- /dev/null +++ b/backend/resources/migrations/postgres/20260624120000_oauth_apis.up.sql @@ -0,0 +1,26 @@ +CREATE TABLE apis ( + id UUID NOT NULL PRIMARY KEY, + created_at TIMESTAMPTZ NOT NULL, + updated_at TIMESTAMPTZ, + name TEXT NOT NULL, + audience TEXT NOT NULL UNIQUE +); + +CREATE TABLE api_permissions ( + id UUID NOT NULL PRIMARY KEY, + created_at TIMESTAMPTZ NOT NULL, + api_id UUID NOT NULL REFERENCES apis(id) ON DELETE CASCADE, + key TEXT NOT NULL, + name TEXT NOT NULL, + description TEXT, + UNIQUE (api_id, key) +); + +CREATE INDEX idx_api_permissions_api_id ON api_permissions(api_id); + +CREATE TABLE oidc_clients_allowed_api_permissions ( + oidc_client_id TEXT NOT NULL REFERENCES oidc_clients(id) ON DELETE CASCADE, + api_permission_id UUID NOT NULL REFERENCES api_permissions(id) ON DELETE CASCADE, + subject_type TEXT NOT NULL CHECK (subject_type IN ('user', 'client')), + PRIMARY KEY (oidc_client_id, api_permission_id, subject_type) +); diff --git a/backend/resources/migrations/sqlite/20260624120000_oauth_apis.down.sql b/backend/resources/migrations/sqlite/20260624120000_oauth_apis.down.sql new file mode 100644 index 00000000..1534b305 --- /dev/null +++ b/backend/resources/migrations/sqlite/20260624120000_oauth_apis.down.sql @@ -0,0 +1,9 @@ +PRAGMA foreign_keys=OFF; +BEGIN; + +DROP TABLE IF EXISTS oidc_clients_allowed_api_permissions; +DROP TABLE IF EXISTS api_permissions; +DROP TABLE IF EXISTS apis; + +COMMIT; +PRAGMA foreign_keys=ON; diff --git a/backend/resources/migrations/sqlite/20260624120000_oauth_apis.up.sql b/backend/resources/migrations/sqlite/20260624120000_oauth_apis.up.sql new file mode 100644 index 00000000..e5e320a0 --- /dev/null +++ b/backend/resources/migrations/sqlite/20260624120000_oauth_apis.up.sql @@ -0,0 +1,32 @@ +PRAGMA foreign_keys=OFF; +BEGIN; + +CREATE TABLE apis ( + id TEXT NOT NULL PRIMARY KEY, + created_at DATETIME NOT NULL, + updated_at DATETIME, + name TEXT NOT NULL, + audience TEXT NOT NULL UNIQUE +); + +CREATE TABLE api_permissions ( + id TEXT NOT NULL PRIMARY KEY, + created_at DATETIME NOT NULL, + api_id TEXT NOT NULL REFERENCES apis(id) ON DELETE CASCADE, + key TEXT NOT NULL, + name TEXT NOT NULL, + description TEXT, + UNIQUE (api_id, key) +); + +CREATE INDEX idx_api_permissions_api_id ON api_permissions(api_id); + +CREATE TABLE oidc_clients_allowed_api_permissions ( + oidc_client_id TEXT NOT NULL REFERENCES oidc_clients(id) ON DELETE CASCADE, + api_permission_id TEXT NOT NULL REFERENCES api_permissions(id) ON DELETE CASCADE, + subject_type TEXT NOT NULL CHECK (subject_type IN ('user', 'client')), + PRIMARY KEY (oidc_client_id, api_permission_id, subject_type) +); + +COMMIT; +PRAGMA foreign_keys=ON; diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 5df36178..e638d05b 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -531,6 +531,35 @@ "user_has_no_passkeys_yet": "This user has no passkeys yet.", "replay_protection": "Replay Protection", "replay_protection_description": "If enabled the provided token can only be used once. If your provider uses the same token multiple times, you may need to disable this option.", + "apis": "APIs", + "create_api": "Create API", + "create_a_new_api_description": "Create a new API that clients can request access tokens for.", + "add_api": "Add API", + "manage_apis": "Manage APIs", + "api_resource": "Resource", + "api_resource_description": "A unique URI that identifies this API resource. Clients request it with the resource parameter. It can't be changed later.", + "api_permissions": "Permissions", + "api_permissions_description": "The permissions (scopes) that clients can request for this API.", + "api_permission_key": "Permission", + "add_permission": "Add permission", + "type": "Type", + "api_created_successfully": "API created successfully", + "api_updated_successfully": "API updated successfully", + "api_deleted_successfully": "API deleted successfully", + "api_permissions_updated_successfully": "Permissions updated successfully", + "are_you_sure_you_want_to_delete_this_api": "Are you sure you want to delete this API? Clients will lose access to its permissions.", + "api_access": "API access", + "api_access_description": "Select which API permissions this client may request on behalf of users (user-delegated access) and for itself via the client credentials grant (client access).", + "api_access_updated_successfully": "API access updated successfully", + "no_apis_defined_yet": "No APIs have been defined yet. APIs allow clients to request access tokens for specific resources and permissions.", + "access_an_api_on_your_behalf": "Access an API on your behalf", + "api_name": "API Name", + "access": "Access", + "user_delegated_access": "User-delegated access", + "client_access": "Client access (M2M)", + "client_access_unavailable_for_public_clients": "Public clients can't use the client credentials grant, so client access is not available.", + "permissions_granted_count": "{granted} / {total} permissions granted", + "select_the_permissions_this_client_may_request": "Select the permissions this client may request on behalf of the signed-in user (user-delegated access) and for itself without a user via the client credentials grant (client access).", "i_have_a_longer_code": "I have a longer code", "pkce_supported_client_title": "This client supports PKCE", "pkce_supported_client_description": "This client supports Proof Key for Code Exchange (PKCE). PKCE is a security feature that helps protect against certain attacks during the OAuth 2.0 authorization process. It's recommended to enable it." diff --git a/frontend/src/lib/components/form/form-input.svelte b/frontend/src/lib/components/form/form-input.svelte index ffb62dee..9fa412a5 100644 --- a/frontend/src/lib/components/form/form-input.svelte +++ b/frontend/src/lib/components/form/form-input.svelte @@ -32,6 +32,7 @@ children, onInput, labelFor, + readonly = false, inputClass, ...restProps }: HTMLAttributes & @@ -43,6 +44,7 @@ disabled?: boolean; inputClass?: string; type?: 'text' | 'password' | 'email' | 'number' | 'checkbox' | 'date' | 'url'; + readonly?: boolean; onInput?: (e: FormInputEvent) => void; } = $props(); @@ -64,7 +66,7 @@ {#if docsLink} @@ -91,6 +93,7 @@ bind:value={input.value} {disabled} oninput={(e) => onInput?.(e)} + {readonly} /> {/if} {/if} diff --git a/frontend/src/lib/components/scope-list.svelte b/frontend/src/lib/components/scope-list.svelte index fadeb6f3..83eb038d 100644 --- a/frontend/src/lib/components/scope-list.svelte +++ b/frontend/src/lib/components/scope-list.svelte @@ -1,13 +1,19 @@ - + {#if scopes.includes('email')} {/if} @@ -25,4 +31,11 @@ description={m.view_the_groups_you_are_a_member_of()} /> {/if} + {#each customScopes as scope} + + {/each} diff --git a/frontend/src/lib/components/ui/card/card-description.svelte b/frontend/src/lib/components/ui/card/card-description.svelte index 54805ad3..293d8250 100644 --- a/frontend/src/lib/components/ui/card/card-description.svelte +++ b/frontend/src/lib/components/ui/card/card-description.svelte @@ -13,7 +13,7 @@

{@render children?.()} diff --git a/frontend/src/lib/services/apis-service.ts b/frontend/src/lib/services/apis-service.ts new file mode 100644 index 00000000..29789722 --- /dev/null +++ b/frontend/src/lib/services/apis-service.ts @@ -0,0 +1,56 @@ +import type { + Api, + ApiCreate, + ApiListItem, + ApiPermissionInput, + ApiUpdate, + ClientApiAccess +} from '$lib/types/api.type'; +import type { ListRequestOptions, Paginated } from '$lib/types/list-request.type'; +import APIService from './api-service'; + +export default class ApisService extends APIService { + list = async (options?: ListRequestOptions) => { + const res = await this.api.get('/apis', { params: options }); + return res.data as Paginated; + }; + + listAll = async () => { + const res = await this.api.get('/apis', { params: { pagination: { page: 1, limit: 1000 } } }); + return (res.data as Paginated).data; + }; + + get = async (id: string) => { + const res = await this.api.get(`/apis/${id}`); + return res.data as Api; + }; + + create = async (api: ApiCreate) => { + const res = await this.api.post('/apis', api); + return res.data as Api; + }; + + update = async (id: string, api: ApiUpdate) => { + const res = await this.api.put(`/apis/${id}`, api); + return res.data as Api; + }; + + remove = async (id: string) => { + await this.api.delete(`/apis/${id}`); + }; + + updatePermissions = async (id: string, permissions: ApiPermissionInput[]) => { + const res = await this.api.put(`/apis/${id}/permissions`, { permissions }); + return res.data as Api; + }; + + getClientAccess = async (clientId: string) => { + const res = await this.api.get(`/api-access/${clientId}`); + return res.data as ClientApiAccess; + }; + + updateClientAccess = async (clientId: string, access: ClientApiAccess) => { + const res = await this.api.put(`/api-access/${clientId}`, access); + return res.data as ClientApiAccess; + }; +} diff --git a/frontend/src/lib/types/api.type.ts b/frontend/src/lib/types/api.type.ts new file mode 100644 index 00000000..7f03a29e --- /dev/null +++ b/frontend/src/lib/types/api.type.ts @@ -0,0 +1,38 @@ +export type ApiPermission = { + id: string; + key: string; + name: string; + description?: string; +}; + +export type Api = { + id: string; + name: string; + resource: string; + createdAt: string; + permissions: ApiPermission[]; +}; + +export type ApiListItem = Omit & { + permissionCount: number; +}; + +export type ApiCreate = { + name: string; + resource: string; +}; + +export type ApiUpdate = { + name: string; +}; + +export type ApiPermissionInput = { + key: string; + name: string; + description: string; +}; + +export type ClientApiAccess = { + userDelegatedPermissionIds: string[]; + clientPermissionIds: string[]; +}; diff --git a/frontend/src/lib/types/oidc.type.ts b/frontend/src/lib/types/oidc.type.ts index 729d4788..d78f3a23 100644 --- a/frontend/src/lib/types/oidc.type.ts +++ b/frontend/src/lib/types/oidc.type.ts @@ -43,7 +43,10 @@ export type OidcClientWithAllowedUserGroupsCount = OidcClient & { allowedUserGroupsCount: number; }; -export type OidcClientUpdate = Omit; +export type OidcClientUpdate = Omit< + OidcClient, + 'id' | 'logoURL' | 'hasLogo' | 'hasDarkLogo' | 'pkceSupported' +>; export type OidcClientCreate = OidcClientUpdate & { id?: string; }; @@ -61,6 +64,7 @@ export type OidcClientCreateWithLogo = OidcClientCreate & { export type OidcDeviceCodeInfo = { scope: string[]; + scopeInfo: InteractionScopeInfo[]; authorizationRequired: boolean; reauthenticationRequired: boolean; client: OidcClientMetaData; @@ -72,9 +76,16 @@ export type AccessibleOidcClient = OidcClientMetaData & { export type InteractionStep = 'authenticate' | 'select_account' | 'reauthenticate' | 'consent'; +export type InteractionScopeInfo = { + key: string; + name: string; + description?: string; +}; + export type InteractionSession = { id: string; scopes: string[]; + scopeInfo: InteractionScopeInfo[]; client: OidcClientMetaData; currentStep?: InteractionStep; requiredSteps: InteractionStep[]; diff --git a/frontend/src/routes/device/+page.svelte b/frontend/src/routes/device/+page.svelte index c8763a18..e4d8c046 100644 --- a/frontend/src/routes/device/+page.svelte +++ b/frontend/src/routes/device/+page.svelte @@ -121,8 +121,8 @@

{:else if authorizationRequired}
- - + +

- +

diff --git a/frontend/src/routes/interaction/+page.svelte b/frontend/src/routes/interaction/+page.svelte index b0371d95..24734ebe 100644 --- a/frontend/src/routes/interaction/+page.svelte +++ b/frontend/src/routes/interaction/+page.svelte @@ -170,7 +170,7 @@ {:else if currentStep === 'consent'}
- +

- +

diff --git a/frontend/src/routes/settings/+layout.svelte b/frontend/src/routes/settings/+layout.svelte index 7076e2d8..1e20120e 100644 --- a/frontend/src/routes/settings/+layout.svelte +++ b/frontend/src/routes/settings/+layout.svelte @@ -34,6 +34,7 @@ { href: '/settings/admin/users', label: m.users() }, { href: '/settings/admin/user-groups', label: m.user_groups() }, { href: '/settings/admin/oidc-clients', label: m.oidc_clients() }, + { href: '/settings/admin/apis', label: m.apis() }, { href: '/settings/admin/api-keys', label: m.api_keys() }, { href: '/settings/admin/application-configuration', label: m.application_configuration() } ]; diff --git a/frontend/src/routes/settings/admin/apis/+page.svelte b/frontend/src/routes/settings/admin/apis/+page.svelte new file mode 100644 index 00000000..67fb1c30 --- /dev/null +++ b/frontend/src/routes/settings/admin/apis/+page.svelte @@ -0,0 +1,83 @@ + + + + {m.apis()} + + +
+ + +
+
+ + + {m.create_api()} + + {m.create_a_new_api_description()} +
+ {#if !expandAddApi} + + {:else} + + {/if} +
+ + {#if expandAddApi} +
+ + + +
+ {/if} + +
+ +
+ + + + + {m.manage_apis()} + + + + + + +
diff --git a/frontend/src/routes/settings/admin/apis/[id]/+page.svelte b/frontend/src/routes/settings/admin/apis/[id]/+page.svelte new file mode 100644 index 00000000..a99f2e0d --- /dev/null +++ b/frontend/src/routes/settings/admin/apis/[id]/+page.svelte @@ -0,0 +1,88 @@ + + + + {api.name} + + +
+ +
+ + + + {m.general()} + + + + + + + + +
+ +
+ diff --git a/frontend/src/routes/settings/admin/apis/[id]/+page.ts b/frontend/src/routes/settings/admin/apis/[id]/+page.ts new file mode 100644 index 00000000..c9e97c54 --- /dev/null +++ b/frontend/src/routes/settings/admin/apis/[id]/+page.ts @@ -0,0 +1,7 @@ +import ApisService from '$lib/services/apis-service'; +import type { PageLoad } from './$types'; + +export const load: PageLoad = async ({ params }) => { + const api = await new ApisService().get(params.id); + return { api }; +}; diff --git a/frontend/src/routes/settings/admin/apis/[id]/api-permissions-input.svelte b/frontend/src/routes/settings/admin/apis/[id]/api-permissions-input.svelte new file mode 100644 index 00000000..1530885a --- /dev/null +++ b/frontend/src/routes/settings/admin/apis/[id]/api-permissions-input.svelte @@ -0,0 +1,44 @@ + + +
+ {#each permissions as _, i} +
+ + + + +
+ {/each} +
+{#if permissions.length < limit} + +{/if} diff --git a/frontend/src/routes/settings/admin/apis/api-form.svelte b/frontend/src/routes/settings/admin/apis/api-form.svelte new file mode 100644 index 00000000..94b5cfae --- /dev/null +++ b/frontend/src/routes/settings/admin/apis/api-form.svelte @@ -0,0 +1,65 @@ + + +
+
+ + +
+
+ +
+
diff --git a/frontend/src/routes/settings/admin/apis/api-list.svelte b/frontend/src/routes/settings/admin/apis/api-list.svelte new file mode 100644 index 00000000..2f2310ac --- /dev/null +++ b/frontend/src/routes/settings/admin/apis/api-list.svelte @@ -0,0 +1,74 @@ + + + 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 4270bcf1..39197c8a 100644 --- a/frontend/src/routes/settings/admin/oidc-clients/[id]/+page.svelte +++ b/frontend/src/routes/settings/admin/oidc-clients/[id]/+page.svelte @@ -23,6 +23,7 @@ import { backNavigate } from '../../users/navigate-back-util'; import OidcForm from '../oidc-client-form.svelte'; import OidcClientPreviewModal from '../oidc-client-preview-modal.svelte'; + import ApiAccessCard from './api-access-card.svelte'; import ScimResourceProviderForm from './scim-resource-provider-form.svelte'; let { data } = $props(); @@ -315,6 +316,9 @@ > + + + + import { goto } from '$app/navigation'; + import CopyToClipboard from '$lib/components/copy-to-clipboard.svelte'; + import { Button } from '$lib/components/ui/button'; + import { Spinner } from '$lib/components/ui/spinner'; + import * as Table from '$lib/components/ui/table'; + import { m } from '$lib/paraglide/messages'; + import ApisService from '$lib/services/apis-service'; + import type { Api } from '$lib/types/api.type'; + import { axiosErrorToast } from '$lib/utils/error-util'; + import { onMount } from 'svelte'; + import { toast } from 'svelte-sonner'; + import ApiPermissionsModal from './api-permissions-modal.svelte'; + + let { clientId, isPublicClient }: { clientId: string; isPublicClient: boolean } = $props(); + + const apisService = new ApisService(); + + let apis = $state([]); + let userSelected = $state>(new Set()); + let clientSelected = $state>(new Set()); + let loading = $state(true); + + let editingApi = $state(null); + let modalOpen = $state(false); + + onMount(async () => { + try { + const [list, access] = await Promise.all([ + apisService.listAll(), + apisService.getClientAccess(clientId) + ]); + apis = await Promise.all(list.map((a) => apisService.get(a.id))); + userSelected = new Set(access.userDelegatedPermissionIds); + clientSelected = new Set(access.clientPermissionIds); + } catch (e) { + axiosErrorToast(e); + } finally { + loading = false; + } + }); + + function grantedCount(api: Api, selected: Set) { + return api.permissions.filter((p) => selected.has(p.id)).length; + } + + function openEdit(api: Api) { + editingApi = api; + modalOpen = true; + } + + function allowedIdsFor(api: Api, selected: Set) { + return api.permissions.filter((p) => selected.has(p.id)).map((p) => p.id); + } + + async function saveApi(api: Api, userIds: string[], clientIds: string[]) { + // Grants of other APIs stay untouched, and for public clients the (never editable) client grants are sent back unchanged + const otherUser = [...userSelected].filter((id) => !api.permissions.some((p) => p.id === id)); + const otherClient = [...clientSelected].filter( + (id) => !api.permissions.some((p) => p.id === id) + ); + const res = await apisService.updateClientAccess(clientId, { + userDelegatedPermissionIds: [...otherUser, ...userIds], + clientPermissionIds: isPublicClient ? [...clientSelected] : [...otherClient, ...clientIds] + }); + userSelected = new Set(res.userDelegatedPermissionIds); + clientSelected = new Set(res.clientPermissionIds); + toast.success(m.api_access_updated_successfully()); + } + + +{#if loading} +
+ +
+{:else if apis.length === 0} +
+

{m.no_apis_defined_yet()}

+ +
+{:else} + + + + {m.api_name()} + {m.user_delegated_access()} + {#if !isPublicClient} + {m.client_access()} + {/if} + + + + + {#each apis as api} + + +
+ {api.name} +
+ + {api.resource} + +
+
+ + + {m.permissions_granted_count({ + granted: String(grantedCount(api, userSelected)), + total: String(api.permissions.length) + })} + + {#if !isPublicClient} + + {m.permissions_granted_count({ + granted: String(grantedCount(api, clientSelected)), + total: String(api.permissions.length) + })} + + {/if} + + + + + {/each} + + +{/if} + +{#if editingApi} + saveApi(editingApi!, userIds, clientIds)} + /> +{/if} diff --git a/frontend/src/routes/settings/admin/oidc-clients/[id]/api-permissions-modal.svelte b/frontend/src/routes/settings/admin/oidc-clients/[id]/api-permissions-modal.svelte new file mode 100644 index 00000000..d2e88842 --- /dev/null +++ b/frontend/src/routes/settings/admin/oidc-clients/[id]/api-permissions-modal.svelte @@ -0,0 +1,153 @@ + + +{#snippet KeyCell({ item }: { item: ApiPermission })} + {item.key} +{/snippet} + +{#snippet UserDelegatedCell({ item }: { item: ApiPermission })} + (workingUser = toggle(workingUser, item.id, checked))} + /> +{/snippet} + +{#snippet ClientAccessCell({ item }: { item: ApiPermission })} + + (workingClient = toggle(workingClient, item.id, checked))} + /> +{/snippet} + + + + + {api.name} + + {m.select_the_permissions_this_client_may_request()} + {#if !showClientAccess} + {m.client_access_unavailable_for_public_clients()} + {/if} + + + + + +
+ + +
+ + diff --git a/tests/data.ts b/tests/data.ts index e7357925..5eb0e5a0 100644 --- a/tests/data.ts +++ b/tests/data.ts @@ -82,6 +82,26 @@ export const oidcClients = { } }; +export const apis = { + orders: { + id: 'f6a8b3c1-2d4e-4a6b-8c9d-0e1f2a3b4c5d', + name: 'Orders API', + resource: 'https://api.orders.test', + permissions: { + readOrders: { + id: '1a2b3c4d-5e6f-4a7b-8c9d-0e1f2a3b4c5d', + key: 'read:orders', + name: 'Read orders' + }, + writeOrders: { + id: '2b3c4d5e-6f7a-4b8c-9d0e-1f2a3b4c5d6e', + key: 'write:orders', + name: 'Write orders' + } + } + } +}; + export const userGroups = { developers: { id: 'c7ae7c01-28a3-4f3c-9572-1ee734ea8368', diff --git a/tests/resources/export/database.json b/tests/resources/export/database.json index 0fcbfb4b..5cd27203 100644 --- a/tests/resources/export/database.json +++ b/tests/resources/export/database.json @@ -1,8 +1,47 @@ { "provider": "sqlite", "version": 20260726153900, - "tableOrder": ["users", "user_groups", "oidc_clients", "signup_tokens"], + "tableOrder": ["users", "user_groups", "oidc_clients", "signup_tokens", "apis", "api_permissions", "oidc_clients_allowed_api_permissions"], "tables": { + "apis": [ + { + "id": "f6a8b3c1-2d4e-4a6b-8c9d-0e1f2a3b4c5d", + "created_at": "2025-11-25T12:39:02Z", + "updated_at": null, + "name": "Orders API", + "audience": "https://api.orders.test" + } + ], + "api_permissions": [ + { + "id": "1a2b3c4d-5e6f-4a7b-8c9d-0e1f2a3b4c5d", + "created_at": "2025-11-25T12:39:02Z", + "api_id": "f6a8b3c1-2d4e-4a6b-8c9d-0e1f2a3b4c5d", + "key": "read:orders", + "name": "Read orders", + "description": "Read order data" + }, + { + "id": "2b3c4d5e-6f7a-4b8c-9d0e-1f2a3b4c5d6e", + "created_at": "2025-11-25T12:39:02Z", + "api_id": "f6a8b3c1-2d4e-4a6b-8c9d-0e1f2a3b4c5d", + "key": "write:orders", + "name": "Write orders", + "description": "Create and modify orders" + } + ], + "oidc_clients_allowed_api_permissions": [ + { + "oidc_client_id": "606c7782-f2b1-49e5-8ea9-26eb1b06d018", + "api_permission_id": "1a2b3c4d-5e6f-4a7b-8c9d-0e1f2a3b4c5d", + "subject_type": "user" + }, + { + "oidc_client_id": "606c7782-f2b1-49e5-8ea9-26eb1b06d018", + "api_permission_id": "2b3c4d5e-6f7a-4b8c-9d0e-1f2a3b4c5d6e", + "subject_type": "client" + } + ], "api_keys": [ { "created_at": "2025-12-21T19:12:03Z", diff --git a/tests/specs/api.spec.ts b/tests/specs/api.spec.ts new file mode 100644 index 00000000..dd5a1aef --- /dev/null +++ b/tests/specs/api.spec.ts @@ -0,0 +1,315 @@ +import test, { expect } from '@playwright/test'; +import * as jose from 'jose'; +import { apis, oidcClients } from '../data'; +import { cleanupBackend } from '../utils/cleanup.util'; +import * as oidcUtil from '../utils/oidc.util'; + +test.beforeEach(async () => await cleanupBackend()); + +function tokenScopes(claims: jose.JWTPayload): string[] { + if (Array.isArray((claims as Record).scp)) { + return (claims as Record).scp as string[]; + } + if (typeof claims.scope === 'string') { + return claims.scope.split(' '); + } + return []; +} + +function tokenAudiences(claims: jose.JWTPayload): string[] { + if (Array.isArray(claims.aud)) return claims.aud; + if (typeof claims.aud === 'string') return [claims.aud]; + return []; +} + +// --------------------------------------------------------------------------- +// Admin UI +// --------------------------------------------------------------------------- + +test('Lists the preseeded API', async ({ page }) => { + await page.goto('/settings/admin/apis'); + + const row = page.getByRole('row', { name: apis.orders.name }); + await expect(row).toBeVisible(); + await expect(row).toContainText(apis.orders.resource); +}); + +test('Create API', async ({ page }) => { + await page.goto('/settings/admin/apis'); + + await page.getByRole('button', { name: 'Add API' }).click(); + await page.getByLabel('Name', { exact: true }).fill('Billing API'); + await page.getByLabel('Resource').fill('https://api.billing.test'); + await page.getByRole('button', { name: 'Save' }).click(); + + await expect(page.locator('[data-type="success"]')).toHaveText('API created successfully'); + await page.waitForURL('/settings/admin/apis/*'); + + await expect(page.getByLabel('Name', { exact: true })).toHaveValue('Billing API'); + await expect(page.getByLabel('Resource')).toHaveValue('https://api.billing.test'); +}); + +test('Cannot create an API with the issuer as resource', async ({ page }) => { + const { issuer } = await page.request + .get('/.well-known/openid-configuration') + .then((r) => r.json()); + + await page.goto('/settings/admin/apis'); + await page.getByRole('button', { name: 'Add API' }).click(); + await page.getByLabel('Name', { exact: true }).fill('Reserved API'); + await page.getByLabel('Resource').fill(issuer); + await page.getByRole('button', { name: 'Save' }).click(); + + await expect(page.locator('[data-type="error"]')).toContainText('reserved'); +}); + +test('Edit the name of an API', async ({ page }) => { + await page.goto(`/settings/admin/apis/${apis.orders.id}`); + + await page.getByLabel('Name', { exact: true }).fill('Orders API renamed'); + await page.getByRole('button', { name: 'Save' }).nth(0).click(); + + await expect(page.locator('[data-type="success"]')).toHaveText('API updated successfully'); + + await page.reload(); + await expect(page.getByLabel('Name', { exact: true })).toHaveValue('Orders API renamed'); +}); + +test('Add a permission to an API', async ({ page }) => { + await page.goto(`/settings/admin/apis/${apis.orders.id}`); + + // The seeded API already has permissions, so the button reads "Add another" + await page.getByRole('button', { name: 'Add another' }).click(); + await page.getByPlaceholder('Permission', { exact: true }).last().fill('ship:orders'); + await page.getByPlaceholder('Name', { exact: true }).last().fill('Ship orders'); + await page.getByRole('button', { name: 'Save' }).nth(1).click(); + + await expect(page.locator('[data-type="success"]')).toHaveText( + 'Permissions updated successfully' + ); + + await page.reload(); + // The two seeded permissions plus the newly added one + await expect(page.getByPlaceholder('Permission', { exact: true })).toHaveCount(3); +}); + +test('Delete an API', async ({ page }) => { + await page.goto('/settings/admin/apis'); + + await page.getByRole('row', { name: apis.orders.name }).getByRole('button').click(); + await page.getByRole('menuitem', { name: 'Delete' }).click(); + await page.getByRole('button', { name: 'Delete' }).click(); + + await expect(page.locator('[data-type="success"]')).toHaveText('API deleted successfully'); + await expect(page.getByRole('row', { name: apis.orders.name })).not.toBeVisible(); +}); + +test('Grant a client user-delegated and client access to API permissions', async ({ page }) => { + // Nextcloud has no API access granted by default + await page.goto(`/settings/admin/oidc-clients/${oidcClients.nextcloud.id}`); + + // Expand the API access card, then edit the Orders API row + await page.getByText('API access', { exact: true }).click(); + await page + .getByRole('row', { name: apis.orders.name }) + .getByRole('button', { name: 'Edit' }) + .click(); + + // Grant read:orders and write:orders on behalf of users, but only write:orders for the client itself + const dialog = page.getByRole('dialog'); + await dialog + .getByRole('checkbox', { + name: `User-delegated access: ${apis.orders.permissions.readOrders.name}` + }) + .click(); + await dialog + .getByRole('checkbox', { + name: `User-delegated access: ${apis.orders.permissions.writeOrders.name}` + }) + .click(); + await dialog + .getByRole('checkbox', { + name: `Client access (M2M): ${apis.orders.permissions.writeOrders.name}` + }) + .click(); + await dialog.getByRole('button', { name: 'Save' }).click(); + + await expect(page.locator('[data-type="success"]')).toHaveText('API access updated successfully'); + // Both subject types keep their own count: 2 / 2 user-delegated, 1 / 2 client access + const row = page.getByRole('row', { name: apis.orders.name }); + await expect(row).toContainText('2 / 2'); + await expect(row).toContainText('1 / 2'); +}); + +// --------------------------------------------------------------------------- +// Authorization flow with the RFC 8707 resource parameter +// --------------------------------------------------------------------------- + +test('Authorization with a resource parameter issues a token audienced to that API', async ({ + page, + baseURL +}) => { + const client = oidcClients.immich; + const api = apis.orders; + + const params = new URLSearchParams({ + client_id: client.id, + response_type: 'code', + scope: 'openid email read:orders', + resource: api.resource, + redirect_uri: client.callbackUrl, + state: 'nXx-6Qr-owc1SHBa', + nonce: 'P1gN3PtpKHJgKUVcLpLjm' + }); + + const callbackUrl = await oidcUtil.interceptCallbackRedirect( + page, + new URL(client.callbackUrl).pathname, + async () => { + await page.goto(`/authorize?${params.toString()}`); + await page.getByRole('button', { name: 'Sign in' }).click(); + } + ); + const code = callbackUrl.searchParams.get('code'); + expect(code).toBeTruthy(); + + const res = await oidcUtil.exchangeCode(page, { + grant_type: 'authorization_code', + redirect_uri: client.callbackUrl, + code: code!, + client_id: client.id, + client_secret: client.secret + }); + expect(res.access_token).toBeTruthy(); + + const claims = jose.decodeJwt(res.access_token!); + expect(tokenAudiences(claims)).toContain(api.resource); + // Because openid was requested alongside the resource, the token also carries the issuer audience so it can still reach /userinfo + expect(tokenAudiences(claims)).toContain(baseURL); + expect(tokenScopes(claims)).toContain(api.permissions.readOrders.key); + + // The same token can be presented at userinfo, by the client's explicit opt-in of requesting openid + const userinfo = await page.request.get('/api/oidc/userinfo', { + headers: { Authorization: 'Bearer ' + res.access_token } + }); + expect(userinfo.status()).toBe(200); +}); + +test('Consent screen shows the friendly permission name for a resource request', async ({ + page +}) => { + const client = oidcClients.immich; + const api = apis.orders; + + const params = new URLSearchParams({ + client_id: client.id, + response_type: 'code', + scope: 'openid read:orders', + resource: api.resource, + redirect_uri: client.callbackUrl, + state: 'nXx-6Qr-owc1SHBa' + }); + await page.goto(`/authorize?${params.toString()}`); + + const scopeList = page.getByTestId('scopes'); + await expect(scopeList).toBeVisible(); + // The permission's friendly name is shown, not the raw scope key + await expect(scopeList.getByText(api.permissions.readOrders.name, { exact: true })).toBeVisible(); +}); + +test('Requesting a custom scope without its resource is rejected with invalid_scope', async ({ + page +}) => { + const client = oidcClients.immich; + + // The client is allowed read:orders, but it is requested without the resource parameter + const params = new URLSearchParams({ + client_id: client.id, + response_type: 'code', + scope: 'openid read:orders', + redirect_uri: client.callbackUrl, + state: 'nXx-6Qr-owc1SHBa' + }); + + const callbackUrl = await oidcUtil.interceptCallbackRedirect( + page, + new URL(client.callbackUrl).pathname, + async () => { + await page.goto(`/authorize?${params.toString()}`); + } + ); + + expect(callbackUrl.searchParams.get('error')).toBe('invalid_scope'); + expect(callbackUrl.searchParams.get('state')).toBe('nXx-6Qr-owc1SHBa'); +}); + +// --------------------------------------------------------------------------- +// Separation of user-delegated and client (machine-to-machine) access +// --------------------------------------------------------------------------- + +test('Client credentials issues a token for a client-granted permission', async ({ page }) => { + const client = oidcClients.immich; + const api = apis.orders; + + // write:orders is granted to Immich for client access + const res = await oidcUtil.exchangeCode(page, { + grant_type: 'client_credentials', + client_id: client.id, + client_secret: client.secret, + scope: api.permissions.writeOrders.key, + resource: api.resource + }); + expect(res.access_token).toBeTruthy(); + + const claims = jose.decodeJwt(res.access_token!); + expect(tokenAudiences(claims)).toContain(api.resource); + expect(tokenScopes(claims)).toContain(api.permissions.writeOrders.key); +}); + +test('Client credentials cannot mint a permission that is only user-delegated', async ({ + page +}) => { + const client = oidcClients.immich; + const api = apis.orders; + + // read:orders is only granted for user-delegated access + const res = await oidcUtil.exchangeCode(page, { + grant_type: 'client_credentials', + client_id: client.id, + client_secret: client.secret, + scope: api.permissions.readOrders.key, + resource: api.resource + }); + + expect(res.access_token).toBeFalsy(); + expect(res.error).toBe('invalid_scope'); +}); + +test('Authorization on behalf of a user cannot request a client-only permission', async ({ + page +}) => { + const client = oidcClients.immich; + const api = apis.orders; + + // write:orders is only granted for client access, so users cannot be asked to delegate it + const params = new URLSearchParams({ + client_id: client.id, + response_type: 'code', + scope: `openid ${api.permissions.writeOrders.key}`, + resource: api.resource, + redirect_uri: client.callbackUrl, + state: 'nXx-6Qr-owc1SHBa' + }); + + const callbackUrl = await oidcUtil.interceptCallbackRedirect( + page, + new URL(client.callbackUrl).pathname, + async () => { + await page.goto(`/authorize?${params.toString()}`); + } + ); + + // The authorize endpoint collapses every resource-targeted scope/resource failure into a generic invalid_request + expect(callbackUrl.searchParams.get('error')).toBe('invalid_request'); + expect(callbackUrl.searchParams.get('state')).toBe('nXx-6Qr-owc1SHBa'); +}); diff --git a/tests/specs/oidc.spec.ts b/tests/specs/oidc.spec.ts index b3461b87..4ca4d382 100644 --- a/tests/specs/oidc.spec.ts +++ b/tests/specs/oidc.spec.ts @@ -398,7 +398,8 @@ test.describe('Introspection endpoint', () => { expect(introspectionBody.active).toBe(true); expect(introspectionBody.iss).toBe(baseURL); expect(introspectionBody.sub).toBe(users.tim.id); - expect(introspectionBody.aud).toStrictEqual([oidcClients.nextcloud.id]); + // An identity access token is audienced to the client and additionally to the issuer, so it can be presented at /userinfo + expect(introspectionBody.aud).toStrictEqual([oidcClients.nextcloud.id, baseURL]); }); test('succeeds with federated client credentials', async ({ page, request, baseURL }) => { @@ -427,7 +428,8 @@ test.describe('Introspection endpoint', () => { expect(introspectionBody.active).toBe(true); expect(introspectionBody.iss).toBe(baseURL); expect(introspectionBody.sub).toBe(users.tim.id); - expect(introspectionBody.aud).toStrictEqual([oidcClients.federated.id]); + // An identity access token is audienced to the client and additionally to the issuer, so it can be presented at /userinfo + expect(introspectionBody.aud).toStrictEqual([oidcClients.federated.id, baseURL]); }); test('fails with client credentials for wrong app', async ({ request }) => { diff --git a/tests/specs/scim.spec.ts b/tests/specs/scim.spec.ts index c4711c23..2b7ccafe 100644 --- a/tests/specs/scim.spec.ts +++ b/tests/specs/scim.spec.ts @@ -5,7 +5,7 @@ import { oidcClients, userGroups, users } from '../data'; async function configureOidcClient(page: Page) { await page.goto(`/settings/admin/oidc-clients/${oidcClients.scim.id}`); - await page.getByRole('button', { name: 'Expand card' }).nth(1).click(); + await page.getByText('SCIM Provisioning', { exact: true }).click(); await page .getByLabel('SCIM Endpoint') @@ -29,7 +29,7 @@ test.describe('SCIM Configuration', () => { test('Enable SCIM for OIDC client', async ({ page }) => { await page.goto(`/settings/admin/oidc-clients/${oidcClients.scim.id}`); - await page.getByRole('button', { name: 'Expand card' }).nth(1).click(); + await page.getByText('SCIM Provisioning', { exact: true }).click(); await page.getByLabel('SCIM Endpoint').fill('http://scim.provider/api'); await page.getByLabel('SCIM Token').fill('supersecrettoken');