feat: support multiple client secrets per OIDC client

Fixes #1659
This commit is contained in:
ItalyPaleAle
2026-08-09 17:38:53 -07:00
parent 875e7f0848
commit 379be0dbbc
27 changed files with 1366 additions and 152 deletions

View File

@@ -34,7 +34,10 @@ func NewOidcController(group *gin.RouterGroup, authMiddleware *middleware.AuthMi
group.DELETE("/oidc/clients/:id", authMiddleware.Add(), httpserver.Handle(oc.deleteClientHandler))
group.PUT("/oidc/clients/:id/allowed-user-groups", authMiddleware.Add(), httpserver.Handle(oc.updateAllowedUserGroupsHandler))
group.POST("/oidc/clients/:id/secret", authMiddleware.Add(), httpserver.Handle(oc.createClientSecretHandler))
group.POST("/oidc/clients/:id/secret", authMiddleware.Add(), httpserver.Handle(oc.replaceClientSecretHandler))
group.GET("/oidc/clients/:id/secrets", authMiddleware.Add(), httpserver.Handle(oc.listClientSecretsHandler))
group.POST("/oidc/clients/:id/secrets", authMiddleware.Add(), httpserver.Handle(oc.createClientSecretHandler))
group.DELETE("/oidc/clients/:id/secrets/:secretId", authMiddleware.Add(), httpserver.Handle(oc.deleteClientSecretHandler))
group.GET("/oidc/clients/:id/logo", httpserver.Handle(oc.getClientLogoHandler))
group.DELETE("/oidc/clients/:id/logo", authMiddleware.Add(), httpserver.Handle(oc.deleteClientLogoHandler))
@@ -161,7 +164,8 @@ func (oc *OidcController) listClientsHandler(c *gin.Context) error {
// @Router /api/oidc/clients [post]
func (oc *OidcController) createClientHandler(c *gin.Context) error {
var input dto.OidcClientCreateDto
if err := httpserver.BindJSON(c, &input); err != nil {
err := httpserver.BindJSON(c, &input)
if err != nil {
return err
}
@@ -171,7 +175,8 @@ func (oc *OidcController) createClientHandler(c *gin.Context) error {
}
var clientDto dto.OidcClientWithAllowedUserGroupsDto
if err := dto.MapStruct(client, &clientDto); err != nil {
err = dto.MapStruct(client, &clientDto)
if err != nil {
return err
}
@@ -208,7 +213,8 @@ func (oc *OidcController) deleteClientHandler(c *gin.Context) error {
// @Router /api/oidc/clients/{id} [put]
func (oc *OidcController) updateClientHandler(c *gin.Context) error {
var input dto.OidcClientUpdateDto
if err := httpserver.BindJSON(c, &input); err != nil {
err := httpserver.BindJSON(c, &input)
if err != nil {
return err
}
@@ -218,7 +224,8 @@ func (oc *OidcController) updateClientHandler(c *gin.Context) error {
}
var clientDto dto.OidcClientWithAllowedUserGroupsDto
if err := dto.MapStruct(client, &clientDto); err != nil {
err = dto.MapStruct(client, &clientDto)
if err != nil {
return err
}
@@ -241,7 +248,8 @@ func (oc *OidcController) refreshClientMetadataHandler(c *gin.Context) error {
}
var clientDto dto.OidcClientWithAllowedUserGroupsDto
if err := dto.MapStruct(client, &clientDto); err != nil {
err = dto.MapStruct(client, &clientDto)
if err != nil {
return err
}
@@ -250,23 +258,24 @@ func (oc *OidcController) refreshClientMetadataHandler(c *gin.Context) error {
return nil
}
// createClientSecretHandler godoc
// @Summary Create client secret
// @Description Set or generate a new secret for an OIDC client
// replaceClientSecretHandler godoc
// @Summary Replace client secrets
// @Description Set or generate a new secret for an OIDC client, removing all of its existing secrets. Deprecated: use POST /api/oidc/clients/{id}/secrets to add a secret without invalidating the existing ones.
// @Tags OIDC
// @Accept json
// @Produce json
// @Param id path string true "Client ID"
// @Param payload body dto.OidcClientSecretDto false "Client secret"
// @Param payload body dto.OidcClientSecretCreateDto false "Client secret"
// @Success 200 {object} object "{ \"secret\": \"string\" }"
// @Deprecated true
// @Router /api/oidc/clients/{id}/secret [post]
func (oc *OidcController) createClientSecretHandler(c *gin.Context) error {
var input dto.OidcClientSecretDto
func (oc *OidcController) replaceClientSecretHandler(c *gin.Context) error {
var input dto.OidcClientSecretCreateDto
if err := httpserver.BindOptionalJSON(c, &input); err != nil {
return err
}
secret, err := oc.oidcService.CreateClientSecret(c.Request.Context(), c.Param("id"), input)
_, secret, err := oc.oidcService.ReplaceClientSecrets(c.Request.Context(), c.Param("id"), input)
if err != nil {
return err
}
@@ -275,6 +284,81 @@ func (oc *OidcController) createClientSecretHandler(c *gin.Context) error {
return nil
}
// listClientSecretsHandler godoc
// @Summary List client secrets
// @Description List the secrets of an OIDC client, without disclosing their values
// @Tags OIDC
// @Produce json
// @Param id path string true "Client ID"
// @Success 200 {array} dto.OidcClientSecretDto "Client secrets"
// @Router /api/oidc/clients/{id}/secrets [get]
func (oc *OidcController) listClientSecretsHandler(c *gin.Context) error {
secrets, err := oc.oidcService.ListClientSecrets(c.Request.Context(), c.Param("id"))
if err != nil {
return err
}
var secretsDto []dto.OidcClientSecretDto
err = dto.MapStructList(secrets, &secretsDto)
if err != nil {
return err
}
c.JSON(http.StatusOK, secretsDto)
return nil
}
// createClientSecretHandler godoc
// @Summary Create client secret
// @Description Add a new secret to an OIDC client, leaving the existing ones usable. The value is only returned by this endpoint and cannot be retrieved later.
// @Tags OIDC
// @Accept json
// @Produce json
// @Param id path string true "Client ID"
// @Param payload body dto.OidcClientSecretCreateDto false "Client secret"
// @Success 201 {object} dto.OidcClientSecretCreatedDto "Created client secret"
// @Router /api/oidc/clients/{id}/secrets [post]
func (oc *OidcController) createClientSecretHandler(c *gin.Context) error {
var input dto.OidcClientSecretCreateDto
err := httpserver.BindOptionalJSON(c, &input)
if err != nil {
return err
}
created, secret, err := oc.oidcService.CreateClientSecret(c.Request.Context(), c.Param("id"), input)
if err != nil {
return err
}
var secretDto dto.OidcClientSecretCreatedDto
err = dto.MapStruct(created, &secretDto)
if err != nil {
return err
}
secretDto.Secret = secret
c.JSON(http.StatusCreated, secretDto)
return nil
}
// deleteClientSecretHandler godoc
// @Summary Delete client secret
// @Description Delete a single secret of an OIDC client, making it immediately unusable
// @Tags OIDC
// @Param id path string true "Client ID"
// @Param secretId path string true "Client secret ID"
// @Success 204 "No content"
// @Router /api/oidc/clients/{id}/secrets/{secretId} [delete]
func (oc *OidcController) deleteClientSecretHandler(c *gin.Context) error {
err := oc.oidcService.DeleteClientSecret(c.Request.Context(), c.Param("id"), c.Param("secretId"))
if err != nil {
return err
}
c.Status(http.StatusNoContent)
return nil
}
// getClientLogoHandler godoc
// @Summary Get client logo
// @Description Get the logo image for an OIDC client

View File

@@ -64,12 +64,34 @@ type OidcClientCreateDto struct {
ID string `json:"id" binding:"omitempty,client_id,min=2,max=128"`
}
// OidcClientSecretDto describes a client secret without disclosing its value, which is only ever returned right after the secret is created
type OidcClientSecretDto struct {
ID string `json:"id"`
// Prefix holds the first few characters of the secret in clear text, and is empty for secrets migrated from the single-secret column
Prefix string `json:"prefix"`
CreatedAt datatype.DateTime `json:"createdAt"`
ExpiresAt *datatype.DateTime `json:"expiresAt"`
IsActive bool `json:"isActive"`
}
// OidcClientSecretCreateDto is the request body for creating a new client secret
type OidcClientSecretCreateDto struct {
// Secret allows callers to supply their own value instead of having Pocket ID generate one
Secret string `json:"secret" binding:"omitempty,min=16,printascii"`
// ExpiresAt makes the secret unusable after the given time (if nil, secrets don't expire)
ExpiresAt *datatype.DateTime `json:"expiresAt"`
}
// OidcClientSecretCreatedDto is returned when a secret is created, and is the only response that contains the secret's value
type OidcClientSecretCreatedDto struct {
OidcClientSecretDto
Secret string `json:"secret"`
}
type OidcClientCredentialsDto struct {
FederatedIdentities []OidcClientFederatedIdentityDto `json:"federatedIdentities,omitempty"`
// Secrets is read-only: secrets are managed through the dedicated client secret endpoints and any value sent by a client is ignored
Secrets []OidcClientSecretDto `json:"secrets,omitempty"`
}
type OidcClientFederatedIdentityDto struct {

View File

@@ -3,10 +3,14 @@ package dto
import (
"encoding/json"
"testing"
"time"
"github.com/gin-gonic/gin/binding"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/pocket-id/pocket-id/backend/internal/model"
datatype "github.com/pocket-id/pocket-id/backend/internal/model/types"
)
func TestOidcClientUpdateDto_tokenLifetimes(t *testing.T) {
@@ -70,3 +74,35 @@ func TestOidcClientUpdateDto_tokenLifetimes(t *testing.T) {
})
}
}
func TestOidcClientDto_secrets(t *testing.T) {
expired := datatype.DateTime(time.Now().Add(-time.Hour))
valid := datatype.DateTime(time.Now().Add(time.Hour))
client := model.OidcClient{
Base: model.Base{ID: "client-id"},
Name: "Test Client",
Credentials: model.OidcClientCredentials{
Secrets: []model.OidcClientSecret{
{ID: "active", Algorithm: model.OidcClientSecretHashSHA256, Hash: "hash-1", Prefix: "abcd"},
{ID: "expiring", Algorithm: model.OidcClientSecretHashSHA256, Hash: "hash-2", Prefix: "efgh", ExpiresAt: &valid},
{ID: "expired", Algorithm: model.OidcClientSecretHashSHA256, Hash: "hash-3", Prefix: "ijkl", ExpiresAt: &expired},
},
},
}
var clientDto OidcClientDto
require.NoError(t, MapStruct(client, &clientDto))
require.Len(t, clientDto.Credentials.Secrets, 3)
assert.Equal(t, "active", clientDto.Credentials.Secrets[0].ID)
assert.Equal(t, "abcd", clientDto.Credentials.Secrets[0].Prefix)
assert.True(t, clientDto.Credentials.Secrets[0].IsActive)
assert.True(t, clientDto.Credentials.Secrets[1].IsActive)
assert.False(t, clientDto.Credentials.Secrets[2].IsActive)
// Serializing the client must never disclose the hashes of its secrets
serialized, err := json.Marshal(clientDto)
require.NoError(t, err)
assert.NotContains(t, string(serialized), "hash-1")
assert.Contains(t, string(serialized), `"isActive":true`)
}

View File

@@ -3,6 +3,7 @@ package model
import (
"database/sql/driver"
"encoding/json"
"time"
datatype "github.com/pocket-id/pocket-id/backend/internal/model/types"
"github.com/pocket-id/pocket-id/backend/internal/utils"
@@ -34,6 +35,11 @@ const (
MinTokenDurationMinutes int64 = 1
// MaxTokenDurationMinutes is the longest configurable token lifetime
MaxTokenDurationMinutes int64 = 365 * 24 * 60
// MaxOidcClientSecrets is the number of secrets a single client can have at the same time, including expired ones
MaxOidcClientSecrets = 20
// OidcClientSecretPrefixLength is how many leading characters of a client secret are kept in clear text so admins can tell secrets apart
OidcClientSecretPrefixLength = 4
)
type OidcClient struct {
@@ -41,7 +47,6 @@ type OidcClient struct {
Name string `sortable:"true"`
Description string
Secret string
CallbackURLs datatype.StringList
LogoutCallbackURLs datatype.StringList
ImageType *string
@@ -88,6 +93,65 @@ func (c OidcClient) IsMetadataDocument() bool {
type OidcClientCredentials struct { //nolint:recvcheck
FederatedIdentities []OidcClientFederatedIdentity `json:"federatedIdentities,omitempty"`
Secrets []OidcClientSecret `json:"secrets,omitempty"`
}
// OidcClientSecretHashAlgorithm identifies how the hash of a client secret was computed
type OidcClientSecretHashAlgorithm string
const (
// OidcClientSecretHashSHA256 is used by every client secret generated by Pocket ID
// A plain hash is enough because client secrets are generated with enough entropy that they cannot be brute-forced
OidcClientSecretHashSHA256 OidcClientSecretHashAlgorithm = "sha256"
// OidcClientSecretHashBcrypt is only found on secrets migrated from the single-secret column that Pocket ID used before it supported multiple secrets
// Those hashes cannot be converted to SHA-256, so they are kept as-is and verified with bcrypt until the admin rotates the secret
OidcClientSecretHashBcrypt OidcClientSecretHashAlgorithm = "bcrypt"
)
// OidcClientSecret is a single client secret of an OIDC client, stored hashed in the credentials JSON document
type OidcClientSecret struct {
// ID uniquely identifies the secret within the client, so it can be deleted without knowing its value
ID string `json:"id"`
// Algorithm used to compute Hash
Algorithm OidcClientSecretHashAlgorithm `json:"alg"`
// Hash of the secret's value, hex-encoded for SHA-256 secrets and in modular crypt format for bcrypt ones
Hash string `json:"hash"`
// Prefix contains the first few characters of the secret in clear text, so admins can match a secret to the app that uses it
// It is empty for secrets migrated from the single-secret column, whose value was never stored
Prefix string `json:"prefix,omitempty"`
CreatedAt datatype.DateTime `json:"createdAt"`
ExpiresAt *datatype.DateTime `json:"expiresAt,omitempty"`
}
// IsActive reports whether the secret can still be used to authenticate the client
func (s OidcClientSecret) IsActive() bool {
return !s.IsExpiredAt(time.Now())
}
// IsExpiredAt reports whether the secret is expired at the given time
// Secrets without an expiration date are always valid
func (s OidcClientSecret) IsExpiredAt(now time.Time) bool {
return s.ExpiresAt != nil && !now.Before(s.ExpiresAt.ToTime())
}
// EncodedHash returns the secret's hash with its algorithm prepended, in the format "<alg>:<hash>"
// Fosite compares hashes without any other context, so the algorithm has to travel alongside the hash
func (s OidcClientSecret) EncodedHash() []byte {
return []byte(string(s.Algorithm) + ":" + s.Hash)
}
// ActiveSecrets returns the secrets that have not expired yet, ordered from the most recently created to the oldest
func (occ OidcClientCredentials) ActiveSecrets() []OidcClientSecret {
now := time.Now()
active := make([]OidcClientSecret, 0, len(occ.Secrets))
for _, secret := range occ.Secrets {
if !secret.IsExpiredAt(now) {
active = append(active, secret)
}
}
return active
}
type OidcClientFederatedIdentity struct {

View File

@@ -19,8 +19,28 @@ func (c Client) GetID() string {
return c.ID
}
// GetHashedSecret returns the hash of the client's most recently created secret that is still active
// Fosite checks this one first and then falls back to GetRotatedHashes for the others
func (c Client) GetHashedSecret() []byte {
return []byte(c.Secret)
active := c.Credentials.ActiveSecrets()
if len(active) == 0 {
return nil
}
return active[0].EncodedHash()
}
// GetRotatedHashes returns the hashes of every active secret except the one returned by GetHashedSecret
func (c Client) GetRotatedHashes() [][]byte {
active := c.Credentials.ActiveSecrets()
if len(active) <= 1 {
return nil
}
hashes := make([][]byte, len(active)-1)
for i, secret := range active[1:] {
hashes[i] = secret.EncodedHash()
}
return hashes
}
func (c Client) GetRedirectURIs() []string {

View File

@@ -0,0 +1,49 @@
package oidc
import (
"context"
"crypto/sha256"
"crypto/subtle"
"encoding/hex"
"errors"
"strings"
"golang.org/x/crypto/bcrypt"
"github.com/pocket-id/pocket-id/backend/internal/model"
)
// errUnsupportedHashAlgorithm is returned when a stored hash carries an algorithm this build cannot verify
var errUnsupportedHashAlgorithm = errors.New("unsupported client secret hash algorithm")
// clientSecretHasher verifies client secrets against the hashes stored in the client's credentials document
// Fosite hands it the encoded hashes produced by model.OidcClientSecret.EncodedHash, which carry the algorithm they were computed with
type clientSecretHasher struct{}
// Compare checks a presented client secret against a stored, algorithm-prefixed hash
func (clientSecretHasher) Compare(_ context.Context, hash []byte, data []byte) error {
algorithm, encoded, found := strings.Cut(string(hash), ":")
if !found {
return errUnsupportedHashAlgorithm
}
switch model.OidcClientSecretHashAlgorithm(algorithm) {
case model.OidcClientSecretHashSHA256:
sum := sha256.Sum256(data)
if subtle.ConstantTimeCompare([]byte(hex.EncodeToString(sum[:])), []byte(encoded)) != 1 {
return errors.New("client secret does not match")
}
return nil
case model.OidcClientSecretHashBcrypt:
// Legacy hashes migrated from the single-secret column, which cannot be re-hashed because the secret's value is not recoverable
return bcrypt.CompareHashAndPassword([]byte(encoded), data)
default:
return errUnsupportedHashAlgorithm
}
}
// Hash is required by the fosite.Hasher interface but is never called, because Pocket ID hashes secrets when it stores them
func (clientSecretHasher) Hash(_ context.Context, data []byte) ([]byte, error) {
sum := sha256.Sum256(data)
return []byte(string(model.OidcClientSecretHashSHA256) + ":" + hex.EncodeToString(sum[:])), nil
}

View File

@@ -0,0 +1,99 @@
package oidc
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/crypto/bcrypt"
"github.com/pocket-id/pocket-id/backend/internal/model"
"github.com/pocket-id/pocket-id/backend/internal/utils"
)
func TestClientSecretHasherCompare(t *testing.T) {
const value = "client-secret-value"
bcryptHash, err := bcrypt.GenerateFromPassword([]byte(value), bcrypt.MinCost)
require.NoError(t, err)
for _, test := range []struct {
name string
hash string
data string
wantErr bool
}{
{
name: "matching SHA-256 hash",
hash: "sha256:" + utils.CreateSha256Hash(value),
data: value,
},
{
name: "SHA-256 hash of another secret",
hash: "sha256:" + utils.CreateSha256Hash(value),
data: "some-other-value",
wantErr: true,
},
{
name: "bcrypt hash migrated from the single-secret column",
hash: "bcrypt:" + string(bcryptHash),
data: value,
},
{
name: "bcrypt hash of another secret",
hash: "bcrypt:" + string(bcryptHash),
data: "some-other-value",
wantErr: true,
},
{
name: "hash without an algorithm",
hash: utils.CreateSha256Hash(value),
data: value,
wantErr: true,
},
{
name: "unknown algorithm",
hash: "md5:" + utils.CreateSha256Hash(value),
data: value,
wantErr: true,
},
{
name: "empty hash, as returned for a client without secrets",
hash: "",
data: value,
wantErr: true,
},
} {
t.Run(test.name, func(t *testing.T) {
err := clientSecretHasher{}.Compare(t.Context(), []byte(test.hash), []byte(test.data))
if test.wantErr {
require.Error(t, err)
} else {
require.NoError(t, err)
}
})
}
}
func TestClientSecretHasherHash(t *testing.T) {
const value = "client-secret-value"
hash, err := clientSecretHasher{}.Hash(t.Context(), []byte(value))
require.NoError(t, err)
assert.Equal(t, "sha256:"+utils.CreateSha256Hash(value), string(hash))
// A hash produced by the hasher is accepted by its own comparison
err = clientSecretHasher{}.Compare(t.Context(), hash, []byte(value))
require.NoError(t, err)
}
func TestClientSecretHasherAcceptsModelEncoding(t *testing.T) {
const value = "client-secret-value"
secret := model.OidcClientSecret{
Algorithm: model.OidcClientSecretHashSHA256,
Hash: utils.CreateSha256Hash(value),
}
err := clientSecretHasher{}.Compare(t.Context(), secret.EncodedHash(), []byte(value))
require.NoError(t, err)
}

View File

@@ -1,13 +1,17 @@
package oidc
import (
"strconv"
"testing"
"time"
"github.com/ory/fosite"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/pocket-id/pocket-id/backend/internal/model"
datatype "github.com/pocket-id/pocket-id/backend/internal/model/types"
"github.com/pocket-id/pocket-id/backend/internal/utils"
)
// Interface assertions
@@ -15,8 +19,26 @@ var (
_ fosite.Client = (*Client)(nil)
_ fosite.ResponseModeClient = (*Client)(nil)
_ fosite.ClientWithCustomTokenLifespans = (*Client)(nil)
_ fosite.ClientWithSecretRotation = (*Client)(nil)
)
// testClientCredentials returns credentials with a single never-expiring secret with the given value
func testClientCredentials(values ...string) model.OidcClientCredentials {
credentials := model.OidcClientCredentials{
Secrets: make([]model.OidcClientSecret, len(values)),
}
for i, value := range values {
credentials.Secrets[i] = model.OidcClientSecret{
ID: "secret-" + strconv.Itoa(i),
Algorithm: model.OidcClientSecretHashSHA256,
Hash: utils.CreateSha256Hash(value),
Prefix: value[:model.OidcClientSecretPrefixLength],
CreatedAt: datatype.DateTime(time.Now()),
}
}
return credentials
}
func TestClientGetEffectiveLifespan(t *testing.T) {
client := Client{OidcClient: model.OidcClient{
AccessTokenDurationMinutes: 2 * 60,
@@ -51,3 +73,41 @@ func TestClientGetEffectiveLifespan(t *testing.T) {
require.Equal(t, fallback, client.GetEffectiveLifespan(fosite.GrantTypeAuthorizationCode, fosite.AccessToken, fallback))
require.Equal(t, fallback, client.GetEffectiveLifespan(fosite.GrantTypeAuthorizationCode, fosite.RefreshToken, fallback))
}
func TestClientSecretHashes(t *testing.T) {
past := datatype.DateTime(time.Now().Add(-time.Hour))
future := datatype.DateTime(time.Now().Add(time.Hour))
older := datatype.DateTime(time.Now().Add(-48 * time.Hour))
newer := datatype.DateTime(time.Now().Add(-time.Minute))
t.Run("client without secrets", func(t *testing.T) {
client := Client{}
assert.Nil(t, client.GetHashedSecret())
assert.Empty(t, client.GetRotatedHashes())
})
t.Run("expired secrets are never returned", func(t *testing.T) {
client := Client{OidcClient: model.OidcClient{Credentials: model.OidcClientCredentials{
Secrets: []model.OidcClientSecret{
{ID: "expired", Algorithm: model.OidcClientSecretHashSHA256, Hash: "expired-hash", ExpiresAt: &past},
},
}}}
assert.Nil(t, client.GetHashedSecret())
assert.Empty(t, client.GetRotatedHashes())
})
t.Run("the most recent active secret comes first", func(t *testing.T) {
client := Client{OidcClient: model.OidcClient{Credentials: model.OidcClientCredentials{
Secrets: []model.OidcClientSecret{
{ID: "older", Algorithm: model.OidcClientSecretHashSHA256, Hash: "older-hash", CreatedAt: older},
{ID: "expired", Algorithm: model.OidcClientSecretHashSHA256, Hash: "expired-hash", CreatedAt: newer, ExpiresAt: &past},
{ID: "newer", Algorithm: model.OidcClientSecretHashSHA256, Hash: "newer-hash", CreatedAt: newer, ExpiresAt: &future},
},
}}}
assert.Equal(t, "sha256:newer-hash", string(client.GetHashedSecret()))
rotated := client.GetRotatedHashes()
require.Len(t, rotated, 1)
assert.Equal(t, "sha256:older-hash", string(rotated[0]))
})
}

View File

@@ -57,6 +57,7 @@ func newProvider(store *Store, authenticator *federatedClientAuthenticator, sign
GlobalSecret: secret,
JWTScopeClaimKey: jwt.JWTScopeFieldBoth,
ClientResolver: clientResolver,
ClientSecretsHasher: clientSecretHasher{},
}
keyGetter := func(context.Context) (interface{}, error) {

View File

@@ -20,10 +20,10 @@ import (
fositeoauth2 "github.com/ory/fosite/handler/oauth2"
fositejwt "github.com/ory/fosite/token/jwt"
"github.com/stretchr/testify/require"
"golang.org/x/crypto/bcrypt"
"gorm.io/gorm"
"github.com/pocket-id/pocket-id/backend/internal/model"
datatype "github.com/pocket-id/pocket-id/backend/internal/model/types"
testutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing"
)
@@ -49,12 +49,10 @@ func TestTokenHandlerClientCredentialsGrant(t *testing.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),
Credentials: testClientCredentials(clientPlain),
IsPublic: false,
AccessTokenDurationMinutes: 2 * 60,
}).Error)
@@ -111,13 +109,11 @@ func TestTokenHandlerClientCredentialsDropsIdentityScopes(t *testing.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,
Base: model.Base{ID: clientID},
Name: "Client Credentials Client",
Credentials: testClientCredentials(clientPlain),
IsPublic: false,
}).Error)
provider, err := newProvider(NewStore(db, nil), nil, testTokenSigner{key: key}, Config{
@@ -166,13 +162,11 @@ func TestTokenHandlerClientCredentialsUsesClientSubjectGrants(t *testing.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,
Base: model.Base{ID: clientID},
Name: "Client Credentials Client",
Credentials: testClientCredentials(clientPlain),
IsPublic: false,
}).Error)
apiAccess := fakeAPIAccess{allowed: map[string]map[SubjectType][]string{
@@ -239,13 +233,11 @@ func TestTokenHandlerClientCredentialsDefaultsResourceScopes(t *testing.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,
Base: model.Base{ID: clientID},
Name: "Client Credentials Client",
Credentials: testClientCredentials(clientPlain),
IsPublic: false,
}).Error)
apiAccess := fakeAPIAccess{allowed: map[string]map[SubjectType][]string{
@@ -731,3 +723,124 @@ func TestTokenHandlerRefreshGrantPreservesAudienceAndScope(t *testing.T) {
require.Equal(t, "invalid_scope", body["error"])
})
}
// TestTokenHandlerClientSecretRotation drives the token endpoint with each of a client's secrets,
// to check that every active secret authenticates the client while expired ones are rejected.
func TestTokenHandlerClientSecretRotation(t *testing.T) {
gin.SetMode(gin.TestMode)
const (
baseURL = "https://issuer.example.com"
secret = "test-secret"
clientID = "rotation-client"
currentPlain = "current-secret-value"
olderPlain = "older-secret-value"
expiredPlain = "expired-secret-value"
)
db := testutils.NewDatabaseForTest(t)
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
require.NoError(t, err)
credentials := testClientCredentials(currentPlain, olderPlain, expiredPlain)
expiredAt := datatype.DateTime(time.Now().Add(-time.Hour))
credentials.Secrets[2].ExpiresAt = &expiredAt
require.NoError(t, db.Create(&model.OidcClient{
Base: model.Base{ID: clientID},
Name: "Secret Rotation Client",
Credentials: credentials,
IsPublic: false,
}).Error)
provider, err := newProvider(NewStore(db, nil), nil, testTokenSigner{key: key}, Config{
BaseURL: baseURL,
TokenBaseURL: baseURL,
Secret: []byte(secret),
}, nil)
require.NoError(t, err)
handler := newTokenHandler(provider, newClaimsService(db, nil, baseURL, nil), nil)
requestToken := func(t *testing.T, clientSecret string) map[string]any {
t.Helper()
form := url.Values{"grant_type": {"client_credentials"}}
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, clientSecret)
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
}
for _, test := range []struct {
name string
clientSecret string
wantToken bool
}{
{name: "most recently created secret", clientSecret: currentPlain, wantToken: true},
{name: "secret being rotated out", clientSecret: olderPlain, wantToken: true},
{name: "expired secret", clientSecret: expiredPlain},
{name: "secret that was never configured", clientSecret: "not-a-secret"},
} {
t.Run(test.name, func(t *testing.T) {
body := requestToken(t, test.clientSecret)
if test.wantToken {
require.NotEmpty(t, body["access_token"], "expected a token, got error: %v", body["error"])
} else {
require.Empty(t, body["access_token"])
require.Equal(t, "invalid_client", body["error"])
}
})
}
}
// TestTokenHandlerClientWithoutSecrets checks that a confidential client that has no secrets at all cannot authenticate.
func TestTokenHandlerClientWithoutSecrets(t *testing.T) {
gin.SetMode(gin.TestMode)
const (
baseURL = "https://issuer.example.com"
secret = "test-secret"
clientID = "no-secrets-client"
)
db := testutils.NewDatabaseForTest(t)
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
require.NoError(t, err)
require.NoError(t, db.Create(&model.OidcClient{
Base: model.Base{ID: clientID},
Name: "Client Without Secrets",
IsPublic: false,
}).Error)
provider, err := newProvider(NewStore(db, nil), nil, testTokenSigner{key: key}, Config{
BaseURL: baseURL,
TokenBaseURL: baseURL,
Secret: []byte(secret),
}, nil)
require.NoError(t, err)
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()))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
req.SetBasicAuth(clientID, "")
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.Empty(t, body["access_token"])
require.Equal(t, "invalid_client", body["error"])
}

View File

@@ -103,6 +103,26 @@ func (s *TestService) initExternalIdP() error {
return nil
}
// seededClientCredentials returns credentials holding a single client secret with the given value, which E2E tests use to authenticate as the client
func seededClientCredentials(secretID string, value string) model.OidcClientCredentials {
return model.OidcClientCredentials{
Secrets: seededClientSecrets(secretID, value),
}
}
// seededClientSecrets returns a single never-expiring client secret with the given value
func seededClientSecrets(secretID string, value string) []model.OidcClientSecret {
return []model.OidcClientSecret{
{
ID: secretID,
Algorithm: model.OidcClientSecretHashSHA256,
Hash: utils.CreateSha256Hash(value),
Prefix: value[:model.OidcClientSecretPrefixLength],
CreatedAt: datatype.DateTime(time.Now()),
},
}
}
//nolint:gocognit
func (s *TestService) SeedDatabase(baseURL string) error {
err := s.db.Transaction(func(tx *gorm.DB) error {
@@ -181,7 +201,7 @@ func (s *TestService) SeedDatabase(baseURL string) error {
Name: "Nextcloud",
Description: "This is an example description for Nextcloud",
LaunchURL: new("https://nextcloud.local"),
Secret: "$2a$10$9dypwot8nGuCjT6wQWWpJOckZfRprhe2EkwpKizxS/fpVHrOLEJHC", // w2mUeZISmEvIDMEDvpY0PnxQIpj1m3zY
Credentials: seededClientCredentials("2f1b8f1a-1d3e-4f0c-9c1a-000000000001", "w2mUeZISmEvIDMEDvpY0PnxQIpj1m3zY"),
CallbackURLs: datatype.StringList{"http://nextcloud.localhost/auth/callback"},
LogoutCallbackURLs: datatype.StringList{"http://nextcloud.localhost/auth/logout/callback"},
ImageType: new("png"),
@@ -192,7 +212,7 @@ func (s *TestService) SeedDatabase(baseURL string) error {
ID: "606c7782-f2b1-49e5-8ea9-26eb1b06d018",
},
Name: "Immich",
Secret: "$2a$10$Ak.FP8riD1ssy2AGGbG.gOpnp/rBpymd74j0nxNMtW0GG1Lb4gzxe", // PYjrE9u4v9GVqXKi52eur0eb2Ci4kc0x
Credentials: seededClientCredentials("2f1b8f1a-1d3e-4f0c-9c1a-000000000002", "PYjrE9u4v9GVqXKi52eur0eb2Ci4kc0x"),
CallbackURLs: datatype.StringList{"http://immich.localhost/auth/callback"},
CreatedByID: new(users[1].ID),
IsGroupRestricted: true,
@@ -205,7 +225,7 @@ func (s *TestService) SeedDatabase(baseURL string) error {
ID: "7c21a609-96b5-4011-9900-272b8d31a9d1",
},
Name: "Tailscale",
Secret: "$2a$10$xcRReBsvkI1XI6FG8xu/pOgzeF00bH5Wy4d/NThwcdi3ZBpVq/B9a", // n4VfQeXlTzA6yKpWbR9uJcMdSx2qH0Lo
Credentials: seededClientCredentials("2f1b8f1a-1d3e-4f0c-9c1a-000000000003", "n4VfQeXlTzA6yKpWbR9uJcMdSx2qH0Lo"),
CallbackURLs: datatype.StringList{"http://tailscale.localhost/auth/callback"},
LogoutCallbackURLs: datatype.StringList{"http://tailscale.localhost/auth/logout/callback"},
IsGroupRestricted: true,
@@ -219,11 +239,11 @@ func (s *TestService) SeedDatabase(baseURL string) error {
ID: "c48232ff-ff65-45ed-ae96-7afa8a9b443b",
},
Name: "Federated",
Secret: "$2a$10$Ak.FP8riD1ssy2AGGbG.gOpnp/rBpymd74j0nxNMtW0GG1Lb4gzxe", // PYjrE9u4v9GVqXKi52eur0eb2Ci4kc0x
CallbackURLs: datatype.StringList{"http://federated.localhost/auth/callback"},
CreatedByID: new(users[1].ID),
AllowedUserGroups: []model.UserGroup{},
Credentials: model.OidcClientCredentials{
Secrets: seededClientSecrets("2f1b8f1a-1d3e-4f0c-9c1a-000000000004", "PYjrE9u4v9GVqXKi52eur0eb2Ci4kc0x"),
FederatedIdentities: []model.OidcClientFederatedIdentity{
{
Issuer: "https://external-idp.local",
@@ -239,7 +259,7 @@ func (s *TestService) SeedDatabase(baseURL string) error {
ID: "c46d2090-37a0-4f2b-8748-6aa53b0c1afa",
},
Name: "SCIM Client",
Secret: "$2a$10$h4wfa8gI7zavDAxwzSq1sOwYU4e8DwK1XZ8ZweNnY5KzlJ3Iz.qdK", // nQbiuMRG7FpdK2EnDd5MBivWQeKFXohn
Credentials: seededClientCredentials("2f1b8f1a-1d3e-4f0c-9c1a-000000000005", "nQbiuMRG7FpdK2EnDd5MBivWQeKFXohn"),
CallbackURLs: datatype.StringList{"http://scimclient.localhost/auth/callback"},
CreatedByID: new(users[0].ID),
IsGroupRestricted: true,
@@ -253,7 +273,7 @@ func (s *TestService) SeedDatabase(baseURL string) error {
ID: "a1b2c3d4-e5f6-7890-abcd-ef0000000001",
},
Name: "PAR Test Client",
Secret: "$2a$10$9dypwot8nGuCjT6wQWWpJOckZfRprhe2EkwpKizxS/fpVHrOLEJHC", // w2mUeZISmEvIDMEDvpY0PnxQIpj1m3zY
Credentials: seededClientCredentials("2f1b8f1a-1d3e-4f0c-9c1a-000000000006", "w2mUeZISmEvIDMEDvpY0PnxQIpj1m3zY"),
CallbackURLs: datatype.StringList{"http://par-client.localhost/auth/callback"},
CreatedByID: new(users[0].ID),
},
@@ -262,7 +282,7 @@ func (s *TestService) SeedDatabase(baseURL string) error {
ID: "e1f2a3b4-c5d6-7890-abcd-ef0000000002",
},
Name: "Skip Consent Client",
Secret: "$2a$10$9dypwot8nGuCjT6wQWWpJOckZfRprhe2EkwpKizxS/fpVHrOLEJHC", // w2mUeZISmEvIDMEDvpY0PnxQIpj1m3zY
Credentials: seededClientCredentials("2f1b8f1a-1d3e-4f0c-9c1a-000000000007", "w2mUeZISmEvIDMEDvpY0PnxQIpj1m3zY"),
CallbackURLs: datatype.StringList{"http://skip-consent.localhost/auth/callback"},
CreatedByID: new(users[0].ID),
// Trusted client that bypasses the consent screen by default

View File

@@ -0,0 +1,88 @@
package service
import (
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/crypto/bcrypt"
"gorm.io/gorm"
"github.com/pocket-id/pocket-id/backend/internal/model"
testutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing"
)
// versionBeforeMultipleClientSecrets is the migration version right before client secrets moved into the credentials document
const versionBeforeMultipleClientSecrets = 20260802120000
// TestMigrateClientSecretsToCredentials checks that the secret of every existing client is preserved as an entry of the credentials document
func TestMigrateClientSecretsToCredentials(t *testing.T) {
const legacySecret = "legacy-client-secret"
legacyHash, err := bcrypt.GenerateFromPassword([]byte(legacySecret), bcrypt.MinCost)
require.NoError(t, err)
createdAt := time.Now().Add(-72 * time.Hour).Truncate(time.Second)
db := testutils.NewDatabaseForTestWithMigrationSeed(t, versionBeforeMultipleClientSecrets, func(t *testing.T, db *gorm.DB) {
// A client with a secret and no other credentials
err := db.Exec(
`INSERT INTO oidc_clients (id, created_at, name, secret, callback_urls, is_public, pkce_enabled) VALUES (?, ?, ?, ?, ?, ?, ?)`,
"client-with-secret", createdAt.Unix(), "With Secret", string(legacyHash), `["https://example.com/callback"]`, false, false,
).Error
require.NoError(t, err)
// A client whose credentials document already holds a federated identity, which the migration must preserve
err = db.Exec(
`INSERT INTO oidc_clients (id, created_at, name, secret, callback_urls, is_public, pkce_enabled, credentials) VALUES (?, ?, ?, ?, ?, ?, ?, ?)`,
"client-with-federated-identity", createdAt.Unix(), "With Federated Identity", string(legacyHash), `["https://example.com/callback"]`, false, false,
`{"federatedIdentities":[{"issuer":"https://issuer.example.com"}]}`,
).Error
require.NoError(t, err)
// A public client, which never had a secret
err = db.Exec(
`INSERT INTO oidc_clients (id, created_at, name, secret, callback_urls, is_public, pkce_enabled) VALUES (?, ?, ?, ?, ?, ?, ?)`,
"public-client", createdAt.Unix(), "Public", "", `["https://example.com/callback"]`, true, true,
).Error
require.NoError(t, err)
})
// The legacy column is gone once the migration has run
ok := db.Migrator().HasColumn(&model.OidcClient{}, "secret")
assert.False(t, ok)
var clients []model.OidcClient
err = db.Find(&clients).Error
require.NoError(t, err)
byID := make(map[string]model.OidcClient, len(clients))
for _, client := range clients {
byID[client.ID] = client
}
require.Len(t, byID, 3)
// The migrated secret keeps its bcrypt hash, has no expiration, and carries no prefix because the value was never stored
migrated := byID["client-with-secret"].Credentials.Secrets
require.Len(t, migrated, 1)
assert.NotEmpty(t, migrated[0].ID)
assert.Equal(t, model.OidcClientSecretHashBcrypt, migrated[0].Algorithm)
assert.Equal(t, string(legacyHash), migrated[0].Hash)
assert.Empty(t, migrated[0].Prefix)
assert.Nil(t, migrated[0].ExpiresAt)
assert.True(t, migrated[0].IsActive())
assert.Equal(t, createdAt.UTC(), migrated[0].CreatedAt.UTC())
// Existing federated identities survive the migration alongside the new secret
withFederated := byID["client-with-federated-identity"].Credentials
require.Len(t, withFederated.Secrets, 1)
assert.Equal(t, string(legacyHash), withFederated.Secrets[0].Hash)
require.Len(t, withFederated.FederatedIdentities, 1)
assert.Equal(t, "https://issuer.example.com", withFederated.FederatedIdentities[0].Issuer)
// Clients that never had a secret do not get an empty one
assert.Empty(t, byID["public-client"].Credentials.Secrets)
// Every migrated secret gets its own identifier
assert.NotEqual(t, migrated[0].ID, withFederated.Secrets[0].ID)
}

View File

@@ -10,10 +10,11 @@ import (
"net/http"
"net/url"
"path"
"slices"
"strings"
"time"
"golang.org/x/crypto/bcrypt"
"github.com/google/uuid"
"gorm.io/gorm"
"gorm.io/gorm/clause"
@@ -320,7 +321,33 @@ func (s *OidcService) DeleteClient(ctx context.Context, clientID string) error {
return nil
}
func (s *OidcService) CreateClientSecret(ctx context.Context, clientID string, input dto.OidcClientSecretDto) (string, error) {
// ListClientSecrets returns all secrets configured for a client, including the expired ones
func (s *OidcService) ListClientSecrets(ctx context.Context, clientID string) ([]model.OidcClientSecret, error) {
client, err := s.getClientInternal(ctx, clientID, s.db, false)
if err != nil {
return nil, err
}
return client.Credentials.Secrets, nil
}
// CreateClientSecret adds a new secret to a client and returns both the stored record and the secret's value, which is not recoverable afterwards
func (s *OidcService) CreateClientSecret(ctx context.Context, clientID string, input dto.OidcClientSecretCreateDto) (model.OidcClientSecret, string, error) {
return s.createClientSecretInternal(ctx, clientID, input, false)
}
// ReplaceClientSecrets removes every existing secret of a client and creates a new one
// It backs the deprecated single-secret endpoint, whose documented behavior is that creating a secret invalidates the previous one
func (s *OidcService) ReplaceClientSecrets(ctx context.Context, clientID string, input dto.OidcClientSecretCreateDto) (model.OidcClientSecret, string, error) {
return s.createClientSecretInternal(ctx, clientID, input, true)
}
func (s *OidcService) createClientSecretInternal(ctx context.Context, clientID string, input dto.OidcClientSecretCreateDto, replaceExisting bool) (model.OidcClientSecret, string, error) {
// An expiration date in the past would create a secret that can never be used
if input.ExpiresAt != nil && !input.ExpiresAt.ToTime().After(time.Now()) {
return model.OidcClientSecret{}, "", apperror.ValidationMessage("The expiration date of a client secret must be in the future")
}
tx := s.db.Begin()
defer func() {
tx.Rollback()
@@ -328,41 +355,102 @@ func (s *OidcService) CreateClientSecret(ctx context.Context, clientID string, i
client, err := s.getClientInternal(ctx, clientID, tx, true)
if err != nil {
return "", err
return model.OidcClientSecret{}, "", fmt.Errorf("error retrieving OIDC client: %w", err)
}
if client.IsPublic {
return "", apperror.ValidationMessage("Cannot create a secret for a public client")
return model.OidcClientSecret{}, "", apperror.ValidationMessage("Cannot create a secret for a public client")
}
if replaceExisting {
client.Credentials.Secrets = nil
} else if len(client.Credentials.Secrets) >= model.MaxOidcClientSecrets {
return model.OidcClientSecret{}, "", apperror.ValidationMessage(fmt.Sprintf("A client cannot have more than %d secrets", model.MaxOidcClientSecrets))
}
// Callers may supply their own value, otherwise one with enough entropy is generated here
clientSecret := input.Secret
if clientSecret == "" {
clientSecret, err = utils.GenerateRandomAlphanumericString(32)
if err != nil {
return "", err
return model.OidcClientSecret{}, "", fmt.Errorf("failed to generate client secret: %w", err)
}
}
hashedSecret, err := bcrypt.GenerateFromPassword([]byte(clientSecret), bcrypt.DefaultCost)
if err != nil {
return "", err
// Only the hash and a short prefix are persisted, so this is the last time the value is available
secret := model.OidcClientSecret{
ID: uuid.New().String(),
Algorithm: model.OidcClientSecretHashSHA256,
Hash: utils.CreateSha256Hash(clientSecret),
Prefix: clientSecretPrefix(clientSecret),
CreatedAt: datatype.DateTime(time.Now()),
ExpiresAt: input.ExpiresAt,
}
client.Credentials.Secrets = append(client.Credentials.Secrets, secret)
client.Secret = string(hashedSecret)
err = tx.
WithContext(ctx).
Save(&client).
Model(&client).
Select("Credentials").
Updates(&client).
Error
if err != nil {
return "", err
return model.OidcClientSecret{}, "", fmt.Errorf("failed to update OIDC client: %w", err)
}
err = tx.Commit().Error
if err != nil {
return "", err
return model.OidcClientSecret{}, "", fmt.Errorf("failed to commit transaction: %w", err)
}
return clientSecret, nil
return secret, clientSecret, nil
}
// DeleteClientSecret removes a single secret from a client, making it immediately unusable
func (s *OidcService) DeleteClientSecret(ctx context.Context, clientID string, secretID string) error {
tx := s.db.Begin()
defer func() {
tx.Rollback()
}()
client, err := s.getClientInternal(ctx, clientID, tx, true)
if err != nil {
return fmt.Errorf("error retrieving OIDC client: %w", err)
}
countBefore := len(client.Credentials.Secrets)
client.Credentials.Secrets = slices.DeleteFunc(client.Credentials.Secrets, func(secret model.OidcClientSecret) bool {
return secret.ID == secretID
})
if len(client.Credentials.Secrets) == countBefore {
return apperror.NotFound("Client secret")
}
err = tx.
WithContext(ctx).
Model(&client).
Select("Credentials").
Updates(&client).
Error
if err != nil {
return fmt.Errorf("failed to update OIDC client: %w", err)
}
err = tx.Commit().Error
if err != nil {
return fmt.Errorf("failed to commit transaction: %w", err)
}
return nil
}
// clientSecretPrefix returns the leading characters of a secret that are stored in clear text to help admins tell secrets apart
func clientSecretPrefix(clientSecret string) string {
if len(clientSecret) <= model.OidcClientSecretPrefixLength {
return ""
}
return clientSecret[:model.OidcClientSecretPrefixLength]
}
func (s *OidcService) GetClientLogo(ctx context.Context, clientID string, light bool) (io.ReadCloser, int64, string, error) {

View File

@@ -6,8 +6,7 @@ import (
"strconv"
"strings"
"testing"
"golang.org/x/crypto/bcrypt"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -668,16 +667,138 @@ func TestOidcService_CreateClientSecret_withCustomSecret(t *testing.T) {
require.NoError(t, err)
customSecret := "custom-client-secret-with-a-minimum-length"
input := dto.OidcClientSecretDto{Secret: customSecret}
input := dto.OidcClientSecretCreateDto{Secret: customSecret}
secret, err := s.CreateClientSecret(t.Context(), client.ID, input)
created, secret, err := s.CreateClientSecret(t.Context(), client.ID, input)
require.NoError(t, err)
assert.Equal(t, customSecret, secret)
assert.Equal(t, "cust", created.Prefix)
assert.Nil(t, created.ExpiresAt)
assert.True(t, created.IsActive())
var fetched model.OidcClient
err = db.First(&fetched, "id = ?", client.ID).Error
require.NoError(t, err)
require.NoError(t, bcrypt.CompareHashAndPassword([]byte(fetched.Secret), []byte(customSecret)))
require.Len(t, fetched.Credentials.Secrets, 1)
assert.Equal(t, created.ID, fetched.Credentials.Secrets[0].ID)
assert.Equal(t, model.OidcClientSecretHashSHA256, fetched.Credentials.Secrets[0].Algorithm)
assert.Equal(t, utils.CreateSha256Hash(customSecret), fetched.Credentials.Secrets[0].Hash)
}
func TestOidcService_CreateClientSecret_multipleSecrets(t *testing.T) {
db := testutils.NewDatabaseForTest(t)
s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil)
require.NoError(t, err)
client := model.OidcClient{Name: "Test Client"}
err = db.Create(&client).Error
require.NoError(t, err)
// Adding a second secret leaves the first one in place
first, _, err := s.CreateClientSecret(t.Context(), client.ID, dto.OidcClientSecretCreateDto{})
require.NoError(t, err)
expiresAt := datatype.DateTime(time.Now().Add(24 * time.Hour))
second, _, err := s.CreateClientSecret(t.Context(), client.ID, dto.OidcClientSecretCreateDto{ExpiresAt: &expiresAt})
require.NoError(t, err)
secrets, err := s.ListClientSecrets(t.Context(), client.ID)
require.NoError(t, err)
require.Len(t, secrets, 2)
assert.Equal(t, first.ID, secrets[0].ID)
assert.Equal(t, second.ID, secrets[1].ID)
assert.Equal(t, expiresAt.ToTime().Unix(), secrets[1].ExpiresAt.ToTime().Unix())
// Deleting one secret keeps the other usable
err = s.DeleteClientSecret(t.Context(), client.ID, first.ID)
require.NoError(t, err)
secrets, err = s.ListClientSecrets(t.Context(), client.ID)
require.NoError(t, err)
require.Len(t, secrets, 1)
assert.Equal(t, second.ID, secrets[0].ID)
// Deleting an unknown secret is reported as not found
err = s.DeleteClientSecret(t.Context(), client.ID, first.ID)
require.Error(t, err)
}
func TestOidcService_CreateClientSecret_expirationInThePast(t *testing.T) {
db := testutils.NewDatabaseForTest(t)
s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil)
require.NoError(t, err)
client := model.OidcClient{Name: "Test Client"}
err = db.Create(&client).Error
require.NoError(t, err)
expiresAt := datatype.DateTime(time.Now().Add(-time.Minute))
_, _, err = s.CreateClientSecret(t.Context(), client.ID, dto.OidcClientSecretCreateDto{ExpiresAt: &expiresAt})
require.Error(t, err)
}
func TestOidcService_CreateClientSecret_limit(t *testing.T) {
db := testutils.NewDatabaseForTest(t)
s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil)
require.NoError(t, err)
client := model.OidcClient{Name: "Test Client"}
err = db.Create(&client).Error
require.NoError(t, err)
for range model.MaxOidcClientSecrets {
_, _, err = s.CreateClientSecret(t.Context(), client.ID, dto.OidcClientSecretCreateDto{})
require.NoError(t, err)
}
_, _, err = s.CreateClientSecret(t.Context(), client.ID, dto.OidcClientSecretCreateDto{})
require.Error(t, err)
// The deprecated endpoint replaces every existing secret with a single new one
_, _, err = s.ReplaceClientSecrets(t.Context(), client.ID, dto.OidcClientSecretCreateDto{})
require.NoError(t, err)
secrets, err := s.ListClientSecrets(t.Context(), client.ID)
require.NoError(t, err)
assert.Len(t, secrets, 1)
}
func TestOidcService_CreateClientSecret_preservesFederatedIdentities(t *testing.T) {
db := testutils.NewDatabaseForTest(t)
s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil)
require.NoError(t, err)
client := model.OidcClient{
Name: "Test Client",
CallbackURLs: datatype.StringList{"https://example.com/callback"},
Credentials: model.OidcClientCredentials{
FederatedIdentities: []model.OidcClientFederatedIdentity{{Issuer: "https://issuer.example.com"}},
},
}
err = db.Create(&client).Error
require.NoError(t, err)
_, _, err = s.CreateClientSecret(t.Context(), client.ID, dto.OidcClientSecretCreateDto{})
require.NoError(t, err)
// Updating the client must not drop the secrets managed by the dedicated endpoints
_, err = s.UpdateClient(t.Context(), client.ID, dto.OidcClientUpdateDto{
Name: "Test Client",
CallbackURLs: []string{"https://example.com/callback"},
Credentials: dto.OidcClientCredentialsDto{
FederatedIdentities: []dto.OidcClientFederatedIdentityDto{{Issuer: "https://other.example.com"}},
},
})
require.NoError(t, err)
var fetched model.OidcClient
require.NoError(t, db.First(&fetched, "id = ?", client.ID).Error)
require.Len(t, fetched.Credentials.Secrets, 1)
require.Len(t, fetched.Credentials.FederatedIdentities, 1)
assert.Equal(t, "https://other.example.com", fetched.Credentials.FederatedIdentities[0].Issuer)
}
func TestOidcService_UpdateClient_description(t *testing.T) {

View File

@@ -0,0 +1,16 @@
ALTER TABLE oidc_clients ADD COLUMN secret TEXT;
-- Only secrets that were migrated up from this same column can be restored, since the old column stores a bcrypt hash
-- Secrets created while multiple secrets were supported are hashed with SHA-256 and are dropped here
UPDATE oidc_clients
SET secret = (
SELECT secret_entry ->> 'hash'
FROM jsonb_array_elements(credentials -> 'secrets') AS secret_entry
WHERE secret_entry ->> 'alg' = 'bcrypt'
LIMIT 1
)
WHERE jsonb_typeof(credentials -> 'secrets') = 'array';
UPDATE oidc_clients
SET credentials = credentials - 'secrets'
WHERE jsonb_typeof(credentials -> 'secrets') = 'array';

View File

@@ -0,0 +1,14 @@
-- Move the single client secret into the credentials document, as the only entry of the new "secrets" array
-- Migrated secrets keep their bcrypt hash because the secret's value is not recoverable, and they never expire so that existing integrations keep working
UPDATE oidc_clients
SET credentials = COALESCE(credentials, '{}'::jsonb) || jsonb_build_object(
'secrets', jsonb_build_array(jsonb_build_object(
'id', gen_random_uuid()::text,
'alg', 'bcrypt',
'hash', secret,
'createdAt', to_char(COALESCE(created_at, now()) AT TIME ZONE 'UTC', 'YYYY-MM-DD"T"HH24:MI:SS"Z"')
))
)
WHERE secret IS NOT NULL AND secret <> '';
ALTER TABLE oidc_clients DROP COLUMN secret;

View File

@@ -0,0 +1,28 @@
PRAGMA foreign_keys= OFF;
BEGIN;
ALTER TABLE oidc_clients ADD COLUMN secret TEXT;
-- Only secrets that were migrated up from this same column can be restored, since the old column stores a bcrypt hash
-- Secrets created while multiple secrets were supported are hashed with SHA-256 and are dropped here
UPDATE oidc_clients
SET secret = (
SELECT json_extract(value, '$.hash')
FROM json_each(json_extract(credentials, '$.secrets'))
WHERE json_extract(value, '$.alg') = 'bcrypt'
LIMIT 1
)
WHERE credentials IS NOT NULL
AND credentials != ''
AND json_valid(credentials)
AND json_type(credentials, '$.secrets') = 'array';
UPDATE oidc_clients
SET credentials = json_remove(credentials, '$.secrets')
WHERE credentials IS NOT NULL
AND credentials != ''
AND json_valid(credentials)
AND json_type(credentials, '$.secrets') = 'array';
COMMIT;
PRAGMA foreign_keys= ON;

View File

@@ -0,0 +1,31 @@
PRAGMA foreign_keys= OFF;
BEGIN;
-- Move the single client secret into the credentials document, as the only entry of the new "secrets" array
-- Migrated secrets keep their bcrypt hash because the secret's value is not recoverable, and they never expire so that existing integrations keep working
UPDATE oidc_clients
SET credentials = json_set(
COALESCE(NULLIF(credentials, ''), '{}'),
'$.secrets',
json_array(json_object(
-- Generate a UUID
'id', lower(
hex(randomblob(4)) || '-' ||
hex(randomblob(2)) || '-4' ||
substr(hex(randomblob(2)), 2) || '-' ||
substr('89ab', abs(random()) % 4 + 1, 1) || substr(hex(randomblob(2)), 2) || '-' ||
hex(randomblob(6))
),
'alg', 'bcrypt',
'hash', secret,
'createdAt', strftime('%Y-%m-%dT%H:%M:%SZ', COALESCE(created_at, strftime('%s', 'now')), 'unixepoch')
))
)
WHERE secret IS NOT NULL
AND secret != ''
AND (credentials IS NULL OR credentials = '' OR json_valid(credentials));
ALTER TABLE oidc_clients DROP COLUMN secret;
COMMIT;
PRAGMA foreign_keys= ON;

View File

@@ -331,8 +331,6 @@
"cimd_url_allowlist_description": "Restrict which client ID metadata document URLs are accepted. {#link href=|https://pocket-id.org/docs/advanced/callback-url-wildcards|}Wildcards{/link} are supported. An empty list blocks all URLs.",
"refresh": "Refresh",
"oidc_client_metadata_refreshed_successfully": "Client metadata document refreshed successfully",
"create_new_client_secret": "Create new client secret",
"are_you_sure_you_want_to_create_a_new_client_secret": "Are you sure you want to create a new client secret? The old one will be invalidated.",
"generate": "Generate",
"new_client_secret_created_successfully": "New client secret created successfully",
"oidc_client_name": "OIDC Client {name}",
@@ -612,5 +610,17 @@
"or": "or",
"visit_and_enter": "Visit {url} and enter:",
"oidc": "OIDC",
"device_login_request_expired": "Your QR-Code has expired. Please start the sign-in process again."
"device_login_request_expired": "Your QR-Code has expired. Please start the sign-in process again.",
"credentials": "Credentials",
"client_secrets": "Client secrets",
"client_secrets_description": "Secrets that the app uses to authenticate itself. Multiple secrets can be active at the same time, so one can be rotated without downtime.",
"no_client_secrets_yet": "This app has no client secrets yet.",
"public_clients_cannot_have_secrets": "Public clients cannot have client secrets.",
"add_client_secret": "Add client secret",
"no_expiration": "No expiration",
"days_count": "{count} days",
"client_secrets_limit_reached": "An app cannot have more than 20 client secrets.",
"delete_client_secret": "Delete client secret",
"are_you_sure_you_want_to_delete_this_client_secret": "Are you sure you want to delete this client secret? Apps using it will no longer be able to authenticate.",
"client_secret_deleted_successfully": "Client secret deleted successfully"
}

View File

@@ -7,6 +7,8 @@ import type {
OidcClient,
OidcClientCreate,
OidcClientMetaData,
OidcClientSecret,
OidcClientSecretCreated,
OidcClientUpdate,
OidcClientWithAllowedUserGroups,
OidcClientWithAllowedUserGroupsCount,
@@ -87,8 +89,20 @@ class OidcService extends APIService {
cachedOidcClientLogo.bustCache(id, light);
};
createClientSecret = async (id: string) =>
(await this.api.post(`/oidc/clients/${encodeClientIdParam(id)}/secret`)).data.secret as string;
listClientSecrets = async (id: string) =>
(await this.api.get(`/oidc/clients/${encodeClientIdParam(id)}/secrets`))
.data as OidcClientSecret[];
createClientSecret = async (id: string, expiresAt: Date | null) =>
(
await this.api.post(`/oidc/clients/${encodeClientIdParam(id)}/secrets`, {
expiresAt: expiresAt?.toISOString() ?? null
})
).data as OidcClientSecretCreated;
deleteClientSecret = async (id: string, secretId: string) => {
await this.api.delete(`/oidc/clients/${encodeClientIdParam(id)}/secrets/${secretId}`);
};
updateAllowedUserGroups = async (id: string, userGroupIds: string[]) => {
const res = await this.api.put(`/oidc/clients/${encodeClientIdParam(id)}/allowed-user-groups`, {

View File

@@ -1,17 +1,28 @@
import { writable } from 'svelte/store';
const clientSecretStore = writable<string | null>(null);
// Holds the clear-text value of the client secrets created during the current page visit, keyed by secret ID.
// The server never returns those values again, so they are shown until the user navigates away and then forgotten.
const clientSecretStore = writable<Record<string, string>>({});
const set = (user: string) => {
clientSecretStore.set(user);
const set = (secretId: string, secret: string) => {
clientSecretStore.update((secrets) => ({ ...secrets, [secretId]: secret }));
};
const remove = (secretId: string) => {
clientSecretStore.update((secrets) => {
const remaining = { ...secrets };
delete remaining[secretId];
return remaining;
});
};
const clear = () => {
clientSecretStore.set(null);
clientSecretStore.set({});
};
export default {
subscribe: clientSecretStore.subscribe,
set,
remove,
clear
};

View File

@@ -21,8 +21,23 @@ export type OidcClientFederatedIdentity = {
replayProtection: boolean;
};
export type OidcClientSecret = {
id: string;
// The first characters of the secret, empty for secrets created before Pocket ID supported multiple secrets
prefix: string;
createdAt: string;
expiresAt: string | null;
isActive: boolean;
};
// The clear-text value of a secret is only returned when it is created and cannot be retrieved afterwards
export type OidcClientSecretCreated = OidcClientSecret & {
secret: string;
};
export type OidcClientCredentials = {
federatedIdentities: OidcClientFederatedIdentity[];
secrets?: OidcClientSecret[];
};
export type OidcDiscoveryConfiguration = {

View File

@@ -32,10 +32,7 @@
: Promise.resolve();
await Promise.all([logoPromise, darkLogoPromise]);
if (!createdClient.isPublic) {
const clientSecret = await oidcService.createClientSecret(createdClient.id);
clientSecretStore.set(clientSecret);
}
// A new client starts without any secret: the admin creates the ones they need from the credentials tab
goto(`/settings/admin/oidc-clients/${encodeClientIdParam(createdClient.id)}`);
toast.success(m.oidc_client_created_successfully());
return true;

View File

@@ -16,17 +16,13 @@
import type {
OidcClientCreateWithLogo,
OidcClientCredentials,
OidcClientSecret,
OidcClientTokenLifetimes
} from '$lib/types/oidc.type';
import type { ScimServiceProviderCreate } from '$lib/types/scim.type';
import { cachedOidcClientLogo } from '$lib/utils/cached-image-util';
import { axiosErrorToast } from '$lib/utils/error-util';
import {
LucideChevronLeft,
LucideInfo,
LucideRefreshCcw,
LucideTriangleAlert
} from '@lucide/svelte';
import { LucideChevronLeft, LucideInfo, LucideTriangleAlert } from '@lucide/svelte';
import { toast } from 'svelte-sonner';
import { slide } from 'svelte/transition';
import { backNavigate } from '../../users/navigate-back-util';
@@ -34,6 +30,7 @@
import OidcClientPreviewModal from '../oidc-client-preview-modal.svelte';
import ApiAccessCard from './api-access-card.svelte';
import OidcClientFederatedCredentialsCard from './oidc-client-federated-credentials-card.svelte';
import OidcClientSecretsCard from './oidc-client-secrets-card.svelte';
import OidcClientTokenLifetimesCard from './oidc-client-token-lifetimes-card.svelte';
import ScimResourceProviderForm from './scim-resource-provider-form.svelte';
@@ -43,6 +40,9 @@
allowedUserGroupIds: data.client.allowedUserGroups.map((g) => g.id)
});
// Secrets are managed by their own endpoints, so they are kept out of the client object that the forms below submit
let clientSecrets = $state<OidcClientSecret[]>(data.client.credentials?.secrets ?? []);
let scimServiceProvider = $state(data.scimServiceProvider);
let showAllDetails = $state(false);
let showPreview = $state(false);
@@ -174,26 +174,6 @@
});
}
async function createClientSecret() {
openConfirmDialog({
title: m.create_new_client_secret(),
message: m.are_you_sure_you_want_to_create_a_new_client_secret(),
confirm: {
label: m.generate(),
destructive: true,
action: async () => {
try {
const clientSecret = await oidcService.createClientSecret(client.id);
clientSecretStore.set(clientSecret);
toast.success(m.new_client_secret_created_successfully());
} catch (e) {
axiosErrorToast(e);
}
}
}
});
}
async function updateUserGroupClients(allowedGroups: string[]) {
await oidcService
.updateAllowedUserGroups(client.id, allowedGroups)
@@ -281,6 +261,7 @@
<LucideTriangleAlert class="ml-0.5 size-4 text-yellow-600 dark:text-yellow-400" />
{/if}</Tabs.Trigger
>
<Tabs.Trigger value="credentials">{m.credentials()}</Tabs.Trigger>
<Tabs.Trigger value="api-access">{m.api_access()}</Tabs.Trigger>
<Tabs.Trigger value="scim">{m.scim_provisioning()}</Tabs.Trigger>
<Tabs.Trigger value="preview">{m.oidc_data_preview()}</Tabs.Trigger>
@@ -302,32 +283,6 @@
</span>
</CopyToClipboard>
</div>
{#if !client.isPublic}
<div class="mt-1 mb-2 flex flex-col sm:flex-row sm:items-center">
<Field.Label class="w-52">{m.client_secret()}</Field.Label>
{#if $clientSecretStore}
<CopyToClipboard value={$clientSecretStore}>
<span class="text-muted-foreground text-sm" data-testid="client-secret">
{$clientSecretStore}
</span>
</CopyToClipboard>
{:else}
<div>
<span class="text-muted-foreground text-sm" data-testid="client-secret"
>••••••••••••••••••••••••••••••••</span
>
<Button
class="ml-2"
onclick={createClientSecret}
size="sm"
variant="ghost"
aria-label="Create new client secret"
><LucideRefreshCcw class="size-3" /></Button
>
</div>
{/if}
</div>
{/if}
{#if showAllDetails}
<div transition:slide>
{#each Object.entries(setupDetails) as [key, value] (key)}
@@ -359,6 +314,10 @@
</Card.Root>
<OidcClientTokenLifetimesCard {client} callback={updateTokenLifetimes} />
</Tabs.Content>
<Tabs.Content value="credentials" id="credentials" class="flex flex-col gap-4">
<OidcClientSecretsCard {client} bind:secrets={clientSecrets} />
<OidcClientFederatedCredentialsCard {client} callback={updateFederatedCredentials} />
</Tabs.Content>

View File

@@ -0,0 +1,195 @@
<script lang="ts">
import { openConfirmDialog } from '$lib/components/confirm-dialog';
import CopyToClipboard from '$lib/components/copy-to-clipboard.svelte';
import { Badge } from '$lib/components/ui/badge';
import { Button, buttonVariants } from '$lib/components/ui/button';
import * as Card from '$lib/components/ui/card';
import * as DropdownMenu from '$lib/components/ui/dropdown-menu';
import * as Select from '$lib/components/ui/select';
import * as Table from '$lib/components/ui/table';
import { m } from '$lib/paraglide/messages';
import OidcService from '$lib/services/oidc-service';
import clientSecretStore from '$lib/stores/client-secret-store';
import type { OidcClient, OidcClientSecret } from '$lib/types/oidc.type';
import { axiosErrorToast } from '$lib/utils/error-util';
import { LucideEllipsis, LucidePlus, LucideTrash2 } from '@lucide/svelte';
import { toast } from 'svelte-sonner';
let {
client,
secrets = $bindable()
}: {
client: OidcClient;
secrets: OidcClientSecret[];
} = $props();
const oidcService = new OidcService();
// Mirrors model.MaxOidcClientSecrets on the backend, which rejects any secret above this count
const maxSecrets = 20;
// A secret can only be given an expiration when it is created, so this is not bound to any existing secret
const expirationOptions = [
{ value: 'never', label: m.no_expiration() },
{ value: '30', label: m.days_count({ count: '30' }) },
{ value: '90', label: m.days_count({ count: '90' }) },
{ value: '180', label: m.days_count({ count: '180' }) },
{ value: '360', label: m.days_count({ count: '360' }) },
{ value: '720', label: m.days_count({ count: '720' }) }
];
let expiration = $state('never');
let isCreating = $state(false);
const selectedExpirationLabel = $derived(
expirationOptions.find((option) => option.value === expiration)!.label
);
const limitReached = $derived(secrets.length >= maxSecrets);
function formatDate(date: string | null) {
if (!date) return m.never();
return new Date(date).toLocaleString();
}
function expirationDate() {
if (expiration === 'never') return null;
const millisecondsPerDay = 24 * 60 * 60 * 1000;
return new Date(Date.now() + Number(expiration) * millisecondsPerDay);
}
async function createSecret() {
isCreating = true;
try {
const created = await oidcService.createClientSecret(client.id, expirationDate());
// The value is kept in the store, so it survives switching between tabs but not leaving the page
clientSecretStore.set(created.id, created.secret);
secrets = [...secrets, created];
expiration = 'never';
toast.success(m.new_client_secret_created_successfully());
} catch (e) {
axiosErrorToast(e);
} finally {
isCreating = false;
}
}
function deleteSecret(secret: OidcClientSecret) {
openConfirmDialog({
title: m.delete_client_secret(),
message: m.are_you_sure_you_want_to_delete_this_client_secret(),
confirm: {
label: m.delete(),
destructive: true,
action: async () => {
try {
await oidcService.deleteClientSecret(client.id, secret.id);
clientSecretStore.remove(secret.id);
secrets = secrets.filter((s) => s.id !== secret.id);
toast.success(m.client_secret_deleted_successfully());
} catch (e) {
axiosErrorToast(e);
}
}
}
});
}
</script>
<Card.Root data-testid="client-secrets-card">
<Card.Header>
<Card.Title>{m.client_secrets()}</Card.Title>
<Card.Description>{m.client_secrets_description()}</Card.Description>
</Card.Header>
<Card.Content>
{#if client.isPublic}
<p class="text-muted-foreground text-sm">{m.public_clients_cannot_have_secrets()}</p>
{:else}
{#if secrets.length > 0}
<Table.Root>
<Table.Header>
<Table.Row>
<Table.Head>{m.client_secret()}</Table.Head>
<Table.Head>{m.created()}</Table.Head>
<Table.Head>{m.expires_at()}</Table.Head>
<Table.Head class="w-12"></Table.Head>
</Table.Row>
</Table.Header>
<Table.Body>
{#each secrets as secret (secret.id)}
{@const revealed = $clientSecretStore[secret.id]}
<Table.Row data-testid="client-secret-row">
<Table.Cell>
{#if revealed}
<CopyToClipboard value={revealed}>
<span class="font-mono text-sm break-all" data-testid="client-secret">
{revealed}
</span>
</CopyToClipboard>
{:else}
<span
class="text-muted-foreground font-mono text-sm"
data-testid="client-secret">{secret.prefix}{'•'.repeat(8)}</span
>
{/if}
{#if !secret.isActive}
<Badge class="ml-2" variant="destructive">{m.expired()}</Badge>
{/if}
</Table.Cell>
<Table.Cell class="text-muted-foreground text-sm"
>{formatDate(secret.createdAt)}</Table.Cell
>
<Table.Cell class="text-muted-foreground text-sm"
>{formatDate(secret.expiresAt)}</Table.Cell
>
<Table.Cell class="text-right">
<DropdownMenu.Root>
<DropdownMenu.Trigger
class={buttonVariants({ variant: 'ghost', size: 'icon' })}
>
<LucideEllipsis class="size-4" />
<span class="sr-only">{m.toggle_menu()}</span>
</DropdownMenu.Trigger>
<DropdownMenu.Content align="end">
<DropdownMenu.Item class="text-red-500!" onclick={() => deleteSecret(secret)}>
<LucideTrash2 class="mr-2 size-4" />
{m.delete()}
</DropdownMenu.Item>
</DropdownMenu.Content>
</DropdownMenu.Root>
</Table.Cell>
</Table.Row>
{/each}
</Table.Body>
</Table.Root>
{:else}
<p class="text-muted-foreground text-sm">{m.no_client_secrets_yet()}</p>
{/if}
<div class="mt-5 flex flex-col justify-end gap-3 sm:flex-row sm:items-center">
<Select.Root type="single" bind:value={expiration}>
<Select.Trigger
class="w-full sm:w-48"
aria-label={m.expiration()}
placeholder={m.expiration()}
>
{selectedExpirationLabel}
</Select.Trigger>
<Select.Content>
{#each expirationOptions as option (option.value)}
<Select.Item value={option.value}>{option.label}</Select.Item>
{/each}
</Select.Content>
</Select.Root>
<Button onclick={createSecret} disabled={limitReached || isCreating}>
<LucidePlus class="mr-2 size-4" />
{m.add_client_secret()}
</Button>
</div>
{#if limitReached}
<p class="text-muted-foreground mt-2 text-end text-xs">
{m.client_secrets_limit_reached()}
</p>
{/if}
{/if}
</Card.Content>
</Card.Root>

View File

@@ -1,6 +1,6 @@
{
"provider": "sqlite",
"version": 20260802120000,
"version": 20260807120000,
"tableOrder": [
"users",
"user_groups",
@@ -92,7 +92,7 @@
"client_type": "standard",
"created_at": "2025-11-25T12:39:02Z",
"created_by_id": "f4b89dc2-62fb-46bf-9f5f-c34f4eafe93e",
"credentials": "e30=",
"credentials": "eyJzZWNyZXRzIjpbeyJpZCI6IjJmMWI4ZjFhLTFkM2UtNGYwYy05YzFhLTAwMDAwMDAwMDAwMSIsImFsZyI6InNoYTI1NiIsImhhc2giOiJkZjMzMWMxOGRhMTM0NDMwOTRkYzJjODQ5NzFkNWNlZmIzNDY0ZTk3OTk5YzM5YmMwMzg0NWVhY2NjNjE3MGEyIiwicHJlZml4IjoidzJtVSIsImNyZWF0ZWRBdCI6IjIwMjUtMTEtMjVUMTI6Mzk6MDJaIn1dfQ==",
"dark_image_type": null,
"id": "3654a746-35d4-4321-ac61-0bdcff2b4055",
"image_type": "png",
@@ -109,7 +109,6 @@
"requires_pushed_authorization_requests": false,
"requires_reauthentication": false,
"refresh_token_duration_minutes": 43200,
"secret": "$2a$10$9dypwot8nGuCjT6wQWWpJOckZfRprhe2EkwpKizxS/fpVHrOLEJHC",
"skip_consent": false
},
{
@@ -118,7 +117,7 @@
"client_type": "standard",
"created_at": "2025-11-25T12:39:02Z",
"created_by_id": "1cd19686-f9a6-43f4-a41f-14a0bf5b4036",
"credentials": "e30=",
"credentials": "eyJzZWNyZXRzIjpbeyJpZCI6IjJmMWI4ZjFhLTFkM2UtNGYwYy05YzFhLTAwMDAwMDAwMDAwMiIsImFsZyI6InNoYTI1NiIsImhhc2giOiI4ODNiNzFjZjk1MWZlODk3ZTEwMjAxMTdiMTZjOGE2ZTY0MjllMWI5NGViZWY3ZGQ5NjM0OGJjZjc4ZThhMjI0IiwicHJlZml4IjoiUFlqciIsImNyZWF0ZWRBdCI6IjIwMjUtMTEtMjVUMTI6Mzk6MDJaIn1dfQ==",
"dark_image_type": null,
"id": "606c7782-f2b1-49e5-8ea9-26eb1b06d018",
"image_type": null,
@@ -135,7 +134,6 @@
"requires_pushed_authorization_requests": false,
"requires_reauthentication": false,
"refresh_token_duration_minutes": 43200,
"secret": "$2a$10$Ak.FP8riD1ssy2AGGbG.gOpnp/rBpymd74j0nxNMtW0GG1Lb4gzxe",
"skip_consent": false
},
{
@@ -144,7 +142,7 @@
"client_type": "standard",
"created_at": "2025-11-25T12:39:02Z",
"created_by_id": "f4b89dc2-62fb-46bf-9f5f-c34f4eafe93e",
"credentials": "e30=",
"credentials": "eyJzZWNyZXRzIjpbeyJpZCI6IjJmMWI4ZjFhLTFkM2UtNGYwYy05YzFhLTAwMDAwMDAwMDAwMyIsImFsZyI6InNoYTI1NiIsImhhc2giOiIzZWY5MzU5YWVlNmRiYjNlNGNhOTYxMTQxZGY3ZjZiYTA2ZGJiOTE5MDBiMGM4MWZhNjk5NjZlNDU4ZjA3MGQ2IiwicHJlZml4IjoibjRWZiIsImNyZWF0ZWRBdCI6IjIwMjUtMTEtMjVUMTI6Mzk6MDJaIn1dfQ==",
"dark_image_type": null,
"id": "7c21a609-96b5-4011-9900-272b8d31a9d1",
"image_type": null,
@@ -161,7 +159,6 @@
"requires_pushed_authorization_requests": false,
"requires_reauthentication": false,
"refresh_token_duration_minutes": 43200,
"secret": "$2a$10$xcRReBsvkI1XI6FG8xu/pOgzeF00bH5Wy4d/NThwcdi3ZBpVq/B9a",
"skip_consent": false
},
{
@@ -170,7 +167,7 @@
"client_type": "standard",
"created_at": "2025-11-25T12:39:02Z",
"created_by_id": "1cd19686-f9a6-43f4-a41f-14a0bf5b4036",
"credentials": "eyJmZWRlcmF0ZWRJZGVudGl0aWVzIjpbeyJpc3N1ZXIiOiJodHRwczovL2V4dGVybmFsLWlkcC5sb2NhbCIsInN1YmplY3QiOiJjNDgyMzJmZi1mZjY1LTQ1ZWQtYWU5Ni03YWZhOGE5YjQ0M2IiLCJhdWRpZW5jZSI6ImFwaTovL1BvY2tldElEIiwiandrcyI6Imh0dHA6Ly9sb2NhbGhvc3Q6MTQxMS9hcGkvZXh0ZXJuYWxpZHAvandrcy5qc29uIn1dfQ==",
"credentials": "eyJmZWRlcmF0ZWRJZGVudGl0aWVzIjpbeyJpc3N1ZXIiOiJodHRwczovL2V4dGVybmFsLWlkcC5sb2NhbCIsInN1YmplY3QiOiJjNDgyMzJmZi1mZjY1LTQ1ZWQtYWU5Ni03YWZhOGE5YjQ0M2IiLCJhdWRpZW5jZSI6ImFwaTovL1BvY2tldElEIiwiandrcyI6Imh0dHA6Ly9sb2NhbGhvc3Q6MTQxMS9hcGkvZXh0ZXJuYWxpZHAvandrcy5qc29uIn1dLCJzZWNyZXRzIjpbeyJpZCI6IjJmMWI4ZjFhLTFkM2UtNGYwYy05YzFhLTAwMDAwMDAwMDAwNCIsImFsZyI6InNoYTI1NiIsImhhc2giOiI4ODNiNzFjZjk1MWZlODk3ZTEwMjAxMTdiMTZjOGE2ZTY0MjllMWI5NGViZWY3ZGQ5NjM0OGJjZjc4ZThhMjI0IiwicHJlZml4IjoiUFlqciIsImNyZWF0ZWRBdCI6IjIwMjUtMTEtMjVUMTI6Mzk6MDJaIn1dfQ==",
"dark_image_type": null,
"id": "c48232ff-ff65-45ed-ae96-7afa8a9b443b",
"image_type": null,
@@ -187,7 +184,6 @@
"requires_pushed_authorization_requests": false,
"requires_reauthentication": false,
"refresh_token_duration_minutes": 43200,
"secret": "$2a$10$Ak.FP8riD1ssy2AGGbG.gOpnp/rBpymd74j0nxNMtW0GG1Lb4gzxe",
"skip_consent": false
},
{
@@ -212,8 +208,8 @@
"requires_pushed_authorization_requests": false,
"requires_reauthentication": false,
"refresh_token_duration_minutes": 43200,
"secret": "$2a$10$h4wfa8gI7zavDAxwzSq1sOwYU4e8DwK1XZ8ZweNnY5KzlJ3Iz.qdK",
"skip_consent": false
"skip_consent": false,
"credentials": "eyJzZWNyZXRzIjpbeyJpZCI6IjJmMWI4ZjFhLTFkM2UtNGYwYy05YzFhLTAwMDAwMDAwMDAwNSIsImFsZyI6InNoYTI1NiIsImhhc2giOiJiZWM3ZjA4YTI4MDhlZmNkYmU2ZmIwYjFmM2EwZjFhMDFjNjYwMGQxMTc1MjYyOTNjMTlhOWRlZDNkOGVjMGZiIiwicHJlZml4IjoiblFiaSIsImNyZWF0ZWRBdCI6IjIwMjUtMTEtMjVUMTI6Mzk6MDJaIn1dfQ=="
},
{
"access_token_duration_minutes": 60,
@@ -221,7 +217,7 @@
"client_type": "standard",
"created_at": "2025-11-25T12:39:02Z",
"created_by_id": "f4b89dc2-62fb-46bf-9f5f-c34f4eafe93e",
"credentials": "e30=",
"credentials": "eyJzZWNyZXRzIjpbeyJpZCI6IjJmMWI4ZjFhLTFkM2UtNGYwYy05YzFhLTAwMDAwMDAwMDAwNiIsImFsZyI6InNoYTI1NiIsImhhc2giOiJkZjMzMWMxOGRhMTM0NDMwOTRkYzJjODQ5NzFkNWNlZmIzNDY0ZTk3OTk5YzM5YmMwMzg0NWVhY2NjNjE3MGEyIiwicHJlZml4IjoidzJtVSIsImNyZWF0ZWRBdCI6IjIwMjUtMTEtMjVUMTI6Mzk6MDJaIn1dfQ==",
"dark_image_type": null,
"id": "a1b2c3d4-e5f6-7890-abcd-ef0000000001",
"image_type": null,
@@ -238,7 +234,6 @@
"requires_pushed_authorization_requests": false,
"requires_reauthentication": false,
"refresh_token_duration_minutes": 43200,
"secret": "$2a$10$9dypwot8nGuCjT6wQWWpJOckZfRprhe2EkwpKizxS/fpVHrOLEJHC",
"skip_consent": false
},
{
@@ -247,7 +242,7 @@
"client_type": "standard",
"created_at": "2025-11-25T12:39:02Z",
"created_by_id": "f4b89dc2-62fb-46bf-9f5f-c34f4eafe93e",
"credentials": "e30=",
"credentials": "eyJzZWNyZXRzIjpbeyJpZCI6IjJmMWI4ZjFhLTFkM2UtNGYwYy05YzFhLTAwMDAwMDAwMDAwNyIsImFsZyI6InNoYTI1NiIsImhhc2giOiJkZjMzMWMxOGRhMTM0NDMwOTRkYzJjODQ5NzFkNWNlZmIzNDY0ZTk3OTk5YzM5YmMwMzg0NWVhY2NjNjE3MGEyIiwicHJlZml4IjoidzJtVSIsImNyZWF0ZWRBdCI6IjIwMjUtMTEtMjVUMTI6Mzk6MDJaIn1dfQ==",
"dark_image_type": null,
"id": "e1f2a3b4-c5d6-7890-abcd-ef0000000002",
"image_type": null,
@@ -264,7 +259,6 @@
"requires_pushed_authorization_requests": false,
"requires_reauthentication": false,
"refresh_token_duration_minutes": 43200,
"secret": "$2a$10$9dypwot8nGuCjT6wQWWpJOckZfRprhe2EkwpKizxS/fpVHrOLEJHC",
"skip_consent": true
}
],

View File

@@ -1,6 +1,7 @@
import test, { expect, Page } from '@playwright/test';
import { oidcClients, userGroups } from '../data';
import { cleanupBackend } from '../utils/cleanup.util';
import * as oidcUtil from '../utils/oidc.util';
test.beforeEach(async () => await cleanupBackend());
@@ -37,7 +38,6 @@ test.describe('Create OIDC client', () => {
);
const resolvedClientId = (await page.getByTestId('client-id').innerText()).trim();
const clientSecret = (await page.getByTestId('client-secret').innerText()).trim();
if (clientId) {
expect(resolvedClientId).toBe(clientId);
@@ -45,8 +45,6 @@ test.describe('Create OIDC client', () => {
expect(resolvedClientId).toMatch(/^[\w-]{36}$/);
}
expect(clientSecret).toMatch(/^\w{32}$/);
await expect(page.getByLabel('Name')).toHaveValue(oidcClient.name);
await expect(page.getByLabel('Description')).toHaveValue(oidcClient.description);
await expect(page.getByTestId('callback-url-1')).toHaveValue(oidcClient.callbackUrl);
@@ -166,7 +164,7 @@ test('Update OIDC client token lifetimes', async ({ page }) => {
test('Update OIDC client federated credentials', async ({ page }) => {
const client = oidcClients.nextcloud;
await page.goto(`/settings/admin/oidc-clients/${client.id}`);
await page.goto(`/settings/admin/oidc-clients/${client.id}#credentials`);
const card = page.getByTestId('federated-credentials-card');
await card.getByRole('button', { name: 'Create', exact: true }).click();
@@ -188,6 +186,7 @@ test('Update OIDC client federated credentials', async ({ page }) => {
await expect(card.getByLabel('Audience')).toHaveValue('https://pocket-id.example.com');
// Saving the main client form must preserve credentials managed by the separate card
await page.locator('[role="tab"][data-value="general"]').click();
const description = page.getByLabel('Description');
await description.fill('Updated without replacing federated credentials');
const clientForm = description.locator('xpath=ancestor::form');
@@ -199,21 +198,87 @@ test('Update OIDC client federated credentials', async ({ page }) => {
await clientForm.getByRole('button', { name: 'Save' }).click();
expect((await formUpdate).ok()).toBeTruthy();
await page.reload();
await page.goto(`/settings/admin/oidc-clients/${client.id}#credentials`);
await expect(card.getByLabel('Issuer')).toHaveValue('https://issuer.example.com');
});
test('Create new OIDC client secret', async ({ page }) => {
test('Create and delete OIDC client secrets', async ({ page }) => {
const oidcClient = oidcClients.nextcloud;
await page.goto(`/settings/admin/oidc-clients/${oidcClient.id}`);
await page.goto(`/settings/admin/oidc-clients/${oidcClient.id}#credentials`);
await page.getByLabel('Create new client secret').click();
await page.getByRole('button', { name: 'Generate' }).click();
const card = page.getByTestId('client-secrets-card');
// The seeded client already has the secret the other tests authenticate with
await expect(card.getByTestId('client-secret-row')).toHaveCount(1);
await card.getByRole('button', { name: 'Add client secret' }).click();
await expect(page.locator('[data-type="success"]')).toHaveText(
'New client secret created successfully'
);
// The new secret is the only one whose value is shown in full, and only until the page is left
await expect(card.getByTestId('client-secret-row')).toHaveCount(2);
const createdSecret = (await card.getByTestId('client-secret').nth(1).innerText()).trim();
expect(createdSecret).toMatch(/^\w{32}$/);
// Both secrets authenticate the client, so it can be rotated without downtime
for (const secret of [oidcClient.secret, createdSecret]) {
const res = await oidcUtil.exchangeCode(page, {
grant_type: 'client_credentials',
client_id: oidcClient.id,
client_secret: secret
});
expect(res.access_token).toBeTruthy();
}
// After a reload only the stored prefix is left
await page.reload();
await expect(card.getByTestId('client-secret').nth(1)).toHaveText(
`${createdSecret.substring(0, 4)}••••••••`
);
await card
.getByTestId('client-secret-row')
.nth(1)
.getByRole('button', { name: 'Toggle menu' })
.click();
await page.getByRole('menuitem', { name: 'Delete' }).click();
await page.getByRole('button', { name: 'Delete' }).click();
await expect(page.locator('[data-type="success"]')).toHaveText(
'Client secret deleted successfully'
);
await expect(card.getByTestId('client-secret-row')).toHaveCount(1);
// The deleted secret can no longer authenticate the client
const res = await oidcUtil.exchangeCode(page, {
grant_type: 'client_credentials',
client_id: oidcClient.id,
client_secret: createdSecret
});
expect(res.access_token).toBeFalsy();
});
test('Client secrets can be created with an expiration', async ({ page }) => {
const oidcClient = oidcClients.nextcloud;
await page.goto(`/settings/admin/oidc-clients/${oidcClient.id}#credentials`);
const card = page.getByTestId('client-secrets-card');
await card.getByRole('combobox').click();
await page.getByRole('option', { name: '90 days' }).click();
await card.getByRole('button', { name: 'Add client secret' }).click();
await expect(page.locator('[data-type="success"]')).toHaveText(
'New client secret created successfully'
);
expect((await page.getByTestId('client-secret').textContent())?.length).toBe(32);
const secrets = await page.request
.get(`/api/oidc/clients/${oidcClient.id}/secrets`)
.then((r) => r.json());
expect(secrets).toHaveLength(2);
const expiresAt = new Date(secrets[1].expiresAt).getTime();
const expected = Date.now() + 90 * 24 * 60 * 60 * 1000;
expect(Math.abs(expiresAt - expected)).toBeLessThan(5 * 60 * 1000);
expect(secrets[1].isActive).toBe(true);
});
test('Delete OIDC client', async ({ page }) => {