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

fix: correctly check app state on authentication (#8630)

Browse Source 
# Which Problems Are Solved

In Zitadel, even after an organization is deactivated, associated
projects, respectively their applications remain active. Users across
other organizations can still log in and access through these
applications, leading to unauthorized access.
Additionally, if a project was deactivated access to applications was
also still possible.

# How the Problems Are Solved

- Correctly check the status of the organization and related project. 
(Corresponding functions have been renamed to `Active...`)
This commit is contained in:
Livio Spring
2024-09-17 13:34:14 +02:00
committed by GitHub
parent 77aa02a521
commit d01bd1c51a
13 changed files with 299 additions and 146 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
internal/api/oidc/client.go
View File

@@ -50,13 +50,10 @@ func (o *OPStorage) GetClientByClientID(ctx context.Context, id string) (_ op.Cl
err = oidcError(err)
span.EndWithError(err)
}()
client, err := o.query.GetOIDCClientByID(ctx, id, false)
client, err := o.query.ActiveOIDCClientByID(ctx, id, false)
if err != nil {
return nil, err
}
if client.State != domain.AppStateActive {
return nil, zerrors.ThrowPreconditionFailed(nil, "OIDC-sdaGg", "client is not active")
}
return ClientFromBusiness(client, o.defaultLoginURL, o.defaultLoginURLV2), nil
}
@@ -979,16 +976,13 @@ func (s *Server) VerifyClient(ctx context.Context, r *op.Request[op.ClientCreden
if err != nil {
return nil, err
}
client, err := s.query.GetOIDCClientByID(ctx, clientID, assertion)
client, err := s.query.ActiveOIDCClientByID(ctx, clientID, assertion)
if zerrors.IsNotFound(err) {
return nil, oidc.ErrInvalidClient().WithParent(err).WithReturnParentToClient(authz.GetFeatures(ctx).DebugOIDCParentError).WithDescription("client not found")
return nil, oidc.ErrInvalidClient().WithParent(err).WithReturnParentToClient(authz.GetFeatures(ctx).DebugOIDCParentError).WithDescription("no active client not found")
}
if err != nil {
return nil, err // defaults to server error
}
if client.State != domain.AppStateActive {
return nil, oidc.ErrInvalidClient().WithDescription("client is not active")
}
if client.Settings == nil {
client.Settings = &query.OIDCSettings{
AccessTokenLifetime: s.defaultAccessTokenLifetime,

12
internal/api/oidc/integration_test/client_test.go
View File

@@ -196,9 +196,13 @@ func TestServer_VerifyClient(t *testing.T) {
sessionID, sessionToken, startTime, changeTime := Instance.CreateVerifiedWebAuthNSession(t, CTXLOGIN, User.GetUserId())
project, err := Instance.CreateProject(CTX)
require.NoError(t, err)
projectInactive, err := Instance.CreateProject(CTX)
require.NoError(t, err)
inactiveClient, err := Instance.CreateOIDCInactivateClient(CTX, redirectURI, logoutRedirectURI, project.GetId())
require.NoError(t, err)
inactiveProjectClient, err := Instance.CreateOIDCInactivateProjectClient(CTX, redirectURI, logoutRedirectURI, projectInactive.GetId())
require.NoError(t, err)
nativeClient, err := Instance.CreateOIDCNativeClient(CTX, redirectURI, logoutRedirectURI, project.GetId(), false)
require.NoError(t, err)
basicWebClient, err := Instance.CreateOIDCWebClientBasic(CTX, redirectURI, logoutRedirectURI, project.GetId())
@@ -240,6 +244,14 @@ func TestServer_VerifyClient(t *testing.T) {
},
wantErr: true,
},
{
name: "client inactive (project) error",
client: clientDetails{
authReqClientID: nativeClient.GetClientId(),
clientID: inactiveProjectClient.GetClientId(),
},
wantErr: true,
},
{
name: "native client success",
client: clientDetails{

2
internal/api/oidc/introspect.go
View File

@@ -213,7 +213,7 @@ func (s *Server) clientFromCredentials(ctx context.Context, cc *op.ClientCredent
if err != nil {
return nil, err
}
client, err = s.query.GetIntrospectionClientByID(ctx, clientID, assertion)
client, err = s.query.ActiveIntrospectionClientByID(ctx, clientID, assertion)
if errors.Is(err, sql.ErrNoRows) {
return nil, oidc.ErrUnauthorizedClient().WithParent(err).WithReturnParentToClient(authz.GetFeatures(ctx).DebugOIDCParentError)
}

10
internal/api/saml/storage.go
View File

@@ -55,13 +55,10 @@ type Storage struct {
}
func (p *Storage) GetEntityByID(ctx context.Context, entityID string) (*serviceprovider.ServiceProvider, error) {
app, err := p.query.AppBySAMLEntityID(ctx, entityID)
app, err := p.query.ActiveAppBySAMLEntityID(ctx, entityID)
if err != nil {
return nil, err
}
if app.State != domain.AppStateActive {
return nil, zerrors.ThrowPreconditionFailed(nil, "SAML-sdaGg", "app is not active")
}
return serviceprovider.NewServiceProvider(
app.ID,
&serviceprovider.Config{
@@ -72,13 +69,10 @@ func (p *Storage) GetEntityByID(ctx context.Context, entityID string) (*servicep
}
func (p *Storage) GetEntityIDByAppID(ctx context.Context, appID string) (string, error) {
app, err := p.query.AppByID(ctx, appID)
app, err := p.query.AppByID(ctx, appID, true)
if err != nil {
return "", err
}
if app.State != domain.AppStateActive {
return "", zerrors.ThrowPreconditionFailed(nil, "SAML-sdaGg", "app is not active")
}
return app.SAMLConfig.EntityID, nil
}