andwati/zitadel
1
0
Fork 0
mirror of https://github.com/andwati/zitadel.git synced 2026-11-10 08:05:12 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: prevent possible nil pointer panics (#11728)

Browse Source 
# Which Problems Are Solved

We found multiple cases where potentially a panic occurred or could
occur:
- when de/encrypting certain information
- returning apps without any configuration type (?)
- apple IdPs without a proper private key

# How the Problems Are Solved

- Added nil checks
- Check private key format for apple IdPs
- Added necessary helper function

# Additional Changes

Fixed i18n yaml where the IDP errors were indented under `org` instead
of directly under `errors`.

# Additional Context

- requires backport to v4.x

---------

Co-authored-by: Marco A. <[email protected]>
This commit is contained in:
Livio Spring
2026-03-16 14:30:17 +00:00
committed by GitHub
co-authored by Marco A.
parent 7c0e1750f3
commit 500821ce0c
33 changed files with 440 additions and 264 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/command/instance_idp.go
+9
View File
@@ -12,6 +12,7 @@ import (
"github.com/zitadel/zitadel/internal/crypto"
"github.com/zitadel/zitadel/internal/domain"
"github.com/zitadel/zitadel/internal/eventstore"
"github.com/zitadel/zitadel/internal/idp/providers/apple"
"github.com/zitadel/zitadel/internal/idp/providers/saml"
"github.com/zitadel/zitadel/internal/repository/instance"
"github.com/zitadel/zitadel/internal/zerrors"
@@ -1668,6 +1669,9 @@ func (c *Commands) prepareAddInstanceAppleProvider(a *instance.Aggregate, writeM
if len(provider.PrivateKey) == 0 {
return nil, zerrors.ThrowInvalidArgument(nil, "INST-GVD4n", "Errors.IDP.PrivateKeyMissing")
}
if _, err := apple.BytesToPrivateKey(provider.PrivateKey); err != nil {
return nil, zerrors.ThrowInvalidArgument(err, "INST-Fk38d", "Errors.IDP.InvalidPrivateKey")
}
return func(ctx context.Context, filter preparation.FilterToQueryReducer) ([]eventstore.Command, error) {
events, err := filter(ctx, writeModel.Query())
if err != nil {
@@ -1713,6 +1717,11 @@ func (c *Commands) prepareUpdateInstanceAppleProvider(a *instance.Aggregate, wri
if provider.KeyID = strings.TrimSpace(provider.KeyID); provider.KeyID == "" {
return nil, zerrors.ThrowInvalidArgument(nil, "INST-Gh4z2", "Errors.IDP.KeyIDMissing")
}
if len(provider.PrivateKey) > 0 {
if _, err := apple.BytesToPrivateKey(provider.PrivateKey); err != nil {
return nil, zerrors.ThrowInvalidArgument(err, "INST-eWSDf", "Errors.IDP.InvalidPrivateKey")
}
}
return func(ctx context.Context, filter preparation.FilterToQueryReducer) ([]eventstore.Command, error) {
events, err := filter(ctx, writeModel.Query())
if err != nil {
internal/command/instance_idp_test.go
+67 -44
View File
@@ -2,6 +2,9 @@ package command
import (
"context"
"crypto/ecdsa"
"crypto/elliptic"
"crypto/rand"
"errors"
"testing"
"time"
@@ -4893,6 +4896,14 @@ func TestCommandSide_UpdateInstanceLDAPIDP(t *testing.T) {
}
}
var (
privateKeyPKCS8 = func() []byte {
privateKey, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
data, _ := crypto.PrivateKeyToBytesPKCS8(privateKey)
return data
}()
)
func TestCommandSide_AddInstanceAppleIDP(t *testing.T) {
type fields struct {
eventstore func(*testing.T) *eventstore.Eventstore
@@ -4906,7 +4917,7 @@ func TestCommandSide_AddInstanceAppleIDP(t *testing.T) {
type res struct {
id string
want *domain.ObjectDetails
err func(error) bool
err error
}
tests := []struct {
name string
@@ -4925,9 +4936,7 @@ func TestCommandSide_AddInstanceAppleIDP(t *testing.T) {
provider: AppleProvider{},
},
res{
err: func(err error) bool {
return errors.Is(err, zerrors.ThrowInvalidArgument(nil, "INST-jkn3w", "Errors.IDP.ClientIDMissing"))
},
err: zerrors.ThrowInvalidArgument(nil, "INST-jkn3w", "Errors.IDP.ClientIDMissing"),
},
},
{
@@ -4943,9 +4952,7 @@ func TestCommandSide_AddInstanceAppleIDP(t *testing.T) {
},
},
res{
err: func(err error) bool {
return errors.Is(err, zerrors.ThrowInvalidArgument(nil, "INST-Ffg32", "Errors.IDP.TeamIDMissing"))
},
err: zerrors.ThrowInvalidArgument(nil, "INST-Ffg32", "Errors.IDP.TeamIDMissing"),
},
},
{
@@ -4962,13 +4969,11 @@ func TestCommandSide_AddInstanceAppleIDP(t *testing.T) {
},
},
res{
err: func(err error) bool {
return errors.Is(err, zerrors.ThrowInvalidArgument(nil, "INST-GDjm5", "Errors.IDP.KeyIDMissing"))
},
err: zerrors.ThrowInvalidArgument(nil, "INST-GDjm5", "Errors.IDP.KeyIDMissing"),
},
},
{
"invalid privateKey",
"no privateKey",
fields{
eventstore: expectEventstore(),
idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "id1"),
@@ -4982,10 +4987,27 @@ func TestCommandSide_AddInstanceAppleIDP(t *testing.T) {
},
},
res{
err: func(err error) bool {
return errors.Is(err, zerrors.ThrowInvalidArgument(nil, "INST-GVD4n", "Errors.IDP.PrivateKeyMissing"))
err: zerrors.ThrowInvalidArgument(nil, "INST-GVD4n", "Errors.IDP.PrivateKeyMissing"),
},
},
{
"invalid privateKey",
fields{
eventstore: expectEventstore(),
idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "id1"),
},
args{
ctx: authz.WithInstanceID(context.Background(), "instance1"),
provider: AppleProvider{
ClientID: "clientID",
TeamID: "teamID",
KeyID: "keyID",
PrivateKey: []byte("invalid"),
},
},
res{
err: zerrors.ThrowInvalidArgument(crypto.ErrEmpty, "INST-Fk38d", "Errors.IDP.InvalidPrivateKey"),
},
},
{
name: "ok",
@@ -5003,7 +5025,7 @@ func TestCommandSide_AddInstanceAppleIDP(t *testing.T) {
CryptoType: crypto.TypeEncryption,
Algorithm: "enc",
KeyID: "id",
Crypted: []byte("privateKey"),
Crypted: privateKeyPKCS8,
},
nil,
idp.Options{},
@@ -5019,7 +5041,7 @@ func TestCommandSide_AddInstanceAppleIDP(t *testing.T) {
ClientID: "clientID",
TeamID: "teamID",
KeyID: "keyID",
PrivateKey: []byte("privateKey"),
PrivateKey: privateKeyPKCS8,
},
},
res: res{
@@ -5043,7 +5065,7 @@ func TestCommandSide_AddInstanceAppleIDP(t *testing.T) {
CryptoType: crypto.TypeEncryption,
Algorithm: "enc",
KeyID: "id",
Crypted: []byte("privateKey"),
Crypted: privateKeyPKCS8,
},
[]string{"name", "email"},
idp.Options{
@@ -5064,7 +5086,7 @@ func TestCommandSide_AddInstanceAppleIDP(t *testing.T) {
ClientID: "clientID",
TeamID: "teamID",
KeyID: "keyID",
PrivateKey: []byte("privateKey"),
PrivateKey: privateKeyPKCS8,
Scopes: []string{"name", "email"},
IDPOptions: idp.Options{
IsCreationAllowed: true,
@@ -5088,12 +5110,7 @@ func TestCommandSide_AddInstanceAppleIDP(t *testing.T) {
idpConfigEncryption: tt.fields.secretCrypto,
}
id, got, err := c.AddInstanceAppleProvider(tt.args.ctx, tt.args.provider)
if tt.res.err == nil {
assert.NoError(t, err)
}
if tt.res.err != nil && !tt.res.err(err) {
t.Errorf("got wrong err: %v ", err)
}
assert.ErrorIs(t, err, tt.res.err)
if tt.res.err == nil {
assert.Equal(t, tt.res.id, id)
assertObjectDetails(t, tt.res.want, got)
@@ -5114,7 +5131,7 @@ func TestCommandSide_UpdateInstanceAppleIDP(t *testing.T) {
}
type res struct {
want *domain.ObjectDetails
err func(error) bool
err error
}
tests := []struct {
name string
@@ -5132,9 +5149,7 @@ func TestCommandSide_UpdateInstanceAppleIDP(t *testing.T) {
provider: AppleProvider{},
},
res{
err: func(err error) bool {
return errors.Is(err, zerrors.ThrowInvalidArgument(nil, "INST-FRHBH", "Errors.IDMissing"))
},
err: zerrors.ThrowInvalidArgument(nil, "INST-FRHBH", "Errors.IDMissing"),
},
},
{
@@ -5148,9 +5163,7 @@ func TestCommandSide_UpdateInstanceAppleIDP(t *testing.T) {
provider: AppleProvider{},
},
res{
err: func(err error) bool {
return errors.Is(err, zerrors.ThrowInvalidArgument(nil, "INST-SFm4l", "Errors.IDP.ClientIDMissing"))
},
err: zerrors.ThrowInvalidArgument(nil, "INST-SFm4l", "Errors.IDP.ClientIDMissing"),
},
},
{
@@ -5166,9 +5179,7 @@ func TestCommandSide_UpdateInstanceAppleIDP(t *testing.T) {
},
},
res{
err: func(err error) bool {
return errors.Is(err, zerrors.ThrowInvalidArgument(nil, "INST-SG34t", "Errors.IDP.TeamIDMissing"))
},
err: zerrors.ThrowInvalidArgument(nil, "INST-SG34t", "Errors.IDP.TeamIDMissing"),
},
},
{
@@ -5185,10 +5196,27 @@ func TestCommandSide_UpdateInstanceAppleIDP(t *testing.T) {
},
},
res{
err: func(err error) bool {
return errors.Is(err, zerrors.ThrowInvalidArgument(nil, "INST-Gh4z2", "Errors.IDP.KeyIDMissing"))
err: zerrors.ThrowInvalidArgument(nil, "INST-Gh4z2", "Errors.IDP.KeyIDMissing"),
},
},
{
"invalid key",
fields{
eventstore: expectEventstore(),
},
args{
ctx: authz.WithInstanceID(context.Background(), "instance1"),
id: "id1",
provider: AppleProvider{
ClientID: "clientID",
TeamID: "teamID",
KeyID: "keyID",
PrivateKey: []byte("invalid"),
},
},
res{
err: zerrors.ThrowInvalidArgument(nil, "INST-eWSDf", "Errors.IDP.InvalidPrivateKey"),
},
},
{
name: "not found",
@@ -5207,7 +5235,7 @@ func TestCommandSide_UpdateInstanceAppleIDP(t *testing.T) {
},
},
res: res{
err: zerrors.IsNotFound,
err: zerrors.ThrowNotFound(nil, "INST-SG3bh", "Errors.IDPConfig.NotExisting"),
},
},
{
@@ -5282,7 +5310,7 @@ func TestCommandSide_UpdateInstanceAppleIDP(t *testing.T) {
CryptoType: crypto.TypeEncryption,
Algorithm: "enc",
KeyID: "id",
Crypted: []byte("newPrivateKey"),
Crypted: privateKeyPKCS8,
}),
idp.ChangeAppleScopes([]string{"name", "email"}),
idp.ChangeAppleOptions(idp.OptionChanges{
@@ -5306,7 +5334,7 @@ func TestCommandSide_UpdateInstanceAppleIDP(t *testing.T) {
ClientID: "clientID2",
TeamID: "teamID2",
KeyID: "keyID2",
PrivateKey: []byte("newPrivateKey"),
PrivateKey: privateKeyPKCS8,
Scopes: []string{"name", "email"},
IDPOptions: idp.Options{
IsCreationAllowed: true,
@@ -5328,12 +5356,7 @@ func TestCommandSide_UpdateInstanceAppleIDP(t *testing.T) {
idpConfigEncryption: tt.fields.secretCrypto,
}
got, err := c.UpdateInstanceAppleProvider(tt.args.ctx, tt.args.id, tt.args.provider)
if tt.res.err == nil {
assert.NoError(t, err)
}
if tt.res.err != nil && !tt.res.err(err) {
t.Errorf("got wrong err: %v ", err)
}
assert.ErrorIs(t, err, tt.res.err)
if tt.res.err == nil {
assertObjectDetails(t, tt.res.want, got)
}
internal/command/org_idp.go
+9
View File
@@ -10,6 +10,7 @@ import (
"github.com/zitadel/zitadel/internal/crypto"
"github.com/zitadel/zitadel/internal/domain"
"github.com/zitadel/zitadel/internal/eventstore"
"github.com/zitadel/zitadel/internal/idp/providers/apple"
"github.com/zitadel/zitadel/internal/idp/providers/saml"
"github.com/zitadel/zitadel/internal/repository/org"
"github.com/zitadel/zitadel/internal/zerrors"
@@ -1642,6 +1643,9 @@ func (c *Commands) prepareAddOrgAppleProvider(a *org.Aggregate, writeModel *OrgA
if len(provider.PrivateKey) == 0 {
return nil, zerrors.ThrowInvalidArgument(nil, "ORG-GVD4n", "Errors.IDP.PrivateKeyMissing")
}
if _, err := apple.BytesToPrivateKey(provider.PrivateKey); err != nil {
return nil, zerrors.ThrowInvalidArgument(err, "ORG-Fk38d", "Errors.IDP.InvalidPrivateKey")
}
return func(ctx context.Context, filter preparation.FilterToQueryReducer) ([]eventstore.Command, error) {
events, err := filter(ctx, writeModel.Query())
if err != nil {
@@ -1687,6 +1691,11 @@ func (c *Commands) prepareUpdateOrgAppleProvider(a *org.Aggregate, writeModel *O
if provider.KeyID = strings.TrimSpace(provider.KeyID); provider.KeyID == "" {
return nil, zerrors.ThrowInvalidArgument(nil, "ORG-Gh4z2", "Errors.IDP.KeyIDMissing")
}
if len(provider.PrivateKey) > 0 {
if _, err := apple.BytesToPrivateKey(provider.PrivateKey); err != nil {
return nil, zerrors.ThrowInvalidArgument(err, "ORG-eWSDf", "Errors.IDP.InvalidPrivateKey")
}
}
return func(ctx context.Context, filter preparation.FilterToQueryReducer) ([]eventstore.Command, error) {
events, err := filter(ctx, writeModel.Query())
if err != nil {
internal/command/org_idp_test.go
+58 -45
View File
@@ -4966,7 +4966,7 @@ func TestCommandSide_AddOrgAppleIDP(t *testing.T) {
type res struct {
id string
want *domain.ObjectDetails
err func(error) bool
err error
}
tests := []struct {
name string
@@ -4986,9 +4986,7 @@ func TestCommandSide_AddOrgAppleIDP(t *testing.T) {
provider: AppleProvider{},
},
res{
err: func(err error) bool {
return errors.Is(err, zerrors.ThrowInvalidArgument(nil, "ORG-jkn3w", "Errors.IDP.ClientIDMissing"))
},
err: zerrors.ThrowInvalidArgument(nil, "ORG-jkn3w", "Errors.IDP.ClientIDMissing"),
},
},
{
@@ -5005,9 +5003,7 @@ func TestCommandSide_AddOrgAppleIDP(t *testing.T) {
},
},
res{
err: func(err error) bool {
return errors.Is(err, zerrors.ThrowInvalidArgument(nil, "ORG-Ffg32", "Errors.IDP.TeamIDMissing"))
},
err: zerrors.ThrowInvalidArgument(nil, "ORG-Ffg32", "Errors.IDP.TeamIDMissing"),
},
},
{
@@ -5025,13 +5021,11 @@ func TestCommandSide_AddOrgAppleIDP(t *testing.T) {
},
},
res{
err: func(err error) bool {
return errors.Is(err, zerrors.ThrowInvalidArgument(nil, "ORG-GDjm5", "Errors.IDP.KeyIDMissing"))
},
err: zerrors.ThrowInvalidArgument(nil, "ORG-GDjm5", "Errors.IDP.KeyIDMissing"),
},
},
{
"invalid privateKey",
"no privateKey",
fields{
eventstore: expectEventstore(),
idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "id1"),
@@ -5046,10 +5040,28 @@ func TestCommandSide_AddOrgAppleIDP(t *testing.T) {
},
},
res{
err: func(err error) bool {
return errors.Is(err, zerrors.ThrowInvalidArgument(nil, "ORG-GVD4n", "Errors.IDP.PrivateKeyMissing"))
err: zerrors.ThrowInvalidArgument(nil, "ORG-GVD4n", "Errors.IDP.PrivateKeyMissing"),
},
},
{
"invalid privateKey",
fields{
eventstore: expectEventstore(),
idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "id1"),
},
args{
ctx: context.Background(),
resourceOwner: "org1",
provider: AppleProvider{
ClientID: "clientID",
TeamID: "teamID",
KeyID: "keyID",
PrivateKey: []byte("invalid"),
},
},
res{
err: zerrors.ThrowInvalidArgument(crypto.ErrEmpty, "ORG-Fk38d", "Errors.IDP.InvalidPrivateKey"),
},
},
{
name: "ok",
@@ -5067,7 +5079,7 @@ func TestCommandSide_AddOrgAppleIDP(t *testing.T) {
CryptoType: crypto.TypeEncryption,
Algorithm: "enc",
KeyID: "id",
Crypted: []byte("privateKey"),
Crypted: privateKeyPKCS8,
},
nil,
idp.Options{},
@@ -5084,7 +5096,7 @@ func TestCommandSide_AddOrgAppleIDP(t *testing.T) {
ClientID: "clientID",
TeamID: "teamID",
KeyID: "keyID",
PrivateKey: []byte("privateKey"),
PrivateKey: privateKeyPKCS8,
},
},
res: res{
@@ -5108,7 +5120,7 @@ func TestCommandSide_AddOrgAppleIDP(t *testing.T) {
CryptoType: crypto.TypeEncryption,
Algorithm: "enc",
KeyID: "id",
Crypted: []byte("privateKey"),
Crypted: privateKeyPKCS8,
},
[]string{"name", "email"},
idp.Options{
@@ -5130,7 +5142,7 @@ func TestCommandSide_AddOrgAppleIDP(t *testing.T) {
ClientID: "clientID",
TeamID: "teamID",
KeyID: "keyID",
PrivateKey: []byte("privateKey"),
PrivateKey: privateKeyPKCS8,
Scopes: []string{"name", "email"},
IDPOptions: idp.Options{
IsCreationAllowed: true,
@@ -5154,12 +5166,7 @@ func TestCommandSide_AddOrgAppleIDP(t *testing.T) {
idpConfigEncryption: tt.fields.secretCrypto,
}
id, got, err := c.AddOrgAppleProvider(tt.args.ctx, tt.args.resourceOwner, tt.args.provider)
if tt.res.err == nil {
assert.NoError(t, err)
}
if tt.res.err != nil && !tt.res.err(err) {
t.Errorf("got wrong err: %v ", err)
}
assert.ErrorIs(t, err, tt.res.err)
if tt.res.err == nil {
assert.Equal(t, tt.res.id, id)
assertObjectDetails(t, tt.res.want, got)
@@ -5181,7 +5188,7 @@ func TestCommandSide_UpdateOrgAppleIDP(t *testing.T) {
}
type res struct {
want *domain.ObjectDetails
err func(error) bool
err error
}
tests := []struct {
name string
@@ -5200,9 +5207,7 @@ func TestCommandSide_UpdateOrgAppleIDP(t *testing.T) {
provider: AppleProvider{},
},
res{
err: func(err error) bool {
return errors.Is(err, zerrors.ThrowInvalidArgument(nil, "ORG-FRHBH", "Errors.IDMissing"))
},
err: zerrors.ThrowInvalidArgument(nil, "ORG-FRHBH", "Errors.IDMissing"),
},
},
{
@@ -5217,9 +5222,7 @@ func TestCommandSide_UpdateOrgAppleIDP(t *testing.T) {
provider: AppleProvider{},
},
res{
err: func(err error) bool {
return errors.Is(err, zerrors.ThrowInvalidArgument(nil, "ORG-SFm4l", "Errors.IDP.ClientIDMissing"))
},
err: zerrors.ThrowInvalidArgument(nil, "ORG-SFm4l", "Errors.IDP.ClientIDMissing"),
},
},
{
@@ -5236,9 +5239,7 @@ func TestCommandSide_UpdateOrgAppleIDP(t *testing.T) {
},
},
res{
err: func(err error) bool {
return errors.Is(err, zerrors.ThrowInvalidArgument(nil, "ORG-SG34t", "Errors.IDP.TeamIDMissing"))
},
err: zerrors.ThrowInvalidArgument(nil, "ORG-SG34t", "Errors.IDP.TeamIDMissing"),
},
},
{
@@ -5256,10 +5257,27 @@ func TestCommandSide_UpdateOrgAppleIDP(t *testing.T) {
},
},
res{
err: func(err error) bool {
return errors.Is(err, zerrors.ThrowInvalidArgument(nil, "ORG-Gh4z2", "Errors.IDP.KeyIDMissing"))
err: zerrors.ThrowInvalidArgument(nil, "ORG-Gh4z2", "Errors.IDP.KeyIDMissing"),
},
},
{
"invalid key",
fields{
eventstore: expectEventstore(),
},
args{
ctx: context.Background(),
id: "id1",
provider: AppleProvider{
ClientID: "clientID",
TeamID: "teamID",
KeyID: "keyID",
PrivateKey: []byte("invalid"),
},
},
res{
err: zerrors.ThrowInvalidArgument(nil, "ORG-eWSDf", "Errors.IDP.InvalidPrivateKey"),
},
},
{
name: "not found",
@@ -5279,7 +5297,7 @@ func TestCommandSide_UpdateOrgAppleIDP(t *testing.T) {
},
},
res: res{
err: zerrors.IsNotFound,
err: zerrors.ThrowNotFound(nil, "ORG-SG3bh", "Errors.IDPConfig.NotExisting"),
},
},
{
@@ -5298,7 +5316,7 @@ func TestCommandSide_UpdateOrgAppleIDP(t *testing.T) {
CryptoType: crypto.TypeEncryption,
Algorithm: "enc",
KeyID: "id",
Crypted: []byte("privateKey"),
Crypted: privateKeyPKCS8,
},
nil,
idp.Options{},
@@ -5355,7 +5373,7 @@ func TestCommandSide_UpdateOrgAppleIDP(t *testing.T) {
CryptoType: crypto.TypeEncryption,
Algorithm: "enc",
KeyID: "id",
Crypted: []byte("newPrivateKey"),
Crypted: privateKeyPKCS8,
}),
idp.ChangeAppleScopes([]string{"name", "email"}),
idp.ChangeAppleOptions(idp.OptionChanges{
@@ -5380,7 +5398,7 @@ func TestCommandSide_UpdateOrgAppleIDP(t *testing.T) {
ClientID: "clientID2",
TeamID: "teamID2",
KeyID: "keyID2",
PrivateKey: []byte("newPrivateKey"),
PrivateKey: privateKeyPKCS8,
Scopes: []string{"name", "email"},
IDPOptions: idp.Options{
IsCreationAllowed: true,
@@ -5402,12 +5420,7 @@ func TestCommandSide_UpdateOrgAppleIDP(t *testing.T) {
idpConfigEncryption: tt.fields.secretCrypto,
}
got, err := c.UpdateOrgAppleProvider(tt.args.ctx, tt.args.resourceOwner, tt.args.id, tt.args.provider)
if tt.res.err == nil {
assert.NoError(t, err)
}
if tt.res.err != nil && !tt.res.err(err) {
t.Errorf("got wrong err: %v ", err)
}
assert.ErrorIs(t, err, tt.res.err)
if tt.res.err == nil {
assertObjectDetails(t, tt.res.want, got)
}