TheArchive/zitadel
1
0
Fork 0
mirror of https://github.com/zitadel/zitadel.git synced 2025-08-11 19:07:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: correctly check user state (#8631)

Browse Source 
# Which Problems Are Solved

ZITADEL's user account deactivation mechanism did not work correctly
with service accounts. Deactivated service accounts retained the ability
to request tokens, which could lead to unauthorized access to
applications and resources.

# How the Problems Are Solved

Additionally to checking the user state on the session API and login UI,
the state is checked on all oidc session methods resulting in a new
token or when returning the user information (userinfo, introspection,
id_token / access_token and saml attributes)
This commit is contained in:
Livio Spring
2024-09-17 15:21:49 +02:00
committed by GitHub
parent ca1914e235
commit 5b40af79f0
10 changed files with 520 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

362
internal/command/oidc_session_test.go
View File

@@ -205,6 +205,103 @@ func TestCommands_CreateOIDCSessionFromAuthRequest(t *testing.T) {
err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-Flk38", "Errors.Session.NotExisting"),
},
},
{
"user not active",
fields{
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
authrequest.NewAddedEvent(context.Background(), &authrequest.NewAggregate("V2_authRequestID", "instanceID").Aggregate,
"loginClient",
"clientID",
"redirectURI",
"state",
"nonce",
[]string{"openid", "offline_access"},
[]string{"audience"},
domain.OIDCResponseTypeCode,
domain.OIDCResponseModeQuery,
&domain.OIDCCodeChallenge{
Challenge: "challenge",
Method: domain.CodeChallengeMethodS256,
},
[]domain.Prompt{domain.PromptNone},
[]string{"en", "de"},
gu.Ptr(time.Duration(0)),
gu.Ptr("loginHint"),
gu.Ptr("hintUserID"),
true,
),
),
eventFromEventPusher(
authrequest.NewCodeAddedEvent(context.Background(), &authrequest.NewAggregate("V2_authRequestID", "instanceID").Aggregate),
),
eventFromEventPusher(
authrequest.NewSessionLinkedEvent(context.Background(), &authrequest.NewAggregate("V2_authRequestID", "instanceID").Aggregate,
"sessionID",
"userID",
testNow,
[]domain.UserAuthMethodType{domain.UserAuthMethodTypePassword},
),
),
),
expectFilter(
eventFromEventPusher(
session.NewAddedEvent(context.Background(),
&session.NewAggregate("sessionID", "instance1").Aggregate,
&domain.UserAgent{
FingerprintID: gu.Ptr("fp1"),
IP: net.ParseIP("1.2.3.4"),
Description: gu.Ptr("firefox"),
Header: http.Header{"foo": []string{"bar"}},
},
),
),
eventFromEventPusher(
session.NewUserCheckedEvent(context.Background(), &session.NewAggregate("sessionID", "instanceID").Aggregate,
"userID", "org1", testNow, &language.Afrikaans),
),
eventFromEventPusher(
session.NewPasswordCheckedEvent(context.Background(), &session.NewAggregate("sessionID", "instanceID").Aggregate,
testNow),
),
),
expectFilter(
user.NewHumanAddedEvent(
context.Background(),
&user.NewAggregate("userID", "org1").Aggregate,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.Afrikaans,
domain.GenderUnspecified,
"email",
false,
),
user.NewUserDeactivatedEvent(
context.Background(),
&user.NewAggregate("userID", "org1").Aggregate,
),
),
),
idGenerator: mock.NewIDGeneratorExpectIDs(t),
defaultAccessTokenLifetime: time.Hour,
defaultRefreshTokenLifetime: 7 * 24 * time.Hour,
defaultRefreshTokenIdleLifetime: 24 * time.Hour,
keyAlgorithm: crypto.CreateMockEncryptionAlg(gomock.NewController(t)),
},
args{
ctx: authz.WithInstanceID(context.Background(), "instanceID"),
authRequestID: "V2_authRequestID",
complianceCheck: mockAuthRequestComplianceChecker(nil),
needRefreshToken: true,
},
res{
err: zerrors.ThrowPreconditionFailed(nil, "OIDCS-kj3g2", "Errors.User.NotActive"),
},
},
{
"add successful",
fields{
@@ -266,6 +363,21 @@ func TestCommands_CreateOIDCSessionFromAuthRequest(t *testing.T) {
testNow),
),
),
expectFilter(
user.NewHumanAddedEvent(
context.Background(),
&user.NewAggregate("userID", "org1").Aggregate,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.Afrikaans,
domain.GenderUnspecified,
"email",
false,
),
),
expectFilter(), // token lifetime
expectPush(
authrequest.NewCodeExchangedEvent(context.Background(), &authrequest.NewAggregate("V2_authRequestID", "instanceID").Aggregate),
@@ -382,6 +494,21 @@ func TestCommands_CreateOIDCSessionFromAuthRequest(t *testing.T) {
testNow),
),
),
expectFilter(
user.NewHumanAddedEvent(
context.Background(),
&user.NewAggregate("userID", "org1").Aggregate,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.Afrikaans,
domain.GenderUnspecified,
"email",
false,
),
),
expectFilter(), // token lifetime
expectPush(
oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
@@ -521,10 +648,81 @@ func TestCommands_CreateOIDCSession(t *testing.T) {
},
wantErr: io.ErrClosedPipe,
},
{
name: "not active user",
fields: fields{
eventstore: expectEventstore(
expectFilter(
user.NewHumanAddedEvent(
context.Background(),
&user.NewAggregate("userID", "org1").Aggregate,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.Afrikaans,
domain.GenderUnspecified,
"email",
false,
),
user.NewUserDeactivatedEvent(
context.Background(),
&user.NewAggregate("userID", "org1").Aggregate,
),
),
),
idGenerator: mock.NewIDGeneratorExpectIDs(t),
defaultAccessTokenLifetime: time.Hour,
defaultRefreshTokenLifetime: 7 * 24 * time.Hour,
defaultRefreshTokenIdleLifetime: 24 * time.Hour,
keyAlgorithm: crypto.CreateMockEncryptionAlg(gomock.NewController(t)),
},
args: args{
ctx: context.Background(),
userID: "userID",
resourceOwner: "org1",
clientID: "clientID",
audience: []string{"audience"},
scope: []string{"openid", "offline_access"},
authMethods: []domain.UserAuthMethodType{domain.UserAuthMethodTypePassword},
authTime: testNow,
nonce: "nonce",
preferredLanguage: &language.Afrikaans,
userAgent: &domain.UserAgent{
FingerprintID: gu.Ptr("fp1"),
IP: net.ParseIP("1.2.3.4"),
Description: gu.Ptr("firefox"),
Header: http.Header{"foo": []string{"bar"}},
},
reason: domain.TokenReasonAuthRequest,
actor: &domain.TokenActor{
UserID: "user2",
Issuer: "foo.com",
},
needRefreshToken: false,
},
wantErr: zerrors.ThrowPreconditionFailed(nil, "OIDCS-kj3g2", "Errors.User.NotActive"),
},
{
name: "without refresh token",
fields: fields{
eventstore: expectEventstore(
expectFilter(
user.NewHumanAddedEvent(
context.Background(),
&user.NewAggregate("userID", "org1").Aggregate,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.Afrikaans,
domain.GenderUnspecified,
"email",
false,
),
),
expectFilter(), // token lifetime
expectPush(
oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
@@ -606,6 +804,21 @@ func TestCommands_CreateOIDCSession(t *testing.T) {
name: "with refresh token",
fields: fields{
eventstore: expectEventstore(
expectFilter(
user.NewHumanAddedEvent(
context.Background(),
&user.NewAggregate("userID", "org1").Aggregate,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.Afrikaans,
domain.GenderUnspecified,
"email",
false,
),
),
expectFilter(), // token lifetime
expectPush(
oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
@@ -689,6 +902,21 @@ func TestCommands_CreateOIDCSession(t *testing.T) {
name: "with sessionID",
fields: fields{
eventstore: expectEventstore(
expectFilter(
user.NewHumanAddedEvent(
context.Background(),
&user.NewAggregate("userID", "org1").Aggregate,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.Afrikaans,
domain.GenderUnspecified,
"email",
false,
),
),
expectFilter(), // token lifetime
expectPush(
oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
@@ -772,6 +1000,21 @@ func TestCommands_CreateOIDCSession(t *testing.T) {
name: "impersonation not allowed",
fields: fields{
eventstore: expectEventstore(
expectFilter(
user.NewHumanAddedEvent(
context.Background(),
&user.NewAggregate("userID", "org1").Aggregate,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.Afrikaans,
domain.GenderUnspecified,
"email",
false,
),
),
expectFilter(), // token lifetime
),
idGenerator: mock.NewIDGeneratorExpectIDs(t, "oidcSessionID"),
@@ -813,6 +1056,21 @@ func TestCommands_CreateOIDCSession(t *testing.T) {
name: "impersonation allowed",
fields: fields{
eventstore: expectEventstore(
expectFilter(
user.NewHumanAddedEvent(
context.Background(),
&user.NewAggregate("userID", "org1").Aggregate,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.Afrikaans,
domain.GenderUnspecified,
"email",
false,
),
),
expectFilter(), // token lifetime
expectPush(
user.NewUserImpersonatedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate, "clientID", &domain.TokenActor{
@@ -1067,6 +1325,63 @@ func TestCommands_ExchangeOIDCSessionRefreshAndAccessToken(t *testing.T) {
err: zerrors.ThrowPreconditionFailed(nil, "OIDCS-3jt2w", "Errors.OIDCSession.RefreshTokenInvalid"),
},
},
{
"user not active",
fields{
eventstore: expectEventstore(
expectFilter(
eventFromEventPusherWithCreationDateNow(
oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
"userID", "org1", "sessionID", "clientID", []string{"audience"}, []string{"openid", "profile", "offline_access"},
[]domain.UserAuthMethodType{domain.UserAuthMethodTypePassword}, testNow, "nonce", &language.Afrikaans,
&domain.UserAgent{FingerprintID: gu.Ptr("browserFP")},
),
),
eventFromEventPusherWithCreationDateNow(
oidcsession.NewAccessTokenAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
"at_accessTokenID", []string{"openid", "profile", "offline_access"}, time.Hour, domain.TokenReasonAuthRequest, nil),
),
eventFromEventPusherWithCreationDateNow(
oidcsession.NewRefreshTokenAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
"rt_refreshTokenID", 7*24*time.Hour, 24*time.Hour),
),
),
expectFilter(
user.NewHumanAddedEvent(
context.Background(),
&user.NewAggregate("userID", "org1").Aggregate,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.Afrikaans,
domain.GenderUnspecified,
"email",
false,
),
user.NewUserDeactivatedEvent(
context.Background(),
&user.NewAggregate("userID", "org1").Aggregate,
),
),
),
idGenerator: mock.NewIDGeneratorExpectIDs(t),
defaultAccessTokenLifetime: time.Hour,
defaultRefreshTokenLifetime: 7 * 24 * time.Hour,
defaultRefreshTokenIdleLifetime: 24 * time.Hour,
keyAlgorithm: crypto.CreateMockEncryptionAlg(gomock.NewController(t)),
},
args{
ctx: authz.WithInstanceID(context.Background(), "instanceID"),
refreshToken: "VjJfb2lkY1Nlc3Npb25JRC1ydF9yZWZyZXNoVG9rZW5JRDp1c2VySUQ", //V2_oidcSessionID:rt_refreshTokenID:userID
scope: []string{"openid", "offline_access"},
complianceCheck: mockRefreshTokenComplianceChecker(nil),
},
res{
err: zerrors.ThrowPreconditionFailed(nil, "OIDCS-J39h2", "Errors.User.NotActive"),
},
},
{
"refresh successful",
fields{
@@ -1088,6 +1403,21 @@ func TestCommands_ExchangeOIDCSessionRefreshAndAccessToken(t *testing.T) {
"rt_refreshTokenID", 7*24*time.Hour, 24*time.Hour),
),
),
expectFilter(
user.NewHumanAddedEvent(
context.Background(),
&user.NewAggregate("userID", "org1").Aggregate,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.Afrikaans,
domain.GenderUnspecified,
"email",
false,
),
),
expectFilter(), // token lifetime
expectPush(
oidcsession.NewAccessTokenAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
@@ -1153,7 +1483,7 @@ func TestCommands_ExchangeOIDCSessionRefreshAndAccessToken(t *testing.T) {
func TestCommands_OIDCSessionByRefreshToken(t *testing.T) {
type fields struct {
eventstore *eventstore.Eventstore
eventstore func(*testing.T) *eventstore.Eventstore
idGenerator id.Generator
defaultAccessTokenLifetime time.Duration
defaultRefreshTokenLifetime time.Duration
@@ -1177,7 +1507,7 @@ func TestCommands_OIDCSessionByRefreshToken(t *testing.T) {
{
"invalid refresh token format error",
fields{
eventstore: eventstoreExpect(t),
eventstore: expectEventstore(),
keyAlgorithm: crypto.CreateMockEncryptionAlg(gomock.NewController(t)),
},
args{
@@ -1191,7 +1521,7 @@ func TestCommands_OIDCSessionByRefreshToken(t *testing.T) {
{
"inactive session error",
fields{
eventstore: eventstoreExpect(t,
eventstore: expectEventstore(
expectFilter(),
),
keyAlgorithm: crypto.CreateMockEncryptionAlg(gomock.NewController(t)),
@@ -1207,7 +1537,7 @@ func TestCommands_OIDCSessionByRefreshToken(t *testing.T) {
{
"invalid refresh token error",
fields{
eventstore: eventstoreExpect(t,
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
@@ -1235,7 +1565,7 @@ func TestCommands_OIDCSessionByRefreshToken(t *testing.T) {
{
"expired refresh token error",
fields{
eventstore: eventstoreExpect(t,
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
@@ -1267,7 +1597,7 @@ func TestCommands_OIDCSessionByRefreshToken(t *testing.T) {
{
"get successful",
fields{
eventstore: eventstoreExpect(t,
eventstore: expectEventstore(
expectFilter(
eventFromEventPusherWithCreationDateNow(
oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
@@ -1316,7 +1646,7 @@ func TestCommands_OIDCSessionByRefreshToken(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c := &Commands{
eventstore: tt.fields.eventstore,
eventstore: tt.fields.eventstore(t),
idGenerator: tt.fields.idGenerator,
defaultAccessTokenLifetime: tt.fields.defaultAccessTokenLifetime,
defaultRefreshTokenLifetime: tt.fields.defaultRefreshTokenLifetime,
@@ -1348,7 +1678,7 @@ func TestCommands_OIDCSessionByRefreshToken(t *testing.T) {
func TestCommands_RevokeOIDCSessionToken(t *testing.T) {
type fields struct {
eventstore *eventstore.Eventstore
eventstore func(*testing.T) *eventstore.Eventstore
keyAlgorithm crypto.EncryptionAlgorithm
}
type args struct {
@@ -1368,7 +1698,7 @@ func TestCommands_RevokeOIDCSessionToken(t *testing.T) {
{
"invalid token",
fields{
eventstore: eventstoreExpect(t),
eventstore: expectEventstore(),
keyAlgorithm: crypto.CreateMockEncryptionAlg(gomock.NewController(t)),
},
args{
@@ -1382,7 +1712,7 @@ func TestCommands_RevokeOIDCSessionToken(t *testing.T) {
{
"refresh_token inactive",
fields{
eventstore: eventstoreExpect(t,
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
@@ -1407,7 +1737,7 @@ func TestCommands_RevokeOIDCSessionToken(t *testing.T) {
{
"refresh_token invalid client",
fields{
eventstore: eventstoreExpect(t,
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
@@ -1432,7 +1762,7 @@ func TestCommands_RevokeOIDCSessionToken(t *testing.T) {
{
"refresh_token revoked",
fields{
eventstore: eventstoreExpect(t,
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
@@ -1468,7 +1798,7 @@ func TestCommands_RevokeOIDCSessionToken(t *testing.T) {
{
"access_token inactive session",
fields{
eventstore: eventstoreExpect(t,
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
@@ -1493,7 +1823,7 @@ func TestCommands_RevokeOIDCSessionToken(t *testing.T) {
{
"access_token invalid client",
fields{
eventstore: eventstoreExpect(t,
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
@@ -1518,7 +1848,7 @@ func TestCommands_RevokeOIDCSessionToken(t *testing.T) {
{
"access_token revoked",
fields{
eventstore: eventstoreExpect(t,
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
@@ -1555,7 +1885,7 @@ func TestCommands_RevokeOIDCSessionToken(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c := &Commands{
eventstore: tt.fields.eventstore,
eventstore: tt.fields.eventstore(t),
keyAlgorithm: tt.fields.keyAlgorithm,
}
err := c.RevokeOIDCSessionToken(tt.args.ctx, tt.args.token, tt.args.clientID)